abstractdog commented on code in PR #6443:
URL: https://github.com/apache/hive/pull/6443#discussion_r3219776096
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1428,12 +1427,16 @@ public void setTableName(String tableName) {
this.tableName = tableName;
}
- public List<String> getColName() {
- return colName;
+ public List<FieldSchema> getColumnSchemas() {
+ return columnSchemas;
+ }
+
+ public void setColumnSchemas(List<FieldSchema> columnSchemas) {
+ this.columnSchemas = columnSchemas;
}
- public void setColName(List<String> colName) {
- this.colName = colName;
+ public List<String> getColName() {
+ return columnSchemas == null ? null :
Utilities.getColumnNamesFromFieldSchema(columnSchemas);
Review Comment:
if by static code analysis we can be sure that `getColName` is not called
e.g. 100 times during compilation, this is going to be fine, I'm inclined to
accept your analysis here
my point was only to completely remove the separate `colName` and `colType`
variables from method signatures where it's possible, and only have them as
separate variables where it's really needed (e.g. where EXPLAIN plan is used),
e.g.:
```
@Explain(displayName = "Columns")
public List<String> getColName() {
return colName;
}
```
so I can see that TaskCompiler uses them separately only for passing them to
ColumnStatsDesc, which just complicates code, so what about passing schemas to
ColumnStatsDesc, and we can resolve names/types there like:
```
@Explain(displayName = "Column Types")
public List<String> getColType() {
return ...the logic parsing types for schemas...;
}
```
--
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]
