node-postgres: Translate pg sql NULL to javascript undefined

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?

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 23
  • Comments: 18 (12 by maintainers)

Most upvoted comments

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.

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.

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.

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}

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

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.

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

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

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).