JSON null value
Hello! I believe there is an edge case missing when mapping JSON values in the driver.
Postgres makes a distinction between an SQL NULL and a JSON null. When querying this in Dart, those 2 are mapped to a Dart null, making it impossible to distinguish them.
We could have a constant value in the package for this special value. A few ways we could represent it.
const JsonNull j = JsonNull.instance
const JsonNull j = kJsonNull
SELECT null as c1, 'null'::jsonb as c2, '"null"'::jsonb as c3
This is an interesting edge case.
dart:convert's json.decode('null').runtimeType is Null and its value is null. I think the current behavior is aligned with what I'd expect from a Dart ecosystem package, and as a default I'd keep the current behavior.
However, nullable SQL JSON column being a special case, I am also fine to have a "distinguished" NULL value in the package, but not as a default (esp. because it would be a breaking change for both v2 and v3 users). We should have a configuration setting or better a type serialization override that handles this as a special value.
@isoos I think it's totally fair to leave it as an optional flag to the encode/decode behavior., considering the v3 breaking change is really recent.
I was just tinkering with the json null handling in the encode and decode part of the code and it seems to work just fine with very minimal changes. I'm not that confident to dig into the optional flag for this behavior though, as the encoder and decoder are used in a really long chain of functions that use it.
Should I open a draft PR with the encode/decode changes + a few tests, and later add a proper opt-in mechanism on top?
@davidmartos96: how urgent is this change? I'd prefer the type override over config option, but that requires a bit of refactoring first.
@isoos Sorry, re-reading your comment you are referring to make the SQL NULL the special value, not the Json null. That sounds better actually.
It's not urgent at all. We found this behavior when adapting the ElectricSQL Postgres type mapping support to the Dart client, so that it can be used seemless in backend and frontend apps.
What's the API you have in mind?
Tell the package that a particular TypeOid needs to come wrapped in a sealed class SQLNull | SQLValue(...) ? Or would it be global?
This is a bit harder to make it consistent than I've expected (and also non-breaking or not-much-breaking). The ideas that I am playing with:
- json-specific null value (an empty class) of
JsonNullwhich make a non-NULLSQL value of jsonnull - generic null value (an empty class) of
SqlNull, which makesNULLSQL value, whilenullwould make a jsonnull
On top of it, we should be consistent while writing and then reading the same cells: they should use the same types/values.
I'm not entirely sure what's the best approach to this. I could argue with both strict and lenient approaches, but I'd rather collect more examples on how other libraries do it and how it feels using them.
@isoos In what way would it be breaking?
I was under the impression it would be some opt-in configuration to the TypeRegistry class.
I agree some research could be good. This is something I found before posting this issue. It's from a popular Java library. https://github.com/jOOQ/jOOQ/issues/9607
@davidmartos96: should we close this?
@davidmartos96: should we close this?
Yes, feel free to close this. Thank you!