date:20181205

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11565 )

Change subject: IMPALA-7659: Populate NULL count while computing column stats
..


Patch Set 7:

It'd still be good to do a sanity check for performance, just on your dev 
machine, if possible, but that shouldn't block getting this in.


--
To view, visit http://gerrit.cloudera.org:8080/11565
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68f8b4c3756eb1980ce299a602a7d56db1e507a
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 06 Dec 2018 01:45:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7473: fix crash when printing decimal with -v=3

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12029 )

Change subject: IMPALA-7473: fix crash when printing decimal with -v=3
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3528/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/12029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I772681b78cda27d6c2086b91151ca8bdcd901828
Gerrit-Change-Number: 12029
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 06 Dec 2018 01:43:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7659: Populate NULL count while computing column stats

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11565 )

Change subject: IMPALA-7659: Populate NULL count while computing column stats
..


Patch Set 7: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@251
PS6, Line 251:
> Agreed. Let's get this in, then tackle the NDV=0 as a separate issue.
IMPALA-1003 doesn't include many breadcrumbs. I do wonder if there were other, 
more severe, issues back when it was disabled - e.g. codegen was disabled for 
the whole aggregation or something.

I just quickly tried an experiment measuring the time added to the aggregation 
for each additional aggregate function of this nature and the delta from each 
added function is very small.



--
To view, visit http://gerrit.cloudera.org:8080/11565
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68f8b4c3756eb1980ce299a602a7d56db1e507a
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 06 Dec 2018 01:43:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12018 )

Change subject: IMPALA-7914: Base class for statement-like AST nodes
..


Patch Set 5:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/1551/ : Initial code 
review checks failed. See linked job for details on the failure.


--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 23:15:13 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes

Hello Bharath Vissapragada, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7914: Base class for statement-like AST nodes
..

IMPALA-7914: Base class for statement-like AST nodes

In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.

To prepare for that goal, this patch introduces a new StmtNode class
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.

Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.

Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 72 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/5
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11955 )

Change subject: IMPALA-7844: HAVING clause cannot support ordinals
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1550/ : 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/11955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
Gerrit-Change-Number: 11955
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 23:07:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7914: Base class for statement-like AST nodes

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12018 )

Change subject: IMPALA-7914: Base class for statement-like AST nodes
..


Patch Set 4:

Turns out a base class is even more useful than a base interface.


--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 23:05:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7844: HAVING clause cannot support ordinals

Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7844: HAVING clause cannot support ordinals
..

IMPALA-7844: HAVING clause cannot support ordinals

The SELECT statement has two clauses that take lists of columns and/or
aliases: ORDER BY and GROUP BY. Each element is a name, a table.column
reference or a number, which represents the ordinal number of a column.
Thus, "GROUP BY a, 2, c" is unambiguous.

SELECT also has a number of predicate clauses: WHERE and HAVING. In
these, aliases are possible (though seldom suppored), but ordinals are
ambiguous: is "WHERE 1 = 2" a reference to two constants, two columns by
ordinal, or a combination? No SQL dialect supports ordinals in WHERE or
HAVING for this reason.

Impala seems to have adopted a rather odd convention: if the HAVING
predicate has only one element (no opeators), than that one element can
be an ordinal or alias. Thus "HAVING a" and "HAVING 1" are valid, only
if alias a or the column at ordinal 1 are Boolean. However,
"HAVING a = 1" and "HAVING 1 = 2" are not valid.  This is unusal
because HAVING is normally a predicate: "HAVING a = 10".

This fix removes the attempt to support ordinals in the HAVING clause,
and treats HAVING like WHERE, rather than trying to treat it like
ORDER BY or GROUP BY. The fix retains the limited form of alias
support.

Refactored the "ordinal or alias" code to allow resolution of only
aliases (for HAVING).

Reworded a few error messages for greater clarity.

Testing:

* Refactored AnalyzeStmtsTest to split apart the alias and ordinals
  cases for easier debugging.
* Disabled the tests for ordinals in HAVING.
* Added new tests to verify that integers in HAVING act like
  (unsupported) integer constants.

Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
5 files changed, 283 insertions(+), 137 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/11955/4
--
To view, visit http://gerrit.cloudera.org:8080/11955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b9f9e8c60fe2b25e20c57c2ffc31d8e59d5861
Gerrit-Change-Number: 11955
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-5973: Provide query plan in JSON format

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11974 )

Change subject: IMPALA-5973: Provide query plan in JSON format
..


Patch Set 3:

Pranay, turns out it would be very handy indeed to serialize the query tree to 
JSON for testing. Your change focuses on the plan tree after planning. My need 
is for the query tree after analysis. Let's coordinate to see if we can use the 
same JSON serialization mechanism for both.


--
To view, visit http://gerrit.cloudera.org:8080/11974
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f07e2223be58b7323e1ea2fa44b62626cdf4944
Gerrit-Change-Number: 11974
Gerrit-PatchSet: 3
Gerrit-Owner: Pranay Singh (320)
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Pranay Singh (320)
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:44:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11888 )

Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..


