You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2019/08/05 22:26:12 UTC
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14012
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_exprs.py
11 files changed, 276 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/14012/1
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.h
File be/src/service/query-options.h:
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.h@190
PS2, Line 190:
> nit: Do we need spaces here according our code style?
Removed the spaces for these new lines. I don't think the spaces are needed for any code style thing.
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@824
PS2, Line 824: Invalid statement expression limit
> nit: can we also print the value?
Added the actual value here and for the max statement length bytes.
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@825
PS2, Line 825: Value
> typo: Valid
Done
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@838
PS2, Line 838: value
> nit: values
Done
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@462
PS1, Line 462: boolean isExplain = analysisResult_.isExplainStmt();
> nit: Probably worth mentioning that we want to enforce this before the rewr
Done
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2861
PS2, Line 2861: String errorStr = String.format("Exceeded the statement expression limit (%s)\n" +
> nit: use "%d" for int.
Switched to %d. I don't know the history, but it makes sense to use %d.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2435
PS1, Line 2435: String repCols20 = getRepeatedColumnReference("int_col", 20, true);
> Verify that analyzer.numStmtExprs_ is accounted properly?
Added asserts for the AnalyzesOk() calls to verify numStmtExprs_ is exactly what we expect. The AnalysisError() cases already include the numStmtExprs_ in the error message.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2493
PS1, Line 2493: StringBuilder inList = new StringBuilder();
> what about constant expressions in the IN lists? foo IN (2*3, 3*4...) ? It
Good point, I added tests for arithmetic expressions 1*2*3*4. This found a bug where these expressions could be double-counted due to the Exprs being cloned and reanalyzed. I added a per-Expr variable to track whether the Expr had been counted. Each Expr will only be counted once. This also required some logic to skip counting these expressions when in a WITH clause (it gets accounted separately).
I added a test that has an IN list with an arithmetic expression.
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2443
PS2, Line 2443: // WHERE clause
> nit: For coverage, maybe test the expression in the WHERE clause?
The way I'm thinking about this is that the SQL without the WHERE has 20 expressions and adding the WHERE with bool_col adds one expression, so it would only fail if the WHERE were counted. I added a comment here.
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@139
PS1, Line 139: # This takes 20+ minutes, so only run it on exhaustive.
> Is the intention to test the default limits here? If not, we can probably s
I wanted this to test as close to the default limits as possible. It is unclear how much value this test has. In my runs, it doesn't seem to impact the total runtime of the exhaustive tests by much.
http://gerrit.cloudera.org:8080/#/c/14012/2/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/2/tests/query_test/test_exprs.py@216
PS2, Line 216:
> flake8: E261 at least two spaces before inline comment
Done
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 01:07:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Michael Ho, Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14012
to look at the new patch set (#4).
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
This also changes the logging in tests/common/impala_connection.py
to limit the total SQL size that it will print to 128KB. This is
prevents the JUnitXML (which includes this logging) from being too
large. Existing tests do not run SQL larger than about 80KB, so
this only applies to tests added in this change that run multi-MB
SQLs to verify limits.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/common/impala_connection.py
M tests/query_test/test_exprs.py
13 files changed, 379 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/14012/4
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/3/tests/common/impala_connection.py
File tests/common/impala_connection.py:
http://gerrit.cloudera.org:8080/#/c/14012/3/tests/common/impala_connection.py@49
PS3, Line 49: def log_sql_stmt(sql_stmt):
flake8: E302 expected 2 blank lines, found 0
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 01:08:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/3/tests/common/impala_connection.py
File tests/common/impala_connection.py:
http://gerrit.cloudera.org:8080/#/c/14012/3/tests/common/impala_connection.py@49
PS3, Line 49: def log_sql_stmt(sql_stmt):
> flake8: E302 expected 2 blank lines, found 0
Done
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:54:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@457
PS1, Line 457: timeline_.markEvent("Initial analysis finished");
The current test failure is due to the introduction of this timeline event. I used this for testing, but we need to decide whether this is useful in general.
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 08 Aug 2019 17:19:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@147
PS1, Line 147: test_max_statement_size
Thinking of adding an assert verifying that this takes less than 1 minute (or some time limit).
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@167
PS1, Line 167: test_statement_expression_limit
Same here, assert that this takes less than 1 minute (or whatever).
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:45:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4784/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 22:59:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4734/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:26:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4144/ : 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/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:51:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Michael Ho, Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14012
to look at the new patch set (#2).
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_exprs.py
12 files changed, 277 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/14012/2
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4208/ : 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/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 16:52:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/2/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/2/tests/query_test/test_exprs.py@216
PS2, Line 216:
flake8: E261 at least two spaces before inline comment
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 16:12:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
(4 comments)
lgtm overall. Some general comments.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@462
PS1, Line 462: analysisResult_.analyzer_.checkStmtExprLimit();
nit: Probably worth mentioning that we want to enforce this before the rewrites because rewrites are costly with long expression chains.
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2435
PS1, Line 2435: String repCols20 = getRepeatedColumnReference("int_col", 20, true);
Verify that analyzer.numStmtExprs_ is accounted properly?
http://gerrit.cloudera.org:8080/#/c/14012/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2493
PS1, Line 2493: StringBuilder inList = new StringBuilder();
what about constant expressions in the IN lists? foo IN (2*3, 3*4...) ? It looks to me like they are accounted, add tests for a mix of Literal and Const expressions?
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@139
PS1, Line 139: # This takes 20+ minutes, so only run it on exhaustive.
Is the intention to test the default limits here? If not, we can probably set a non-default lower expression limit to save time and memory.
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 17:11:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 6: Code-Review+2
Carry +2
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:28:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4734/
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 03:57:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 04:36:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4786/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:33:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2:
(6 comments)
LGTM. Just leave some "nit" comments.
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.h
File be/src/service/query-options.h:
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.h@190
PS2, Line 190:
nit: Do we need spaces here according our code style?
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@824
PS2, Line 824: Invalid statement expression limit
nit: can we also print the value?
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@825
PS2, Line 825: Value
typo: Valid
http://gerrit.cloudera.org:8080/#/c/14012/2/be/src/service/query-options.cc@838
PS2, Line 838: value
nit: values
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2861
PS2, Line 2861: String errorStr = String.format("Exceeded the statement expression limit (%s)\n" +
nit: use "%d" for int.
Looks like we use "%s" for INTs in many places. Are there any reasons?
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/14012/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2443
PS2, Line 2443: // WHERE clause
nit: For coverage, maybe test the expression in the WHERE clause?
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Aug 2019 03:00:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4240/ : 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/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 23:34:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/14012/1/tests/query_test/test_exprs.py@216
PS1, Line 216:
flake8: E261 at least two spaces before inline comment
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:27:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 05:17:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4244/ : 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/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 01:08:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 03:05:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Michael Ho, Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14012
to look at the new patch set (#6).
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
This also changes the logging in tests/common/impala_connection.py
to limit the total SQL size that it will print to 128KB. This is
prevents the JUnitXML (which includes this logging) from being too
large. Existing tests do not run SQL larger than about 80KB, so
this only applies to tests added in this change that run multi-MB
SQLs to verify limits.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/common/impala_connection.py
M tests/query_test/test_exprs.py
13 files changed, 381 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/14012/6
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4765/
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 20:19:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h
File be/src/service/query-options.h:
http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h@185
PS5, Line 185: static const int64_t SPILLABLE_BUFFER_LIMIT = 1LL << 40; // 1 TB
nit: not related to your change but indents off?
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:02:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h
File be/src/service/query-options.h:
http://gerrit.cloudera.org:8080/#/c/14012/5/be/src/service/query-options.h@185
PS5, Line 185: static const int64_t SPILLABLE_BUFFER_LIMIT = 1LL << 40; // 1 TB
> nit: not related to your change but indents off?
Went ahead and fixed this
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 00:27:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
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/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
This also changes the logging in tests/common/impala_connection.py
to limit the total SQL size that it will print to 128KB. This is
prevents the JUnitXML (which includes this logging) from being too
large. Existing tests do not run SQL larger than about 80KB, so
this only applies to tests added in this change that run multi-MB
SQLs to verify limits.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Reviewed-on: http://gerrit.cloudera.org:8080/14012
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/common/impala_connection.py
M tests/query_test/test_exprs.py
13 files changed, 381 insertions(+), 11 deletions(-)
Approvals:
Joe McDonnell: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Michael Ho, Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14012
to look at the new patch set (#3).
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
IMPALA-4551: Limit the size of SQL statements
Various BI tools generate and run SQL. When used incorrectly or
misconfigured, the tools can generate extremely large SQLs.
Some of these SQL statements reach 10s of megabytes. Large SQL
statements impose costs throughout execution, including
statement rewrite logic in the frontend and codegen in the
backend. The resource usage of these statements can impact
the stability of the system or the ability to run other SQL
statements.
This implements two new query options that provide controls
to reject large SQL statements.
- The first, MAX_STATEMENT_LENGTH_BYTES is a cap on the
total size of the SQL statement (in bytes). It is
applied before any parsing or analysis. It uses a
default value of 16MB.
- The second, STATEMENT_EXPRESSION_LIMIT, is a limit on
the total number of expressions in a statement or any
views that it references. The limit is applied upon the
first round of analysis, but it is not reapplied when
statement rewrite rules are applied. Certain expressions
such as literals in IN lists or VALUES clauses are not
analyzed and do not count towards the limit. It uses
a default value of 250,000.
The two are complementary. Since enforcing the statement
expression limit requires parsing and analyzing the
statement, the MAX_STATEMENT_LENGTH_BYTES sets an upper
bound on the size of statement that needs to be parsed
and analyzed. Testing confirms that even statements
approaching 16MB get through the first round of analysis
within a few seconds and then are rejected.
This also changes the logging in tests/common/impala_connection.py
to limit the total SQL size that it will print to 128KB. This is
prevents the JUnitXML (which includes this logging) from being too
large. Existing tests do not run SQL larger than about 80KB, so
this only applies to tests added in this change that run multi-MB
SQLs to verify limits.
Testing:
- This adds frontend tests that verify the low level
semantics about how expressions are counted and verifies
that the expression limits are enforced.
- This adds end-to-end tests that verify both the
MAX_STATEMENT_LENGTH_BYTES and STATEMENT_EXPRESSION_LIMIT
at their defaults values.
- There is also an end-to-end test that runs in exhaustive
mode that runs a SQL with close to 250,000 expressions.
Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
---
M be/src/service/impala-server.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/common/impala_connection.py
M tests/query_test/test_exprs.py
13 files changed, 377 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/14012/3
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4776/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 01:07:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4765/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 16:12:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4551: Limit the size of SQL statements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14012 )
Change subject: IMPALA-4551: Limit the size of SQL statements
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4228/ : 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/14012
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5675fb4a08c1dc51ae5bcf467cbb969cc064602c
Gerrit-Change-Number: 14012
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 01:49:14 +0000
Gerrit-HasComments: No