fsk119 commented on code in PR #27108:
URL: https://github.com/apache/flink/pull/27108#discussion_r2492839344


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/expressions/converter/ExpressionConverter.java:
##########
@@ -235,27 +255,96 @@ public RexNode visit(TypeLiteralExpression typeLiteral) {
     }
 
     @Override
-    public RexNode visit(Expression other) {
+    public RexNode visit(TableReferenceExpression tableRef) {
+        final LogicalType tableArgType = 
tableRef.getOutputDataType().getLogicalType();
+        final RelDataType rowType = typeFactory.buildRelNodeRowType((RowType) 
tableArgType);
+        final int[] partitionKeys;
+        if (tableRef.getQueryOperation() instanceof PartitionQueryOperation) {
+            final PartitionQueryOperation partitionOperation =
+                    (PartitionQueryOperation) tableRef.getQueryOperation();
+            partitionKeys = partitionOperation.getPartitionKeys();
+        } else {
+            partitionKeys = new int[0];
+        }
+        final RexTableArgCall tableArgCall =
+                new RexTableArgCall(rowType, inputStack.size(), partitionKeys, 
new int[0]);
+        inputStack.add(relBuilder.build());

Review Comment:
   Can we do like this?
   
   ```
     Collections.reverse(resolvedArgs);
     final List<RelNode> inputStack = new ArrayList<>();
     final List<RexNode> rexNodeArgs =
             resolvedArgs.stream()
                     .map(QueryOperationConverter.this::convertExprToRexNode)
                     .peek(
                             expr -> {
                                 if (expr instanceof RexTableArgCall) {
                                     inputStack.add(relBuilder.build());
                                 }
                             })
                     .collect(Collectors.toList());
     Collections.reverse(rexNodeArgs);
   
   ```
   And in the ExpressionConverter:
   
   ```
    @Override
       public RexNode visit(TableReferenceExpression tableRef) {
           final LogicalType tableArgType = 
tableRef.getOutputDataType().getLogicalType();
           final RelDataType rowType = 
typeFactory.buildRelNodeRowType((RowType) tableArgType);
           final int[] partitionKeys;
           if (tableRef.getQueryOperation() instanceof PartitionQueryOperation) 
{
               final PartitionQueryOperation partitionOperation =
                       (PartitionQueryOperation) tableRef.getQueryOperation();
               partitionKeys = partitionOperation.getPartitionKeys();
           } else {
               partitionKeys = new int[0];
           }
           final RexTableArgCall tableArgCall =
                   new RexTableArgCall(rowType, relBuilder.size() - 1, 
partitionKeys, new int[0]);
           //        inputStack.add(relBuilder.build());
           return tableArgCall;
       }
   ```
   
   After the change, I find no test in `ProcessTableFunctionSemanticTests` 
fails. 
   
   I think the logic for `ptf(table t1, table t2, arg)` is:
   
   1. When building the FunctionQueryOperation, the planner first pushes 
      all operands used in the RelNode tree onto the stack. As a result, 
      the stack head is t2, followed by t1.
   2. We then parse the operands in reverse order. For operand table t2, 
      the current stack size is 2, so we can build a RexTableArgCall to 
      refer to the stack head. After visiting, we pop the stack head.
   
   WDYT?



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