Pg-promise: Consider making it easier to generate dynamic inserts/updates with optional columns

[noinkling]

This is a followup to my comment in #191

One of the nice things things about paramatized queries is that you can often directly supply a user-provided object to INSERT and UPDATE queries like so:

db.none(`
  INSERT INTO books (title, author)
  VALUES ($[title], $[author]);
`, req.params)

``js db.none(
UPDATE books
SET title = $[title], author = $[author]
WHERE id = $[id];
`, req.params)

Since the column names are hardcoded, any additional (maybe malicious) properties on `req.params` will simply be ignored.

However these sorts of queries often have columns which are optional, in other words, they are intentionally nullable or have a set default. The above queries will obviously fail, if say, the `author` property doesn't exist on the `req.params` object, so the columns need to be supplied dynamically to the query based on what's supplied in the object. For that we can use `helpers.insert` and `helpers.update` like so:
```js
const query = pgp.helpers.insert(req.params, null, 'books');
db.none(query);

This will only set columns that exist on the source object. The problem is, now we have lost the benefit of automatically excluding properties we don't want. If we simply supply a list of column names to the helper (helpers.insert(req.params, ['title', 'author'], 'books')) we're back where we started (error about author not existing on the object). What I really want is for the column list to act as a whitelist.

Column objects allow some flexibility to allow this, i.e. for UPDATE queries:

const skip = ({ exists }) => !exists;

const columns = new pgp.helpers.ColumnSet([
  'title',
  { name: 'author', skip: skip }
], { table: 'books' });

const query = pgp.helpers.update(req.params, columns) + ' WHERE id = $[id]';
db.none(query, req.params.id);

But it's pretty clunky, and even worse for INSERT queries since the helper doesn't respect the skip option, so we have to dynamically determine if we should set DEFAULT (as a raw value) instead:

const setDefault = col => (
  col.exists ? col.value : { _rawDBType: true, formatDBType: () => 'DEFAULT' }
);

const columns = new pgp.helpers.ColumnSet([
  'title',
  { name: 'author', init: setDefault }
], { table: 'books' });
const query = pgp.helpers.insert(req.params, columns);
db.none(query);

This seems like quite a messy and complex workaround, especially compared to the non-dynamic version. It would be nice if there was a way to achieve the same thing more easily, for example maybe a whitelist option for ColumnSet:

new helpers.ColumnSet(['title', 'author'], { whitelist: true });

Or, since that wouldn't let you differentiate between required and option columns, maybe an optional option for Column:

new helpers.Column({
  name: 'author',
  optional: true    // like `skip: ({ exists }) => !exists` except recognized by `helpers.insert`
});

You could maybe even include a special shorthand syntax like there is for the cnd option?

Of course you could stop at helpers.insert(req.params, null, 'books') and say that it's the developer's job to do their own filtering on the source object, but I believe it's in the scope of the helpers to provide the functionality I've outlined here.

Sorry for the long post, and thanks for your consideration.

[vitaly-t]

What you are suggesting would only work for a single-row insert, but not for multi-row inserts, and thus offer little value. See #239

[vitaly-t]

This seems like quite a messy and complex workaround

Why? You can define that logic only once, in a nice, reusable way:

var rawText = text => ({_rawDBType: true, formatDBType: () => text});

var optCol = name => ({name, init: c => c.exists ? c.value : rawText('DEFAULT')});

and then you can declare your optional columns like this:

new pgp.helpers.ColumnSet(['title', optCol('author')], { table: 'books' });

This solution will work for both single-row inserts and multi-row inserts, and it is very extensible. And as I said earlier, it would offer little value trying to add something that will only work in one specific scenario. It is best to keep the protocol to the minimum / to what's necessary, imo.

And again, what you suggested isn't flexible enough, because when a property does exist, we need full control over the value, which is what property init gives us.

@noinkling back at ya :wink:

[noinkling]

What you are suggesting would only work for a single-row insert, but not for multi-row inserts, and thus offer little value. See #239

As you show in your next post, if you're generating a multi-row insert, it could just set DEFAULT for the relevant records instead of omitting the column, same effect right? I mean, even for single-row insert that would be fine too if it makes the implementation cleaner/easier.

Why? You can define that logic only once, in a nice, reusable way:

I do like this (thanks for making it look pretty), but it really is just a matter of convenience and having something obvious instead of the developer having to spend time working out and implementing something like this themselves. I mean, even if you just included that optCol function on helpers (maybe with a different name) I would be happy:

var pgp = require(...);
var { insert, update, optional } = pgp.helpers;

var query = insert(req.params, ['title', optional('author')], 'books');

It could maybe even be made into a tag function: optional`author`.

[vitaly-t]

The example for optCol is just one possible variation. There are many other properties supported by type ColumnConfig, and I'd rather not bottle up those into a limited-purpose function, giving you the complete freedom of how the type is used.

I think this issue has been discussed at length ;)

[noinkling]

You'd still have that freedom though right? This would just be convenience for a relatively common use-case.

Anyway I respect your decision, thanks for the well-maintained project.

[vitaly-t]

@noinkling after a bit more thinking I've realized that I've missed something trivial in the example that I provided earlier...

This logic that we do: init: c => c.exists ? c.value : rawText('DEFAULT'), it exactly replicates the logic for property def, i.e. your type optCol can be defined in a much simpler way:

var optCol = name => ({name, def: rawText('DEFAULT')});

[noinkling]

Thanks 👍 For some dumb reason it didn't occur to me that def would respect the custom type (maybe because I was too busy seeing if I could make the mod option work dynamically).

For reference my pared-down implementation looks something like this now, in case it's useful to someone else, or you want to add an example in the docs or something:

const DEFAULT = { formatDBType: () => 'DEFAULT', _rawDBType: true };
const skip = ({exists}) => !exists;

exports.optionalColumn = (name) => ({
  name: Array.isArray(name) ? name[0] : name,    // Allows use as a template string tag
  def: DEFAULT,
  skip: skip
});

Usage:

const { db, pgp } = require('./db');
const optional = require('./db/utils').optionalColumn;

const columns = new pgp.helpers.ColumnSet([
  `title`,
  optional `author`    // Really easy to add/remove optional status with this syntax
]);

I included skip so it also works with single-object helpers.update. The only thing I have to watch out for is that I don't use it with the multiple-object form, because that doesn't work with skip (for obvious syntactical reasons) and behaves more like an INSERT, falling back to DEFAULT on omitted properties, which probably isn't desirable.

[vitaly-t]

@noinkling a related question was published on StackOverflow today. It wasn't you, right? 😄

[noinkling]

@vitaly-t Nope.

[vitaly-t]

F.Y.I. Important breaking change in v6.5.0.

[noinkling]

Thanks for the heads-up.

[vitaly-t]

This makes a good reusable example of an optional column that's skipped for updates and provides DEFAULT for inserts:

const optional = name => ({
    name: Array.isArray(name) ? name[0] : name,    // Allows use as a template string tag
    def: {toPostgres: () => 'DEFAULT', _rawType: true},
    skip: c => !c.exists
});

Using it we can define optional columns in two ways:

optional `name` // ES6 syntax

optional('name') // ES5 syntax

@noinkling I published it here to be used as a reference for later :smile:

[vitaly-t]

Version 7.2.0 changed it a bit.