zabetak commented on code in PR #3843:
URL: https://github.com/apache/hive/pull/3843#discussion_r1043287199
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/PlanModifierForASTConv.java:
##########
@@ -71,7 +73,10 @@ public class PlanModifierForASTConv {
public static RelNode convertOpTree(RelNode rel, List<FieldSchema>
resultSchema, boolean alignColumns)
throws CalciteSemanticException {
if (rel instanceof HiveValues) {
- return rel;
+ RelDataTypeFactory typeFactory = rel.getCluster().getTypeFactory();
+ List<String> fieldNames =
resultSchema.stream().map(FieldSchema::getName).collect(Collectors.toList());
+ fieldNames = SqlValidatorUtil.uniquify(fieldNames,
typeFactory.getTypeSystem().isSchemaCaseSensitive());
Review Comment:
Is it necessary to create uniq field names? I am asking cause it seems that
due to this the CLI header in some existing tests change.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveValues.java:
##########
@@ -43,4 +45,18 @@ public HiveValues(RelOptCluster cluster, RelDataType
rowType, ImmutableList<Immu
public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
return new HiveValues(getCluster(), getRowType(), tuples, getTraitSet());
}
+
+ public RelNode copy(List<String> newColumnNames) throws
CalciteSemanticException {
+ if (newColumnNames.size() != getRowType().getFieldCount()) {
+ throw new CalciteSemanticException("The number of new column names and
columns in the schema does not match!");
+ }
+
+ RelDataTypeFactory.Builder builder =
getCluster().getTypeFactory().builder();
Review Comment:
Maybe you can exploit the `builder.uniquify()` method to make the code
slightly more readable.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/PlanModifierForASTConv.java:
##########
@@ -71,7 +73,10 @@ public class PlanModifierForASTConv {
public static RelNode convertOpTree(RelNode rel, List<FieldSchema>
resultSchema, boolean alignColumns)
throws CalciteSemanticException {
if (rel instanceof HiveValues) {
- return rel;
+ RelDataTypeFactory typeFactory = rel.getCluster().getTypeFactory();
+ List<String> fieldNames =
resultSchema.stream().map(FieldSchema::getName).collect(Collectors.toList());
+ fieldNames = SqlValidatorUtil.uniquify(fieldNames,
typeFactory.getTypeSystem().isSchemaCaseSensitive());
+ return ((HiveValues) rel).copy(fieldNames);
Review Comment:
I suppose the intention is to make something similar to
`renameTopLevelSelectInResultSchema` but in that case shouldn't be the logic of
naming the columns the same?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
