raminqaf commented on code in PR #28276:
URL: https://github.com/apache/flink/pull/28276#discussion_r3324350087
##########
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:
If table is optional
```suggestion
| [ LATERAL ] [TABLE] '(' functionName '(' expression [, expression ]* ')'
')'
```
##########
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:
AFAK we only add the english text as place holder and let folks translate it
to Chinese
##########
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:
Do we really need so many javadocs and comments on tests? Claude loves to do
it but IMO test should be clean enough to read with a good function name
##########
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:
```suggestion
| [ LATERAL ] [TABLE] '(' functionName '(' expression [, expression ]* ')'
')'
```
##########
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:
Seems like a good candidate for parasitized tests. WDYT?
##########
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:
Agree this is danger zone ⚠️
--
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]
