abstractdog commented on code in PR #6443:
URL: https://github.com/apache/hive/pull/6443#discussion_r3217901670
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java:
##########
@@ -103,37 +104,32 @@ private boolean shouldRewrite(ASTNode tree) {
}
/**
- * Get the names of the columns that support column statistics.
+ * Get the Field Schemas of the columns that support column statistics.
*/
- private static List<String> getColumnNamesSupportingStats(Table tbl) {
- List<String> colNames = new ArrayList<>();
+ private static List<FieldSchema> getStatsEligibleFieldSchemas(Table tbl) {
+ List<FieldSchema> result = new ArrayList<>();
for (FieldSchema col : tbl.getCols()) {
String type = col.getType();
TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString(type);
boolean isSupported =
ColumnStatsAutoGatherContext.isColumnSupported(typeInfo.getCategory(), () ->
typeInfo);
if (isSupported) {
- colNames.add(col.getName());
+ result.add(col);
}
}
- return colNames;
+ return result;
}
- private List<String> getColumnName(ASTNode tree) throws SemanticException {
-
- switch (tree.getChildCount()) {
- case 2:
- return getColumnNamesSupportingStats(tbl);
- case 3:
- int numCols = tree.getChild(2).getChildCount();
- List<String> colName = new ArrayList<>(numCols);
- for (int i = 0; i < numCols; i++) {
- colName.add(getUnescapedName((ASTNode) tree.getChild(2).getChild(i)));
- }
- return colName;
- default:
- throw new SemanticException("Internal error. Expected number of children
of ASTNode to be"
- + " either 2 or 3. Found : " + tree.getChildCount());
+ private List<String> getExplicitColumnNamesFromAst(ASTNode tree) throws
SemanticException {
+ if (tree.getChildCount() != 3) {
Review Comment:
@tanishq-chugh, @thomasrebele : `if (tree.getChildCount() != 3)` looks like
black magic to me at first sight :) , can you help the future code readers with
a small code comment here regarding why only 3 is correct? I'm going to accept
it, just need an explanation
The below exception message "Expected number of children of ASTNode should
be 3" doesn't help at all
--
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]
