twalthr commented on code in PR #28675:
URL: https://github.com/apache/flink/pull/28675#discussion_r3613297984
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/stream/sql/ProcessTableFunctionTest.java:
##########
@@ -283,6 +284,49 @@ void testNoSystemArgsAllowedForTablePtf() {
"Disabling system arguments is not supported
for user-defined PTF."));
}
+ @Test
+ void testOnTimeArgRejectedForDisabledPtf() {
Review Comment:
Use the error spec below for all these throwing tests.
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/stream/sql/MLPredictTableFunctionTest.java:
##########
@@ -103,4 +104,30 @@ public void testInputTableIsCdcStream() {
.hasMessageContaining(
"StreamPhysicalMLPredictTableFunction doesn't support
consuming update and delete changes which is produced by node
TableSourceScan(table=[[default_catalog, default_database, CdcTable]],
fields=[a, b])");
}
+
+ @Test
+ void testOnTimeArgumentNotAllowed() {
+ // ML_PREDICT disables the implicit system arguments. Supplying
`on_time` must be rejected
+ // at the SQL level even though ML_PREDICT is handled by a dedicated
optimizer rule.
+ String sql =
+ "SELECT * FROM TABLE(ML_PREDICT(INPUT => TABLE MyTable, MODEL
=> MODEL MyModel, "
Review Comment:
please remove everwhere
```suggestion
"SELECT * FROM ML_PREDICT(INPUT => TABLE MyTable, MODEL =>
MODEL MyModel, "
```
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/SystemTypeInference.java:
##########
@@ -126,6 +127,33 @@ public static boolean
isInvalidUidForProcessTableFunction(String uid) {
return !UID_FORMAT.test(uid);
}
+ /**
+ * Rejects the implicit system arguments ({@code on_time}, {@code uid})
for a function that
+ * disables them via {@link TypeInference#disableSystemArguments()}.
+ *
+ * <p>The system arguments are not part of such a function's signature.
Enforcing this from
+ * every translation path (SQL operand checking and Table API call
resolution) rejects them
+ * consistently, regardless of whether the function is processed by the
generic PTF rule or a
+ * dedicated optimizer rule (e.g. ML_PREDICT, LATERAL SNAPSHOT).
+ */
+ public static void checkNoSystemArguments(
+ boolean sysArgsDisabled,
+ Collection<String> suppliedArgumentNames,
+ String functionName) {
+ if (!sysArgsDisabled) {
+ return;
+ }
+ for (StaticArgument systemArg : PROCESS_TABLE_FUNCTION_SYSTEM_ARGS) {
+ if (suppliedArgumentNames.contains(systemArg.getName())) {
+ throw new ValidationException(
+ String.format(
+ "Invalid function call. The '%s' argument is
not supported for "
+ + "function '%s' because it disables
system arguments.",
Review Comment:
asked AI to improve the message:
```
Invalid function call: The '%s' argument is not supported because function
'%s' does not use system arguments.
```
less internal implementation specific
--
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]
