-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Translate pg sql NULL to javascript undefined #1274
Comments
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 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. |
const replaceNull = (key, value) =>
value === null ?
undefined :
value;
JSON.stringify({ x: null, y: 1.2 }, replaceNull) // {"y":1.2} |
@charmander @brianc |
I use Strongly −1 on implementing this, as a setting or otherwise. |
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). |
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. |
It would be a huge BC break! |
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 |
|
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 |
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
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. |
I thought about this for a bit, and agree with @charmander & @sehrope 100%. We will not change from 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 |
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 |
Going to close this as we're not going to change from |
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 It's also consistent since undefined's are also converted to NULL when inserting/updating data |
@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());
}); |
@rpedela thanks, I think I could also try
|
I don't know, maybe current translation
NULL
tonull
have some reasons, but from my point of viewundefined
is more useful, becauseundefined
omitted inJSON
automatically:@brianc
What do you think about it?
The text was updated successfully, but these errors were encountered: