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

add SQL syntactic sugar to SPQ pipe operators #5479

Merged
merged 2 commits into from
Nov 14, 2024
Merged

add SQL syntactic sugar to SPQ pipe operators #5479

merged 2 commits into from
Nov 14, 2024

Conversation

mccanne
Copy link
Collaborator

@mccanne mccanne commented Nov 14, 2024

This commit adds some syntactic sugar so that UNNEST can be used in place of OVER, ORDER BY in place of SORT, and LIMIT in place of HEAD. We also unified the PEG grammar for the sort operaetor with the existing rules for SQL ORDER BY, and we added syntactic sugar for IS NULL and IS NOT NULL.

This commit adds some syntactic sugar so that UNNEST can be
used in place of OVER, ORDER BY in place of SORT, and LIMIT
in place of HEAD.  We also unified the PEG grammar for the
sort operaetor with the existing rules for SQL ORDER BY, and
we added syntactic sugar for IS NULL and IS NOT NULL.
@mccanne mccanne requested a review from a team November 14, 2024 16:02
Comment on lines +957 to 961

IsNull
= IS _ NULL { return "==", nil }
/ IS _ NOT _ NULL { return "!=", nil }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be its own expression? Once we get to SQL semantics for null values IS NULL will behave differently than foo == null...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just getting the syntax in. When we change the null semantics, we can update this.

data: |
FROM in.jsup
| UNNEST [...assignees,assignee]
| WHERE this IS NOT NULL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel right since currently FROM in.jsup | UNNEST [...assignees,assignee] evaluates to:

{login:"joe"}
{login:"fred"}
error("missing")
{login:"sue"}
{login:"joe"}
{login:"sue"}
error("missing")

or is error("missing") equivalent to null for this IS NOT NULL?

Copy link
Collaborator

@mattnibs mattnibs Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the PartiQL paper

As far as the boolean connectives and IS NULL are concerned a NULL input and a MISSING input behave
identically.

Is this what you are going for here? If so comment withdrawn.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This came from a SQL query where there is no MISSING or error("missing") and the behavior works with our current this!=null semantics and is also consistent with SQL++ semantics, so I think we're ok.

@@ -0,0 +1,26 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change

@mccanne mccanne merged commit f1213fa into main Nov 14, 2024
3 checks passed
@mccanne mccanne deleted the unnest-op branch November 14, 2024 21:13
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.

3 participants