fhueske commented on code in PR #28276:
URL: https://github.com/apache/flink/pull/28276#discussion_r3870528269
##########
docs/content.zh/docs/dev/table/functions/udfs.md:
##########
@@ -1088,7 +1088,7 @@ env.sqlQuery("SELECT GetBeverageName(beverageId) FROM
Beverages");
在 Table API 中，表值函数是通过 `.joinLateral(...)` 或者 `.leftOuterJoinLateral(...)`
来使用的。`joinLateral` 算子会把外表（算子左侧的表）的每一行跟跟表值函数返回的所有行（位于算子右侧）进行
（cross）join。`leftOuterJoinLateral`
算子也是把外表（算子左侧的表）的每一行跟表值函数返回的所有行（位于算子右侧）进行（cross）join，并且如果表值函数返回 0 行也会保留外表的这一行。
-在 SQL 里面用 `JOIN` 或者 以 `ON TRUE` 为条件的 `LEFT JOIN` 来配合 `LATERAL
TABLE(<TableFunction>)` 的使用。
+在 SQL 里面用 `JOIN` 或者 以 `ON TRUE` 为条件的 `LEFT JOIN` 来配合 `LATERAL
<TableFunction>(...)` 的使用（等价的写法为 `LATERAL TABLE(<TableFunction>(...))`）。
Review Comment:
Good point on the convention. In this spot the lines I touched are edits to
text that was already in Chinese (updating the SQL examples to the implicit
form, plus one adjacent sentence), not a newly added section.
I used Claude to adjust the text and validated with Google Translate, so it
shouldn't be too far off 😬 .
##########
flink-table/flink-sql-parser/src/main/codegen/templates/Parser.jj:
##########
@@ -2236,6 +2236,16 @@ SqlNode TableRef3(ExprContext exprContext, boolean
lateral) :
{
tableRef = unnestOp.createCall(s.end(this), (List<SqlNode>) args);
}
+ |
+ // LATERAL with implicit table function call syntax,
+ // e.g. "FROM t, LATERAL fn(...)" instead of "FROM t, LATERAL
TABLE(fn(...))".
+ // The non-LATERAL implicit form is handled by the
CompoundTableIdentifier
+ // branch above.
+ LOOKAHEAD(2)
+ <LATERAL> { lateral = true; }
+ tableName = CompoundTableIdentifier()
+ tableRef = ImplicitTableFunctionCallArgs(tableName)
+ tableRef = addLateral(tableRef, lateral)
Review Comment:
I've update the change to mirror the recent CALCITE-7183
(apache/calcite#5214) change one-to-one.
##########
docs/content.zh/docs/sql/reference/queries/overview.md:
##########
@@ -316,7 +316,8 @@ tableReference:
tablePrimary:
[ TABLE ] tablePath [ dynamicTableOptions ] [systemTimePeriod] [[AS]
correlationName]
- | LATERAL TABLE '(' functionName '(' expression [, expression ]* ')' ')'
+ | [ LATERAL ] functionName '(' expression [, expression ]* ')'
+ | [ LATERAL ] TABLE '(' functionName '(' expression [, expression ]* ')' ')'
Review Comment:
Thanks! I'd prefer to keep these as two separate lines rather than
`[TABLE]`, because collapsing them changes the grammar: `[ LATERAL ] [TABLE]
'(' functionName '(' ... ')' ')'` would also permit the bare-parenthesized `(
fn(args) )`, which the parser does not accept. The two valid forms are
`fn(args)` and `TABLE(fn(args))`, so they need distinct productions.
This also matches Calcite's own `reference.md` in CALCITE-7183
(apache/calcite#5214), which adds the implicit line separately and keeps the
explicit `[ LATERAL ] TABLE '(' ... ')'` line.
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -4018,6 +4018,150 @@ void testOuterApplyFunctionFails() {
.fails("(?s).*Encountered \"\\)\" at .*");
}
+ /**
+ * Overrides the parent {@link
org.apache.calcite.sql.parser.SqlParserTest#testLateral()}
+ * because Flink makes the {@code TABLE} keyword optional inside {@code
LATERAL}. With this
+ * change, {@code LATERAL <identifier>} is the start of an implicit
table-function call; the
+ * parser expects an argument list next, so the error position shifts.
+ */
+ @Test
+ void testLateral() {
+ // This is the only test case that differs from Calcite's
SqlParserTest.testLateral().
+ // LATERAL <identifier> is now interpreted as an implicit table
function
+ // call; the error moves to where the argument list (LPAREN) is
missing.
+ sql("select * from lateral em^p^").fails("(?s)Encountered \"<EOF>\" at
.*");
+
+ // All other test cases are identical to Calcite's
SqlParserTest.testLateral().
+ // LATERAL TABLE <identifier> still fails at the identifier (no
LPAREN).
+ sql("select * from lateral table ^emp^ as e").fails("(?s)Encountered
\"emp\" at .*");
+ sql("select * from lateral table ^scott^.emp").fails("(?s)Encountered
\"scott\" at .*");
+
+ final String expected = "SELECT *\n" + "FROM LATERAL TABLE(`RAMP`(1))";
+
+ // Good: LATERAL TABLE function(arg, arg)
+ sql("select * from lateral table(ramp(1))").ok(expected);
+ sql("select * from lateral table(ramp(1)) as t").ok(expected + " AS
`T`");
+ sql("select * from lateral table(ramp(1)) as t(x)").ok(expected + " AS
`T` (`X`)");
+ // Bad: Parentheses make it look like a sub-query
+ sql("select * from lateral (^table (ramp(1))^)").fails("Expected query
or join");
+
+ // Good: LATERAL (subQuery)
+ final String expected2 = "SELECT *\n" + "FROM LATERAL (SELECT *\n" +
"FROM `EMP`)";
+ sql("select * from lateral (select * from emp)").ok(expected2);
+ sql("select * from lateral (select * from emp) as t").ok(expected2 + "
AS `T`");
+ sql("select * from lateral (select * from emp) as t(x)").ok(expected2
+ " AS `T` (`X`)");
+ }
+
+ /**
+ * Overrides the parent {@link
org.apache.calcite.sql.parser.SqlParserTest#testTemporalTable()}
+ * for the same reason as {@link #testLateral()}: with the implicit
table-function call form,
+ * {@code LATERAL products_temporal} now parses successfully and the error
shifts to the next
+ * unexpected token ({@code for}).
+ */
+ @Test
+ void testTemporalTable() {
Review Comment:
`testLateral` and `testTemporalTable` are overrides of Calcite's
`SqlParserTest`, and I kept them structurally identical to upstream (same mix
and order of `ok()`/`fails()` cases) so that future Calcite syncs stay a
trivial diff. I'd lean toward leaving them as-is for that reason, but happy to
parameterize the pure `fails()` cases if you feel strongly.
##########
docs/content/docs/sql/reference/queries/overview.md:
##########
@@ -316,7 +316,8 @@ tableReference:
tablePrimary:
[ TABLE ] tablePath [ dynamicTableOptions ] [systemTimePeriod] [[AS]
correlationName]
- | LATERAL TABLE '(' functionName '(' expression [, expression ]* ')' ')'
+ | [ LATERAL ] functionName '(' expression [, expression ]* ')'
+ | [ LATERAL ] TABLE '(' functionName '(' expression [, expression ]* ')' ')'
Review Comment:
See above
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -4018,6 +4018,150 @@ void testOuterApplyFunctionFails() {
.fails("(?s).*Encountered \"\\)\" at .*");
}
+ /**
+ * Overrides the parent {@link
org.apache.calcite.sql.parser.SqlParserTest#testLateral()}
+ * because Flink makes the {@code TABLE} keyword optional inside {@code
LATERAL}. With this
+ * change, {@code LATERAL <identifier>} is the start of an implicit
table-function call; the
+ * parser expects an argument list next, so the error position shifts.
+ */
Review Comment:
Agreed — trimmed. Removed the multi-line javadocs and the redundant "this
case differs / all others identical" narration, keeping just a one-line note on
why each method overrides Calcite and a short marker on the single assertion
that actually differs.
--
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]
