circuithub / rel8

Hey! Hey! Can u rel8?
https://rel8.readthedocs.io
Other
154 stars 39 forks source link

Fixes CASE/WHEN expression possibly returning a set #220

Closed ilyakooo0 closed 1 year ago

ilyakooo0 commented 1 year ago

closes #219

From what I understand the problem was that boolExpr was being called directly with a value of "UNNEST(.." which directly produces a set which is then returned in the body of a CASE/WHEN expression.

If you try to create the same problem with publicly exported functions (using maybeTable for example which call boolExpr internally) then you are forced to >>= the UNNESTed rows which results in a subexpression in SQL and the CASE/WHEN ends up returning a single row instead of a set.

MaybeTable used to use guard which woks around the requirement of an extra >>=. The fix just removes the tag check for the just column altogether. This seems fine to me because there can never be a case where tag is non-null and just is null. I e it seems like the guard was redundant.

tomjaguarpaw commented 1 year ago

there can never be a case where tag is non-null and just is null

I'm not following all the details, but what about Just Nothing :: Maybe (Maybe (Expr a))?

ilyakooo0 commented 1 year ago

@tomjaguarpaw in that case we would have a structure like

MaybeTable
  { tag = true
  , just = 
    MaybeTable
      { tag = null
      , just = null 
      }
  }

I read what I wrote again and it made no sense. (:

What I meant to say was: the guard seems to be redundant because all relevant functions check the tag explicitly. This means that the guard is only useful in cases where the tag says that the value should be Just, but is actually null, but in that case the guard would return the "true" value i. e. null.

tomjaguarpaw commented 1 year ago

OK, it's possible I'm making no sense either because I didn't really look closely :)

shane-circuithub commented 1 year ago

I've implemented a different fix for this in #240.