mihaibudiu commented on code in PR #5077:
URL: https://github.com/apache/calcite/pull/5077#discussion_r3554814627


##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -275,13 +277,36 @@ public abstract Result visitInput(RelNode e, int i, 
boolean anon,
   public void addSelect(List<SqlNode> selectList, SqlNode node,
       RelDataType rowType) {
     String name = rowType.getFieldNames().get(selectList.size());
+    addSelect(selectList, node, name);
+  }
+
+  private void addSelect(List<SqlNode> selectList, SqlNode node,
+      String name) {
     @Nullable String alias = SqlValidatorUtil.alias(node);
     if (alias == null || !alias.equals(name)) {
       node = as(node, name);
     }
     selectList.add(node);
   }
 
+  /** Returns field names to use when a result is wrapped as a derived table. 
*/
+  protected List<String> derivedTableFieldNames(RelDataType rowType) {
+    return rowType.getFieldNames();
+  }
+
+  /** Returns a copy of a row type with different field names. */
+  private static RelDataType renameRowTypeFields(RelDataType rowType,

Review Comment:
   I wonder whether this is the right place for this function. It looks like it 
would belong as a method to RelRecordType.



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -275,13 +277,36 @@ public abstract Result visitInput(RelNode e, int i, 
boolean anon,
   public void addSelect(List<SqlNode> selectList, SqlNode node,
       RelDataType rowType) {
     String name = rowType.getFieldNames().get(selectList.size());
+    addSelect(selectList, node, name);
+  }
+
+  private void addSelect(List<SqlNode> selectList, SqlNode node,
+      String name) {
     @Nullable String alias = SqlValidatorUtil.alias(node);
     if (alias == null || !alias.equals(name)) {
       node = as(node, name);
     }
     selectList.add(node);
   }
 
+  /** Returns field names to use when a result is wrapped as a derived table. 
*/
+  protected List<String> derivedTableFieldNames(RelDataType rowType) {

Review Comment:
   Is this function really useful?



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -2444,18 +2499,89 @@ public Context qualifiedContext() {
       return aliasContext(aliases, true);
     }
 
+    /** Returns a result that uses derived-table field names. */
+    private Result forDerivedTable() {
+      if (neededType == null) {
+        return this;
+      }
+      // Internal derived table aliases may need dialect-specific uniqueness.
+      final List<String> fieldNames = derivedTableFieldNames(neededType);
+      if (fieldNames.equals(neededType.getFieldNames())) {
+        return this;
+      }
+      final RelDataType type = renameRowTypeFields(neededType, fieldNames);
+      final SqlNode newNode = withDerivedTableFieldNames(fieldNames);
+      final ImmutableMap.Builder<String, RelDataType> aliasBuilder =
+          ImmutableMap.builder();
+      for (Map.Entry<String, RelDataType> alias : aliases.entrySet()) {
+        aliasBuilder.put(alias.getKey(),
+            alias.getValue() == neededType ? type : alias.getValue());
+      }
+      return new Result(newNode, clauses, neededAlias, type, 
aliasBuilder.build(), anon,
+          ignoreClauses, expectedClauses, expectedRel, forceExplicitAlias);
+    }
+
+    /** Rewrites a result for use in a derived table. */
+    private SqlNode withDerivedTableFieldNames(List<String> fieldNames) {
+      if (node.getKind() == SqlKind.AS) {
+        final SqlCall call = (SqlCall) node;
+        final List<SqlNode> operands = call.getOperandList();
+        final int fieldAliasStart = 2;
+        if (operands.size() == fieldNames.size() + fieldAliasStart) {
+          final List<SqlNode> newOperands = new ArrayList<>(operands.size());
+          newOperands.add(call.operand(0));
+          newOperands.add(call.operand(1));
+          for (String fieldName : fieldNames) {
+            newOperands.add(new SqlIdentifier(fieldName, POS));
+          }
+          return SqlStdOperatorTable.AS.createCall(POS, newOperands);
+        }
+      }
+      return withSelectFieldNames(fieldNames);
+    }
+
+    /** Rewrites a SELECT list to expose the given field names. */

Review Comment:
   expose?
   use the supplied names from `{@code fieldNames}`?
   I suspect the count must match, do you need an assertion someplace about 
this?



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -2444,18 +2499,89 @@ public Context qualifiedContext() {
       return aliasContext(aliases, true);
     }
 
+    /** Returns a result that uses derived-table field names. */
+    private Result forDerivedTable() {
+      if (neededType == null) {
+        return this;
+      }
+      // Internal derived table aliases may need dialect-specific uniqueness.
+      final List<String> fieldNames = derivedTableFieldNames(neededType);
+      if (fieldNames.equals(neededType.getFieldNames())) {
+        return this;
+      }
+      final RelDataType type = renameRowTypeFields(neededType, fieldNames);
+      final SqlNode newNode = withDerivedTableFieldNames(fieldNames);
+      final ImmutableMap.Builder<String, RelDataType> aliasBuilder =
+          ImmutableMap.builder();
+      for (Map.Entry<String, RelDataType> alias : aliases.entrySet()) {
+        aliasBuilder.put(alias.getKey(),
+            alias.getValue() == neededType ? type : alias.getValue());
+      }
+      return new Result(newNode, clauses, neededAlias, type, 
aliasBuilder.build(), anon,
+          ignoreClauses, expectedClauses, expectedRel, forceExplicitAlias);
+    }
+
+    /** Rewrites a result for use in a derived table. */

Review Comment:
   This doc does not describe the parameters, and is not very clear, maybe it 
will become clearer once you define derived tables.



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -2444,18 +2499,89 @@ public Context qualifiedContext() {
       return aliasContext(aliases, true);
     }
 
+    /** Returns a result that uses derived-table field names. */

Review Comment:
   I am not sure the term "derived table" is defined in Calcite, people may 
wonder what it means.
   Maybe you can add a description in the JavaDoc explaining when a table is 
derived.



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