abstractdog commented on code in PR #6443:
URL: https://github.com/apache/hive/pull/6443#discussion_r3220212053
##########
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:
I feel we should have a single constructor, so simply change the existing one
regarding complexity: right, I don't know this codepath in general, so I'm
not sure either if we need to be extra cautious about storing `colName`
separately, but there is a good way to handle this complexity: refactor it to a
new class :D I mean, really, what about `FieldSchemas`, a simple container to
handle this mess, and it can act as `FieldSchemas.getColName()` and
`FieldSchemas.getColType()`: might seem overengineering, but already has some
value, and you can introduce it early, so instead of:
```
private static List<FieldSchema> getStatsEligibleFieldSchemas(Table tbl) {
List<FieldSchema> result = new ArrayList<>();
...
return result;
}
```
you can do:
```
private static List<FieldSchema> getStatsEligibleFieldSchemas(Table tbl) {
List<FieldSchema> result = new ArrayList<>();
...
return new FieldSchemas(result);
}
```
this can already eliminate some functions from the already huge `Utilities`
class
--
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]
