beam
beam copied to clipboard
Make `isJust_` == `not . isNothing_`
Right now isJust_
returns False
for a RowT (Nullable (QGenExpr ctxt be s))
where
data RowT f =
RowT
{ _uuid :: !(C f Text)
, _foo :: !(C f (Maybe Int))
}
deriving anyclass (Beamable)
deriving (Generic)
if the _foo :: !(C f (Maybe Int))
field is null
, even if _uuid :: !(C f Text)
is non-null
.
Perhaps this is intended behaviour, but I find it confusing that a isJust_ x = False
for the same x
for which isNothing_ x = False
.
Please let me know if the PR is missing something, if something is unclear or if I'm plain wrong. :)
Hmm, this is a bit confusing, but I think the current behavior makes sense and/or is at least useful since it distinguishes more values. Can we fix this with better documentation instead?
Hmm, this is a bit confusing, but I think the current behavior makes sense and/or is at least useful since it distinguishes more values. Can we fix this with better documentation instead?
Personally, I disagree that isJust_ x == False
when x :: RowT (Nullable (QGenExpr ctxt be s))
has a single field that is NULL
makes sense, sounds counter-intuitive to me.
As for its usefulness, I no longer have access to the code base where this issue came up, so I'm not sure if the issues we had could be solved by substituting not . isNothing_
.
Documentation sounds great regardless, thanks for picking this up! :smiley:
Perhaps we should rename isJust_
to isEveryFieldJust_
or something longer and more obvious.
This is somewhat intended behavior due to the difficulty of dealing with nullables in SQL... We can't determine if the Maybe came from the fact the row didn't exist, versus the row simply containing a NULL value. It's mentioned somewhere in the docs, but it would probably be worth it to dedicate a whole page to the gotchas with NULL in SQL. Unfortunately, the lack of sum types in SQL is a major pain.