Patch Set 9:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@59
PS9, Line 59:   PartitionSpec partitionSpec, String db, boolean 
authorization) {
We now have two flags: isRefresh, and authorization. Are these disjoint? One is 
true or the other? And, how to they relate to partitionSpec?

Should we define an enum to identify the meaning of the statement rather than 
parsing it from flags?

enum Action{ REFRESH_AUTH, REFRESH_TABLE, REFRESH_ALL, ... };


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@111
PS9, Line 111: if (authorization_) {
The logic here is getting hard to follow. Would be easier using a case 
statement with that enum mentioned above.


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@160
PS9, Line 160: result.append(" AUTHORIZATION");
Same comment as above.


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS9, Line 1140: if (sentryProxy_ != null) {
if (sentryProxy == null) return;

Then, outdent the try/catch block. Result is a bit easier to read.


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex
File fe/src/main/jflex/sql-scanner.flex:

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex@78
PS9, Line 78: keywordMap.put("authorization", 
SqlParserSymbols.KW_AUTHORIZATION);
A persistent concern when adding keywords is that some queries will now fail:

SELECT authorization FROM mySecurityTable

Is there some existing keyword that could be used instead, even if not ideal? 
Maybe REFRESH ROLE METADATA? REFRESH GRANT METADATA?


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@331
PS9, Line 331: AnalyzesOk("refresh authorization", 
createAnalysisCtx(createAuthorizationConfig()));
Ideally, here or elsewhere, we'd inspect the resulting statement to ensure all 
the knick-knacks are set as you expect: flags, fields, etc. Ensure that 
predicates fire as you expect: is/is not a table action is an authorization 
action, etc.


http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@451
PS9, Line 451: AuthorizationConfig authzConfig = 
AuthorizationConfig.createHadoopGroupAuthConfig(
Maybe explain this a bit: what is covered in the file? What does this enabled? 
When should we use this form?


http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:

http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py@547
PS9, Line 547:   self.client.execute("refresh authorization")
Not specific to this test, but what do we return from this statement? Should we 
return a warning or message if no auth is configured? A success message or code 
if successful?



--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:42:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7842: Expose physical plan for unit testing

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11920 )

Change subject: IMPALA-7842: Expose physical plan for unit testing
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1549/ : 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/11920
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c615dbc1d14685a6966c5ca7538777cdc80b74d
Gerrit-Change-Number: 11920
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:31:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7659: Populate NULL count while computing column stats

Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/11565 )

Change subject: IMPALA-7659: Populate NULL count while computing column stats
..

Patch Set 7: Code-Review+1

(2 comments)

My vote is to get this in, then do three things:

1. Use IMPALA-7842 to add cardinality tests based on this feature.
2. Do some refresh metadata performance runs to check performance impact.
3. Tackle the NDV-does-or-does-not-include-nulls issue.

http://gerrit.cloudera.org:8080/#/c/11565/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11565/7//COMMIT_MSG@16
PS7, Line 16: Tests: Updated the affected tests to include the null counts.
> Can we add a couple of tests that verify that cardinality estimates for out
FWIW, IMPALA-7842 provides a starter set of cardinality tests based on exposing
the pre-Thrift plan tree. We can build on those if that patch goes in before
this one.

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@251
PS6, Line 251:
> Was discussing this with Paul offline. We thought that adjusting the NDV be
Agreed. Let's get this in, then tackle the NDV=0 as a separate issue.

I wonder, do we have any data about the original issue: any performance
slowness when adding this additional calculation? If a table has many columns,
and we add a null count for each, how much impact is there on refresh metadata
performance?

--
To view, visit http://gerrit.cloudera.org:8080/11565
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68f8b4c3756eb1980ce299a602a7d56db1e507a
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:18:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7842: Expose physical plan for unit testing

Hello Bharath Vissapragada, Philip Zeyliger, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7842: Expose physical plan for unit testing
..

IMPALA-7842: Expose physical plan for unit testing

The FrontEnd class uses a functional model to generate a plan: pass
in SQL text to the createExecRequest() method and get back a Thrift
plan ready for serialization.

For unit testing, however, we need access to the intermediate physical
plan tree produced by the planner. Inspecting only the Thrift version
results in loss of important details.

This fix introduces a new planner context (PlanCtx) class that passes
information into the planner, and passses the physical plan back out.
Code that used the prior version (pass in a query context and explain
string) are changed to use the new form.

Testing:
* There is no functional change, just a refactoring of the existing
  code. Ran all FE test to verify no regressions.
* Introduces a new test case that uses this feature to verify plan
  cardinality.

Change-Id: I4c615dbc1d14685a6966c5ca7538777cdc80b74d
---
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
A fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
5 files changed, 258 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/11920/7
--
To view, visit http://gerrit.cloudera.org:8080/11920
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c615dbc1d14685a6966c5ca7538777cdc80b74d
Gerrit-Change-Number: 11920
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12018 )

Change subject: IMPALA-7914: Base interface for statement-like AST nodes
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1548/ : 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/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:15:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7842: Expose physical plan for unit testing

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11920 )

Change subject: IMPALA-7842: Expose physical plan for unit testing
..


Patch Set 7:

(1 comment)

Bharath, thanks for the additional review. Corrected the comment you 
identified. Rebased on the latest master.

http://gerrit.cloudera.org:8080/#/c/11920/6/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11920/6/fe/src/main/java/org/apache/impala/service/Frontend.java@1225
PS6, Line 1225: Fills in the EXPLAIN string and option
> I think the explain string is not optional whereas the thrift plan is?
Done



--
To view, visit http://gerrit.cloudera.org:8080/11920
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c615dbc1d14685a6966c5ca7538777cdc80b74d
Gerrit-Change-Number: 11920
Gerrit-PatchSet: 7
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:11:04 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11890 )

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
..


