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/30 18:39:17 UTC
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12009
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insentisitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 744 insertions(+), 516 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/1
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 8
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jan 2019 00:16:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1626/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Mon, 17 Dec 2018 03:31:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12009/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/12009/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@761
PS2, Line 761: "using: ALTER TABLE functional.alltypestiny PARTITION (`year` = 2009, `month` = 1) " +
line too long (94 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 23:11:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#7).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 738 insertions(+), 503 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/7
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 20:06:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 5:
(6 comments)
Few minor comments, lgtm otherwise.
http://gerrit.cloudera.org:8080/#/c/12009/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12009/1//COMMIT_MSG@17
PS1, Line 17: insentisitive
typo
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@116
PS5, Line 116: an identifier
nit: 'ident'
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@124
PS5, Line 124: ANTLRStringStream(ident.toUpperCase()
probably switch to ANTLRNoCaseStringStream to be consistent with Hive?
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: if (SqlScanner.isReserved(ident)) return true;
: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary for the given identifier itself, the quotes
: // are needed if this identifier will be preceded by a ".".
: return Character.isDigit(ident.charAt(0));
simplify to return SqlScanner.isReserved(ident) || Character.isDigit(ident.charAt(0)) ;?
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@169
PS5, Line 169: if (ident.equals("*")) return ident;
Why this special case? (optimization?)
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@186
PS5, Line 186: // Test implicit table aliases with unqualified and fully qualified
Can we do col = ToSqlUtils.identSql(col) here instead?
(same for table)
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 22 Dec 2018 01:58:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1679/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Dec 2018 08:30:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 4:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/1612/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 20:13:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 11: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@147
PS11, Line 147: true if the identifier is an Impala keyword, or if starts
: * with a digit
update.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 11
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 18:57:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
(2 comments)
Just couple nits, but it looks good overall.
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@126
PS1, Line 126: Token t = hiveLexer.nextToken();
Will hiveLexer.nextToken() returns null when there's no token? We should either do a null check or add Preconditions if we're sure it never returns a null. Similarly with L132.
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@149
PS1, Line 149: // Impala's scanner recognizes the ".123" portion of "db.123_tbl" as a decimal,
: // so while the quoting is not necessary for the given identifier itself, the quotes
: // are needed if this identifier will be preceded by a ".".
This is no longer an issue in Impala: https://gerrit.cloudera.org/c/11927/. I'm okay with quoting it since db.3e12 still needs to be quoted.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 19:47:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements using the toSql() call.
Generated SQL is often used during testing or when writing the query
plan. Impala keywords such as "create", when used as identifiers,
must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() quotes the identifier if it is
an Impala or Hive keyword, or if it does not follow the identifier
pattern. The code uses the Hive lexer to detect a keyword. But, the
code contained a flaw: the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not. This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* PlannerTest emits statement SQL to the output file wrapped to 80
columns and sometimes leaves trailing spaces at the end of the line.
Some tools remove that trailing space, resulting in trivial file
differences. Fixed this to remove trailing spaces in order to simplify
file comparisons.
* Tweaked the "In pipelines" output to avoid trailing spaces when no
pipelines are listed.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Reviewed-on: http://gerrit.cloudera.org:8080/12009
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/order.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/small-query-opt.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-planner/queries/PlannerTest/views.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
44 files changed, 1,310 insertions(+), 1,064 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 14
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#6).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 730 insertions(+), 503 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/6
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 6:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/12009/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12009/1//COMMIT_MSG@17
PS1, Line 17: insensitive i
> typo
Done
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@116
PS5, Line 116: an identifier
> nit: 'ident'
"ident" is a common abbreviation, and the name of the parameter, but I think the English term is "identifier." Do we normally use abbreviations in comments?
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@124
PS5, Line 124: upper-case text, but that
> probably switch to ANTLRNoCaseStringStream to be consistent with Hive?
Turns out ANTLRNoCaseStringStream is a non-static inner class inside the ParserDriver class for no good reason other than the author seems to have forgotten to type "static". All that ANTLRNoCaseStringStream does is uppercase each character as it is returned. Uppercasing the entire (short) string up front accomplishes the same goal here, so should be OK.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: /
: public static boolean impalaNeedsQuotes(String ident) {
: return SqlScanner.isReserved(ident) ||
: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary
> Actually, I think IMPALA-941 recently fixed this bad decimal thing.
Made the || change.
Retained this code from lines 131-136 of the original code. Fredy tells me that although his change can handle ".123" as an identifier, it cannot handle ".123e3". So, I'm hesitant to remove this code.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@169
PS5, Line 169: /
> Why this special case? (optimization?)
Since * is not a valid identifier, it would normally be quoted. But, here is is the wildcard character which should not be quoted. So, yes it does need special handling. Added a comment.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@186
PS5, Line 186: String quotedCol = ToSqlUtils.identSql(col);
> Can we do col = ToSqlUtils.identSql(col) here instead?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Dec 2018 07:56:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 7:
(2 comments)
Addressed recent review comments.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@116
PS5, Line 116: an identifier
> I meant to refer to the method argument @ident here. But it is fine. Not to
Took a second look. The text was ambiguous, using "identifier" for two distinct concepts. Reworded to be clearer.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: /
: public static boolean impalaNeedsQuotes(String ident) {
: return SqlScanner.isReserved(ident) ||
: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary
> ya, 123e45 is a valid decimal literal but not .123 part like you mentioned
Reworded the comment and inserted an actual example that illustrates the issue. "3a3" is a fine unquoted identifier. "3e3" must be quoted, else it is a double. So, to be safe, we quote all identifiers that start with a digit.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Jan 2019 00:37:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#10).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements using the toSql() call.
Generated SQL is often used during testing or when writing the query
plan. Impala keywords such as "create", when used as identifiers,
must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() quotes the identifier if it is
an Impala or Hive keyword, or if it does not follow the identifier
pattern. The code uses the Hive lexer to detect a keyword. But, the
code contained a flaw: the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not. This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* PlannerTest emits statement SQL to the output file wrapped to 80
columns and sometimes leaves trailing spaces at the end of the line.
Some tools remove that trailing space, resulting in trivial file
differences. Fixed this to remove trailing spaces in order to simplify
file comparisons.
* Tweaked the "In pipelines" output to avoid trailing spaces when no
pipelines are listed.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/order.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/small-query-opt.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-planner/queries/PlannerTest/views.test
43 files changed, 1,296 insertions(+), 1,053 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/10
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@761
PS1, Line 761: "using: ALTER TABLE functional.alltypestiny PARTITION (`year` = 2009, `month` = 1) " +
line too long (94 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 18:40:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#2).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insentisitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 719 insertions(+), 494 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/2
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#12).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements using the toSql() call.
Generated SQL is often used during testing or when writing the query
plan. Impala keywords such as "create", when used as identifiers,
must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() quotes the identifier if it is
an Impala or Hive keyword, or if it does not follow the identifier
pattern. The code uses the Hive lexer to detect a keyword. But, the
code contained a flaw: the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not. This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* PlannerTest emits statement SQL to the output file wrapped to 80
columns and sometimes leaves trailing spaces at the end of the line.
Some tools remove that trailing space, resulting in trivial file
differences. Fixed this to remove trailing spaces in order to simplify
file comparisons.
* Tweaked the "In pipelines" output to avoid trailing spaces when no
pipelines are listed.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/order.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/small-query-opt.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-planner/queries/PlannerTest/views.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
44 files changed, 1,310 insertions(+), 1,064 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/12
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 12
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1729/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Jan 2019 00:50:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@126
PS1, Line 126: Token t = hiveLexer.nextToken();
> Will hiveLexer.nextToken() returns null when there's no token? We should ei
Actually forget about this, I missed the try/catch. This code is fine.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 19:49:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 12
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:04:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 6: Code-Review+2
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@116
PS5, Line 116: an identifier
> "ident" is a common abbreviation, and the name of the parameter, but I thin
I meant to refer to the method argument @ident here. But it is fine. Not too strong.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@124
PS5, Line 124: upper-case text, but that
> Turns out ANTLRNoCaseStringStream is a non-static inner class inside the Pa
cool, thanks for checking.
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: /
: public static boolean impalaNeedsQuotes(String ident) {
: return SqlScanner.isReserved(ident) ||
: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary
> Made the || change.
ya, 123e45 is a valid decimal literal but not .123 part like you mentioned (e is exponential). Fix the comment?
[localhost:21000] default> select typeof(.123e45);
| typeof(1.23e+44) |
+------------------+
| DOUBLE |
+------------------+
I guess this is true anything simple like `12345`
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Jan 2019 19:32:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1609/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 19:47:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 12:
(1 comment)
Addressed the code review comment.
http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/11/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@147
PS11, Line 147: true if the identifier is an Impala keyword, or if starts
: * with a digit
> update.
Done
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 12
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 20:31:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
This one is just a nuisance fix. Can you take a look? Maybe you know if we need the Hive identifier fix? But, if we have the code, it should actually work.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 18:40:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: if (SqlScanner.isReserved(ident)) return true;
: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary for the given identifier itself, the quotes
: // are needed if this identifier will be preceded by a ".".
: return Character.isDigit(ident.charAt(0));
> simplify to return SqlScanner.isReserved(ident) || Character.isDigit(ident.
Actually, I think IMPALA-941 recently fixed this bad decimal thing.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 22 Dec 2018 02:09:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#5).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insentisitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 718 insertions(+), 495 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/5
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1480/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 19:12:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#11).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements using the toSql() call.
Generated SQL is often used during testing or when writing the query
plan. Impala keywords such as "create", when used as identifiers,
must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() quotes the identifier if it is
an Impala or Hive keyword, or if it does not follow the identifier
pattern. The code uses the Hive lexer to detect a keyword. But, the
code contained a flaw: the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not. This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* PlannerTest emits statement SQL to the output file wrapped to 80
columns and sometimes leaves trailing spaces at the end of the line.
Some tools remove that trailing space, resulting in trivial file
differences. Fixed this to remove trailing spaces in order to simplify
file comparisons.
* Tweaked the "In pipelines" output to avoid trailing spaces when no
pipelines are listed.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/order.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/small-query-opt.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
M testdata/workloads/functional-planner/queries/PlannerTest/views.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
44 files changed, 1,302 insertions(+), 1,059 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/11
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 11
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 9
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jan 2019 01:00:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1861/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 12
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:02:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 6:
Rebased on latest master to resolve merge conflict.
Added a trivial fix for IMPALA-8051: using a comment character in a column name.
Please review the fix and rebase and renew your approval if you agree.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Jan 2019 00:17:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#8).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insensitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
This patch also fixes:
IMPALA-8051: Compute stats fails on a column with comment character in
name
The code uses the Hive lexical analyzer to check names. Since "#" and
"--" are comment characters, a name like "foo#" is parsed as "foo" which
does not need quotes, hence we don't quote "foo#", which causes issues.
Added a special check for "#" and "--" to resolve this issue.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 741 insertions(+), 503 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/8
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 8
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 9:
Looks like there are some failed tests. Paul, can you please take look when you find time. Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 9
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 12 Jan 2019 00:17:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
Pre-commit tests ran: https://jenkins.impala.io/job/pre-review-test/236/console
Tests had a single failure in metadata/test_stats_extrapolation.py, but the test passed in my own environment. Since this change does not affect stats, I'm thinking this may have been a flaky test...
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 18:42:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1601/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 23:44:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 3:
(2 comments)
Addressed review comments. Rebased on master.
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@126
PS1, Line 126: Token t = hiveLexer.nextToken();
> Actually forget about this, I missed the try/catch. This code is fine.
Thanks. Actually, this is the original code, just moved into a separate function for testing.
http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@149
PS1, Line 149: // Impala's scanner recognizes the ".123" portion of "db.123e45" as a decimal,
: // so while the quoting is not necessary for the given identifier itself, the quotes
: // are needed if this identifier will be preceded by a ".".
> This is no longer an issue in Impala: https://gerrit.cloudera.org/c/11927/.
This comment is original, just moved. Thanks for pointing out the correction.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 19:54:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1730/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 8
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Jan 2019 01:07:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 13
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:05:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 13:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3667/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 13
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:05:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#3).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insentisitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 720 insertions(+), 495 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/3
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 1:
Paul, I think you need to rebase this.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 19:47:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 10:
Rebased on latest master. Added a few minor output fixes to remove trailing spaces which cause annoying trivial file differences when comparing expected and actual .test files.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sun, 20 Jan 2019 00:00:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3621/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 9
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jan 2019 01:00:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1833/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sun, 20 Jan 2019 00:31:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 11:
Looks like some e-to-e test "golden" files need updating. Each four-hour test highlights one failure. Will ping once I've nailed them all.
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 11
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 03:05:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1839/ : 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/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 11
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 03:38:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 9: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3621/
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 9
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jan 2019 05:50:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 11:
Pre-review tests now pass again. Please go ahead and re-review. https://jenkins.impala.io/job/pre-review-test/282/
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 11
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Jan 2019 17:08:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12009 )
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
Patch Set 13: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 13
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Jan 2019 01:26:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7905: Hive keywords not quoted for identifiers
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Fredy Wijaya, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12009
to look at the new patch set (#4).
Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................
IMPALA-7905: Hive keywords not quoted for identifiers
Impala often generates SQL for statements in the toSql() call, often
used during testing or when writing the query plan. Impala keywords such
as "create", when used as identifiers, must be quoted:
SELECT `select`, `from` FROM `order` ...
The code in ToSqlUtils.getIdentSql() also quotes the identifier if it is
a Hive keyword. But, that code contained a flaw: it uses the Hive lexer
to detect a keyword, but the lexer expects a case-insentisitive input.
We provide a case sensitive input. As a result, "MONTH" is caught as a
Hive keyword and quoted, but "month" is not.
This patch fixes that flaw.
Testing:
* Refactored getIdentSql() easier testing.
* Added a tests to the recently added ToSqlUtilsTest for this case and
several others.
* Making this change caused the columns `month`, `year`, and `key` to be
quoted when before they were not. Updated many tests as a result.
* Added a new identSql() function, for use in tests, to match the
quoting that Impala uses, and to handle the wildcard, and multi-part
names. Used this in ToSqlTest to handle the quoted names.
* Reran all FE tests.
Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
---
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/hdfs.test
M testdata/workloads/functional-planner/queries/PlannerTest/implicit-joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
M testdata/workloads/functional-planner/queries/PlannerTest/insert.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
M testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-stats-agg.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-key-scans.test
M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-query-options.test
M testdata/workloads/functional-planner/queries/PlannerTest/shuffle-by-distinct-exprs.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
31 files changed, 719 insertions(+), 495 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/12009/4
--
To view, visit http://gerrit.cloudera.org:8080/12009
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>