You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2018/11/20 18:39:16 UTC

[Impala-ASF-CR] IMPALA-7867, part 1: Expose List in TreeNode, parser

Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11954

to look at the new patch set (#2).

Change subject: IMPALA-7867, part 1: Expose List in TreeNode, parser
......................................................................

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.

Tests: This is purely a refactoring, no functionality changed. Ran the
FE unit tests to verify no regressions.

Change-Id: Iebab7dccdb4b2fa0b5ca812beab0e8bdba39f539
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionName.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/catalog/StructType.java
M fe/src/main/java/org/apache/impala/common/TreeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
15 files changed, 99 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11954/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11954
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebab7dccdb4b2fa0b5ca812beab0e8bdba39f539
Gerrit-Change-Number: 11954
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>