[GitHub] [flink] wuchong commented on a diff in pull request #19017: [FLINK-26542][hive] Hive dialect supports queries with multiple same columns in select

[GitBox <gi...@apache.org>]

wuchong commented on code in PR #19017:
URL: https://github.com/apache/flink/pull/19017#discussion_r934110184


##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/copy/HiveParserRowResolver.java:
##########
@@ -170,6 +206,12 @@ public boolean hasTableAlias(String tabAlias) {
     public ColumnInfo get(String tabAlias, String colAlias) throws SemanticException {
         ColumnInfo ret = null;
 
+        if (!isExprResolver && isAmbiguousReference(tabAlias, colAlias)) {
+            String tableName = tabAlias != null ? tabAlias : "";
+            String fullQualifiedName = tableName + "." + colAlias;

Review Comment:
   If `tableName` is null, the qualified name is `.colAlias`. Would be better to eliminate the `.` if tableName is null?



##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/copy/HiveParserRowResolver.java:
##########
@@ -109,6 +133,17 @@ public void put(String tabAlias, String colAlias, ColumnInfo colInfo) {
         }
     }
 
+    private void keepAmbiguousInfo(String colAlias, String tabAlias) {
+        // we keep track of duplicate <tab alias, col alias> so that get can check
+        // for ambiguity
+        LinkedHashMap<String, String> colAliases = ambiguousColumns.get(tabAlias);
+        if (colAliases == null) {
+            colAliases = new LinkedHashMap<>();
+            ambiguousColumns.put(tabAlias, colAliases);
+        }

Review Comment:
   Can be replaced with single method call.
   
   ```java
   LinkedHashMap<String, String> colAliases = ambiguousColumns.computeIfAbsent(tabAlias, k -> new LinkedHashMap<>());
   ```



##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/planner/delegation/hive/copy/HiveParserRowResolver.java:
##########
@@ -109,6 +133,17 @@ public void put(String tabAlias, String colAlias, ColumnInfo colInfo) {
         }
     }
 
+    private void keepAmbiguousInfo(String colAlias, String tabAlias) {

Review Comment:
   Is it guaranteed that the `colAlias` and `tabAlias` are lowercase? Should we lower again in the method? 



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org