Skip to content
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

Fixes CASE/WHEN expression possibly returning a set #220

Closed
wants to merge 1 commit into from

Conversation

ilyakooo0
Copy link

@ilyakooo0 ilyakooo0 commented Dec 19, 2022

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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

@shane-circuithub
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rel8 produces invalid SQL: ERROR: argument of CASE/WHEN must not return a set
3 participants