You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/04/25 07:20:01 UTC
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Hello Todd Lipcon, Impala Public Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/13114
to review the following change.
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
This is a grab-bag of small fixes necessary to get most queries on HDFS
tables passing with the correct plans:
* Change the loading of tables to check for other table types before
checking for FS tables, since without specific checks against
various properties, other tables may look like FS tables and try to
instantiate LocalFsTable incorrectly.
* Return -1 for extrapolated row count -- the previous 0 value was
convincing the planner that it had a valid value.
* Fix up the handling of BuiltinsDb so that we don't depend on
ImpaladCatalog to have been loaded in order to instantiate it.
* Properly handle the case where all partitions are pruned by a
predicate.
With this change, about half of the tests in PlannerTest pass. The tests
that don't pass all rely on views, HBase tables, etc.
Conflicts:
fe/src/main/java/org/apache/impala/analysis/FunctionName.java
Need to Modify:
fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Reviewed-on: http://gerrit.cloudera.org:8080/10798
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionName.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
15 files changed, 67 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/13114/1
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2906/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 25 Apr 2019 08:00:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4064/
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 25 Apr 2019 11:18:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 26 Apr 2019 09:57:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4079/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 26 Apr 2019 06:05:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1:
The conflicts are in fe/src/main/java/org/apache/impala/analysis/FunctionName.java:
88 public void analyze(Analyzer analyzer) throws AnalysisException {
89 if (isAnalyzed_) return;
90 analyzeFnNamePath();
91 if (fn_.isEmpty()) throw new AnalysisException("Function name cannot be empty.");
92 for (int i = 0; i < fn_.length(); ++i) {
93 if (!isValidCharacter(fn_.charAt(i))) {
94 throw new AnalysisException(
95 "Function names must be all alphanumeric or underscore. " +
96 "Invalid name: " + fn_);
97 }
98 }
99 if (Character.isDigit(fn_.charAt(0))) {
100 throw new AnalysisException("Function cannot start with a digit: " + fn_);
101 }
102
103 // Resolve the database for this function.
104 <<<<<<< HEAD
105 if (!isFullyQualified()) {
106 Db builtinDb = ImpaladCatalog.getBuiltinsDb();
107 if (builtinDb.containsFunction(fn_)) {
108 // If it isn't fully qualified and is the same name as a builtin, use
109 // the builtin.
110 db_ = ImpaladCatalog.BUILTINS_DB;
111 isBuiltin_ = true;
112 } else {
113 db_ = analyzer.getDefaultDb();
114 isBuiltin_ = false;
115 =======
116 Db builtinDb = BuiltinsDb.getInstance();
117 if (!isFullyQualified()) {
118 db_ = analyzer.getDefaultDb();
119 if (preferBuiltinsDb && builtinDb.containsFunction(fn_)) {
120 db_ = BuiltinsDb.NAME;
121 >>>>>>> 7f8a4d3... IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
122 }
123 } else {
124 isBuiltin_ = db_.equals(ImpaladCatalog.BUILTINS_DB);
125 }
126 <<<<<<< HEAD
127 =======
128 Preconditions.checkNotNull(db_);
129 isBuiltin_ = db_.equals(BuiltinsDb.NAME) &&
130 builtinDb.containsFunction(fn_);
131 >>>>>>> 7f8a4d3... IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
132 isAnalyzed_ = true;
133 }
The cause is that in the master branch, this function is modified by IMPALA-6724: https://github.com/apache/impala/commit/08d386f0fc62e2991bcc046dde8574f25d421580
This patch is skipped for 2.x.
To resolve the conflicts, we keep everything of HEAD excerpts changing ImpaladCatalog.getBuiltinsDb() to BuiltinsDb.getInstance() and changing ImpaladCatalog.BUILTINS_DB to BuiltinsDb.NAME.
Besides, we also need to do this in fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java:[152,23] since these lines were removed in the master branch too.
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 25 Apr 2019 07:25:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1:
The previous build failure is due to the lake of bin/jenkins/dockerized-impala-bootstrap-and-test.sh in 2.x branch. Rerun it after the job ubuntu-16.04-dockerised-tests is disabled.
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 26 Apr 2019 07:02:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4064/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 25 Apr 2019 07:26:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
This is a grab-bag of small fixes necessary to get most queries on HDFS
tables passing with the correct plans:
* Change the loading of tables to check for other table types before
checking for FS tables, since without specific checks against
various properties, other tables may look like FS tables and try to
instantiate LocalFsTable incorrectly.
* Return -1 for extrapolated row count -- the previous 0 value was
convincing the planner that it had a valid value.
* Fix up the handling of BuiltinsDb so that we don't depend on
ImpaladCatalog to have been loaded in order to instantiate it.
* Properly handle the case where all partitions are pruned by a
predicate.
With this change, about half of the tests in PlannerTest pass. The tests
that don't pass all rely on views, HBase tables, etc.
Conflicts:
fe/src/main/java/org/apache/impala/analysis/FunctionName.java
Need to Modify:
fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Reviewed-on: http://gerrit.cloudera.org:8080/10798
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/13114
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/FunctionName.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
15 files changed, 67 insertions(+), 44 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR](2.x) IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13114 )
Change subject: IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13114
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f603e62b7a013c148c0905ebdec2f4303f9c4e5
Gerrit-Change-Number: 13114
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 17:40:57 +0000
Gerrit-HasComments: No