beam icon indicating copy to clipboard operation
beam copied to clipboard

Make `isJust_` == `not . isNothing_`

Open hovind opened this issue 4 years ago • 4 comments

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

hovind avatar Mar 09 '20 11:03 hovind

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?

kmicklas avatar Aug 09 '20 14:08 kmicklas

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:

hovind avatar Aug 10 '20 16:08 hovind

Perhaps we should rename isJust_ to isEveryFieldJust_ or something longer and more obvious.

3noch avatar Aug 10 '20 17:08 3noch

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.

tathougies avatar Sep 23 '20 18:09 tathougies