Patch Set 3:

This patch touches just one FE unit test file. The failure is in the BE web 
server. Seems a reasonable conclusion that the C++ web server failure is not 
related to a Java unit test change.


--
To view, visit http://gerrit.cloudera.org:8080/11890
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:06:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes

Paul Rogers has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12018 )

Change subject: IMPALA-7914: Base interface for statement-like AST nodes
..


Patch Set 3:

(1 comment)

Bharath, thanks for the good documentation suggestion; went ahead and made the 
change.

http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:

http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS3, Line 25:  * Parse nodes divide into two broad categories: statement-like 
nodes (derived
:  * from this interface) and expression nodes (which derive from 
Expr.)
:  *
:  * Statement-like nodes form the structure of a query and mostly 
retain their
:  * structure during analysis. That is, they are "analyzed in 
place." By contrast,
:  * expressions are often rewritten so that the final expression 
out of the analyzer
:  * may be different than the expression received from the parser. 
As a result, the
:  * analyze interface differs between the two categories.
:  *
:  * Error reporting often wants to emit the user's original SQL 
expression before
:  * rewrites. Statements hold onto both the original and rewritten 
expressions.
:  * Expressions, by contrast don't know if they are original or 
rewritten.
> I think this overview makes more sense in the parent ParseNode class?
Done



--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:32:07 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7738: Implement timeouts for HDFS open calls

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11874 )

Change subject: IMPALA-7738: Implement timeouts for HDFS open calls
..


Patch Set 12:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1547/ : 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/11874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
Gerrit-Change-Number: 11874
Gerrit-PatchSet: 12
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:36:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7927: Enhance Rewritten SQL in test files

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12033 )

Change subject: IMPALA-7927: Enhance Rewritten SQL in test files
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1546/ : 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/12033
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I09ee971a7797c4154bafee5c080ac3c6a1057fc7
Gerrit-Change-Number: 12033
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:34:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes

Hello Bharath Vissapragada, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7914: Base interface for statement-like AST nodes
..

IMPALA-7914: Base interface for statement-like AST nodes

In order to integrate expression rewrites into the analysis phase, the
expression analyze() operation must be able to replace one expression
node with another. Statements, however, are analyzed in place. The two
types of parse nodes thus need different analyze() semantics.

To prepare for that goal, this patch introduces a new StmtNode interface
as the base for all statement-like AST nodes. The existing analyze()
method moves to StmtNode. While Expr still defines this method for now,
the future goal is to change the Expr analyze() semantics.

Tests: This is purely a code restructuring, no functional changes. Reran
all FE tests.

Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
A fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
18 files changed, 72 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/12018/4
--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7473: fix crash when printing decimal with -v=3

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12029 )

Change subject: IMPALA-7473: fix crash when printing decimal with -v=3
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3527/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/12029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I772681b78cda27d6c2086b91151ca8bdcd901828
Gerrit-Change-Number: 12029
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:23:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7473: fix crash when printing decimal with -v=3

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12029 )

Change subject: IMPALA-7473: fix crash when printing decimal with -v=3
..


Patch Set 3: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I772681b78cda27d6c2086b91151ca8bdcd901828
Gerrit-Change-Number: 12029
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:23:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 3: Verified+1

Build Successful

https://jenkins.impala.io/job/gerrit-docs-auto-test/173/ : Doc tests passed.


--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 20:19:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 3:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/173/

Testing docs change - this change appears to modify docs/ and no code. This is 
experimental - please report any issues to tarmstr...@cloudera.com or on this 
JIRA: IMPALA-7317


--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 19:44:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12024 )

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1545/ : 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/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Wed, 05 Dec 2018 19:44:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 3: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:59:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Alex Rodoni has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..

IMPALA-5605: [DOCS] Document how to increase memory resource limit

Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Reviewed-on: http://gerrit.cloudera.org:8080/12032
Reviewed-by: Alex Rodoni 
Tested-by: Alex Rodoni 
---
M docs/topics/impala_troubleshooting.xml
1 file changed, 26 insertions(+), 10 deletions(-)

Approvals:
  Alex Rodoni: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 3: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:57:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Hello Matthew Mulder, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..

IMPALA-5605: [DOCS] Document how to increase memory resource limit

Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
---
M docs/topics/impala_troubleshooting.xml
1 file changed, 26 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/12032/3
--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12032/2/docs/topics/impala_troubleshooting.xml
File docs/topics/impala_troubleshooting.xml:

http://gerrit.cloudera.org:8080/#/c/12032/2/docs/topics/impala_troubleshooting.xml@105
PS2, Line 105: the thread counts limit
> maybe "a resource limit".
Done



--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:57:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7738: Implement timeouts for HDFS open calls

2018-12-05 Thread Joe McDonnell (Code Review)

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11874 )

Change subject: IMPALA-7738: Implement timeouts for HDFS open calls
..


Patch Set 12: Code-Review+2

Carry +2


--
To view, visit http://gerrit.cloudera.org:8080/11874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
Gerrit-Change-Number: 11874
Gerrit-PatchSet: 12
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:31:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7473: fix crash when printing decimal with -v=3

2018-12-05 Thread Philip Zeyliger (Code Review)

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12029 )

Change subject: IMPALA-7473: fix crash when printing decimal with -v=3
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I772681b78cda27d6c2086b91151ca8bdcd901828
Gerrit-Change-Number: 12029
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:38:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1048: show sinks in exec summary

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11967 )

