fhueske commented on code in PR #28675:
URL: https://github.com/apache/flink/pull/28675#discussion_r3542387868


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkCalciteSqlValidator.java:
##########
@@ -456,6 +460,39 @@ private static void 
checkNoNamedAndPositionalMixedArgs(SqlBasicCall call) {
         }
     }
 
+    /**
+     * Rejects the implicit PTF system arguments (on_time, uid) for functions 
that disable them.
+     *
+     * <p>When a PTF sets {@code disableSystemArguments(true)}, the system 
arguments are not part of
+     * its signature. Enforcing this here covers all translation paths 
uniformly.
+     */
+    private static void checkDisabledSystemArgs(SqlBasicCall call) {

Review Comment:
   There are a bunch of PTF argument checks in `FlinkCalciteSqlValidator` 
already and the check at this place is fairly straightforward.
   
   If we want to perform the check at a later point, we would need to include 
system args in signatures of PTFs that disable them because Calcite drops 
unknown named arguments when it converts them into positional arguments. So 
without extending the signatures, we would not know if a function was called 
with system args or not.
   That would mean that we need to refactor quite a bit and add more special 
case handling for PTFs with disabled system args.
   We can move the actual check implementation as a static helper into 
`SystemTypeInference` but it's much easier to call it from 
`FlinkCalciteSqlValidator` than at a later time.



-- 
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]

Reply via email to