IMPALA-7867 (Part 1): Expose List in TreeNode, parser When using Java collections, a common Java best practice is to expose the collection interface, but hide the implementation choice. This pattern allows us to start with a generic implementation (an ArrayList, say), but evolve to a more specific implementation to achieve certain goals (a LinkedList or ImmutableList, say.)
For whatever reason, the Impala FE code exposes ArrayList, HashMap and other implementation choices as variable types and in method signatures. Also, since Java 7, the preferred way to create an array is new ArrayList<>() Replaced older forms: new ArrayList<foo>() // Pre-Java 7 Lists.newArrayList() // Guava form, pre-Java 7 This ticket cleans up two files, and their dependencies: * TreeNode (the root of all parser nodes) * sql-parser.cup (the code which creates the parser nodes) Many other uses exist, and will be submitted as separate patches to keep patches small. In TreeNode, also cleaned up some of the generic expresions, which caused dependencies to change in order to be more type-safe. Tests: This is purely a refactoring, no functionality changed. Ran the FE unit tests to verify no regressions. Change-Id: Iebab7dccdb4b2fa0b5ca812beab0e8bdba39f539 Reviewed-on: http://gerrit.cloudera.org:8080/11954 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/12dc29e5 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/12dc29e5 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/12dc29e5 Branch: refs/heads/master Commit: 12dc29e5ee1ec03bbc1556eca4539645e2b9bbf1 Parents: fcfabe0 Author: Paul Rogers <[email protected]> Authored: Sun Nov 18 15:13:34 2018 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Nov 27 00:04:57 2018 +0000 ---------------------------------------------------------------------- fe/src/main/cup/sql-parser.cup | 81 ++++++++++---------- .../apache/impala/analysis/AnalyticInfo.java | 8 +- .../impala/analysis/DescribeTableStmt.java | 6 +- .../java/org/apache/impala/analysis/Expr.java | 2 +- .../apache/impala/analysis/FunctionName.java | 10 +-- .../org/apache/impala/analysis/QueryStmt.java | 4 +- .../org/apache/impala/analysis/SelectStmt.java | 10 +-- .../org/apache/impala/analysis/SlotRef.java | 3 +- .../org/apache/impala/analysis/UnionStmt.java | 2 +- .../org/apache/impala/analysis/ValuesStmt.java | 3 +- .../org/apache/impala/analysis/WithClause.java | 5 +- .../org/apache/impala/catalog/StructType.java | 7 +- .../java/org/apache/impala/common/TreeNode.java | 40 +++++++--- .../apache/impala/planner/AnalyticPlanner.java | 4 +- .../org/apache/impala/planner/HdfsScanNode.java | 2 +- .../java/org/apache/impala/planner/Planner.java | 4 +- .../org/apache/impala/service/Frontend.java | 4 +- 17 files changed, 103 insertions(+), 92 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/cup/sql-parser.cup ---------------------------------------------------------------------- diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index 50418d0..12fd55d 100644 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -20,7 +20,6 @@ package org.apache.impala.analysis; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import java.math.BigDecimal; -import java.math.BigInteger; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -44,7 +43,6 @@ import org.apache.impala.catalog.StructField; import org.apache.impala.catalog.StructType; import org.apache.impala.catalog.Type; import org.apache.impala.catalog.View; -import org.apache.impala.common.AnalysisException; import org.apache.impala.common.Pair; import org.apache.impala.thrift.TCatalogObjectType; import org.apache.impala.thrift.TDescribeOutputStyle; @@ -67,7 +65,7 @@ parser code {: // list of expected tokens ids from current parsing state // for generating syntax error message - private final List<Integer> expectedTokenIds_ = new ArrayList<Integer>(); + private final List<Integer> expectedTokenIds_ = new ArrayList<>(); // Currently used to tell if it's decimal V1 or V2 mode. // TODO: remove when V1 code is dropped. @@ -110,6 +108,7 @@ parser code {: } // override to save error token + @Override public void syntax_error(java_cup.runtime.Symbol token) { errorToken_ = token; @@ -365,10 +364,10 @@ nonterminal SelectList select_list; nonterminal SelectListItem select_list_item; nonterminal SelectListItem star_expr; nonterminal Expr expr, non_pred_expr, arithmetic_expr, timestamp_arithmetic_expr; -nonterminal ArrayList<Expr> expr_list; +nonterminal List<Expr> expr_list; nonterminal String alias_clause; -nonterminal ArrayList<String> ident_list, primary_keys; -nonterminal ArrayList<String> opt_ident_list, opt_sort_cols; +nonterminal List<String> ident_list, primary_keys; +nonterminal List<String> opt_ident_list, opt_sort_cols; nonterminal TableName table_name; nonterminal ColumnName column_name; nonterminal FunctionName function_name; @@ -376,9 +375,9 @@ nonterminal Expr where_clause; nonterminal Expr predicate, bool_test_expr; nonterminal Predicate between_predicate, comparison_predicate, compound_predicate, in_predicate, like_predicate, exists_predicate; -nonterminal ArrayList<Expr> group_by_clause, opt_partition_by_clause; +nonterminal List<Expr> group_by_clause, opt_partition_by_clause; nonterminal Expr having_clause; -nonterminal ArrayList<OrderByElement> order_by_elements, opt_order_by_clause; +nonterminal List<OrderByElement> order_by_elements, opt_order_by_clause; nonterminal OrderByElement order_by_element; nonterminal Boolean opt_order_param; nonterminal Boolean opt_nulls_order_param; @@ -393,15 +392,15 @@ nonterminal AnalyticWindow.Boundary window_boundary; nonterminal LiteralExpr literal; nonterminal NumericLiteral numeric_literal; nonterminal CaseExpr case_expr; -nonterminal ArrayList<CaseWhenClause> case_when_clause_list; +nonterminal List<CaseWhenClause> case_when_clause_list; nonterminal FunctionParams function_params; -nonterminal ArrayList<String> dotted_path; +nonterminal List<String> dotted_path; nonterminal SlotRef slot_ref; nonterminal FromClause from_clause; -nonterminal ArrayList<TableRef> table_ref_list; +nonterminal List<TableRef> table_ref_list; nonterminal TableSampleClause opt_tablesample; nonterminal WithClause opt_with_clause; -nonterminal ArrayList<View> with_view_def_list; +nonterminal List<View> with_view_def_list; nonterminal View with_view_def; nonterminal TableRef table_ref; nonterminal Subquery subquery; @@ -415,7 +414,7 @@ nonterminal Expr sign_chain_expr; nonterminal InsertStmt insert_stmt, upsert_stmt; nonterminal UpdateStmt update_stmt; nonterminal DeleteStmt delete_stmt; -nonterminal ArrayList<Pair<SlotRef, Expr>> update_set_expr_list; +nonterminal List<Pair<SlotRef, Expr>> update_set_expr_list; nonterminal StatementBase explain_stmt; // Optional partition spec nonterminal PartitionSpec opt_partition_spec; @@ -425,9 +424,9 @@ nonterminal PartitionSpec partition_spec; nonterminal PartitionSet opt_partition_set; // Required partition set nonterminal PartitionSet partition_set; -nonterminal ArrayList<PartitionKeyValue> partition_clause; -nonterminal ArrayList<PartitionKeyValue> static_partition_key_value_list; -nonterminal ArrayList<PartitionKeyValue> partition_key_value_list; +nonterminal List<PartitionKeyValue> partition_clause; +nonterminal List<PartitionKeyValue> static_partition_key_value_list; +nonterminal List<PartitionKeyValue> partition_key_value_list; nonterminal PartitionKeyValue partition_key_value; nonterminal PartitionKeyValue static_partition_key_value; nonterminal Qualifier union_op; @@ -462,13 +461,13 @@ nonterminal List<RangePartition> range_params_list; nonterminal RangePartition range_param; nonterminal Pair<List<Expr>, Boolean> opt_lower_range_val, opt_upper_range_val; -nonterminal ArrayList<KuduPartitionParam> hash_partition_param_list; -nonterminal ArrayList<KuduPartitionParam> partition_param_list; +nonterminal List<KuduPartitionParam> hash_partition_param_list; +nonterminal List<KuduPartitionParam> partition_param_list; nonterminal KuduPartitionParam range_partition_param; nonterminal ColumnDef column_def, view_column_def; -nonterminal ArrayList<ColumnDef> column_def_list, partition_column_defs, +nonterminal List<ColumnDef> column_def_list, partition_column_defs, view_column_def_list, view_column_defs; -nonterminal ArrayList<StructField> struct_field_def_list; +nonterminal List<StructField> struct_field_def_list; // Options for DDL commands - CREATE/DROP/ALTER nonterminal HdfsCachingOp cache_op_val, opt_cache_op_val; nonterminal BigDecimal opt_cache_op_replication; @@ -839,7 +838,7 @@ update_stmt ::= update_set_expr_list ::= slot_ref:slot EQUAL expr:e {: - ArrayList<Pair<SlotRef, Expr>> tmp = + List<Pair<SlotRef, Expr>> tmp = Lists.newArrayList(new Pair<SlotRef, Expr>(slot, e)); RESULT = tmp; :} @@ -906,7 +905,7 @@ opt_ident_list ::= ident_list:ident {: RESULT = ident; :} | /* empty */ - {: RESULT = Lists.newArrayList(); :} + {: RESULT = new ArrayList<>(); :} ; opt_kw_table ::= @@ -1442,7 +1441,7 @@ hash_partition_param ::= {: RESULT = KuduPartitionParam.createHashParam(cols, numPartitions.intValue()); :} | KW_HASH KW_PARTITIONS INTEGER_LITERAL:numPartitions {: - RESULT = KuduPartitionParam.createHashParam(Lists.<String>newArrayList(), + RESULT = KuduPartitionParam.createHashParam(new ArrayList<>(), numPartitions.intValue()); :} ; @@ -1719,7 +1718,7 @@ properties_map ::= column_def_list ::= column_def:col_def {: - ArrayList<ColumnDef> list = Lists.newArrayList(); + List<ColumnDef> list = new ArrayList<>(); list.add(col_def); RESULT = list; :} @@ -1901,7 +1900,7 @@ view_column_defs ::= view_column_def_list ::= view_column_def:col_def {: - ArrayList<ColumnDef> list = new ArrayList<ColumnDef>(); + List<ColumnDef> list = new ArrayList<>(); list.add(col_def); RESULT = list; :} @@ -2025,7 +2024,7 @@ partition_clause ::= partition_key_value_list ::= partition_key_value:item {: - ArrayList<PartitionKeyValue> list = new ArrayList<PartitionKeyValue>(); + List<PartitionKeyValue> list = new ArrayList<>(); list.add(item); RESULT = list; :} @@ -2071,7 +2070,7 @@ opt_partition_spec ::= static_partition_key_value_list ::= static_partition_key_value:item {: - ArrayList<PartitionKeyValue> list = new ArrayList<PartitionKeyValue>(); + List<PartitionKeyValue> list = new ArrayList<>(); list.add(item); RESULT = list; :} @@ -2241,7 +2240,7 @@ with_view_def ::= with_view_def_list ::= with_view_def:v {: - ArrayList<View> list = new ArrayList<View>(); + List<View> list = new ArrayList<>(); list.add(v); RESULT = list; :} @@ -2307,7 +2306,7 @@ union_operand ::= union_operand_list ::= union_operand:operand {: - List<UnionOperand> operands = new ArrayList<UnionOperand>(); + List<UnionOperand> operands = new ArrayList<>(); operands.add(new UnionOperand(operand, null)); RESULT = operands; :} @@ -2345,7 +2344,7 @@ values_stmt ::= values_operand_list ::= LPAREN select_list:selectList RPAREN {: - List<UnionOperand> operands = new ArrayList<UnionOperand>(); + List<UnionOperand> operands = new ArrayList<>(); operands.add(new UnionOperand( new SelectStmt(selectList, null, null, null, null, null, null), null)); RESULT = operands; @@ -2608,7 +2607,7 @@ from_clause ::= table_ref_list ::= table_ref:table opt_plan_hints:hints {: - ArrayList<TableRef> list = new ArrayList<TableRef>(); + List<TableRef> list = new ArrayList<>(); table.setTableHints(hints); list.add(table); RESULT = list; @@ -2703,7 +2702,7 @@ opt_plan_hints ::= plan_hints:hints {: RESULT = hints; :} | /* empty */ - {: RESULT = Lists.newArrayList(); :} + {: RESULT = new ArrayList<>(); :} ; plan_hints ::= @@ -2731,7 +2730,7 @@ plan_hint ::= plan_hint_list ::= plan_hint:hint {: - ArrayList<PlanHint> hints = Lists.newArrayList(hint); + List<PlanHint> hints = Lists.newArrayList(hint); RESULT = hints; :} | plan_hint_list:hints COMMA plan_hint:hint @@ -2754,7 +2753,7 @@ opt_tablesample ::= ident_list ::= ident_or_default:ident {: - ArrayList<String> list = new ArrayList<String>(); + List<String> list = new ArrayList<>(); list.add(ident); RESULT = list; :} @@ -2768,7 +2767,7 @@ ident_list ::= expr_list ::= expr:e {: - ArrayList<Expr> list = new ArrayList<Expr>(); + List<Expr> list = new ArrayList<>(); list.add(e); RESULT = list; :} @@ -2810,7 +2809,7 @@ opt_order_by_clause ::= order_by_elements ::= order_by_element:e {: - ArrayList<OrderByElement> list = new ArrayList<OrderByElement>(); + List<OrderByElement> list = new ArrayList<>(); list.add(e); RESULT = list; :} @@ -2891,7 +2890,7 @@ case_expr ::= case_when_clause_list ::= KW_WHEN expr:whenExpr KW_THEN expr:thenExpr {: - ArrayList<CaseWhenClause> list = new ArrayList<CaseWhenClause>(); + List<CaseWhenClause> list = new ArrayList<>(); list.add(new CaseWhenClause(whenExpr, thenExpr)); RESULT = list; :} @@ -2985,7 +2984,7 @@ function_call_expr ::= function_name:fn_name LPAREN RPAREN {: RESULT = FunctionCallExpr.createExpr( - fn_name, new FunctionParams(new ArrayList<Expr>()), parser.getQueryOptions()); + fn_name, new FunctionParams(new ArrayList<>()), parser.getQueryOptions()); :} | function_name:fn_name LPAREN function_params:params RPAREN {: RESULT = FunctionCallExpr.createExpr(fn_name, params, parser.getQueryOptions()); :} @@ -3112,7 +3111,7 @@ timestamp_arithmetic_expr ::= // Report parsing failure on keyword interval. parser.parseError("interval", SqlParserSymbols.KW_INTERVAL); } - ArrayList<String> fnNamePath = functionName.getFnNamePath(); + List<String> fnNamePath = functionName.getFnNamePath(); if (fnNamePath.size() > 1) { // This production should not accept fully qualified function names throw new Exception("interval should not be qualified by database name"); @@ -3318,7 +3317,7 @@ slot_ref ::= dotted_path ::= ident_or_default:ident {: - ArrayList<String> list = new ArrayList<String>(); + List<String> list = new ArrayList<>(); list.add(ident); RESULT = list; :} @@ -3390,7 +3389,7 @@ struct_field_def ::= struct_field_def_list ::= struct_field_def:field_def {: - ArrayList<StructField> list = new ArrayList<StructField>(); + List<StructField> list = new ArrayList<>(); list.add(field_def); RESULT = list; :} http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java b/fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java index 26ce3f8..09d72d9 100644 --- a/fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java +++ b/fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java @@ -38,7 +38,7 @@ public class AnalyticInfo extends AggregateInfoBase { // All unique analytic exprs of a select block. Used to populate // super.aggregateExprs_ based on AnalyticExpr.getFnCall() for each analytic expr // in this list. - private final ArrayList<Expr> analyticExprs_; + private final ArrayList<AnalyticExpr> analyticExprs_; // Intersection of the partition exps of all the analytic functions. private final List<Expr> commonPartitionExprs_; @@ -46,7 +46,7 @@ public class AnalyticInfo extends AggregateInfoBase { // map from analyticExprs_ to their corresponding analytic tuple slotrefs private final ExprSubstitutionMap analyticTupleSmap_; - private AnalyticInfo(ArrayList<Expr> analyticExprs) { + private AnalyticInfo(ArrayList<AnalyticExpr> analyticExprs) { super(new ArrayList<Expr>(), new ArrayList<FunctionCallExpr>()); analyticExprs_ = Expr.cloneList(analyticExprs); // Extract the analytic function calls for each analytic expr. @@ -68,7 +68,7 @@ public class AnalyticInfo extends AggregateInfoBase { commonPartitionExprs_ = Expr.cloneList(other.commonPartitionExprs_); } - public ArrayList<Expr> getAnalyticExprs() { return analyticExprs_; } + public ArrayList<AnalyticExpr> getAnalyticExprs() { return analyticExprs_; } public ExprSubstitutionMap getSmap() { return analyticTupleSmap_; } public List<Expr> getCommonPartitionExprs() { return commonPartitionExprs_; } @@ -77,7 +77,7 @@ public class AnalyticInfo extends AggregateInfoBase { * smaps. */ static public AnalyticInfo create( - ArrayList<Expr> analyticExprs, Analyzer analyzer) { + ArrayList<AnalyticExpr> analyticExprs, Analyzer analyzer) { Preconditions.checkState(analyticExprs != null && !analyticExprs.isEmpty()); Expr.removeDuplicates(analyticExprs); AnalyticInfo result = new AnalyticInfo(analyticExprs); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java index 65d9c75..1e7c645 100644 --- a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java @@ -17,13 +17,11 @@ package org.apache.impala.analysis; -import java.util.ArrayList; import java.util.List; import org.apache.commons.lang3.StringUtils; import org.apache.impala.analysis.Path.PathType; import org.apache.impala.authorization.Privilege; -import org.apache.impala.authorization.PrivilegeRequest; import org.apache.impala.authorization.PrivilegeRequestBuilder; import org.apache.impala.catalog.FeTable; import org.apache.impala.catalog.StructType; @@ -54,7 +52,7 @@ public class DescribeTableStmt extends StatementBase { private final TDescribeOutputStyle outputStyle_; /// "."-separated path from the describe statement. - private final ArrayList<String> rawPath_; + private final List<String> rawPath_; /// The resolved path to describe, set after analysis. private Path path_; @@ -66,7 +64,7 @@ public class DescribeTableStmt extends StatementBase { /// Only set when describing a path to a nested collection. private StructType resultStruct_; - public DescribeTableStmt(ArrayList<String> rawPath, TDescribeOutputStyle outputStyle) { + public DescribeTableStmt(List<String> rawPath, TDescribeOutputStyle outputStyle) { Preconditions.checkNotNull(rawPath); Preconditions.checkArgument(!rawPath.isEmpty()); rawPath_ = rawPath; http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/Expr.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java index a17877e..6141a11 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Expr.java +++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java @@ -1031,7 +1031,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl return this; } - public static ArrayList<Expr> resetList(ArrayList<Expr> l) { + public static List<Expr> resetList(List<Expr> l) { for (int i = 0; i < l.size(); ++i) { l.set(i, l.get(i).reset()); } http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/FunctionName.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionName.java b/fe/src/main/java/org/apache/impala/analysis/FunctionName.java index aeaa6e1..f3b3d5d 100644 --- a/fe/src/main/java/org/apache/impala/analysis/FunctionName.java +++ b/fe/src/main/java/org/apache/impala/analysis/FunctionName.java @@ -17,13 +17,13 @@ package org.apache.impala.analysis; -import java.util.ArrayList; +import java.util.List; import org.apache.impala.catalog.BuiltinsDb; -import org.apache.impala.catalog.Catalog; import org.apache.impala.catalog.Db; import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TFunctionName; + import com.google.common.base.Joiner; import com.google.common.base.Preconditions; @@ -33,7 +33,7 @@ import com.google.common.base.Preconditions; */ public class FunctionName { // Only set for parsed function names. - private final ArrayList<String> fnNamePath_; + private final List<String> fnNamePath_; // Set/validated during analysis. private String db_; @@ -45,7 +45,7 @@ public class FunctionName { * C'tor for parsed function names. The function names could be invalid. The validity * is checked during analysis. */ - public FunctionName(ArrayList<String> fnNamePath) { + public FunctionName(List<String> fnNamePath) { fnNamePath_ = fnNamePath; } @@ -75,7 +75,7 @@ public class FunctionName { public String getFunction() { return fn_; } public boolean isFullyQualified() { return db_ != null; } public boolean isBuiltin() { return isBuiltin_; } - public ArrayList<String> getFnNamePath() { return fnNamePath_; } + public List<String> getFnNamePath() { return fnNamePath_; } @Override public String toString() { http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java index a561cbd..50d32c1 100644 --- a/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java @@ -49,7 +49,7 @@ public abstract class QueryStmt extends StatementBase { protected WithClause withClause_; - protected ArrayList<OrderByElement> orderByElements_; + protected List<OrderByElement> orderByElements_; protected LimitElement limitElement_; // For a select statment: @@ -98,7 +98,7 @@ public abstract class QueryStmt extends StatementBase { // returns a single row. protected boolean isRuntimeScalar_ = false; - QueryStmt(ArrayList<OrderByElement> orderByElements, LimitElement limitElement) { + QueryStmt(List<OrderByElement> orderByElements, LimitElement limitElement) { orderByElements_ = orderByElements; sortInfo_ = null; limitElement_ = limitElement == null ? new LimitElement(null, null) : limitElement; http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java index 449a16e..f60396a 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java @@ -53,10 +53,10 @@ public class SelectStmt extends QueryStmt { // BEGIN: Members that need to be reset() protected SelectList selectList_; - protected final ArrayList<String> colLabels_; // lower case column labels + protected final List<String> colLabels_; // lower case column labels protected final FromClause fromClause_; protected Expr whereClause_; - protected ArrayList<Expr> groupingExprs_; + protected List<Expr> groupingExprs_; protected Expr havingClause_; // original having clause // havingClause with aliases and agg output resolved @@ -81,8 +81,8 @@ public class SelectStmt extends QueryStmt { SelectStmt(SelectList selectList, FromClause fromClause, - Expr wherePredicate, ArrayList<Expr> groupingExprs, - Expr havingPredicate, ArrayList<OrderByElement> orderByElements, + Expr wherePredicate, List<Expr> groupingExprs, + Expr havingPredicate, List<OrderByElement> orderByElements, LimitElement limitElement) { super(orderByElements, limitElement); selectList_ = selectList; @@ -840,7 +840,7 @@ public class SelectStmt extends QueryStmt { private void createAnalyticInfo() throws AnalysisException { // collect AnalyticExprs from the SELECT and ORDER BY clauses - ArrayList<Expr> analyticExprs = Lists.newArrayList(); + ArrayList<AnalyticExpr> analyticExprs = Lists.newArrayList(); TreeNode.collect(resultExprs_, AnalyticExpr.class, analyticExprs); if (sortInfo_ != null) { TreeNode.collect(sortInfo_.getSortExprs(), AnalyticExpr.class, http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/SlotRef.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java index 8860871..bd3d543 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java +++ b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java @@ -17,7 +17,6 @@ package org.apache.impala.analysis; -import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -41,7 +40,7 @@ public class SlotRef extends Expr { // Results of analysis. private SlotDescriptor desc_; - public SlotRef(ArrayList<String> rawPath) { + public SlotRef(List<String> rawPath) { super(); rawPath_ = rawPath; label_ = ToSqlUtils.getPathSql(rawPath_); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java index 8e73178..8aafc67 100644 --- a/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java @@ -162,7 +162,7 @@ public class UnionStmt extends QueryStmt { ///////////////////////////////////////// public UnionStmt(List<UnionOperand> operands, - ArrayList<OrderByElement> orderByElements, LimitElement limitElement) { + List<OrderByElement> orderByElements, LimitElement limitElement) { super(orderByElements, limitElement); Preconditions.checkNotNull(operands); Preconditions.checkState(operands.size() > 0); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java b/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java index 8bce93a..06a1c67 100644 --- a/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java @@ -17,7 +17,6 @@ package org.apache.impala.analysis; -import java.util.ArrayList; import java.util.List; import com.google.common.base.Preconditions; @@ -34,7 +33,7 @@ import static org.apache.impala.analysis.ToSqlOptions.DEFAULT; public class ValuesStmt extends UnionStmt { public ValuesStmt(List<UnionOperand> operands, - ArrayList<OrderByElement> orderByElements, LimitElement limitElement) { + List<OrderByElement> orderByElements, LimitElement limitElement) { super(operands, orderByElements, limitElement); } http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/analysis/WithClause.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/WithClause.java b/fe/src/main/java/org/apache/impala/analysis/WithClause.java index f0548c4..2e4d590 100644 --- a/fe/src/main/java/org/apache/impala/analysis/WithClause.java +++ b/fe/src/main/java/org/apache/impala/analysis/WithClause.java @@ -17,7 +17,6 @@ package org.apache.impala.analysis; -import java.util.ArrayList; import java.util.List; import org.apache.impala.authorization.PrivilegeRequest; @@ -53,12 +52,12 @@ public class WithClause implements ParseNode { ///////////////////////////////////////// // BEGIN: Members that need to be reset() - private final ArrayList<View> views_; + private final List<View> views_; // END: Members that need to be reset() ///////////////////////////////////////// - public WithClause(ArrayList<View> views) { + public WithClause(List<View> views) { Preconditions.checkNotNull(views); Preconditions.checkState(!views.isEmpty()); views_ = views; http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/catalog/StructType.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/StructType.java b/fe/src/main/java/org/apache/impala/catalog/StructType.java index 54507ec..77d4648 100644 --- a/fe/src/main/java/org/apache/impala/catalog/StructType.java +++ b/fe/src/main/java/org/apache/impala/catalog/StructType.java @@ -19,6 +19,7 @@ package org.apache.impala.catalog; import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -36,9 +37,9 @@ import com.google.common.collect.Maps; */ public class StructType extends Type { private final HashMap<String, StructField> fieldMap_ = Maps.newHashMap(); - private final ArrayList<StructField> fields_; + private final List<StructField> fields_; - public StructType(ArrayList<StructField> fields) { + public StructType(List<StructField> fields) { Preconditions.checkNotNull(fields); fields_ = fields; for (int i = 0; i < fields_.size(); ++i) { @@ -74,7 +75,7 @@ public class StructType extends Type { fieldMap_.put(field.getName().toLowerCase(), field); } - public ArrayList<StructField> getFields() { return fields_; } + public List<StructField> getFields() { return fields_; } public StructField getField(String fieldName) { return fieldMap_.get(fieldName.toLowerCase()); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/common/TreeNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/common/TreeNode.java b/fe/src/main/java/org/apache/impala/common/TreeNode.java index b89f133..c5e7a87 100644 --- a/fe/src/main/java/org/apache/impala/common/TreeNode.java +++ b/fe/src/main/java/org/apache/impala/common/TreeNode.java @@ -28,7 +28,7 @@ import com.google.common.base.Predicate; * Generic tree structure. Only concrete subclasses of this can be instantiated. */ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { - protected ArrayList<NodeType> children_ = new ArrayList<NodeType>(); + protected List<NodeType> children_ = new ArrayList<>(); public NodeType getChild(int i) { return hasChild(i) ? children_.get(i) : null; @@ -48,20 +48,25 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { public boolean hasChild(int i) { return children_.size() > i; } public void setChild(int index, NodeType n) { children_.set(index, n); } - public ArrayList<NodeType> getChildren() { return children_; } + public List<NodeType> getChildren() { return children_; } /** * Return list of all nodes of the tree rooted at 'this', obtained * through pre-order traversal. + * + * Warning: this method is type-unsafe: it returns a list of nodes + * of the requested type, but does not verify that the actual + * nodes are indeed of that type. */ - public <C extends TreeNode<NodeType>> ArrayList<C> getNodesPreOrder() { - ArrayList<C> result = new ArrayList<C>(); + public <C extends TreeNode<NodeType>> List<C> getNodesPreOrder() { + List<C> result = new ArrayList<>(); getNodesPreOrderAux(result); return result; } + @SuppressWarnings("unchecked") protected <C extends TreeNode<NodeType>> void getNodesPreOrderAux( - ArrayList<C> result) { + List<C> result) { result.add((C) this); for (NodeType child: children_) child.getNodesPreOrderAux(result); } @@ -69,15 +74,20 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { /** * Return list of all nodes of the tree rooted at 'this', obtained * through post-order traversal. + * + * Warning: this method is type-unsafe: it returns a list of nodes + * of the requested type, but does not verify that the actual + * nodes are indeed of that type. */ - public <C extends TreeNode<NodeType>> ArrayList<C> getNodesPostOrder() { - ArrayList<C> result = new ArrayList<C>(); + public <C extends TreeNode<NodeType>> List<C> getNodesPostOrder() { + List<C> result = new ArrayList<>(); getNodesPostOrderAux(result); return result; } + @SuppressWarnings("unchecked") protected <C extends TreeNode<NodeType>> void getNodesPostOrderAux( - ArrayList<C> result) { + List<C> result) { for (NodeType child: children_) child.getNodesPostOrderAux(result); result.add((C) this); } @@ -97,6 +107,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { * This node is checked first, followed by its children in order. If the node * itself matches, the children are skipped. */ + @SuppressWarnings("unchecked") public <C extends TreeNode<NodeType>, D extends C> void collect( Predicate<? super C> predicate, Collection<D> matches) { // TODO: the semantics of this function are very strange. contains() @@ -116,8 +127,9 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { * This node is checked first, followed by its children in order. If the node * itself is of class 'cl', the children are skipped. */ + @SuppressWarnings("unchecked") public <C extends TreeNode<NodeType>, D extends C> void collect( - Class cl, Collection<D> matches) { + Class<D> cl, Collection<D> matches) { if (cl.equals(getClass())) { matches.add((D) this); return; @@ -130,6 +142,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { * This node is checked first, followed by its children in order. All nodes * that match in the subtree are added. */ + @SuppressWarnings("unchecked") public <C extends TreeNode<NodeType>, D extends C> void collectAll( Predicate<? super C> predicate, List<D> matches) { if (predicate.apply((C) this)) matches.add((D) this); @@ -150,13 +163,14 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { * into 'matches' */ public static <C extends TreeNode<C>, D extends C> void collect( - Collection<C> nodeList, Class cl, Collection<D> matches) { + Collection<C> nodeList, Class<D> cl, Collection<D> matches) { for (C node: nodeList) node.collect(cl, matches); } /** * Return true if this node or any of its children satisfy 'predicate'. */ + @SuppressWarnings("unchecked") public <C extends TreeNode<NodeType>> boolean contains( Predicate<? super C> predicate) { if (predicate.apply((C) this)) return true; @@ -167,7 +181,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { /** * Return true if this node or any of its children is an instance of class 'cl'. */ - public boolean contains(Class cl) { + public <C extends TreeNode<NodeType>> boolean contains(Class<C> cl) { if (cl.equals(getClass())) return true; for (NodeType child: children_) if (child.contains(cl)) return true; return false; @@ -196,7 +210,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { * Return true if any node in nodeList contains children of class cl. */ public static <C extends TreeNode<C>> boolean contains( - List<C> nodeList, Class cl) { + List<C> nodeList, Class<? extends C> cl) { for (C node: nodeList) if (node.contains(cl)) return true; return false; } @@ -204,6 +218,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { /** * Returns the first node/child of class cl (depth-first traversal). */ + @SuppressWarnings("unchecked") public <C extends NodeType> C findFirstOf(Class<C> cl) { if (this.getClass().equals(cl)) return (C) this; for (NodeType child: children_) { @@ -216,6 +231,7 @@ public abstract class TreeNode<NodeType extends TreeNode<NodeType>> { /** * Visitor pattern accept method */ + @SuppressWarnings("unchecked") public <C extends TreeNode<NodeType>> void accept(Visitor<C> visitor) { visitor.visit((C) this); for (NodeType p: children_) p.accept(visitor); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java b/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java index 3685bf4..13f8d44 100644 --- a/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java +++ b/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java @@ -292,7 +292,7 @@ public class AnalyticPlanner { // we materialize those rhs TupleIsNullPredicates, which are then substituted // by a SlotRef into the sort's tuple in ancestor nodes (IMPALA-1519). if (inputSmap != null) { - List<Expr> tupleIsNullPreds = Lists.newArrayList(); + List<TupleIsNullPredicate> tupleIsNullPreds = Lists.newArrayList(); for (Expr rhsExpr: inputSmap.getRhs()) { // Ignore substitutions that are irrelevant at this plan node and its ancestors. if (!rhsExpr.isBoundByTupleIds(input.getTupleIds())) continue; @@ -591,7 +591,7 @@ public class AnalyticPlanner { * Extract a minimal set of WindowGroups from analyticExprs. */ private List<WindowGroup> collectWindowGroups() { - List<Expr> analyticExprs = analyticInfo_.getAnalyticExprs(); + List<AnalyticExpr> analyticExprs = analyticInfo_.getAnalyticExprs(); List<WindowGroup> groups = Lists.newArrayList(); for (int i = 0; i < analyticExprs.size(); ++i) { AnalyticExpr analyticExpr = (AnalyticExpr) analyticExprs.get(i); http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java index 950382c..1f4c2bd 100644 --- a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java +++ b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java @@ -538,7 +538,7 @@ public class HdfsScanNode extends ScanNode { if (slotRef.getDesc().isArrayPosRef()) return; if (inPred.isNotIn()) return; - ArrayList<Expr> children = inPred.getChildren(); + List<Expr> children = inPred.getChildren(); LiteralExpr min = null; LiteralExpr max = null; for (int i = 1; i < children.size(); ++i) { http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/planner/Planner.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/Planner.java b/fe/src/main/java/org/apache/impala/planner/Planner.java index 59d3b8c..eea26e7 100644 --- a/fe/src/main/java/org/apache/impala/planner/Planner.java +++ b/fe/src/main/java/org/apache/impala/planner/Planner.java @@ -246,7 +246,7 @@ public class Planner { * Uses a default level of EXTENDED, unless overriden by the * 'explain_level' query option. */ - public String getExplainString(ArrayList<PlanFragment> fragments, + public String getExplainString(List<PlanFragment> fragments, TQueryExecRequest request) { // use EXTENDED by default for all non-explain statements TExplainLevel explainLevel = TExplainLevel.EXTENDED; @@ -262,7 +262,7 @@ public class Planner { * explicit explain level. * Includes the estimated resource requirements from the request if set. */ - public String getExplainString(ArrayList<PlanFragment> fragments, + public String getExplainString(List<PlanFragment> fragments, TQueryExecRequest request, TExplainLevel explainLevel) { StringBuilder str = new StringBuilder(); boolean hasHeader = false; http://git-wip-us.apache.org/repos/asf/impala/blob/12dc29e5/fe/src/main/java/org/apache/impala/service/Frontend.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java index 8aeafb1..f6aa123 100644 --- a/fe/src/main/java/org/apache/impala/service/Frontend.java +++ b/fe/src/main/java/org/apache/impala/service/Frontend.java @@ -1049,7 +1049,7 @@ public class Frontend { private TPlanExecInfo createPlanExecInfo(PlanFragment planRoot, Planner planner, TQueryCtx queryCtx, TQueryExecRequest queryExecRequest) { TPlanExecInfo result = new TPlanExecInfo(); - ArrayList<PlanFragment> fragments = planRoot.getNodesPreOrder(); + List<PlanFragment> fragments = planRoot.getNodesPreOrder(); // collect ScanNodes List<ScanNode> scanNodes = Lists.newArrayList(); @@ -1147,7 +1147,7 @@ public class Frontend { // create EXPLAIN output after setting everything else result.setQuery_ctx(queryCtx); // needed by getExplainString() - ArrayList<PlanFragment> allFragments = planRoots.get(0).getNodesPreOrder(); + List<PlanFragment> allFragments = planRoots.get(0).getNodesPreOrder(); explainString.append(planner.getExplainString(allFragments, result)); result.setQuery_plan(explainString.toString());