Change subject: IMPALA-1048: show sinks in exec summary
..


Patch Set 13:

Anyone want to +2?


--
To view, visit http://gerrit.cloudera.org:8080/11967
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3fdf7bacae8ff597b255da65af453e174ba53544
Gerrit-Change-Number: 11967
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:32:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7473: fix crash when printing decimal with -v=3

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12029 )

Change subject: IMPALA-7473: fix crash when printing decimal with -v=3
..


Patch Set 2: Code-Review+1

Carry +1. Anyone want to +2?


--
To view, visit http://gerrit.cloudera.org:8080/12029
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I772681b78cda27d6c2086b91151ca8bdcd901828
Gerrit-Change-Number: 12029
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:32:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7738: Implement timeouts for HDFS open calls

2018-12-05 Thread Joe McDonnell (Code Review)

Hello Michael Ho, Philip Zeyliger, Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7738: Implement timeouts for HDFS open calls
..

IMPALA-7738: Implement timeouts for HDFS open calls

This is part 1 of a push to add timeouts for all HDFS operations.
It adds timeouts for opening an HDFS file handle.

It introduces a new SynchronousThreadPool, which executes
an operation in a thread pool and waits up to a specified
timeout for the operation to complete. This type of thread
pool can accept any subclass of SynchronousWorkItem, and
a single thread pool can process different types of work
items. It is tested by a new test case in thread-pool-test.

This also introduces a new HdfsMonitor which implements
timeouts for HDFS operations, currently limited to
hdfsOpenFile(). This is implemented using a SynchronousThreadPool.
The timeout for hdfs operations is specified by
hdfs_operation_timeout_sec, which defaults to 5 minutes.

Testing:
1. Added a test to thread-pool-test for the new
   SynchronousThreadPool.
2. Core tests
3. Added a custom cluster test that does "kill -STOP"
   for the NameNode and verifies that a subsequent
   hdfsOpenFile operation times out.

Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
---
M be/src/common/status.h
M be/src/exec/base-sequence-scanner.cc
M be/src/runtime/io/CMakeLists.txt
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/hdfs-file-reader.h
A be/src/runtime/io/hdfs-monitored-ops.cc
A be/src/runtime/io/hdfs-monitored-ops.h
M be/src/runtime/runtime-state.cc
M be/src/util/thread-pool-test.cc
M be/src/util/thread-pool.h
M common/thrift/generate_error_codes.py
A tests/custom_cluster/test_hdfs_timeout.py
16 files changed, 582 insertions(+), 86 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/11874/12
--
To view, visit http://gerrit.cloudera.org:8080/11874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
Gerrit-Change-Number: 11874
Gerrit-PatchSet: 12
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-7738: Implement timeouts for HDFS open calls

2018-12-05 Thread Joe McDonnell (Code Review)

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11874 )

Change subject: IMPALA-7738: Implement timeouts for HDFS open calls
..


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11874/11/be/src/util/thread-pool.h
File be/src/util/thread-pool.h:

http://gerrit.cloudera.org:8080/#/c/11874/11/be/src/util/thread-pool.h@272
PS11, Line 272:   /// Otherwise, it returns the status returned by 
ExecuteImpl().
> s/ExecuteImpl/Execute?
Done


http://gerrit.cloudera.org:8080/#/c/11874/11/tests/custom_cluster/test_hdfs_timeout.py
File tests/custom_cluster/test_hdfs_timeout.py:

http://gerrit.cloudera.org:8080/#/c/11874/11/tests/custom_cluster/test_hdfs_timeout.py@43
PS11, Line 43: # Find the NameNode's pid via jps
 : jps_output = check_output(["jps"])
 : namenode_pid = None
 : for line in jps_output.split("\n"):
 :   pid, procname = line.split(" ")
 :   if procname == "NameNode":
 : namenode_pid = pid
 : break
 : assert(namenode_pid is not None)
> This is totally fine. You can semi-equivalently do: "pgrep -f namenode.Name
That's cleaner, switched this over to that.



--
To view, visit http://gerrit.cloudera.org:8080/11874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia14403ca5f3f19c6d5f61b9ab2306b0ad3267454
Gerrit-Change-Number: 11874
Gerrit-PatchSet: 11
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:31:04 +
Gerrit-HasComments: Yes

[native-toolchain-CR] IMPALA-7924: Add Thrift 0.11.0 to the toolchain

2018-12-05 Thread Philip Zeyliger (Code Review)

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12034 )

Change subject: IMPALA-7924: Add Thrift 0.11.0 to the toolchain
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idb07f43e70844b11ec79c708cab3820628a44cd2
Gerrit-Change-Number: 12034
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:22:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5605: [DOCS] Document how to increase memory resource limit

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12032 )

Change subject: IMPALA-5605: [DOCS] Document how to increase memory resource 
limit
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12032/2/docs/topics/impala_troubleshooting.xml
File docs/topics/impala_troubleshooting.xml:

http://gerrit.cloudera.org:8080/#/c/12032/2/docs/topics/impala_troubleshooting.xml@105
PS2, Line 105: the thread counts limit
maybe "a resource limit".



--
To view, visit http://gerrit.cloudera.org:8080/12032
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dde3b87f4c809df74a01952206dbf69c111e333
Gerrit-Change-Number: 12032
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:14:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code

2018-12-05 Thread Philip Zeyliger (Code Review)

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12036 )

