date:20170728

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-5722: Fix string to decimal cast
..

IMPALA-5722: Fix string to decimal cast

When converting a string to a decimal, we didn't handle the case where
the exponent is a large negative number. The function for computing the
divisor returns -1 when the exponent is too large. The problem is fixed
by making sure that the divisor is positive.

Testing:
-Added decimal tests.

Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
---
M be/src/runtime/decimal-test.cc
M be/src/util/decimal-util.h
M be/src/util/string-parser.h
3 files changed, 31 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4939, IMPALA-4939: Decimal V2 multiplication

Taras Bobrovytsky has uploaded a new patch set (#4).

Change subject: IMPALA-4939, IMPALA-4939: Decimal V2 multiplication
..

IMPALA-4939, IMPALA-4939: Decimal V2 multiplication

Implement the new DECIMAL return type rules for multiply expressions,
active when query option DECIMAL_V2=1. The algorithm for determining
the type of the result of multiplication is described in the JIRA.

DECIMAL V1:

+---+
| typeof(cast('0.1' as decimal(38,38)) * cast('0.1' as decimal(38,38))) |
+---+
| DECIMAL(38,38)|
+---+

+---+
| typeof(cast('0.1' as decimal(38,15)) * cast('0.1' as decimal(38,15))) |
+---+
| DECIMAL(38,30)|
+---+

DECIMAL V2:

+---+
| typeof(cast('0.1' as decimal(38,38)) * cast('0.1' as decimal(38,38))) |
+---+
| DECIMAL(38,37)|
+---+

+---+
| typeof(cast('0.1' as decimal(38,15)) * cast('0.1' as decimal(38,15))) |
+---+
| DECIMAL(38,6) |
+---+

In this patch, we also fix the early multiplication overflow. We compute
an int256 intermediate value, which we then attempt to scale down and
round to int128.

Benchmarks:

Query:
select
  sum(l_quantity * l_tax) +
  sum(l_extendedprice * l_discount)
from lineitem_big;

Before:
DECIMAL_V2 disabled: 6.68s
DECIMAL_V2 enabled : 6.77s

After:
DECIMAL_V2 disabled: 6.66s
DECIMAL_V2 enabled : 6.58s

In the above benchmark, we are selecting from lineitem_big, which has
about 100 times as many rows as the normal lineitem.

Query:
select
  sum(l_quantity * l_tax * cast(1 as decimal(38, 37))) +
  sum(l_extendedprice * l_discount * cast(1 as decimal(38, 37)))
from lineitem

Before:
DECIMAL_V2 disabled: 0.23s
DECIMAL_V2 enabled : 0.45s

After:
DECIMAL_V2 disabled: 2.65s
DECIMAL_V2 enabled : 2.54s

The query is slower after the patch because the intermediate result is
int256.

Change-Id: I37ad6232d7953bd75c18dc86e665b2b501a1ebe1
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/util/bit-util.h
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
4 files changed, 253 insertions(+), 58 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I37ad6232d7953bd75c18dc86e665b2b501a1ebe1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-4939, IMPALA-4939: Decimal V2 multiplication

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4939, IMPALA-4939: Decimal V2 multiplication
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7438/1/be/src/runtime/decimal-value.inline.h
File be/src/runtime/decimal-value.inline.h:

Line 301:   // doubling such a value would overflow in two's complement.  
However, we
> A while loop will perform terribly, but there are intrinsics that can be us
Added a TODO.


http://gerrit.cloudera.org:8080/#/c/7438/1/be/src/util/bit-util.h
File be/src/util/bit-util.h:

Line 65: return value < 0 ? -1 : 1;
> I think we want to think carefully about using the Sign function with int25
I included the benchmark result in the commit message.


http://gerrit.cloudera.org:8080/#/c/7438/1/be/src/util/decimal-util.h
File be/src/util/decimal-util.h:

Line 336
> Really?  That's crazy.
I removed this function so that the function on line 58 would get called.


http://gerrit.cloudera.org:8080/#/c/7438/1/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
File fe/src/main/java/org/apache/impala/analysis/TypesUtil.java:

Line 260: resultPrecision = p1 + p2 + 1;
> I think the 0.999 * 0.999 case is a fluke - we are sacrificing precision fo
This is more of a product question. I'll ask Greg about this later.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I37ad6232d7953bd75c18dc86e665b2b501a1ebe1
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4939, IMPALA-4939: Decimal V2 multiplication

Taras Bobrovytsky has uploaded a new patch set (#4).

Change subject: IMPALA-4939, IMPALA-4939: Decimal V2 multiplication
..

IMPALA-4939, IMPALA-4939: Decimal V2 multiplication

Implement the new DECIMAL return type rules for multiply expressions,
active when query option DECIMAL_V2=1. The algorithm for determining
the type of the result of multiplication is described in the JIRA.

DECIMAL V1:

+---+
| typeof(cast('0.1' as decimal(38,38)) * cast('0.1' as decimal(38,38))) |
+---+
| DECIMAL(38,38)|
+---+

+---+
| typeof(cast('0.1' as decimal(38,15)) * cast('0.1' as decimal(38,15))) |
+---+
| DECIMAL(38,30)|
+---+

DECIMAL V2:

+---+
| typeof(cast('0.1' as decimal(38,38)) * cast('0.1' as decimal(38,38))) |
+---+
| DECIMAL(38,37)|
+---+

+---+
| typeof(cast('0.1' as decimal(38,15)) * cast('0.1' as decimal(38,15))) |
+---+
| DECIMAL(38,6) |
+---+

In this patch, we also fix the early multiplication overflow. We compute
an int256 intermediate value, which we then attempt to scale down and
round to int128.

Benchmarks:

Query:
select
  sum(l_quantity * l_tax) +
  sum(l_extendedprice * l_discount)
from lineitem_big;

Before:
DECIMAL_V2 disabled: 6.68s
DECIMAL_V2 enabled : 6.77s

After:
DECIMAL_V2 disabled: 6.66s
DECIMAL_V2 enabled : 6.58s

In the above benchmark, we are selecting from lineitem_big, which has
about 100 times as many rows as the normal lineitem.

Query:
select
  sum(l_quantity * l_tax * cast(1 as decimal(38, 37))) +
  sum(l_extendedprice * l_discount * cast(1 as decimal(38, 37)))
from lineitem

Before:
DECIMAL_V2 disabled: 0.23s
DECIMAL_V2 enabled : 0.45s

After:
DECIMAL_V2 disabled: 2.65s
DECIMAL_V2 enabled : 2.54s

The query is slower after the patch because the intermediate result is
int256.

Change-Id: I37ad6232d7953bd75c18dc86e665b2b501a1ebe1
---
M be/src/exprs/expr-test.cc
M be/src/runtime/decimal-value.inline.h
M be/src/util/bit-util.h
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
4 files changed, 253 insertions(+), 58 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I37ad6232d7953bd75c18dc86e665b2b501a1ebe1
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden

[Impala-ASF-CR] IMPALA-5722: Fix string to decimal cast

2017-07-28 Thread Jim Apple (Code Review)

Jim Apple has posted comments on this change.

Change subject: IMPALA-5722: Fix string to decimal cast
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3894: Change the behavior parsing date "YY"

Tianyi Wang has posted comments on this change.

Change subject: IMPALA-3894: Change the behavior parsing date "YY"
..


Patch Set 8:

(7 comments)

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

Line 159:   if (!context->IsArgConstant(1)) {
> We should be able to do this during the Prepare() phase of this function. I
Done


http://gerrit.cloudera.org:8080/#/c/7530/4/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

Line 449: if (tok_len <= 2) realign_year = true;
> You can do this on one line (see https://google.github.io/styleguide/cppgui
Done


PS4, Line 549: r parsed be YY, th
> Let's factor this common subexpression out into a separate variable for rea
Done


Line 551: date parsed_date(dt_result->year, dt_result->month, 
dt_result->day);
> Could you add a comment giving an example of what the value might be at thi
Done


Line 555: // For example if the century start at 1937 but dt_result->year = 
1936,
> Could you add a comment giving an example of when we need to increment by 1
Done


http://gerrit.cloudera.org:8080/#/c/7530/4/be/src/runtime/timestamp-parse-util.h
File be/src/runtime/timestamp-parse-util.h:

Line 139:   TimestampValue now;
> nit: add a "." for consistency with other comments.
Done


Line 140: 
> Let's just store a copy of the TimestampValue inline instead of the pointer
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3894: Change the behavior parsing date "YY"

Tianyi Wang has uploaded a new patch set (#8).

Change subject: IMPALA-3894: Change the behavior parsing date "YY"
..

IMPALA-3894: Change the behavior parsing date "YY"

This patch change the behavor when running
unix_timestamp(string, string) function.
Before the change Impala directly adds 2000 to the year parsed.
Behavior after change is the same as Hive's,
shifting the parsed date into the interval
[current time - 80 years, current time + 20 years).
In 2017, given query
> select from_unixtime(unix_timestamp('31-AUG-94', 'dd-MMM-yy'),'MMdd');
Impala would output 20940831 before the change
and 19940831 with this patch applied.
unix_timestamp function with other forms of parameters is not affected.

Test: add 4 test cases to timestamp-test.cc, testing the
edge cases of 2-year format.

Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
---
M be/src/exprs/timestamp-functions.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
M be/src/runtime/timestamp-test.cc
4 files changed, 38 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/7530/8
-- 
To view, visit http://gerrit.cloudera.org:8080/7530
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-3894: Change the behavior parsing date "YY"

Tianyi Wang has uploaded a new patch set (#5).

Change subject: IMPALA-3894: Change the behavior parsing date "YY"
..

IMPALA-3894: Change the behavior parsing date "YY"

This patch change the behavor when running
unix_timestamp(string, string) function.
Before the change Impala directly adds 2000 to the year parsed.
Behavior after change is the same as Hive's,
shifting the parsed date into the interval
[current time - 80 years, current time + 20 years).
In 2017, given query
> select from_unixtime(unix_timestamp('31-AUG-94', 'dd-MMM-yy'),'MMdd');
Impala would output 20940831 before the change
and 19940831 with this patch applied.
unix_timestamp function with other forms of parameters is not affected.

Test: add 4 test cases to timestamp-test.cc, testing the
edge cases of 2-year format.

Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
---
M be/src/exprs/timestamp-functions.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
M be/src/runtime/timestamp-test.cc
4 files changed, 39 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

Lars Volker has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 8:

Hit another flaky test :( - Trying again.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[native-toolchain-CR] Bump breakpad version to include fix for IMPALA-4226

Lars Volker has posted comments on this change.

Change subject: Bump breakpad version to include fix for IMPALA-4226
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c4685b9a010a19bf13e7399ae3520b9bdadb298
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5394: Handle blocked HS2 connections

2017-07-28 Thread John Sherman (Code Review)

John Sherman has uploaded a new patch set (#4).

Change subject: IMPALA-5394: Handle blocked HS2 connections
..

IMPALA-5394: Handle blocked HS2 connections

- TThreadPoolServer calls getTransport() on a client from the Server
  thread (the thread that does the accepts).
  - TSaslServerTransport->getTransport() calls TSaslTransport->open()
  - TSaslServerTransport->open() tries to negotiate SASL which calls
read/write
- If read/write blocks, the TThreadPoolServer cannot accept
  connections
- Set the underlying TSocket's recvTimeout and sendTimeout before the
  TSaslServerTransport->open() and reset them to 0 after open()
  completes.
- Added sasl_connect_tcp_timeout_ms flag that defaults to 30
  milliseconds (5 minutes)
- Changed the Thrift server type for hs2 connections from ThreadPool
  to Threaded to take advantage of the AcceptQueueServer
  implementation.
- Add the ability for the TAcceptQueueServer to limit the maximum
  number of tasks at a time
- Removed the previously unenforced thread limit requests from
  StatestoreService, StatestoreSubscriber, CatalogService.

Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
---
M be/src/catalog/catalogd-main.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/transport/TSaslServerTransport.cpp
M common/thrift/metrics.json
10 files changed, 77 insertions(+), 36 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Sherman 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: John Sherman 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil

[native-toolchain-CR] Bump breakpad version to include fix for IMPALA-4226

Lars Volker has posted comments on this change.

Change subject: Bump breakpad version to include fix for IMPALA-4226
..


Patch Set 1:

Patches applied cleanly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c4685b9a010a19bf13e7399ae3520b9bdadb298
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[native-toolchain-CR] Bump breakpad version to include fix for IMPALA-4226

Lars Volker has uploaded a new change for review.

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

Change subject: Bump breakpad version to include fix for IMPALA-4226
..

Bump breakpad version to include fix for IMPALA-4226

Change-Id: I7c4685b9a010a19bf13e7399ae3520b9bdadb298
---
M buildall.sh
A 
source/breakpad/breakpad-1b704857f1e78a864e6942e613457e55f1aecb60-patches/0001-Add-basic-support-for-dwz-dwarf-extension.patch
A 
source/breakpad/breakpad-1b704857f1e78a864e6942e613457e55f1aecb60-patches/0002-Build-breakpad-ffe3e47-on-ppc64le.patch
3 files changed, 648 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c4685b9a010a19bf13e7399ae3520b9bdadb298
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker

[Impala-ASF-CR] IMPALA-5722: Fix string to decimal cast

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5722: Fix string to decimal cast
..


Patch Set 2: Code-Review+1

Looks good to me. Will give Jim a chance to weigh in if he has further comments.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5722: Fix string to decimal cast

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-5722: Fix string to decimal cast
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7517/1/be/src/runtime/decimal-test.cc
File be/src/runtime/decimal-test.cc:

Line 305:   // When the absolute value of the exponent becomes too large, it is 
considered
> This seems a bit weird - it seems like it should be underflow. It seems lik
Changed the be code so that a large negative number would lead to an underflow 
(instead of overflow).


http://gerrit.cloudera.org:8080/#/c/7517/1/be/src/util/decimal-util.h
File be/src/util/decimal-util.h:

Line 63:   DCHECK_GE(result * 10, result);
> The generic integer overflow support wasn't added until gcc5 I believe. Agr
Added a TODO.


http://gerrit.cloudera.org:8080/#/c/7517/1/be/src/util/string-parser.h
File be/src/util/string-parser.h:

PS1, Line 225: DCHECK
> DCHECK_EQ
DCHECK_EQ doesn't work with int128. Added a comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5722: Fix string to decimal cast

Taras Bobrovytsky has uploaded a new patch set (#2).

Change subject: IMPALA-5722: Fix string to decimal cast
..

IMPALA-5722: Fix string to decimal cast

When converting a string to a decimal, we didn't handle the case where
the exponent is a large negative number. The function for computing the
divisor returns -1 when the exponent is too large. The problem is fixed
by making sure that the divisor is positive.

Testing:
-Added decimal tests.

Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
---
M be/src/runtime/decimal-test.cc
M be/src/util/decimal-util.h
M be/src/util/string-parser.h
3 files changed, 31 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f5eb5b64a6924af2e8eb7f9a50da67015757efe
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5636: Change the metadata in parquet

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5636: Change the metadata in parquet
..


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py

To support KRPC on legacy platforms with version of OpenSSL
older than 1.0.1, we may need to use libssl from the toolchain.
This change makes toolchain boostrapping to also download
OpenSSL 1.0.1p.

Testing: private packaging build.

Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Reviewed-on: http://gerrit.cloudera.org:8080/7532
Reviewed-by: Henry Robinson 
Tested-by: Impala Public Jenkins
---
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
2 files changed, 3 insertions(+), 3 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil

[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/949/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py

Lars Volker has posted comments on this change.

Change subject: IMPALA-5009: Clean up test_insert_parquet.py
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7518/2/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:

PS2, Line 407: tmpdir
> Looks like you missed a spot.
Thanks for catching this, fixed it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

Henry Robinson has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 27: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 27
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


IMPALA-4669: [KUTIL] Add kudu_util library to the build.

A few miscellaneous changes to allow kudu_util to compile with Impala.

Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.

Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.

Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.

Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.

Finally, a few changes to allow compilation on RHEL5:

* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
  it does not compile due to other #includes in env_posix.cc. We disable
  the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
  it doesn't exist.

In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.

This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.

Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson 
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/common/config.h.in
A be/src/common/kudu_version.cc
M be/src/common/logging.cc
M be/src/exec/kudu-util.h
A be/src/kudu/gutil
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/cache.cc
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/locks.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M be/src/kudu/util/net/socket.cc
M be/src/kudu/util/subprocess.cc
M bin/run_clang_tidy.sh
21 files changed, 203 insertions(+), 46 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 28
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 27: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 27
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5009: Clean up test insert parquet.py

Hello Michael Brown,

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

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

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

Change subject: IMPALA-5009: Clean up test_insert_parquet.py
..

IMPALA-5009: Clean up test_insert_parquet.py

Replace make_tmp_dir with py.test's own tmpdir

Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2
---
M tests/query_test/test_insert_parquet.py
1 file changed, 71 insertions(+), 74 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia84c78d7ff74cc7fdb3d782060caa5e52d0cd7d2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4086: Add benchmark for simple scheduler

Lars Volker has posted comments on this change.

Change subject: IMPALA-4086: Add benchmark for simple scheduler
..


Patch Set 8: Code-Review+2

Rebased the change after hitting the same Kudu issue again. Carrying Tim's +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89ec1c6c1828bb0b86d1e13ce4dfc5a8df865c2e
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5636: Change the metadata in parquet

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5636: Change the metadata in parquet
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

2017-07-28 Thread Pooja Nilangekar (Code Review)

Pooja Nilangekar has posted comments on this change.

Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

PS1, Line 335: SkipStreamIfEosr();
> Just a passing comment - if this DCHECK fails, all it will tell us via the 
I uploaded another patch but didn't publish it. I've moved the checks to a 
single function and I am now checking both conditions.


http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.h
File be/src/exec/scanner-context.h:

PS1, Line 332: 
> consider case where current_idx_ < 0 as well?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-HasComments: Yes

[Impala-ASF-CR] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

2017-07-28 Thread Pooja Nilangekar (Code Review)

Pooja Nilangekar has uploaded a new patch set (#3).

Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream
..

Build a ConcatenatedStreams wrapper for ScannerContext::Stream

The ConcatenatedStreams class keeps track of a vector of streams which
can be utilized by the BaseScalarColumnReader to read transparently
from a list of streams. The ConcatenatedStreams would hide the
different ScanRanges from the caller and provide the abstraction of
a single stream. The class uses a list of streams and a index to point
to the current active stream. The class moves to the next stream once
the current stream reaches its end of stream.

Testing: The existing test_scanners.py test was used to validate the
correctness of the ConcatenatedStreams.

Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/scanner-context.cc
M be/src/exec/scanner-context.h
4 files changed, 111 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar

[Impala-ASF-CR] IMPALA-5636: Change the metadata in parquet

Tianyi Wang has uploaded a new patch set (#5).

Change subject: IMPALA-5636: Change the metadata in parquet
..

IMPALA-5636: Change the metadata in parquet

When writing in parquet format, Impala does not use repetition level.
But the repetition level encoding is set to BIT_PACKED, which is deprecated
and may cause problems when read by other softwares.
Changing it to RLE solves this issue.

Testing: This change is only manually tested.
To test with default testdata loaded:
> create table default.test like tpch_parquet.orders stored as parquet;
> insert into default.random values (0,0,"",0,"","","",0,"");
Then fetch "hdfs://localhost:20500/test-warehouse/test/*.parq" and use
$ java -jar parquet-tools-1.6.0.jar dump /home/tianyi/Downloads/*.parq | grep 
RLE:
to inspect the file. Before the change you would see output like
page 0:  DLE:RLE RLE:BIT_PACKED VLE:PLA [more]... VC:1
and after the change they should be
page 0:  DLE:RLE RLE:RLE VLE:PLA [more]... VC:1

Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No

[Impala-ASF-CR] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

2017-07-28 Thread Pooja Nilangekar (Code Review)

Pooja Nilangekar has uploaded a new patch set (#2).

Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream
..

Build a ConcatenatedStreams wrapper for ScannerContext::Stream

The ConcatenatedStreams class keeps track of a vector of streams which
can be utilized by the BaseScalarColumnReader to read transparently
from a list of streams. The ConcatenatedStreams would hide the
different ScanRanges from the caller and provide the abstraction of
a single stream. The class uses a list of streams and a index to point
to the current active stream. The class moves to the next stream once
the current stream reaches its end of stream.

Testing: The existing test_scanners.py test was used to validate the
correctness of the ConcatenatedStreams.

Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/parquet-column-readers.h
M be/src/exec/scanner-context.cc
M be/src/exec/scanner-context.h
4 files changed, 110 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar

[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Vincent Tran (Code Review)

Vincent Tran has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1: Code-Review+1

Great analysis.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Sailesh Mukil (Code Review)

Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..


Patch Set 1: Code-Review+1

The explanation seems acceptable to me, we won't be missing out on a huge 
optimization by doing this.
Plus, as this is a correctness issue, I too feel this is the right thing to do.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1891: Statestore won't send deletions in initial non-delta topic

2017-07-28 Thread Bikramjeet Vig (Code Review)

Hello Henry Robinson,

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

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

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

Change subject: IMPALA-1891: Statestore won't send deletions in initial 
non-delta topic
..

IMPALA-1891: Statestore won't send deletions in initial non-delta topic

Currently, when a subscriber connects to the statestore, its first
update contains the entire topic including deletions which are useless
because the subscriber has no base to apply those deletions to. This
patch ensures that the deletions are not included in that first update.

Testing:
Modified existing test case to include this check

Change-Id: I3fc525e1f3d960d642fc6356abb75f744cab7c33
---
M be/src/statestore/statestore.cc
M tests/statestore/test_statestore.py
2 files changed, 15 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fc525e1f3d960d642fc6356abb75f744cab7c33
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1: Code-Review+2

No, if it's already in the current toolchain version you don't need a separate 
build.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1891: Statestore won't send deletions in initial non-delta topic

2017-07-28 Thread Bikramjeet Vig (Code Review)

Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-1891: Statestore won't send deletions in initial 
non-delta topic
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7527/1/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

PS1, Line 531: UNLIKELY
> this isn't perf critical, so let's remove UNLIKELY for now.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fc525e1f3d960d642fc6356abb75f744cab7c33
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5336: Fix partition pruning when column is cast

2017-07-28 Thread Matthew Jacobs (Code Review)

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5336: Fix partition pruning when column is cast
..

Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7521/1/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
File fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java:

PS1, Line 185: if (bp.getChild(0).isImplicitCast()) return false;
> Will this also change the behaviour of implicitly casted integer types (e.g
The reason the bug doesn't affect integer types is because all of them are
NumericLiterals, and so the evalBinaryPredicate() will be able to match the
partition key literal with the predicate literal. The 'matching' in the FE uses
java maps, so it works as long as the LiteralExprs are equal (i.e. java
equal()).

With this change, BIGINT INT should remain the same, but INT BIGINT
will fall back to the BE evaluation (it will still be partition pruned, just
calls into the BE to evaluate the predicate). In the first case, the RHS INT
gets promoted to a BIGINT, so no cast is necessary. In the latter case, the RHS
can't be stored in the type of the LHS, so casting the LHS is necessary.

Given that the function evalBinaryPredicate(Expr) relies on the Literals to
match, I think this change is the right thing to do. Keep in mind that if this
returns false, it doesn't mean there won't be partition pruning, just that the
partition pruning won't happen using the partition MD in the FE - it just has
to call into the backend to evaluate the expr. I think that is acceptable.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Vincent Tran
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

2017-07-28 Thread Michael Ho (Code Review)

Michael Ho has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1:

We have been building OpenSSL-1.0.1p all along so the toolchain version isn't 
changed per-se. Please let me know if I should kick off another toolchain build 
to generate a new hash.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

Henry Robinson has posted comments on this change.

Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

PS1, Line 335: DCHECK(ValidateCurrentIndex());
Just a passing comment - if this DCHECK fails, all it will tell us via the logs 
is that ValidateCurrentIndex() was false, because the DCHECK will just print 
the condition that failed. 

This is useful for debugging, but not as useful as knowing *how* it failed (off 
by one error? was the idx badly initialized?). So my suggestion is to have 
ValidateCurrentIndex() look more like this:

   void ValidateCurrentIndex() {
 DCHECK_LE(0, current_idx_);
 DCHECK_GT(streams_.size(), current_idx_);
   }

The compiler will figure out in release mode that the method call does nothing, 
and should be able to optimize it out.


http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.h
File be/src/exec/scanner-context.h:

PS1, Line 332: return current_idx_
consider case where current_idx_ < 0 as well?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5714: Add OpenSSL to bootstrap toolchain.py

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5714: Add OpenSSL to bootstrap_toolchain.py
..


Patch Set 1:

Does the toolchain version need to be bumped as well?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5636: Change the metadata in parquet

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5636: Change the metadata in parquet
..

IMPALA-5636: Change the metadata in parquet

When writing in parquet format, Impala does not use repetition level.
But the repetition level encoding is set to BIT_PACKED, which is deprecated
and may cause problems when read by other softwares.
Changing it to RLE solves this issue.

Testing: This change is only manually tested.
To test with default testdata loaded:
> create table default.test like tpch_parquet.orders stored as parquet;
> insert into default.random values (0,0,"",0,"","","",0,"");
Then fetch "hdfs://localhost:20500/test-warehouse/test/*.parq" and use
$ java -jar parquet-tools-1.6.0.jar dump /home/tianyi/Downloads/*.parq | grep 
RLE:
to inspect the file. Before the change you would see output like
page 0:  DLE:RLE RLE:BIT_PACKED VLE:PLA [more]... VC:1
and after the change they should be
page 0:  DLE:RLE RLE:RLE VLE:PLA [more]... VC:1

Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5636: change the metadata in parquet

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5636: change the metadata in parquet
..

IMPALA-5636: change the metadata in parquet

When writing in parquet format, Impala does not use repetition level.
But the repetition level encoding is set to BIT_PACKED, which is deprecated
and may cause problems when read by other softwares.
Changing it to RLE solves this issue.

Testing: This change is only manually tested.
To test with default testdata loaded:
> create table default.test like tpch_parquet.orders stored as parquet;
> insert into default.random values (0,0,"",0,"","","",0,"");
Then fetch "hdfs://localhost:20500/test-warehouse/test/*.parq" and use
$ java -jar parquet-tools-1.6.0.jar dump /home/tianyi/Downloads/*.parq | grep 
RLE:
to inspect the file. Before the change you would see output like
page 0:  DLE:RLE RLE:BIT_PACKED VLE:PLA [more]... VC:1
and after the change they should be
page 0:  DLE:RLE RLE:RLE VLE:PLA [more]... VC:1

Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
---
M be/src/exec/hdfs-parquet-table-writer.cc
1 file changed, 1 insertion(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5739: Correctly handle sles12 SP2

Tim Armstrong has submitted this change and it was merged.

Change subject: IMPALA-5739: Correctly handle sles12 SP2
..


IMPALA-5739: Correctly handle sles12 SP2

This takes care of the difference in outputs for SLES 12 SP1 and SP2.
For reference here's the outputs in sles12sp1 and sp2:

sles12sp1 # lsb_release -irs
SUSE LINUX 12.1
sles12sp2 # lsb_release -irs
SUSE 12.2

Testing:
Did a full build on SLES12 SP2. Before this patch, a build resulted in:
'Pre-built toolchain archives not available for your platform.'

After this patch:
Toolchain bootstrap complete.

..Followed by a full build.

Change-Id: I005e05b8b66de78e6d53a35a894eb34d89843a62
Reviewed-on: http://gerrit.cloudera.org:8080/7535
Tested-by: Tim Armstrong 
Reviewed-by: Tim Armstrong 
---
M bin/bootstrap_toolchain.py
1 file changed, 1 insertion(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I005e05b8b66de78e6d53a35a894eb34d89843a62
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: hector.aco...@cloudera.com 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5739: Correctly handle sles12 SP2

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5739: Correctly handle sles12 SP2
..


Patch Set 2: Verified+1

Thanks Hector!

I don't think there's any point running our standard pre-commit tests since 
they don't exercise this. I manually verified that toolchain bootstrapping 
works on Ubuntu:

  tarmstrong@tarmstrong-box:~/Impala/incubator-impala$ rm -rf 
toolchain/cmake-3.2.3-p1/
  tarmstrong@tarmstrong-box:~/Impala/incubator-impala$ 
./bin/bootstrap_toolchain.py 
  URL https://native-toolchain.s3.amazonaws.com/build/424-  
b9179e1169/cmake/3.2.3-p1-gcc-4.9.2/cmake-3.2.3-p1-gcc-4.9.2-ec2-package-ubuntu-16-04.tar.gz
  Downloading cmake-3.2.3-p1-gcc-4.9.2-ec2-package-ubuntu-16-04.tar.gz to 
/home/tarmstrong/Impala/incubator-impala/toolchain
  Extracting cmake-3.2.3-p1-gcc-4.9.2-ec2-package-ubuntu-16-04.tar.gz

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I005e05b8b66de78e6d53a35a894eb34d89843a62
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: hector.aco...@cloudera.com 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
..


Patch Set 27:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5
Gerrit-PatchSet: 27
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5739 Correctly handle sles12 SP2

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5739 Correctly handle sles12 SP2
..


Patch Set 1:

(3 comments)

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

Line 7: IMPALA-5739 Correctly handle sles12 SP2
Nit: add a colon after the JIRA number.


Line 16: 
Can you mention testing briefly - I assume you did a full build on sles12 SP2? 
It's just good to know since we don't have automated testing.


Line 18: Signed-off-by: Hector Acosta 
Can you remove the Signed-off-by?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I005e05b8b66de78e6d53a35a894eb34d89843a62
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: hector.aco...@cloudera.com 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5636: changed the format metadata of repetition level
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7514/2//COMMIT_MSG
Commit Message:

Line 7: IMPALA-5636: changed the format metadata of repetition level from 
bit_packing to RLE
> Try to keep the subject line short (if possible, something like 50 characte
Not capitalising the first letter is fine. I looked through the git history and 
there's no consistency (I'm a lower-caser, personally).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5636: changed the format metadata of repetition level
..


Patch Set 3:

Yeah I didn't realise it was a draft. If you address Taras' comments then 
rebase I can start the merge.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5417: make I/O buffer queue fixed-size

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5417: make I/O buffer queue fixed-size
..


Patch Set 6: Code-Review+1

Carry +1 from kwho

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If7cc3f7199f5320db00b7face97a96cdadb6f83f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5417: make I/O buffer queue fixed-size

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5417: make I/O buffer queue fixed-size
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7408/4/be/src/runtime/disk-io-mgr-test.cc
File be/src/runtime/disk-io-mgr-test.cc:

PS4, Line 375: pool_.reset(new ObjectPool);
> Not your change but can't this just be allocated on the stack ?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If7cc3f7199f5320db00b7face97a96cdadb6f83f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5417: make I/O buffer queue fixed-size

Hello Michael Ho,

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

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

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

Change subject: IMPALA-5417: make I/O buffer queue fixed-size
..

IMPALA-5417: make I/O buffer queue fixed-size

This removes the dynamically-varying queue size behaviour in the I/O
manager. The motivation is to bound resource consumption of scans
and make it possible to reserve memory for I/O buffers upfront.

Does some cleanup/documentation of the locking policy. Fix some cases
in ScanRange::GetNext() where members documented as being protected by
ScanRange::lock_ were accessed without holding it. I think the races
were either benign or prevented by holding DiskIoRequestContext::lock_
in practice.

Testing:
Ran exhaustive build.

Perf:
Ran the full set of workloads (TPC-H, TPC-DS, targeted) on a 16 node
cluster. Everything was within normal variance.

Change-Id: If7cc3f7199f5320db00b7face97a96cdadb6f83f
---
M be/src/runtime/disk-io-mgr-internal.h
M be/src/runtime/disk-io-mgr-reader-context.cc
M be/src/runtime/disk-io-mgr-scan-range.cc
M be/src/runtime/disk-io-mgr-test.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/disk-io-mgr.h
6 files changed, 290 insertions(+), 335 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7cc3f7199f5320db00b7face97a96cdadb6f83f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5689: Avoid inverting non-equi left joins

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5689: Avoid inverting non-equi left joins
..


IMPALA-5689: Avoid inverting non-equi left joins

When checking if a join can be inverted, we forgot to also check that
the resulting join would not be a non-equi right semi-join or a
non-equi right outer-join. We currently do not support those kinds of
joins in the backend.

Testing:
-Added a planner test

Change-Id: I91ba66fe30139fcd44d4615a142f183266800aab
Reviewed-on: http://gerrit.cloudera.org:8080/7476
Reviewed-by: Taras Bobrovytsky 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/planner/JoinNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M 
testdata/workloads/functional-planner/queries/PlannerTest/nested-loop-join.test
3 files changed, 80 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Taras Bobrovytsky: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I91ba66fe30139fcd44d4615a142f183266800aab
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5689: Avoid inverting non-equi left joins

2017-07-28 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5689: Avoid inverting non-equi left joins
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I91ba66fe30139fcd44d4615a142f183266800aab
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level

Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-5636: changed the format metadata of repetition level
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7514/2//COMMIT_MSG
Commit Message:

Line 7: IMPALA-5636: changed the format metadata of repetition level from 
bit_packing to RLE
> Try to keep the subject line short (if possible, something like 50 characte
It looks like patch #3 is a draft (it should be published). Patch #3 is only 
visible to those who commented on the patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level