[Impala-ASF-CR] IMPALA-5949: fix test exchange small delay failure

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8111 )

Change subject: IMPALA-5949: fix test_exchange_small_delay failure
..

IMPALA-5949: fix test_exchange_small_delay failure

Avoid running the problematic query with short delays. This combination
doesn't add coverage - the short delay was meant to test behaviour when
multiple batches were sent, but there are deliberately no batches sent
with this query.

Testing:
Ran a build against Isilon, which succeeded. Ran the test in a loop
locally overnight.

Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Reviewed-on: http://gerrit.cloudera.org:8080/8111
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins
---
A 
testdata/workloads/functional-query/queries/QueryTest/exchange-delays-zero-rows.test
M testdata/workloads/functional-query/queries/QueryTest/exchange-delays.test
M tests/custom_cluster/test_exchange_delays.py
3 files changed, 14 insertions(+), 10 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Gerrit-Change-Number: 8111
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5949: fix test exchange small delay failure

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5949: fix test_exchange_small_delay failure
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Gerrit-Change-Number: 8111
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 22 Sep 2017 05:08:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Fri, 22 Sep 2017 03:45:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8117 )

Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..

IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

Testing:

Ran core tests

Perf:

Ran this query a few times:

  set num_nodes=1;
  set mt_dop=1;
  select min(l_returnflag), min(l_linestatus) from biglineitem;
  summary;

I saw a speedup in scan time from ~2.25s -> 2.11s on my machine.

Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Reviewed-on: http://gerrit.cloudera.org:8080/8117
Reviewed-by: Lars Volker 
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins
---
M be/src/exec/parquet-column-readers.cc
1 file changed, 31 insertions(+), 8 deletions(-)

Approvals:
  Lars Volker: Looks good to me, but someone else must approve
  Dan Hecht: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..

IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

Today, Impala populates the 'rawDataSize' property
during COMPUTE STATS for the purpose of extrapolating
row counts based on file sizes.

After this patch Impala will populate 'totalSize' instead of
'rawDataSize'. The 'rawDataSize' is not populated or used.

Intended meaning/use of tblproperties:
- rawDataSize' is the estimated in-memory size of a table
  (without encoding and compression)
- 'totalSize' represents the on-disk size

Using the fields correctly is important for compatibility
with other users of the HMS such as Hive and SparkSQL.
For example, SparkSQL relies on the 'totalSize' for
join ordering.

Testing:
- core/hdfs run passed

Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Reviewed-on: http://gerrit.cloudera.org:8080/8110
Tested-by: Impala Public Jenkins
Reviewed-by: Alex Behm 
---
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
4 files changed, 25 insertions(+), 25 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Alex Behm: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 22 Sep 2017 03:38:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 22 Sep 2017 02:07:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5949: fix test exchange small delay failure

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5949: fix test_exchange_small_delay failure
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1259/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Gerrit-Change-Number: 8111
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 22 Sep 2017 01:07:15 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5844: use a MemPool for expr local allocations

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5844: use a MemPool for expr local allocations
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@13
PS6, Line 13: * They are freed in bulk at various points in query execution.
> we don't have to do it that way though.
Managing the expr allocations in bulk like this makes sense to me - trying to 
track the lifetime of individual allocations would add a lot of complexity and 
runtime overhead and I think also be a breaking changes for the UDF interface.

The operator code is the only code that knows when the results of expr 
evaluation are no longer needed so it makes sense to me that it would be 
responsible for freeing them.

In a lot of cases it would be possible to free the local allocations more 
frequently, e.g. after each expr evaluation (e.g. for EvalConjuncts() where no 
var-len data can be returned) but I think that would add a lot of runtime 
overhead.


http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@31
PS6, Line 31: Remove FunctionContext::ReallocateLocal()
> is that a UDF breaking change?
It was never exposed in udf.h so should be fine - it was only added to support 
StringVal::Resize()


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h@324
PS6, Line 324: expr-managed
> what does that mean, especially given that it can be used by multiple evalu
Not sure of a better way of expressing it.

The lifetime of the memory is tied to the lifetime of the evaluators - it can 
only be freed when the evaluators are closed since it backs the FreePools and a 
few other things.


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h
File be/src/runtime/free-pool.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h@a69
PS6, Line 69:
> why does this go away?
It was moved to FunctionContext, which is the only user of FreePool. With the 
change to the local allocations coming from a MemPool they can't be intercepted 
here any more.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ba5a7542ed90a49a4b5586c040b5985a7d45b61
Gerrit-Change-Number: 8025
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 22 Sep 2017 01:06:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5949: fix test exchange small delay failure

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5949: fix test_exchange_small_delay failure
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Gerrit-Change-Number: 8111
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 22 Sep 2017 01:07:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8112 )

Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to 
HMS.
..

IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.

This improves an existing workaround for HIVE-12730 to avoid
having the HMS wipe stats as a side-effect of an alterTable()
RPC. The new workaround uses DO_NOT_UPDATE_STATS which is
the recommended way of telling the HMS not to recompute/reset
statistics on its own.