Change subject: IMPALA-7924: Generate Thrift 11 Python Code
..


Patch Set 1: Code-Review+1

(2 comments)

Thanks! Only minor stuff from me.

http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@159
PS1, Line 159: message("Using Thrift 11 compiler: ${THRIFT_COMPILER}")
THRIFT11_COMPILER here, probably.


http://gerrit.cloudera.org:8080/#/c/12036/1/common/thrift/CMakeLists.txt@180
PS1, Line 180: set(THRIFT11_PYTHON_ARGS ${THRIFT_INCLUDE_DIR_OPTION} -r --gen 
py -o )
 : set(THRIFT11_PYTHON_ARGS ${THRIFT11_PYTHON_ARGS} 
${THRIFT11_PYTHON_OUTPUT_DIR})
Very minor: you can probably use "\" to do line continuation or just stuff this 
in a single line and ignore character limit. It's a bit hard to parse that this 
is just "+=" effectively.

https://cmake.org/cmake/help/v3.0/manual/cmake-language.7.html#quoted-argument



--
To view, visit http://gerrit.cloudera.org:8080/12036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
Gerrit-Change-Number: 12036
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:12:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12001 )

Change subject: IMPALA-7902: NumericLiteral fixes, refactoring
..


Patch Set 10: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/12001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2
Gerrit-Change-Number: 12001
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:13:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7659: Populate NULL count while computing column stats

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11565 )

Change subject: IMPALA-7659: Populate NULL count while computing column stats
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11565/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11565/7//COMMIT_MSG@16
PS7, Line 16: Tests: Updated the affected tests to include the null counts.
Can we add a couple of tests that verify that cardinality estimates for output 
of "IS NULL" and "IS NOT NULL" are correct?

It would also be good to do a sanity check of the compute stats performance 
change on a largish table. I'd expect the difference will be small but we 
should confirm.



--
To view, visit http://gerrit.cloudera.org:8080/11565
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68f8b4c3756eb1980ce299a602a7d56db1e507a
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 05 Dec 2018 18:11:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7927: Enhance Rewritten SQL in test files

Paul Rogers has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12033


Change subject: IMPALA-7927: Enhance Rewritten SQL in test files
..

IMPALA-7927: Enhance Rewritten SQL in test files

Makes two changes to the rewritten SQL shown in .test files:

1. Show the rewritten HAVING clause. (We have two copies. Before, the
   rewritten SQL displayed the unrewritten copy.)

2. Smap substitution replaces expressions with references to
   materialized slots. This replacement was hidden in the rewritten SQL.
   This change makes the replacement explicit.

Example of HAVING with the above changes:

HAVING $ao$1 /* sum(2 + id) */ > CAST(1 AS BIGINT)

The $ao$1 is the reference to the Aggregation Output slot 1. The column
label (an expression here) is shown in a quote for reference.

More detail in the JIRA ticket.

Testing: Affects only .test files. Reran all FE tests and updated .test
files as needed.

Change-Id: I09ee971a7797c4154bafee5c080ac3c6a1057fc7
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
9 files changed, 83 insertions(+), 58 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/33/12033/1
--
To view, visit http://gerrit.cloudera.org:8080/12033
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I09ee971a7797c4154bafee5c080ac3c6a1057fc7
Gerrit-Change-Number: 12033
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Paul Rogers

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12024 )

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1543/ : 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/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Wed, 05 Dec 2018 17:12:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12036 )

Change subject: IMPALA-7924: Generate Thrift 11 Python Code
..


Patch Set 1:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/1544/ : Initial code 
review checks failed. See linked job for details on the failure.


--
To view, visit http://gerrit.cloudera.org:8080/12036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
Gerrit-Change-Number: 12036
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:56:58 +
Gerrit-HasComments: No

[Impala-ASF-CR](asf-site) Download 3.1.0

2018-12-05 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12010 )

Change subject: Download 3.1.0
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12010
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d0dd660e077858f40a7cf78ad4a479626c1f46
Gerrit-Change-Number: 12010
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:45:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12024 )

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/12024/2/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/12024/2/tests/common/impala_test_suite.py@838
PS2, Line 838: assert expected_state == actual_state
> How about raising Timeout() from common/errors.py, or creating your own exc
Done.

