-
Notifications
You must be signed in to change notification settings - Fork 767
Separate the LAMBDA keyword syntax from the -> lambda syntax #2458
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
base: main
Are you sure you want to change the base?
Changes from all commits
17ee034
571b7ed
ede1099
df89415
752b6e7
7e7bfc7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -535,10 +535,37 @@ pub trait Dialect: Debug + Any { | |
| /// ```sql | ||
| /// SELECT transform(array(1, 2, 3), x -> x + 1); -- returns [2,3,4] | ||
| /// ``` | ||
| /// | ||
| /// This enables both the `->` spelling above and the `LAMBDA` keyword | ||
| /// spelling gated by [`Self::supports_lambda_keyword_syntax`]. A dialect | ||
| /// that uses `->` as a binary operator should override only the latter. | ||
| fn supports_lambda_functions(&self) -> bool { | ||
| false | ||
| } | ||
|
|
||
| /// Returns true if the dialect supports the `LAMBDA` keyword spelling of | ||
| /// lambda functions, for example: | ||
| /// | ||
| /// ```sql | ||
| /// SELECT list_transform([1, 2, 3], lambda x : x + 1); -- returns [2, 3, 4] | ||
| /// ``` | ||
| /// | ||
| /// This spelling does not claim the `->` token, so it can be enabled by | ||
| /// dialects that already give `->` a different meaning — for example JSON | ||
| /// member access. DuckDB uses `->` for both, resolving the ambiguity from | ||
| /// the function signature at bind time rather than while parsing, and | ||
| /// deprecated the arrow lambda form in v1.3 in favour of this one; v2.0 | ||
| /// disables the arrow form by default. | ||
| /// | ||
| /// Defaults to [`Self::supports_lambda_functions`], so dialects supporting | ||
| /// the `->` spelling accept the `LAMBDA` spelling too unless they say | ||
| /// otherwise. | ||
| /// | ||
| /// See <https://duckdb.org/docs/stable/sql/functions/lambda> | ||
| fn supports_lambda_keyword_syntax(&self) -> bool { | ||
|
adriangb marked this conversation as resolved.
|
||
| self.supports_lambda_functions() | ||
| } | ||
|
Comment on lines
+565
to
+567
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One could argue for adding
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arrow-only is already expressible by overriding |
||
|
|
||
| /// Returns true if the dialect supports multiple variable assignment | ||
| /// using parentheses in a `SET` variable declaration. | ||
| /// | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not sure I understood the problem being solved for - the description mentions pg as an example, suggesting there is some ambiguous grammar in play but pg doesnt have lambda syntax to my knowledge?
is there an example syntax that issupported by a dialect and the parser doesnt cover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fair point. I replied in #2458 (comment) but reflecting a bit more I think there's an even stronger framing. Your hesitation comes from PostgreSQL being a bad example. It has no lambda syntax, so the new flag
would be unreachable there and the motivation is then speculative. I've reframed the
description to focus on DuckDB and custom dialects.
For DuckDB this enables fixing a live bug rather than a hypothetical.
DuckDbDialectalready setssupports_lambda_functions() == true, so DuckDB's owndocumented JSON example misparses today:
A bare column becomes a lambda, a qualified one stays JSON access. Both print back as
the same SQL, which is why no round-trip test catches it.
DuckDB itself resolves
->from the function signature at bind time, which a parsercan't do — and that's exactly why they deprecated the arrow form in v1.3 in favor of
lambda x : x + 1, with v2.0 disabling it by default andSET lambda_syntaxtochoose in between. Following that requires the two spellings to be separable, which is
all this PR does.
I've deliberately not flipped
DuckDbDialecthere: it fixes the case above but breaksx -> x > 1in the other direction, so it's worth its own PR. I did add an assertionpinning the current arrow behavior because I found the whole suite stays green when
you flip it. This way if we do flip it we can verify the change in behavior against tests.
This PR also pplies to custom dialects wanting JSON accessors and lambdas at once, which
is what the
derive_dialect!test now covers. This could be a postgres based dialect that wants to add support for lambda functions (our case, usingderive_dialect!) or a completely custom dialect.