adriangb commented on code in PR #2458:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2458#discussion_r3945430442
##########
src/dialect/mod.rs:
##########
@@ -535,10 +535,32 @@ 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, such as PostgreSQL
+ /// and its derivatives, where `->` is JSON member access. 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>
Review Comment:
This is a fair point. I replied in
https://github.com/apache/datafusion-sqlparser-rs/pull/2458#issuecomment-5508739108
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.
`DuckDbDialect` already sets `supports_lambda_functions() == true`, so
DuckDB's own
documented JSON example misparses today:
```rust
// DuckDbDialect
"SELECT j -> 'field' FROM t" // => Expr::Lambda { params: [j], body:
'field' } ❌
"SELECT t.j -> 'field' FROM t" // => BinaryOp { op: Arrow }
✅
```
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 parser
can'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 and `SET
lambda_syntax` to
choose in between. Following that requires the two spellings to be
separable, which is
all this PR does.
I've deliberately not flipped `DuckDbDialect` here: it fixes the case above
but breaks
`x -> x > 1` in the other direction, so it's worth its own PR. I did add an
assertion
pinning 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, using
`derive_dialect!`) or a completely custom dialect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