Using common.errors.Timeout


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@269
PS2, Line 269: wait_for_state
> I feel that "expected_state" would be clearer.
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@295
PS2, Line 295:  "create table {0}.foo as 
{1}".format(unique_database, sleep_query)]:
> I think this would look cleaner if you define the second query outside of t
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@296
PS2, Line 296:   response_json =\
> nit: wrap after the opening (
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@305
PS2, Line 305: for key in ['cpu_user_s', 'rpc_latency', 
'num_remaining_instances',
> Maybe define those as expected_backend_states at the top of the function. T
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@313
PS2, Line 313: response_json =\
> nit: wrap after the opening ( instead.
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@318
PS2, Line 318:   def test_backend_instances(self, unique_database, 
query_options=None):
> Please check the comments on the previous function for applicability here.
Done


http://gerrit.cloudera.org:8080/#/c/12024/2/tests/webserver/test_web_pages.py@354
PS2, Line 354: dict
> Make this a named parameter to make the core more robust against future cha
Done



--
To view, visit http://gerrit.cloudera.org:8080/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:41:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

Hello Lars Volker, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..

IMPALA-7625: test_web_pages.py backend tests are failing

The backend tests in test_web_page.py do some basic validation of
the query_backends and query_finstances endpoints. The tests were
failing due to a race condition in the test itself. The issue is that
the backend endpoints are only usable while a query is running. When
queried against completed queries, the backend endpoints return nothing.
The original test worked by running a "complex" query asynchronously and
then querying the backend endpoints before the query completed. The
assumption was that the query would run long enough for the request to
the backend endpoint to complete. However, that is not always the case
and the query easily completes before the backend endpoints can be
reached.

This patch updates the test so that instead of running a "complex"
query, it runs a query that calls the sleep UDF, guaranteeing the query
will take enough time for the backend endpoints to be reached.

Furthermore, the patch adds additional validation and cleanup of all the
backend endpoint tests.

Testing:

Ran core tests.

Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
---
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_admission_controller.py
M tests/webserver/test_web_pages.py
3 files changed, 79 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/12024/4
--
To view, visit http://gerrit.cloudera.org:8080/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] IMPALA-7924: Generate Thrift 11 Python Code

Sahil Takiar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12036


Change subject: IMPALA-7924: Generate Thrift 11 Python Code
..

IMPALA-7924: Generate Thrift 11 Python Code

Upgrades the version of the toolchain in order to pull in Thrift 0.11.0.
Updates the CMake build to write generated Python code using Thrift 11
to shell/build/thrift-11-gen/gen-py/.

The Thrift 11 Python deserialization code has some big performance
improvements that allow faster parsing of runtime profiles. By adding
the ability to generate the Thrift Python code using Thrift 11 we can
take advantage of the Python performance improvements without going
through a full Thrift upgrade from 0.9 to 0.11.

Testing:
- Ran core tests

Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
---
M CMakeLists.txt
M bin/impala-config.sh
M common/thrift/CMakeLists.txt
3 files changed, 51 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/12036/1
--
To view, visit http://gerrit.cloudera.org:8080/12036
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3432c3e29d28ec3ef6a0a22156a18910f511fed0
Gerrit-Change-Number: 12036
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

Hello Lars Volker, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..

IMPALA-7625: test_web_pages.py backend tests are failing

The backend tests in test_web_page.py do some basic validation of
the query_backends and query_finstances endpoints. The tests were
failing due to a race condition in the test itself. The issue is that
the backend endpoints are only usable while a query is running. When
queried against completed queries, the backend endpoints return nothing.
The original test worked by running a "complex" query asynchronously and
then querying the backend endpoints before the query completed. The
assumption was that the query would run long enough for the request to
the backend endpoint to complete. However, that is not always the case
and the query easily completes before the backend endpoints can be
reached.

This patch updates the test so that instead of running a "complex"
query, it runs a query that calls the sleep UDF, guaranteeing the query
will take enough time for the backend endpoints to be reached.

Furthermore, the patch adds additional validation and cleanup of all the
backend endpoint tests.

Testing:

Ran core tests.

Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
---
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_admission_controller.py
M tests/webserver/test_web_pages.py
3 files changed, 79 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/12024/3
--
To view, visit http://gerrit.cloudera.org:8080/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] IMPALA-7659: Populate NULL count while computing column stats

2018-12-05 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11565 )

Change subject: IMPALA-7659: Populate NULL count while computing column stats
..


Patch Set 6:

(1 comment)

Any reviews on this one? Planning to get this one in before others keep 
updating these tests.

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/11565/6/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@251
PS6, Line 251:   " IS NULL THEN 1 ELSE NULL END)");
> I thought you were doing it in a separate change? https://gerrit.cloudera.o
Was discussing this with Paul offline. We thought that adjusting the NDV before 
writing the stats to HMS is problematic since it might affect other SQL engines 
(like Presto/Hive etc). We don't know if they expect the null count to be 
adjusted already.



--
To view, visit http://gerrit.cloudera.org:8080/11565
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic68f8b4c3756eb1980ce299a602a7d56db1e507a
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:32:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7625: test web pages.py backend tests are failing

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12024 )

Change subject: IMPALA-7625: test_web_pages.py backend tests are failing
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12024/3/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/12024/3/tests/common/impala_test_suite.py@840
PS3, Line 840: %
flake8: E501 line too long (91 > 90 characters)



--
To view, visit http://gerrit.cloudera.org:8080/12024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2092afe1bfaec30cd3e7b0040f06865e43fe62fb
Gerrit-Change-Number: 12024
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:38:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7902: NumericLiteral fixes, refactoring

2018-12-05 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12001 )

Change subject: IMPALA-7902: NumericLiteral fixes, refactoring
..


Patch Set 10:

Tim, I can take a look at this one.


--
To view, visit http://gerrit.cloudera.org:8080/12001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2
Gerrit-Change-Number: 12001
Gerrit-PatchSet: 10
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:34:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7914: Base interface for statement-like AST nodes

2018-12-05 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12018 )

Change subject: IMPALA-7914: Base interface for statement-like AST nodes
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:

http://gerrit.cloudera.org:8080/#/c/12018/3/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@25
PS3, Line 25:  * Parse nodes divide into two broad categories: statement-like 
nodes (derived
:  * from this interface) and expression nodes (which derive from 
Expr.)
:  *
:  * Statement-like nodes form the structure of a query and mostly 
retain their
:  * structure during analysis. That is, they are "analyzed in 
place." By contrast,
:  * expressions are often rewritten so that the final expression 
out of the analyzer
:  * may be different than the expression received from the parser. 
As a result, the
:  * analyze interface differs between the two categories.
:  *
:  * Error reporting often wants to emit the user's original SQL 
expression before
:  * rewrites. Statements hold onto both the original and rewritten 
expressions.
:  * Expressions, by contrast don't know if they are original or 
rewritten.
I think this overview makes more sense in the parent ParseNode class?