Testing:
- core/hdfs run passed

Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Reviewed-on: http://gerrit.cloudera.org:8080/8112
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 9 insertions(+), 6 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to 
HMS.
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 22 Sep 2017 00:53:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-1767 Adds predicate to test boolean values true, false, 
unknown.
..


Patch Set 1:

(11 comments)

Yup, this approach is the winner!

http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/main/cup/sql-parser.cup@2731
PS1, Line 2731: bool_test_expr ::=
More precisely this is a truth_test_pred, right? The rule should also be under 
predicate and not under non_pred_expr


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/main/cup/sql-parser.cup@2742
PS1, Line 2742:   if (!l.toUpperCase().equals("UNKNOWN")) {
add something like server_ident for handling UNKNOWN as an ident


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@612
PS1, Line 612:   public void TestIsNullOrBoolPredicates() throws 
AnalysisException {
Let's add the new tests under a separate TestTruthTestPredicate()


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@617
PS1, Line 617: String[] rhs_options = new String[] {"true", "false", 
"unknown"};
use Java camel-case style: rhsOptions, lhsOptions


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@628
PS1, Line 628:   String literal_lhs = rhs == "unknown" ? "null" : rhs;
rhsTruthVals? these are definitely not literals


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@629
PS1, Line 629:   String negated_rhs = "not " + rhs;
camel case


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@648
PS1, Line 648: AnalysisError("select ('foo' is true)",
also test the NOT variants since those produce a FunctionCallExpr with a 
different fn name


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:

http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@1449
PS1, Line 1449: ArrayList literals = new ArrayList();
Rename literals to boolTestOperations or truthTestVals? Most of these are 
actually keywords and definitely not literals.


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:

http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1257
PS1, Line 1257: // Boolean value test (expanded to istrue/false).
Truth value test


http://gerrit.cloudera.org:8080/#/c/8122/1/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1258
PS1, Line 1258: testToSql("select (true is true)", "SELECT (istrue(TRUE))");
Let's exhaustively try all possibilities for full coverage


http://gerrit.cloudera.org:8080/#/c/8122/1/testdata/workloads/functional-query/queries/QueryTest/exprs.test
File testdata/workloads/functional-query/queries/QueryTest/exprs.test:

http://gerrit.cloudera.org:8080/#/c/8122/1/testdata/workloads/functional-query/queries/QueryTest/exprs.test@74
PS1, Line 74: # boolean test: IS [NOT] (TRUE | FALSE | UNKNOWN)
These tests using literals only are more appropriate in expr-test.cc



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d5fba65ef6c87dfc55a25d2c45246f74eb48c40
Gerrit-Change-Number: 8122
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Fri, 22 Sep 2017 00:47:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5949: fix test exchange small delay failure

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5949: fix test_exchange_small_delay failure
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia75c42be2de600344de7af5a917d7843880ea6de
Gerrit-Change-Number: 8111
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Fri, 22 Sep 2017 00:19:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5844: use a MemPool for expr local allocations

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8025 )

Change subject: IMPALA-5844: use a MemPool for expr local allocations
..


Patch Set 6:

(5 comments)

Had a few high level questions first.

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@13
PS6, Line 13: * They are freed in bulk at various points in query execution.
we don't have to do it that way though.


http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@31
PS6, Line 31: Remove FunctionContext::ReallocateLocal()
is that a UDF breaking change?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h@324
PS6, Line 324: expr-managed
what does that mean, especially given that it can be used by multiple 
evaluators? maybe it helps to explain these in terms of their lifetimes?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exprs/scalar-expr-evaluator.h
File be/src/exprs/scalar-expr-evaluator.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exprs/scalar-expr-evaluator.h@74
PS6, Line 74: Evaluator-managed allocations from this
:   /// evaluator will be from 'expr_mem_pool' while local 
allocations will be from
:   /// 'local_mem_pool'.
that seems pretty tough to understand and distinguish.  what does 
"evaluator-managed" mean?  maybe more clear to explain this in terms of the 
lifetimes?  Oh, I guess that's what you mean by evaluator-managed -- has the 
lifetime of the evaluator?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h
File be/src/runtime/free-pool.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h@a69
PS6, Line 69:
why does this go away?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ba5a7542ed90a49a4b5586c040b5985a7d45b61
Gerrit-Change-Number: 8025
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:53:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1258/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:41:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8117 )

Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:39:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5599: Fix for mis-use of TimestampValue

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8084 )

Change subject: IMPALA-5599: Fix for mis-use of TimestampValue
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8084/4/be/src/util/time-test.cc
File be/src/util/time-test.cc:

http://gerrit.cloudera.org:8080/#/c/8084/4/be/src/util/time-test.cc@85
PS4, Line 85: );
for debugging, it might be helpful to log the 'now_s' raw value in case of 
failure, which you can do use << on the EXPECT_EQ()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9b0ae06f6d94968c87a199625aa3332b26988142
Gerrit-Change-Number: 8084
Gerrit-PatchSet: 4
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:26:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5538: Use explicit catalog versions for deleted objects

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7731 )

