Node-postgres: Translate pg sql NULL to javascript undefined

[arusakov]

I don't know, maybe current translation NULL to null have some reasons, but from my point of view undefined is more useful, because undefined omitted in JSON automatically:

JSON.stringify({ x: null }) // {"x":null}
JSON.stringify({ x: undefined }) // {}

@brianc
What do you think about it?

[brianc]

I don't think it's a _bad_ idea at all, but it has potential to be a very subtle very breaking change if people are relying on strict equality checks for null on some of their query results. I think at this point we should consider the ship sailed on using null instead of undefined for result values & then if you want to strip them off in your own app you could do something like rows.map(row => _.omit(row, (val, key) => val === null or something? That's a kinda unfortunate side-effect of having two empty values in JavaScript.

I tagged to version 7.x so I can think about it when I start working on some backwards incompat changes to the next breaking version....but I'm leaning towards "that is how it is" right now.

[charmander]

null is the correct value. If you want to omit it from JSON, that’s easy enough:

const replaceNull = (key, value) =>
    value === null ?
        undefined :
        value;

JSON.stringify({ x: null, y: 1.2 }, replaceNull) // {"y":1.2}

[arusakov]

@charmander
Do you have real benefits of null usage? I don't know examples.
Also I know projects, that don't use null at all. For example TypeScript

@brianc
Maybe my proposition can be achieved through new setting?

[charmander]

I use null as an intentional “nothing” value, since undefined tends to be a mistake (given it’s what you get by reading a property that doesn’t exist).

Strongly −1 on implementing this, as a setting or otherwise.

[abenhamdine]

Also -1 on this, null is imho the correct value. As charmander said, undefined is more interpreted as the absence of a given property than the absence of value for this property.

Edit: Also when you look at/manipulate your db response, you want to easily distinguish if a column is null or if the column is missing in the response (because of a mistake or because this column has not been included in the select).

[rpedela]

NULL is a real value in the database and has a specific meaning. We shouldn't be tampering with data in a database client library. You can write your own custom type parser(s) if you want to override how NULL is handled.

[vitaly-t]

https://www.youtube.com/watch?v=lITBGjNEp08

[jfsimon]

It would be a huge BC break!

[caub]

Strongly +1, it may be configurable, I don't know if it's there https://github.com/brianc/node-postgres/blob/master/lib/result.js#L60 but easy to make a configurable 'empty' value, I'll try a PR

a JSON.stringify callback is an overhead, and wanting null in json responses doesn't make sense

[charmander]

wanting null in json responses doesn't make sense

• sure it does
• producing JSON directly from rows with JSON.stringify is not the single use case for this module

[caub]

sure it does

why?

reducing the data size sent is important (without overhead), not related to this lib of course, but null or undefined is arbitrary, the JS null doesn't have to correspond to the PG's NULL, they are unrelated

[sehrope]

why?

Because otherwise you can't represent the existence of a key without a value. While null/undefined are semantically similar in most cases in JS, one big difference is how they're handled when enumerating properties or serializing an object to JSON. In both case the undefined value does not show up.

That means that {foo: null} would be serialized to {}.

reducing the data size sent is important, not related to this lib of course, but null or undefined is arbitrary, the JS null doesn't have to correspond to the PG's NULL, they are unrelated

It does have to correspond. Correctness comes before all else. Otherwise you can't even pass the test of round tripping your data (JS => PG => JS => PG ...). You'd lose information (the presence of keys with NULL values).

If you want to "slim things down" then handle it yourself either with a custom type processor or directly in you app layer.

[brianc]

I thought about this for a bit, and agree with @charmander & @sehrope 100%. We will not change from null to undefined as the default value.

Unfortunately I don't think currently you can work around this with a custom type parser. There is a world somewhere in the future where we could make it possible to _override_ this default in your app with a new setting, but since the effort involved is non-trivial and we have more important stuff to work on I don't know when that will happen.

Plus: it's super easy to strip nulls off your results after you get them from the library if that's how you want to go.

Just to reiterate to relax anyone w/ fears of this being a future breaking change: as of now there is no way we are changing from null to undefined. Not only as @sehrope said is it less correct, but the amount of breakage has the potential to be huge and the upside is minimal at best. I've removed this from consideration for 7.0. As always I'm open to a pull request to make this a setting if the changes are non-invasive, but I have some performance concerns since we turn values into null very deep within the parsing code in a very hot code path.

[georgir]

Specifically for the purpose of JSON.stringify it is trivial to use its second argument and replace null with undefined yourself if that's how you want to roll... There's no point to add breaking changes to unrelated libraries for that purpose.

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
JSON.stringify({wtf:null},(key, value)=>value===null?undefined:value)

[brianc]

Going to close this as we're not going to change from null to undefined Thanks for the discussion though!

[caub]

another point where undefined is more interesting is for destructuring and default values

const {rows:[{x={}, y={}}]} = await pool.query(`select 
    (select login from test where login='44' limit 1) AS x, 
    (select access from test20 limit 1) AS y`)
console.log(x, y) // default won't work when x or y is null

and again {x: undefined} is not the same than {}, so people who wants nulls could do JSON.stringify(rows, (k,v)=>v===undefined ? null : v) or the configurable way like discussed, but it's a pain

It's also consistent since undefined's are also converted to NULL when inserting/updating data

[rpedela]

@caub Write a little code to override the behavior using the type parsers. That is your only option, the current default behavior isn't changing.

var pg = require('pg');

// json
pg.types.setTypeParser(114, function (value) {
    // TODO: replace with your custom logic
    return JSON.parse(value.toString());
});

// jsonb
pg.types.setTypeParser(3802, function (value) {
    // TODO: replace with your custom logic
    return JSON.parse(value.toString());
});

[caub]

@rpedela thanks, I think I could also try json_strip_nulls, probably more performant than js's JSON.stringify(.., ...)

WITH stuff AS (
    SELECT * from (VALUES('1',null)) AS t(id,name)
)
select json_agg(json_strip_nulls(row_to_json(ss))) from (select * from stuff) ss;