--
To view, visit http://gerrit.cloudera.org:8080/12018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie565ff02ad74f805a667017ba9bc8c0a2697a97b
Gerrit-Change-Number: 12018
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:26:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7842: Expose physical plan for unit testing

2018-12-05 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11920 )

Change subject: IMPALA-7842: Expose physical plan for unit testing
..


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11920/6/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11920/6/fe/src/main/java/org/apache/impala/service/Frontend.java@1225
PS6, Line 1225: Optionally fills in the EXPLAIN string
I think the explain string is not optional whereas the thrift plan is?



--
To view, visit http://gerrit.cloudera.org:8080/11920
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c615dbc1d14685a6966c5ca7538777cdc80b74d
Gerrit-Change-Number: 11920
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:19:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12004 )

Change subject: IMPALA-7889: Write new logical types in Parquet
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1542/ : 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/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:07:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging

2018-12-05 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11890 )

Change subject: IMPALA-7821: Refactor ExprRewriteRulesTest for easier debugging
..


Patch Set 3:

Gabor, it is not clear from the jira if the crash is deterministic. Should we 
wait for it to be fixed or can we trigger another run?


--
To view, visit http://gerrit.cloudera.org:8080/11890
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99a28b170bc0132424041e51f61ebe5c848c4083
Gerrit-Change-Number: 11890
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Paul Rogers 
Gerrit-Comment-Date: Wed, 05 Dec 2018 16:06:22 +
Gerrit-HasComments: No

[native-toolchain-CR] IMPALA-7924: Add Thrift 0.11.0 to the toolchain

Sahil Takiar has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12034


Change subject: IMPALA-7924: Add Thrift 0.11.0 to the toolchain
..

IMPALA-7924: Add Thrift 0.11.0 to the toolchain

Adds Thrift 0.11.0 to the toolchain. One patch from 0.9.3 is carried
over that preserves compatibility forward compatibility of TLS
protocols. Another path was added (THRIFT-4417), which is required for
fb303.thrift C++ code to build.

Change-Id: Idb07f43e70844b11ec79c708cab3820628a44cd2
---
M buildall.sh
A source/thrift/thrift-0.11.0-patches/0001-TLS-forward-compatibility.patch
A source/thrift/thrift-0.11.0-patches/0002-THRIFT-4417.patch
3 files changed, 82 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/34/12034/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12034
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idb07f43e70844b11ec79c708cab3820628a44cd2
Gerrit-Change-Number: 12034
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar

[Impala-ASF-CR](asf-site) Download 3.1.0

2018-12-05 Thread Zoltan Borok-Nagy (Code Review)

Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12010 )

Change subject: Download 3.1.0
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12010/1/downloads.html
File downloads.html:

http://gerrit.cloudera.org:8080/#/c/12010/1/downloads.html@153
PS1, Line 153:
> Why not include 3.0.1 here?
Just wasn't sure about it. Added it.



--
To view, visit http://gerrit.cloudera.org:8080/12010
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2d0dd660e077858f40a7cf78ad4a479626c1f46
Gerrit-Change-Number: 12010
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 05 Dec 2018 15:55:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR](asf-site) Download 3.1.0

2018-12-05 Thread Zoltan Borok-Nagy (Code Review)

Hello Jim Apple, Impala Public Jenkins,

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

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

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

Change subject: Download 3.1.0
..

Download 3.1.0

Updated download links for release 3.1.0

Change-Id: Id2d0dd660e077858f40a7cf78ad4a479626c1f46
---
M downloads.html
1 file changed, 19 insertions(+), 9 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2d0dd660e077858f40a7cf78ad4a479626c1f46
Gerrit-Change-Number: 12010
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12004 )

Change subject: IMPALA-7889: Write new logical types in Parquet
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1541/ : 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/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 15:44:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12004 )

Change subject: IMPALA-7889: Write new logical types in Parquet
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:

http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@388
PS5, Line 388: found = False
> You could add
Sorry, I forgot this one in patch set 6.



--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 15:33:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12004 )

Change subject: IMPALA-7889: Write new logical types in Parquet
..


Patch Set 6:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/hdfs-parquet-table-writer.cc
File be/src/exec/parquet/hdfs-parquet-table-writer.cc:

http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/hdfs-parquet-table-writer.cc@973
PS5, Line 973: parquet::SchemaElement& col_schema = file_metadata_.schema[i 
+ 1];
> Maybe choose a better name here?
I choose 'col_schema' -  I can look for a new name if others do not like it. 
'schema_element' would be more exact for someone who know Parquet well, but for 
other people 'col_schema' is more informative in my opinion (+ it is shorter).


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.h
File be/src/exec/parquet/parquet-metadata-utils.h:

http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.h@62
PS5, Line 62:   static void FillSchemaElement(const ColumnType& col_type,
> We usually put input parameters first, then output, i.e. move node to the e
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc
File be/src/exec/parquet/parquet-metadata-utils.cc:

http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@153
PS5, Line 153:   return Status::OK();;
> This can be in the anonymous namespace, too
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@283
PS5, Line 283: ErrorMsg msg(TErrorCode::PARQUET_INCOMPATIBLE_DECIMAL, 
filename, schema_element.name,
> Move to the anonymous namespace. Pass output parameters by pointer and move
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@292
PS5, Line 292: th'
> Can you think of a better name for the schema element than "node"?
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@297
PS5, Line 297:   parquet::LogicalType logical_type;
> Should this be a case statement instead?
I think that the special handling needed for strings causes switch/case to 
loose its benefits. I can do it in the next change if you still prefer it.


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@310
PS5, Line 310: 
col_schema->__set_type_length(ParquetPlainEncoder::DecimalSize(col_type));
> Move else to previous line.
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/be/src/exec/parquet/parquet-metadata-utils.cc@311
PS5, Line 311: col_schema->__set_scale(col_type.scale);
> Please add a brief comment that VARCHAR and CHAR are always UTF8 annotated,
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:

http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@362
PS5, Line 362:
> Add function comment (here and for all other new functions
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@376
PS5, Line 376:   @staticmethod
> Maybe you can clean up some of the other uses of os.walk() in this file?
Done - test_def_level_encoding uses os.walk a bit differently (call 
be/util/parquet-reader instead of python functions), so I did not change it.


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@394
PS5, Line 394: assert val is None
> Can you replace the values with the names from parquet.thrift?
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@397
PS5, Line 397:   def _check_no_logical_type(self, schemas, column_name):
> The other types have to be None, right? If so, maybe create a helper _check
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/query_test/test_insert_parquet.py@432
PS5, Line 432: lf
> nit: once
Done


http://gerrit.cloudera.org:8080/#/c/12004/5/tests/util/get_parquet_metadata.py
File tests/util/get_parquet_metadata.py:

http://gerrit.cloudera.org:8080/#/c/12004/5/tests/util/get_parquet_metadata.py@181
PS5, Line 181: for f in files:
> Add (as some uses of this in test_insert_parquet.py do):
Done



--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Wed, 05 Dec 2018 15:26:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

Hello Lars Volker, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7889: Write new logical types in Parquet
..

IMPALA-7889: Write new logical types in Parquet

Fill the LogicalType field in Parquet schemas for columns
that have an associated logical type. ConvertedType still
has to be filled to remain compatible with older readers.

Testing:
- added new tests to check both logical and converted types
  to test_insert_parquet.py

Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/exec/parquet/parquet-metadata-utils.h
M tests/query_test/test_insert_parquet.py
M tests/util/get_parquet_metadata.py
5 files changed, 228 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/7
--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] IMPALA-7889: Write new logical types in Parquet

Hello Lars Volker, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7889: Write new logical types in Parquet
..

IMPALA-7889: Write new logical types in Parquet

Fill the LogicalType field in Parquet schemas for columns
that have an associated logical type. ConvertedType still
has to be filled to remain compatible with older readers.

Testing:
- added new tests to check both logical and converted types
  to test_insert_parquet.py

Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/exec/parquet/parquet-metadata-utils.h
M tests/query_test/test_insert_parquet.py
M tests/util/get_parquet_metadata.py
5 files changed, 225 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12004/6
--
To view, visit http://gerrit.cloudera.org:8080/12004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91
Gerrit-Change-Number: 12004
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker

[Impala-ASF-CR] Build parquet-reader earlier in test-with-docker

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12025 )

Change subject: Build parquet-reader earlier in test-with-docker
..


Patch Set 3: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/12025
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee141ad8b2a700378133a37498e74ddc306dfd57
Gerrit-Change-Number: 12025
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 14:43:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] Build parquet-reader earlier in test-with-docker

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12025 )

Change subject: Build parquet-reader earlier in test-with-docker
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3526/ 
DRY_RUN=false


--
To view, visit http://gerrit.cloudera.org:8080/12025
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee141ad8b2a700378133a37498e74ddc306dfd57
Gerrit-Change-Number: 12025
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 05 Dec 2018 14:43:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7853: Add support to read int64 NANO timestamps from Parquet

2018-12-05 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11984 )

Change subject: IMPALA-7853: Add support to read int64 NANO timestamps from 
Parquet
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1540/ : 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/11984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I932396d8646f43c0b9ca4a6359f164c4d8349d8f
Gerrit-Change-Number: 11984
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 05 Dec 2018 14:29:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7853: Add support to read int64 NANO timestamps from Parquet

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11984 )

Change subject: IMPALA-7853: Add support to read int64 NANO timestamps from 
Parquet
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11984/2/be/src/exec/parquet-common.cc
File be/src/exec/parquet-common.cc:

http://gerrit.cloudera.org:8080/#/c/11984/2/be/src/exec/parquet-common.cc@148
PS2, Line 148:   if (e.type == parquet::Type::INT96 && 
convert_int96_timestamps) needs_conversion = true;
> Still, if you have a function to set variables based on the schema, that fo
I have added convert_int96_timestamps as a parameter to 
GetTimestampInfoFromSchema. If you think that it looks worse now, I can revert 
to the original solution or try to find a new one.



--
To view, visit http://gerrit.cloudera.org:8080/11984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I932396d8646f43c0b9ca4a6359f164c4d8349d8f
Gerrit-Change-Number: 11984
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 05 Dec 2018 13:58:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7853: Add support to read int64 NANO timestamps from Parquet