Change subject: IMPALA-5538: Use explicit catalog versions for deleted objects
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/statestore/statestore.cc@170
PS6, Line 170: total_value_size_bytes_ -= entry_it->second.value().size()
also, does that still make sense (now that we no longer do the SetValue(NULL) 
at line 176 in the old code?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93cb7a033dc8f0d3e0339394b36affe14523274c
Gerrit-Change-Number: 7731
Gerrit-PatchSet: 6
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:17:57 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5538: Use explicit catalog versions for deleted objects

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7731 )

Change subject: IMPALA-5538: Use explicit catalog versions for deleted objects
..


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/catalog/catalog-server.cc@308
PS6, Line 308: DCHECK_GE
should that be DCHECK_GT?  The old conditional to continue on was <= ...


http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/catalog/catalog.h
File be/src/catalog/catalog.h:

http://gerrit.cloudera.org:8080/#/c/7731/6/be/src/catalog/catalog.h@59
PS6, Line 59: Gets all Catalog objects and the metadata that is applicable for 
the given request.
:   /// Always returns all object names that exist in the Catalog, 
but allows for extended
:   /// metadata for objects that were modified after the specified 
version.
is that accurate?  from our discussion, my understanding was that only objects 
with version > from_version would be included.


http://gerrit.cloudera.org:8080/#/c/7731/5/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/7731/5/be/src/statestore/statestore.cc@177
PS5, Line 177:   }
 : }
> The behavior is topic specific and currently only subscribers of catalog-up
What I mean is that topics that are transient do not get values in deleted 
topics.  So, if a subscriber relies on value to process deletion, it better not 
register as transient, right?  Shouldn't that be made explicit?


http://gerrit.cloudera.org:8080/#/c/7731/6/common/thrift/CatalogInternalService.thrift
File common/thrift/CatalogInternalService.thrift:

http://gerrit.cloudera.org:8080/#/c/7731/6/common/thrift/CatalogInternalService.thrift@32
PS6, Line 32:  relative to
does it include objects with changes in version > or >= the from_version?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93cb7a033dc8f0d3e0339394b36affe14523274c
Gerrit-Change-Number: 7731
Gerrit-PatchSet: 6
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:11:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8117 )

Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:06:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5599: Fix for mis-use of TimestampValue

[Zoram Thanga (Code Review)]

Hello Matthew Jacobs, Dan Hecht,

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

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

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

Change subject: IMPALA-5599: Fix for mis-use of TimestampValue
..

IMPALA-5599: Fix for mis-use of TimestampValue

The TimestampValue class is being used for non-database purposes
in many places, such as in log messages.

This change proposes to introduce APIs to convert Unix timetamps
into the corresponding date-time strings. We provide a series of
functions for different input time units, and also give the user
control over the precision of the output date-time string. APIs
are provided to convert to UTC and local time zones. The new APIs
can be used to replace (or instead of) TimestampValue::ToString()
in those places where Unix timestamps are being converted to
strings for printing.

The current commit implements the APIs and replaces calls to
TimestampValue::ToString() in be/src/service.

A new unit test, time-test, has been added to the back-end tests.

Other uses of TimestampValue in be/src/service, such as to track
start and end times of queries, etc., will be analyzed and changed
as required in a follow-up commit.

Change-Id: I9b0ae06f6d94968c87a199625aa3332b26988142
---
M be/src/service/impala-server.cc
M be/src/util/CMakeLists.txt
A be/src/util/time-test.cc
M be/src/util/time.cc
M be/src/util/time.h
5 files changed, 272 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9b0ae06f6d94968c87a199625aa3332b26988142
Gerrit-Change-Number: 8084
Gerrit-PatchSet: 4
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Zoram Thanga 

Re: [Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[黄权隆]

Such a syntax error may occur if a script is updated while it's running.

2017-09-21 3:03 GMT+08:00 Tianyi Wang (Code Review) :

> Tianyi Wang has posted comments on this change.
>
> Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
> ..
>
>
> Patch Set 4:
>
> > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1238/
>
> The build failed on building stage. The error message is
> "./bin/jenkins/build-all-flag-combinations.sh: line 59: syntax error near
> unexpected token `fi". Pretty strange since the script is not touched and
> it builds on my machine.
>
> --
> To view, visit http://gerrit.cloudera.org:8080/8063
> To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
>
> Gerrit-MessageType: comment
> Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
> Gerrit-PatchSet: 4
> Gerrit-Project: Impala-ASF
> Gerrit-Branch: master
> Gerrit-Owner: Tianyi Wang 
> Gerrit-Reviewer: Alex Behm 
> Gerrit-Reviewer: Impala Public Jenkins
> Gerrit-Reviewer: Philip Zeyliger 
> Gerrit-Reviewer: Thomas Tauber-Marshall 
> Gerrit-Reviewer: Tianyi Wang 
> Gerrit-HasComments: No
>

[Impala-ASF-CR] fix query report generator typo

[Michael Brown (Code Review)]

Michael Brown has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8118 )

Change subject: fix query report generator typo
..

fix query report generator typo

Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
Reviewed-on: http://gerrit.cloudera.org:8080/8118
Reviewed-by: Alex Behm 
Tested-by: Michael Brown 
---
M tests/comparison/leopard/templates/index.template
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Michael Brown: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
Gerrit-Change-Number: 8118
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] fix query report generator typo

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8118 )

Change subject: fix query report generator typo
..


Patch Set 1: Verified+1

verified using a GVD wrapper suitable for Impala contributors: 
https://jenkins.impala.io/job/gerrit-verify-dryrun-external/9/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
Gerrit-Change-Number: 8118
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 21 Sep 2017 22:57:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5250: Unify decompressor output length semantics

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8030 )

Change subject: IMPALA-5250: Unify decompressor output_length semantics
..

IMPALA-5250: Unify decompressor output_length semantics

This patch makes the semantics of the output_length parameter in
Codec::ProcessBlock to be the same across all codecs. In existing code
different decompressor treats output_length differently:
1. SnappyDecompressor needs output_length to be greater than or equal
   to the actual decompressed length, but it does not set it to the
   actual decompressed length after decompression.
2. SnappyBlockDecompressor and Lz4Decompressor require output_length to
   be exactly the same as the actual decompressed length, otherwise
   decompression fails.
3. Other decompressors need output_length to be greater than or equal to
   the actual decompressed length and will set it to actual decompressed
   length if oversized.
This inconsistency leads to a bug where the error message is
undeterministic when the compressed block is corrupted. This patch makes
all decompressor behave like a modified version of 3:
Output_length should be greater than or equal to the actual decompressed
length and it will be set to actual decompressed length if oversized. A
decompression failure sets it to 0.
Lz4Decompressor will use the "safe" instead of the "fast" decompression
function, for the latter is insecure with corrupted data and requires
the decompressed length to be known.

Testing: A testcase is added checking that the decompressors can handle
an oversized output buffer correctly. A regression test for the exact
case described in IMPALA-5250 is also added. A benchmark is run on a
16-node cluster testing the performance impact of the LZ4Decompressor
change and no performance regression is found.

Change-Id: Ifd42942b169921a7eb53940c3762bc45bb82a993
Reviewed-on: http://gerrit.cloudera.org:8080/8030
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/util/codec.h
M be/src/util/decompress-test.cc
M be/src/util/decompress.cc
3 files changed, 74 insertions(+), 45 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifd42942b169921a7eb53940c3762bc45bb82a993
Gerrit-Change-Number: 8030
Gerrit-PatchSet: 6
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1257/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 21 Sep 2017 22:02:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5870: Improve explain/profile output for partial sort

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8123


Change subject: IMPALA-5870: Improve explain/profile output for partial sort
..

IMPALA-5870: Improve explain/profile output for partial sort

A recent change (IMPALA-5498) added the ability to do partial sorts,
which divide their input up into runs each of which is sorted
individually, avoiding the need to spill. Some of the debug output
wasn't updated vs. regular sorts, leading to confusion.

For EXPLAIN, this patch removes the 'spill-buffer' mem-estimate for
partial sorts, since they can't spill. It does this by setting the
spillable buffer size in the resource profile to -1. Since the BE
relied on that number to determine the page size for sorts, it
now calculates the page size from the min reservation, which gives
an equivalent value.

For the runtime profile, it removes the counters 'SpilledRuns' and
'MergesPerformed' since they will always be 0, and it renames the
'IntialRunsCreated' counter to 'RunsCreated' since the 'Initial'
refers to the fact that in a regular sort those runs may be spilled
or merged.

It also adds a profile info string 'SortType' that can take the values
'Total', 'TopN', or 'Partial' to reflect the type of exec node being
used.

Example profile snippet for a partial sort:
SORT_NODE (id=2):(Total: 403.261us, non-child: 382.029us, % non-child: 94.73%)
 SortType: Partial
 ExecOption: Codegen Enabled
- NumRowsPerRun: (Avg: 44 (44) ; Min: 44 (44) ; Max: 44 (44) ; Number of 
samples: 1)
- InMemorySortTime: 34.201us
- PeakMemoryUsage: 2.02 MB (2117632)
- RowsReturned: 44 (44)
- RowsReturnedRate: 109.11 K/sec
- RunsCreated: 1 (1)
- SortDataSize: 572.00 B (572)

Testing:
- Manually ran several sorting queries and inspected their explain
  plans and profile

Change-Id: I2b15af78d8299db8edc44ff820c85db1cbe0be1b
---
M be/src/exec/partial-sort-node.cc
M be/src/exec/sort-node.cc
M be/src/exec/topn-node.cc
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M fe/src/main/java/org/apache/impala/planner/SortNode.java
6 files changed, 22 insertions(+), 9 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b15af78d8299db8edc44ff820c85db1cbe0be1b
Gerrit-Change-Number: 8123
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 

[Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8063 )

Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
..

IMPALA-5416: Fix an impala-shell command recursion bug

Impala-shell crashes with 2 source commands on the same line and runs
a command multiple times if it shares the same line with a source
command.
The bug is caused by a misuse of cmdqueue. The cmdqueue member of
cmd.Cmd is used to execute commands not directly from user input in an
event loop. When a 'source' is run, execute_query_list() is called which
also executes the commands in cmdqueue, causing them to be executed
twice.
The fix is for execute_query_list() to not run the commands in cmdqueue.
For the non-interactive case, where the event loop won't be run, we call
execute_query_list() with cmdqueue so that the commands get run.
A test case is added to test_shell_interactive.py.

Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Reviewed-on: http://gerrit.cloudera.org:8080/8063
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 10 insertions(+), 4 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Gerrit-Change-Number: 8063
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Gerrit-Change-Number: 8063
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 21:41:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5966: Fix the result file location of PlannerTest

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5966: Fix the result file location of PlannerTest
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20005bd0421e1bb9f3eedbb003c97d92a8faf110
Gerrit-Change-Number: 8113
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 21:31:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8117


Change subject: IMPALA-5965: avoid per-value switch on NeedsConversionInline() 
in parquet
..

IMPALA-5965: avoid per-value switch on NeedsConversionInline() in parquet

Testing:

Ran core tests

Perf:

Ran this query a few times:

  set num_nodes=1;
  set mt_dop=1;
  select min(l_returnflag), min(l_linestatus) from biglineitem;
  summary;

I saw a speedup in scan time from ~2.25s -> 2.11s on my machine.

Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
---
M be/src/exec/parquet-column-readers.cc
1 file changed, 31 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04fb4ca73978d0100e1eb9835b87d37540b8b645
Gerrit-Change-Number: 8117
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 

[Impala-ASF-CR] IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8122


Change subject: IMPALA-1767 Adds predicate to test boolean values true, false, 
unknown.
..

IMPALA-1767 Adds predicate to test boolean values true, false, unknown.

Adds a new expression to represent the following boolean predicate:
 IS [NOT] (TRUE | FALSE | UNKNOWN)
The expression is expanded in the parser to istrue/false for the checks
against true and false respectively and to isnull for the check against
unknown. Compared to the other approaches (rewrites, extended backend expr),
this change is the simplest. Main downside is that error messages are
in terms of the lowered expression.

Testing:
- fe: parser, tosql, analyze exprs
- e2e: query exprs

Change-Id: I9d5fba65ef6c87dfc55a25d2c45246f74eb48c40
---
M fe/src/main/cup/sql-parser.cup
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
5 files changed, 170 insertions(+), 8 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9d5fba65ef6c87dfc55a25d2c45246f74eb48c40
Gerrit-Change-Number: 8122
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-5908: Allow SET to unset modified query options.

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8070 )

Change subject: IMPALA-5908: Allow SET to unset modified query options.
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8070/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8070/9//COMMIT_MSG@29
PS9, Line 29: For request_pool, this means that setting the default
: request_pool via impalad command line is now a bad idea
> To do what? To have a default pool? That should rely on using the 'default'
okay



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8c383e68064f839cb5000118901dff77b4e5cb9
Gerrit-Change-Number: 8070
Gerrit-PatchSet: 9
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:56:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8110/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8110/1//COMMIT_MSG@9
PS1, Line 9: Today, Impala populates the 'rawDataSize' property
   : during COMPUTE STATS for the purpose of extrapolating
   : row counts based on file sizes.
   :
   : Intended meaning/use of tblproperties:
   : - rawDataSize' is the estimated in-memory size of a table
   :   (without encoding and compression)
   : - 'totalSize' represents the on-disk size
   :
   : Using the fields correctly is important for compatibility
   : with other users of the HMS such as Hive and SparkSQL.
   : For example, SparkSQL relies on the 'totalSize' for
   : join ordering.
> Although this is very informative, I don't think I understand what this com
The title says "Use totalSize tblproperty instead of rawDataSize".

Added extra info in commit msg body.


http://gerrit.cloudera.org:8080/#/c/8110/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/8110/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1178
PS1, Line 1178: droppedRawDataSize
> rename to droppedTotalSize to be consistent with the tblproperty being upda
Good catch. I missed that one.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:53:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Alex Behm (Code Review)]

Hello Bharath Vissapragada, Dimitris Tsirogiannis,

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

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

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

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..

IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

Today, Impala populates the 'rawDataSize' property
during COMPUTE STATS for the purpose of extrapolating
row counts based on file sizes.

After this patch Impala will populate 'totalSize' instead of
'rawDataSize'. The 'rawDataSize' is not populated or used.

Intended meaning/use of tblproperties:
- rawDataSize' is the estimated in-memory size of a table
  (without encoding and compression)
- 'totalSize' represents the on-disk size

Using the fields correctly is important for compatibility
with other users of the HMS such as Hive and SparkSQL.
For example, SparkSQL relies on the 'totalSize' for
join ordering.

Testing:
- core/hdfs run passed

Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
---
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
4 files changed, 25 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7954 )

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/exprs/timestamp-functions-ir.cc
File be/src/exprs/timestamp-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/exprs/timestamp-functions-ir.cc@332
PS2, Line 332:   StringVal result(context, 10);
why does this DCHECK no longer make sense?  are we saying it's trivially true 
now after check HasDate() and with the checks you've added in this commit?


http://gerrit.cloudera.org:8080/#/c/7954/4/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/4/be/src/runtime/timestamp-value.h@173
PS4, Line 173: boost::gregorian::date
i think it would be best to avoid running the constructor given these things 
can throw exceptions, so let's use 'const &' here.


http://gerrit.cloudera.org:8080/#/c/7954/4/be/src/runtime/timestamp-value.h@287
PS4, Line 287:
nit: extraneous space


http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h@86
PS2, Line 86:   /// Constructors that parse from a date/time string. See 
TimestampParser for details
> I think it would be better to do the logging on the caller side, because de
Logging in the caller is fine but I don't see that added to your diff.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:50:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove unused MemPool::peak allocated bytes

[Tim Armstrong (Code Review)]

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

Change subject: Remove unused MemPool::peak_allocated_bytes_
..


Patch Set 1: Verified+1

Build passed but submission failed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99eba01869914c1d1e0a6ed0cab039d904fecc02
Gerrit-Change-Number: 8114
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:47:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] Remove unused MemPool::peak allocated bytes

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8114 )

Change subject: Remove unused MemPool::peak_allocated_bytes_
..

Remove unused MemPool::peak_allocated_bytes_

The value is not used for anything but there is code devoted to updating
it and testing the value.

Testing:
Ran mem-pool-test to confirm it still works.

Change-Id: I99eba01869914c1d1e0a6ed0cab039d904fecc02
Reviewed-on: http://gerrit.cloudera.org:8080/8114
Reviewed-by: anujphadke 
Reviewed-by: Dan Hecht 
Tested-by: Tim Armstrong 
---
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
3 files changed, 2 insertions(+), 32 deletions(-)

Approvals:
  anujphadke: Looks good to me, but someone else must approve
  Dan Hecht: Looks good to me, approved
  Tim Armstrong: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I99eba01869914c1d1e0a6ed0cab039d904fecc02
Gerrit-Change-Number: 8114
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to 
HMS.
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1256/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:47:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8110 )

Change subject: IMPALA-5955: Use totalSize tblproperty instead of rawDataSize.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8110/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8110/1//COMMIT_MSG@9
PS1, Line 9: Today, Impala populates the 'rawDataSize' property
   : during COMPUTE STATS for the purpose of extrapolating
   : row counts based on file sizes.
   :
   : Intended meaning/use of tblproperties:
   : - rawDataSize' is the estimated in-memory size of a table
   :   (without encoding and compression)
   : - 'totalSize' represents the on-disk size
   :
   : Using the fields correctly is important for compatibility
   : with other users of the HMS such as Hive and SparkSQL.
   : For example, SparkSQL relies on the 'totalSize' for
   : join ordering.
Although this is very informative, I don't think I understand what this commit 
changes. Will we be populating both rawDataSize and totalSize, replace one with 
the other, or something else?


http://gerrit.cloudera.org:8080/#/c/8110/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/8110/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1178
PS1, Line 1178: droppedRawDataSize
rename to droppedTotalSize to be consistent with the tblproperty being updated?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7c2c4e1e99b297c849f9f0d18b2bef34ad811c6
Gerrit-Change-Number: 8110
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:21:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5250: Unify decompressor output length semantics

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5250: Unify decompressor output_length semantics
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1255/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd42942b169921a7eb53940c3762bc45bb82a993
Gerrit-Change-Number: 8030
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:19:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5250: Unify decompressor output length semantics

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5250: Unify decompressor output_length semantics
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd42942b169921a7eb53940c3762bc45bb82a993
Gerrit-Change-Number: 8030
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:18:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1649 Pass precision to Print TCounterType::DOUBLE VALUE

[Pranay Singh (Code Review)]

Pranay Singh has abandoned this change. ( http://gerrit.cloudera.org:8080/7725 )

Change subject: IMPALA-1649 Pass precision to Print TCounterType::DOUBLE_VALUE
..


Abandoned

After talking to Tim and Joe, it appears that TUnit::DOUBLE_VALUE is needed and 
cannot be removed. The use of TUnit::DOUBLE_VALUE can be seen in the function 
RuntimeProfile::PrintChildCounters(), which calls PrettyPrinter::Print() for 
printing different TUnit types.
We can't differentiate between the counters that need to be displayed with a 
decimal precision of FIXED_PRECISION from the other TUnit::UNIT types, if we 
remove TUnit::DOUBLE_VALUE . So we do need TUnit::DOUBLE_VALUE to have counters 
that need to be displayed as a floating point values with FIXED_PRECISION.
--
To view, visit http://gerrit.cloudera.org:8080/7725
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Icc63d8a59bdf175341097df143798fb1a957d93f
Gerrit-Change-Number: 7725
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.

[Dimitris Tsirogiannis (Code Review)]

Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8112 )

Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to 
HMS.
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:00:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3360: Codegen inserting into runtime filters

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3360: Codegen inserting into runtime filters
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc
File be/src/exec/filter-context.cc:

http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc@225
PS3, Line 225:   Constant* expr_type_arg = codegen->ConstantToGVPtr(
> So I've been playing around with this more, and I have a theory. I noticed
That sounds plausible. ToNativePtr() makes a stack allocation so it makes sense 
that it would cause problems if you return that pointer from the function. Just 
generating the call and calling ToNativePtr() inline in this function would 
work (I think) since the stack allocation would have the correct lifetime.

Storing the intermediate values to the heap (i.e. result_) I think is the wrong 
path to go down since it will inhibit optimisations (the compiler knows that 
random pointers won't alias the stack allocation and that the write to the 
stack allocation won't be visible outside of the functoin).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79cf23ad92dadaab996a50a2ca07ef9ebe8639bb
Gerrit-Change-Number: 8029
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:50:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5736: Add impala-shell argument to set default query options

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8038 )

Change subject: IMPALA-5736: Add impala-shell argument to set default query 
options
..


Patch Set 1:

> MJ, do you prefer one option with a comma separated list of
 > key=value pairs, or using several options similar to --var ?

I don't have a preference. When I commented on the JIRA I hadn't considered the 
alternative proposed by Phil. I think that sounds reasonable.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26a3b67230c80a99bd246b6af205d558fec9a986
Gerrit-Change-Number: 8038
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:45:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5250: Unify decompressor output length semantics

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5250: Unify decompressor output_length semantics
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd42942b169921a7eb53940c3762bc45bb82a993
Gerrit-Change-Number: 8030
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:43:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.

[Tim Wood (Code Review)]

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

Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for 
Impala.
..


Patch Set 1:

Reviewers, please don't be shy about continuing to comment.  I am working the 
current batch of comments now, so please add more if you plan to.  The sooner I 
have them, the sooner I can address.  Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66
Gerrit-Change-Number: 8102
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wood 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Matthew Mulder 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Wood 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:42:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

[Tim Armstrong (Code Review)]

Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/7245 )

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..

IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

If a scan range is skipped at runtime the scan node skips reading
the range and never figures out the underlying compression codec used
to compress the files. In such a scenario we default the compression
codec to NONE which can be misleading. This change marks these files
as filtered in the scan node profile

e.g. - File Formats: TEXT/NONE:364 TEXT/NONE(Skipped):1460

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Reviewed-on: http://gerrit.cloudera.org:8080/7245
Reviewed-by: Tim Armstrong 
Tested-by: Tim Armstrong 
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
M tests/query_test/test_scanners.py
7 files changed, 71 insertions(+), 20 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-Change-Number: 7245
Gerrit-PatchSet: 12
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-4863/IMPALA-5311: Correctly account the file type and compression codec

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
..


Patch Set 11: Verified+1

Build passed but gerrit was down when it tried to submit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-Change-Number: 7245
Gerrit-PatchSet: 11
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:38:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
..


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1252/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Gerrit-Change-Number: 8063
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:36:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Gerrit-Change-Number: 8063
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:36:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5416: Fix an impala-shell command recursion bug

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8063 )

Change subject: IMPALA-5416: Fix an impala-shell command recursion bug
..


Patch Set 7:

Rebased again.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3
Gerrit-Change-Number: 8063
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:31:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

[Csaba Ringhofer (Code Review)]

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7954/5/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/5/be/src/runtime/timestamp-value.h@173
PS5, Line 173:
This became static to enable other functions to check whether their date is 
valid or not without actually creating a TimestampValue or calling year() to 
generate an exception. I think it would be better if there was a single 
function that does this check, so if 1400 will be changed for some reason ( 
IMPALA-2009 ), there will be no need to change many parts of the code.


http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h
File be/src/runtime/timestamp-value.h:

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-value.h@86
PS2, Line 86: }
> One purpose of FunctionContext is to be the interface back into impala's ru
I think it would be better to do the logging on the caller side, because 
depending on the caller, the format for the log can be different, e.g. unix 
timestamp vs -MM-DD string.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:29:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5966: Fix the result file location of PlannerTest

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5966: Fix the result file location of PlannerTest
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1251/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20005bd0421e1bb9f3eedbb003c97d92a8faf110
Gerrit-Change-Number: 8113
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:28:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5966: Fix the result file location of PlannerTest

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8113 )

Change subject: IMPALA-5966: Fix the result file location of PlannerTest
..


Patch Set 2:

> Did you try it manually and confirm that it works?

I did.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20005bd0421e1bb9f3eedbb003c97d92a8faf110
Gerrit-Change-Number: 8113
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:24:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

[Csaba Ringhofer (Code Review)]

Hello Lars Volker, Dan Hecht,

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

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

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..

IMPALA-5664: Unix time to timestamp conversions may crash Impala

TimestampValue::FromSubsecondUnixTime() and UtcFromUnixTimeMicros()
are incorrect only in case of the last second of 1399, because these
sub-second values are rounded first towards 1400-01-01 00:00:00, which
is accepted as a valid date, and the sub-second part is subtracted
afterwards, leading to a date outside the valid interval. The maximum
case, -12-31 59:59:59 is a bit different, because as I understand,
with nanosecond precision posix times, the maximum value is actually
1-01-01. 00:00:00 minus 1 nanosec.

TimestampValue::FromUnixTimeNanos() can create problematic TimestampValues
both <1400 and 1<=.

These timestamps can cause problems, because most code assumes that if
HasDate/HasTime is true, then it really is a valid timestamp.

To fix this, the posix times are checked in the constructor of
TimestampValue, and if it is outside the valid interval,both time_
and date_ are set to not_a_date_time.

Test:
select cast(-17987443200-0.1 as timestamp);
This query no longer crashes, but returns NULL, similarly to other
< 1400 timestamps.

Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
---
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-stats.inline.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.h
5 files changed, 47 insertions(+), 10 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] fix query report generator typo

[Alex Behm (Code Review)]

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

Change subject: fix query report generator typo
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
Gerrit-Change-Number: 8118
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Thu, 21 Sep 2017 16:26:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5966: Fix the result file location of PlannerTest

[Alex Behm (Code Review)]

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

Change subject: IMPALA-5966: Fix the result file location of PlannerTest
..


Patch Set 2:

Did you try it manually and confirm that it works?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20005bd0421e1bb9f3eedbb003c97d92a8faf110
Gerrit-Change-Number: 8113
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Sep 2017 16:21:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] fix query report generator typo

[Michael Brown (Code Review)]

Michael Brown has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8118


Change subject: fix query report generator typo
..

fix query report generator typo

Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
---
M tests/comparison/leopard/templates/index.template
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8585c5f85781ff0b80292823256fe8866d3f95bd
Gerrit-Change-Number: 8118
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown 

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

[Csaba Ringhofer (Code Review)]

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-test.cc
File be/src/runtime/timestamp-test.cc:

http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-test.cc@637
PS2, Line 637: Sub-second F
> The problem is only with the functions that take sub-second resolution, rig
Done


http://gerrit.cloudera.org:8080/#/c/7954/2/be/src/runtime/timestamp-test.cc@685
PS2, Line 685:
> nit: typo
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Comment-Date: Thu, 21 Sep 2017 15:43:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5664: Unix time to timestamp conversions may crash Impala

[Csaba Ringhofer (Code Review)]

Hello Lars Volker, Dan Hecht,

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

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

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

Change subject: IMPALA-5664: Unix time to timestamp conversions may crash Impala
..

IMPALA-5664: Unix time to timestamp conversions may crash Impala

TimestampValue::FromSubsecondUnixTime() and UtcFromUnixTimeMicros()
are incorrect only in case of the last second of 1399, because these
sub-second values are rounded first towards 1400-01-01 00:00:00, which
is accepted as a valid date, and the sub-second part is subtracted
afterwards, leading to a date outside the valid interval. The maximum
case, -12-31 59:59:59 is a bit different, because as I understand,
with nanosecond precision posix times, the maximum value is actually
1-01-01. 00:00:00 minus 1 nanosec.

TimestampValue::FromUnixTimeNanos() can create problematic TimestampValues
both <1400 and 1<=.

These timestamps can cause problems, because most code assumes that if
HasDate/HasTime is true, then it really is a valid timestamp.

To fix this, the posix times are checked in the constructor of
TimestampValue, and if it is outside the valid interval,both time_
and date_ are set to not_a_date_time.

Test:
select cast(-17987443200-0.1 as timestamp);
This query no longer crashes, but returns NULL, similarly to other
< 1400 timestamps.

Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
---
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-stats.inline.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.h
6 files changed, 50 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77b2f6284d3a597f57e61c17a67c959eff9e38ff
Gerrit-Change-Number: 7954
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] [DOCS] Fill in release note subtopics for Apache Impala 2.10

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/7958 )

Change subject: [DOCS] Fill in release note subtopics for Apache Impala 2.10
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide38c1e1c64dac287b180b39ebb8e7735d457ce3
Gerrit-Change-Number: 7958
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Michael Brown 
Gerrit-Comment-Date: Thu, 21 Sep 2017 15:32:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Fill in release note subtopics for Apache Impala 2.10

[John Russell (Code Review)]

John Russell has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/7958 )

Change subject: [DOCS] Fill in release note subtopics for Apache Impala 2.10
..

[DOCS] Fill in release note subtopics for Apache Impala 2.10

Primarily just pointing to the list of issues in the changelog.
Those cover the different use cases for the different parts
of the release notes -- fixed issues, new features, and
incompatible changes.

Change-Id: Ide38c1e1c64dac287b180b39ebb8e7735d457ce3
---
M docs/impala_keydefs.ditamap
M docs/topics/impala_fixed_issues.xml
M docs/topics/impala_incompatible_changes.xml
M docs/topics/impala_new_features.xml
4 files changed, 54 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide38c1e1c64dac287b180b39ebb8e7735d457ce3
Gerrit-Change-Number: 7958
Gerrit-PatchSet: 3
Gerrit-Owner: John Russell