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

2017-07-27 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:

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

-- 
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-5714: Add OpenSSL to bootstrap toolchain.py

2017-07-27 Thread Michael Ho (Code Review)
Michael Ho has uploaded a new change for review.

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

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
---
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
2 files changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I860b16d8606de1ee472db35a4d8d4e97b57b67ae
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 


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

2017-07-27 Thread Henry Robinson (Code Review)
Henry Robinson has posted comments on this change.

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


Patch Set 1: Code-Review+2

(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.


-- 
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: Henry Robinson 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-3894: Changed the behavior parsing 2-digit year values

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3894: Changed the behavior parsing 2-digit year values
..


Patch Set 4:

(8 comments)

Looks pretty good. No major concerns, we just need some tests and a bit of 
cleanup to make it easier to follow.

http://gerrit.cloudera.org:8080/#/c/7530/4//COMMIT_MSG
Commit Message:

Line 16: > select from_unixtime(unix_timestamp('31-AUG-94', 
'dd-MMM-yy'),'MMdd');
Can you add some tests in expr-test for some different cases to cover all of 
the code paths?

I think you can make the value of "now()" deterministic if you set up a 
RuntimeState(), and pass in a TQueryCtx with now_string set.


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:   dt_ctx->now = context->impl()->state()->now();
We should be able to do this during the Prepare() phase of this function. It 
looks like that's implemented in UnixAndFromUnixPrepare() according to the 
function registry: 
https://github.com/apache/incubator-impala/blob/master/common/function-registry/impala_functions.py#L246


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) {
You can do this on one line (see 
https://google.github.io/styleguide/cppguide.html#Conditionals)

  if (tok_len <= 2) realign_year = true;


PS4, Line 549: dt_ctx.now->date()
Let's factor this common subexpression out into a separate variable for 
readability.


Line 551: dt_result->year += (century_start_year / 100) * 100;
Could you add a comment giving an example of what the value might be at this 
point. It's subtle..


Line 555: if (TimestampValue(parsed_date, parsed_time) <
Could you add a comment giving an example of when we need to increment by 100?


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:   /// Current time to determine the actual year when parsing 2-digit 
year token
nit: add a "." for consistency with other comments.


Line 140:   const TimestampValue* now;
Let's just store a copy of the TimestampValue inline instead of the pointer. 
Makes it easier to understand who owns the memory.


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


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

2017-07-27 Thread John Sherman (Code Review)
John Sherman has posted comments on this change.

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


Patch Set 3:

Thanks, yeah I overthought it. I'll run this through run-all-tests and post the 
patch.

 > My instinct would be just to use the size of tasks_ to control the
 > coordination.
 > 
 > In SetupConnection(), line 168 or so:
 > 
 > {
 > Synchronized s(tasksMonitor_);
 > if (tasks_.size() > max) tasksMonitor_.wait();
 > tasks_.insert(task);
 > }

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
Gerrit-PatchSet: 3
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 
Gerrit-HasComments: No


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

2017-07-27 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change.

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


Patch Set 4: Code-Review+1

(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 ?


-- 
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-3894: Changed the behavior parsing 2-digit year values

2017-07-27 Thread Greg Rahn (Code Review)
Greg Rahn has posted comments on this change.

Change subject: IMPALA-3894: Changed the behavior parsing 2-digit year values
..


Patch Set 4:

IIRC that's the calculation SimpleDateFormat uses for two digit years and 
likely where Hive gets it from.  If tests prove that true, LGTM.

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3894: Changed the behavior parsing 2-digit year values

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3894: Changed the behavior parsing 2-digit year values
..


Patch Set 4:

Greg, you've got a lot of experience looking at date/time functions.

What do you think about emulating Hive's behaviour here? It looks like it 
factors in the current year when determining how to interpret a 2 digit yet.

-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3894: Changed the behavior parsing 2-digit year values

2017-07-27 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new change for review.

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

Change subject: IMPALA-3894: Changed the behavior parsing 2-digit year values
..

IMPALA-3894: Changed the behavior parsing 2-digit year values

This patch changed 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).
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.

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


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5da761255915dc741f1dcc488fd4ef6ecc385896
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 


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

2017-07-27 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

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

-- 
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](asf-site) Fix incorrect email address identified by Nathanael Smith

2017-07-27 Thread Jim Apple (Code Review)
Jim Apple has submitted this change and it was merged.

Change subject: Fix incorrect email address identified by Nathanael Smith
..


Fix incorrect email address identified by Nathanael Smith

While I'm here, change @impala.incubator.apache.org to
@impala.apache.org which already works and will continue to work once
Impala graduates.

Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
Reviewed-on: http://gerrit.cloudera.org:8080/7529
Reviewed-by: Tim Armstrong 
Tested-by: Jim Apple 
---
M community.html
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jim Apple: Verified
  Tim Armstrong: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR](asf-site) Fix incorrect email address identified by Nathanael Smith

2017-07-27 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

Change subject: Fix incorrect email address identified by Nathanael Smith
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Fix incorrect email address identified by Nathanael Smith

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: Fix incorrect email address identified by Nathanael Smith
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR](asf-site) Fix incorrect email address identified by Nathanael Smith

2017-07-27 Thread Jim Apple (Code Review)
Jim Apple has uploaded a new change for review.

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

Change subject: Fix incorrect email address identified by Nathanael Smith
..

Fix incorrect email address identified by Nathanael Smith

While I'm here, change @impala.incubator.apache.org to
@impala.apache.org which already works and will continue to work once
Impala graduates.

Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
---
M community.html
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id227f15a0fdf9882fa4bc2552b869c3e54f4b55a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 


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

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded a new change for review.

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

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/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3fc525e1f3d960d642fc6356abb75f744cab7c33
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 


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

2017-07-27 Thread Henry Robinson (Code Review)
Henry Robinson has posted comments on this change.

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


Patch Set 3:

My instinct would be just to use the size of tasks_ to control the coordination.

In SetupConnection(), line 168 or so:

  { 
Synchronized s(tasksMonitor_);
if (tasks_.size() > max) tasksMonitor_.wait();
tasks_.insert(task);
  }

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
Gerrit-PatchSet: 3
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 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4703: reservation denial debug action

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4703: reservation denial debug action
..


Patch Set 10: Code-Review+2

Rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ied39bb091b12156e5dc61b528c6c0cd8de3fe657
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

2017-07-27 Thread Tim Armstrong (Code Review)
Hello Thomas Tauber-Marshall, Dan Hecht,

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

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

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

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..

IMPALA-4674: Part 2: port backend exec to BufferPool

Always create global BufferPool at startup using 80% of memory and
limit reservations to 80% of query memory (same as BufferedBlockMgr).
The query's initial reservation is computed in the planner, claimed
centrally (managed by the InitialReservations class) and distributed
to query operators from there.

min_spillable_buffer_size and default_spillable_buffer_size query
options control the buffer size that the planner selects for
spilling operators.

Port ExecNodes to use BufferPool:
  * Each ExecNode has to claim its reservation during Open()
  * Port Sorter to use BufferPool.
  * Switch from BufferedTupleStream to BufferedTupleStreamV2
  * Port HashTable to use BufferPool via a Suballocator.

This also makes PAGG memory consumption more efficient (avoid wasting buffers)
and improve the spilling algorithm:
* Allow preaggs to execute with 0 reservation - if streams and hash tables
  cannot be allocated, it will pass through rows.
* Halve the buffer requirement for spilling aggs - avoid allocating
  buffers for aggregated and unaggregated streams simultaneously.
* Rebuild spilled partitions instead of repartitioning (IMPALA-2708)

TODO in follow-up patches:
* Rename BufferedTupleStreamV2 to BufferedTupleStream
* Implement max_row_size query option.

Testing:
* Updated tests to reflect new memory requirements

Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
---
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/analytic-eval-node.cc
M be/src/exec/analytic-eval-node.h
M be/src/exec/exec-node.cc
M be/src/exec/exec-node.h
M be/src/exec/hash-table-test.cc
M be/src/exec/hash-table.cc
M be/src/exec/hash-table.h
M be/src/exec/hash-table.inline.h
M be/src/exec/nested-loop-join-builder.cc
M be/src/exec/partial-sort-node.cc
M be/src/exec/partial-sort-node.h
M be/src/exec/partitioned-aggregation-node-ir.cc
M be/src/exec/partitioned-aggregation-node.cc
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/partitioned-hash-join-builder-ir.cc
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/partitioned-hash-join-builder.h
M be/src/exec/partitioned-hash-join-node-ir.cc
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
M be/src/exec/partitioned-hash-join-node.inline.h
M be/src/exec/sort-node.cc
M be/src/exec/sort-node.h
M be/src/runtime/CMakeLists.txt
D be/src/runtime/buffered-block-mgr-test.cc
D be/src/runtime/buffered-block-mgr.cc
D be/src/runtime/buffered-block-mgr.h
D be/src/runtime/buffered-tuple-stream-test.cc
D be/src/runtime/buffered-tuple-stream.cc
D be/src/runtime/buffered-tuple-stream.h
D be/src/runtime/buffered-tuple-stream.inline.h
M be/src/runtime/bufferpool/buffer-pool.cc
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.h
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/runtime/fragment-instance-state.cc
A be/src/runtime/initial-reservations.cc
A be/src/runtime/initial-reservations.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M be/src/runtime/runtime-filter.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M be/src/runtime/test-env.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/service/client-request-state.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/bloom-filter.h
M be/src/util/static-asserts.cc
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/generate_error_codes.py
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/DataStreamSink.java
M fe/src/main/java/org/apache/impala/planner/EmptySetNode.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseTableSink.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/

[Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool
..


Patch Set 38: Code-Review+2

Rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
Gerrit-PatchSet: 38
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5630: Add Kudu client version as a common metric

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded a new change for review.

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

Change subject: IMPALA-5630: Add Kudu client version as a common metric
..

IMPALA-5630: Add Kudu client version as a common metric

Change-Id: I8bd0e220c030822209b811703ebd0cea699e8268
---
M be/src/util/common-metrics.cc
M be/src/util/common-metrics.h
M common/thrift/metrics.json
3 files changed, 22 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8bd0e220c030822209b811703ebd0cea699e8268
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 


[Impala-ASF-CR] IMPALA-5696: Enable cipher configuration when using TLS / Thrift

2017-07-27 Thread Henry Robinson (Code Review)
Henry Robinson has uploaded a new change for review.

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

Change subject: IMPALA-5696: Enable cipher configuration when using TLS / Thrift
..

IMPALA-5696: Enable cipher configuration when using TLS / Thrift

The 'cipher suite' is a description of the set of algorithms used by SSL
and TLS to execute key exchange, encryption, message authentication, and
random number generation functions. SSL implementations allows the
cipher suite to be configured so that ciphers may be removed from the
whitelist if they are shown to be weak.

* Add a flag --ssl_cipher_list which controls cipher selection for both
  thrift servers and clients. Default is blank, which means use all
  available cipher suites.
* Add ThriftServerBuilder to simplify construction of
  ThriftServers (whose constructors were otherwise getting very long).

Testing: new tests added to thrift-server-test. Test cases added follow:

* A client cannot connect to a server which does not have any ciphers in
  common with it.
* If ciphers are identical on clients and servers, that ssl connections
  can be made.
* Bad cipher strings lead to errors on both client and server.

Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
---
M be/src/benchmarks/network-perf-benchmark.cc
M be/src/catalog/catalogd-main.cc
M be/src/rpc/thrift-client.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/data-stream-test.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
A be/src/testutil/scoped-flag-setter.h
M be/src/util/webserver-test.cc
13 files changed, 399 insertions(+), 144 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I735ae36eebfdf7228f235686c9c69642c3c9d84f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 


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

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

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


Patch Set 3: Code-Review+2

Looks good to me. We're having some issues with the stability of the pre-merge 
verification job, so I'll hold off on started the merge until it's stabilised

-- 
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: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


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

2017-07-27 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 7:

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

-- 
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: 7
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-5722: Fix string to decimal cast

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

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


Patch Set 1:

(2 comments)

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);
> If T is signed, the compiler is permitted to turn this into a no-op: http:/
The generic integer overflow support wasn't added until gcc5 I believe. Agree 
that this isn't the ideal way to check for overflow though - maybe add a TODO 
indicating that the check is imperfect for this reason.


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

Line 221: T divisor = DecimalUtil::GetScaleMultiplier(shift);
> Look at decimal-util.h line 174, where -1 is returned. The function you wer
Thanks, that makes more sense.


-- 
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] Build a ConcatenatedStreams wrapper for ScannerContext::Stream

2017-07-27 Thread Pooja Nilangekar (Code Review)
Pooja Nilangekar has posted comments on this change.

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


Patch Set 2:

(13 comments)

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

Line 16: 
> It's best to document the TODO in the ConcatenatedStreams class itself, sin
Done


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

PS1, Line 330:   SkipStreamIfEosr();
> Is there a reason why you cannot DCHECK here, too?
Done


PS1, Line 346:   uint8_t** buffer, int64_t* out_len, Status* status, bool peek) 
{
 :   SkipStreamIfEosr();
 :   r
> This block looks like a candidate for a method that increments current_idx_
Done


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

PS1, Line 219: boundary_buffer_
> This buffer looks like a good candidate in case you want to stitch together
Should I look into this in a separate change?


PS1, Line 273: Concatenates a list of streams by reading them sequentia
> "Concatenates a list of streams by reading them sequentially." ?
Done


PS1, Line 274: s a wrapper around the functions which are
 :   /// utilized by BaseScalarColumnReader.
> This is a detail of how the class can be used. Try to describe only what it
Done


Line 287: 
> pass by reference
The scope of the vector is causing issues for call by reference.


Line 290: 
> I don't think you need to explicitly define it as empty, since that's the d
Done


PS1, Line 295: ed_len byt
> missing word?
Done


PS1, Line 299: 
> I think you can just say "the current stream", here and elsewhere.
Current stream could be misleading since incase the current stream has hit 
eosr, the current index is advanced to ensure that the bytes are read from a 
following stream which has not hit eosr.


PS1, Line 300: 
> Shouldn't it say "the last stream" since you're concatenating them in order
The current index would reach the last stream only if every stream before it 
has hit eosr. So I was thinking 'every stream' makes it clear.


PS1, Line 313: whic
> from?
Done


Line 317: int current_idx_;
> If you already know all the streams when creating the class, then you could
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pooja Nilangekar 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Pooja Nilangekar 
Gerrit-HasComments: Yes


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

2017-07-27 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: Lars Volker 


[Impala-ASF-CR] IMPALA-4795: Allow fetching function obj from catalog using signature

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-4795: Allow fetching function obj from catalog using 
signature
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7479/1/fe/src/main/java/org/apache/impala/catalog/Catalog.java
File fe/src/main/java/org/apache/impala/catalog/Catalog.java:

PS1, Line 463: TFunction tfn = objectDesc.getFn();
 : Function fn = null;
 : if (tfn.isSetScalar_fn() || tfn.isSetAggregate_fn()) {
 :   Function desc = Function.fromThrift(tfn);
 :   fn = getFunction(desc, 
Function.CompareMode.IS_INDISTINGUISHABLE);
 : } else if (tfn.isSetSignature()) {
 :   Db db = getDb(tfn.getName().getDb_name());
 :   fn = (db == null) ? null : 
db.getFunction(tfn.getSignature());
 : }
> Pulling into a separate fn with a clear interface in the javadoc makes sens
Done


PS1, Line 463: TFunction tfn = objectDesc.getFn();
 : Function fn = null;
 : if (tfn.isSetScalar_fn() || tfn.isSetAggregate_fn()) {
 :   Function desc = Function.fromThrift(tfn);
 :   fn = getFunction(desc, 
Function.CompareMode.IS_INDISTINGUISHABLE);
 : } else if (tfn.isSetSignature()) {
 :   Db db = getDb(tfn.getName().getDb_name());
 :   fn = (db == null) ? null : 
db.getFunction(tfn.getSignature());
 : }
> nit: How about moving this to a new method, getFunction(TFunction fn, mode)
Done


http://gerrit.cloudera.org:8080/#/c/7479/1/fe/src/main/java/org/apache/impala/catalog/Function.java
File fe/src/main/java/org/apache/impala/catalog/Function.java:

Line 360:   throw new IllegalStateException("Expected function type to be 
either UDA or UDF.");
> I believe IMPALA-626 is what caused the problem filed in IMPALA-4795 in the
Done


http://gerrit.cloudera.org:8080/#/c/7479/1/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

PS1, Line 148: input_url = self.CATALOG_OBJECT_URL.format("25000") + \
 : 
"?object_type=FUNCTION&object_name=_impala_builtins.abs(BIGINT)"
 : response = requests.get(input_url)
 : assert response.status_code == requests.codes.ok \
 :and
> nit: I think you could use impalad.get_catalog_object_dump(..)
Done
Also found and fixed a bug in get_catalog_object_dump


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4795: Allow fetching function obj from catalog using signature

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded a new patch set (#2).

Change subject: IMPALA-4795: Allow fetching function obj from catalog using 
signature
..

IMPALA-4795: Allow fetching function obj from catalog using signature

Fixed a bug where the catalog throws a NullPointerException if trying
to fetch a function object using function signature. This exception
prevented the code paths in CatalogOpExecutor::HandleDropFunction and
ImpalaServer::CatalogUpdateCallback to be exercised which prevented
removal of a recreated function object necessary for maintaining
metadata consistency.

Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f
---
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M tests/common/impala_service.py
M tests/query_test/test_udfs.py
M tests/webserver/test_web_pages.py
5 files changed, 42 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 


[Impala-ASF-CR] IMPALA-1882: Remove ORDER BY restriction from first value()/last value()

2017-07-27 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-1882: Remove ORDER BY restriction from 
first_value()/last_value()
..


Patch Set 3: Code-Review+2

Will hold off on submitting gvo until the job looks better

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a3a56833ac062839629353ea240b361bc727d96
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5529: Add additional function signatures for TRUNC()

2017-07-27 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5529: Add additional function signatures for TRUNC()
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I856da9f817b948de3c72af60a0742b128398b4cf
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-1882: Remove ORDER BY restriction from first value()/last value()

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has uploaded a new patch set (#3).

Change subject: IMPALA-1882: Remove ORDER BY restriction from 
first_value()/last_value()
..

IMPALA-1882: Remove ORDER BY restriction from first_value()/last_value()

In order to conform to the ISO SQL specifications, this patch allows
the 'order by' clause to be optional for first_value() and last_value()
analytical functions.

Testing:
1. Added Analyzer tests
2. Added Planner tests which checks that a sort node is not used if both
'order by' and 'partition by' clauses are not used. Also, checks that
if only 'partition by' clause is used then the sorting is done only on
the partition column.
3. Added a query test that checks that the input is not reordered by
these analytic functions if 'order by' clause is not used

Change-Id: I5a3a56833ac062839629353ea240b361bc727d96
---
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
4 files changed, 226 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a3a56833ac062839629353ea240b361bc727d96
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 


[Impala-ASF-CR] IMPALA-1882: Remove ORDER BY restriction from first value()/last value()

2017-07-27 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change.

Change subject: IMPALA-1882: Remove ORDER BY restriction from 
first_value()/last_value()
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7502/2/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java:

PS2, Line 231: String fnName = fn.functionName();
 : return fnName.equals(LAST_VALUE) || 
fnName.equals(FIRST_VALUE)
 : || fnName.equals(LAST_VALUE_IGNORE_NULLS)
 : || fnName.equals(FIRST_VALUE_IGNORE_NULLS);
 :   }
> Please use isAnalyticFn() as the above methods do.
Done


http://gerrit.cloudera.org:8080/#/c/7502/2/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
File testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test:

PS2, Line 2017: first_val
> remove this alias, for simplicity
Done


PS2, Line 2037: first_val
> same
Done


PS2, Line 2057: first_val
> remove; this alias is just confusing given this is the last_value fn :)
Done


PS2, Line 2077: first_val
> same
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a3a56833ac062839629353ea240b361bc727d96
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: Yes


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

2017-07-27 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:

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

-- 
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-5689: Avoid inverting non-equi left joins

2017-07-27 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change.

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


Patch Set 4: Code-Review+2

Rebased and forwarding the +2

-- 
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: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


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

2017-07-27 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has uploaded a new patch set (#4).

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
---
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(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91ba66fe30139fcd44d4615a142f183266800aab
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 


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

2017-07-27 Thread Taras Bobrovytsky (Code Review)
Hello Bharath Vissapragada, Tim Armstrong,

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

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

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

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
---
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(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I91ba66fe30139fcd44d4615a142f183266800aab
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 


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

2017-07-27 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change.

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


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7476/3//COMMIT_MSG
Commit Message:

PS3, Line 7: right joins
> left joins?
Done


PS3, Line 10: resulting join would not be a non-equi left semi or non-equi left
: outer join
> since you are talking about the resulting join, you should mean " the resul
Done


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


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

2017-07-27 Thread David Knupp (Code Review)
David Knupp 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.


-- 
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-5689: Avoid inverting non-equi right joins

2017-07-27 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change.

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


Patch Set 3: Code-Review+2

(1 comment)

Looks like its good to get this in quickly. +2'ing since the change is simple.
(Please fix the commit subject and content before GVO).

http://gerrit.cloudera.org:8080/#/c/7476/3//COMMIT_MSG
Commit Message:

PS3, Line 7: right joins
left joins?


-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change.

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


Patch Set 2: Code-Review+1

-- 
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: No


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

2017-07-27 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

Line 221: T divisor = DecimalUtil::GetScaleMultiplier(shift);
> I think it can be a huge negative number once it overflows.
Look at decimal-util.h line 174, where -1 is returned. The function you were 
looking at, Tim, is called only in the case of int256.


-- 
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-5009: Clean up test insert parquet.py

2017-07-27 Thread Lars Volker (Code Review)
Lars Volker has uploaded a new patch set (#2).

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, 70 insertions(+), 74 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/7518/2
-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 


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

2017-07-27 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change.

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


Patch Set 1:

(2 comments)

Thank you for the reviews.

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

PS1, Line 345: tmpdir
> Nit: for these private helper methods, prefer a more descriptive name than 
The folder is really not more than a temporary storage place that the caller 
has to provide and clean up. It's not used to return any files back to the 
caller. I couldn't think of a better name so I renamed it to tmp_dir to follow 
the naming convention and added a sentence to the docstrings to explain what it 
is used for and that the caller is responsible for cleaning it up. I'm happy to 
rename it though. Let me know if you prefer a different name.


PS1, Line 351: tmpdir.strpath
> +1 to this idea.
Done


-- 
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: 1
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-4086: Add benchmark for simple scheduler

2017-07-27 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 7:

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

-- 
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: 7
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

2017-07-27 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change.

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


Patch Set 7:

Restarting GVO, previous one seems to have hit IMPALA-5733.

-- 
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: 7
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: changed the format metadata of repetition level

2017-07-27 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#3).

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

IMPALA-5636: changed the format metadata of repetition level

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/3
-- 
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: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level from bit packing to RLE

2017-07-27 Thread Tianyi Wang (Code Review)
Tianyi Wang has restored this change.

Change subject: IMPALA-5636: changed the format metadata of repetition level 
from bit_packing to RLE
..


Restored

reopen to do some modifications

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

Gerrit-MessageType: restore
Gerrit-Change-Id: I4112ec88e8f4050d28661d27f9227450288a6756
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 


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

2017-07-27 Thread anujphadke (Code Review)
anujphadke has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

Line 221: T divisor = DecimalUtil::GetScaleMultiplier(shift);
> I must be missing something, but I don't understand how divisor can ever be
I think it can be a huge negative number once it overflows.

Ex when T is an int -
  int result = 1;
  for(int i=0; i <= 11; i++) {
result *= 10;
  }

result is 
-727379968


-- 
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: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

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


Patch Set 1:

(1 comment)

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);
If T is signed, the compiler is permitted to turn this into a no-op: 
http://eel.is/c++draft/expr#4 , https://blog.regehr.org/archives/1520

You could use make_unsigned_t to do the multiplication in the unsigned domain 
and then convert back or you could use 
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html.

Using make_unsigned is tricky for __int128.


-- 
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: Tim Armstrong 
Gerrit-HasComments: Yes


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

2017-07-27 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:

(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. 
BIGINT  INT)?
One question that I didn't quite have an answer/explanation for while working 
on this was that this bug didn't seem to affect implicitly casted integer-type 
partition keys.


-- 
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: Sailesh Mukil 
Gerrit-Reviewer: Vincent Tran 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong 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 like 
this might lead to CastToDecimalVal() returning the wrong thing, since it has 
different behaviour on underflow and overflow.


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

Line 221: T divisor = DecimalUtil::GetScaleMultiplier(shift);
I must be missing something, but I don't understand how divisor can ever be -1, 
since GetScaleMultiplier returns 1 multiplied by "scale" 10s.


PS1, Line 225: DCHECK
DCHECK_EQ


-- 
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: Tim Armstrong 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5529: Add additional function signatures for TRUNC()

2017-07-27 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5529: Add additional function signatures for TRUNC()
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I856da9f817b948de3c72af60a0742b128398b4cf
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Greg Rahn 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-HasComments: No


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

2017-07-27 Thread John Sherman (Code Review)
John Sherman has posted comments on this change.

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


Patch Set 3:

I'd like some advice/feedback on the approach for enforcing fe_service_threads. 
In my local diff, I've added a maxTasks, numTasks and a taskLimitMonitor to the 
TAcceptQueueServer class.
>From there it seems like I have two choices:
1)
a) inside the Task constructor, synchronize with the taskLimitMonitor, check 
the passed in server's numTasks against maxTasks and wait if we've reached the 
limit or increment the server's numTasks.
b) inside the Task destructor synchronize with the taskLimitMonitor, decrement 
the server's numTasks and notifyAll if I'm transistioning from maxTasks to < 
maxTasks.
-- I like this because the code is somewhat symmetrical, but I do not know if 
it is a good idea to do these blocking operations inside a 
constructor/destructor.

2)
a) inside the SetupConnection function before I create a new task, synchronize 
with the tasklimitMonitor and check/block/increment numTasks.
b) inside the Tasks run method, decrement/notify where the Task removes itself 
from the TAcceptQueueServer's tasks set.

My local diff uses approach 1 currently but I'm a bit unsure about the best 
practices around that approach.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56a5f3d9cf931cff14eae7f236fea018236a6255
Gerrit-PatchSet: 3
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 
Gerrit-HasComments: No


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

2017-07-27 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has uploaded a new change for review.

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

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

IMPALA-5336: Fix partition pruning when column is cast

Partition pruning has two mechanisms:
1) Simple predicates (e.g. binary predicates of the form
 ) can be used to derive lists
   of matching partition ids directly from the
   partition key values. This is handled directly in the FE
   and is very efficient for supported simple predicates.
2) General expr evaluation of predicates using the BE (via
   FeSupport). This works for all predicates, so is the
   mechanism used for predicates not supported by (1).

The issue was that (1) was being used when a binary
predicate contained an implicit cast on the SlotRef. While
this is OK when being evaluated by the BE, the simple
mechanism in (1) would not be able to match the partition
key values with the predicate literal because the partition
key values cannot be cast in the FE.

The fix is to force binary predicates involving a cast to be
evaluated in the BE.

Testing: A planner test was added to demonstrate the
expected partition pruning occurs.

Some modifications were made to the functional schema table
stringpartitionkey, so it will be necessary to reload those
tables:

load-data.py -w functional-query --table_names=stringpartitionkey

Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
---
M fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A 
testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
5 files changed, 34 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I94f597a6589f5e34d2b74abcd29be77c4161cd99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 


[Impala-ASF-CR] IMPALA-5636: changed the format metadata of repetition level from bit packing to RLE

2017-07-27 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5636: changed the format metadata of repetition level 
from bit_packing to RLE
..


Patch Set 2:

(2 comments)

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

Line 8: 
I'm ok with manually testing this change. We have testing that we can read back 
Parquet files (e.g. test_insert.py).

Could you include a "Testing:" section in the commit message with the relevant 
lines of parquet-tools output, so reviewers can see the result.


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

Line 172: column_encodings_.insert(Encoding::RLE);
We can just delete this line - it's redundant with line 171.


-- 
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: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

PS1, Line 351: tmpdir.strpath
> Adding on to Michael's comment, rather than passing the whole fixture aroun
+1 to this idea.


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes


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

2017-07-27 Thread David Knupp (Code Review)
David Knupp has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

PS1, Line 351: tmpdir.strpath
Adding on to Michael's comment, rather than passing the whole fixture around, 
you might even consider just passing tmpdir.strpath initially, since that seems 
to be the only thing you ultimately need to access in this method.


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-5714: Add linker's version script for OpenSSL library

2017-07-27 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5714: Add linker's version script for OpenSSL library
..


Patch Set 6: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7484/6/source/openssl/build.sh
File source/openssl/build.sh:

PS6, Line 48: $PWD/..
is there a better variable to use for this directory?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72b39c5e15db268d35210c013e885f36764f1f89
Gerrit-PatchSet: 6
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change.

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


Patch Set 1:

(1 comment)

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

PS1, Line 345: tmpdir
Nit: for these private helper methods, prefer a more descriptive name than 
tmpdir. In the test methods, tmpdir is a fixture that gives you a temporary 
directory. But in the helper methods, the directory is a local path to put 
files from HDFS. Maybe outdir or something else? Also, consider updating the 
docstrings to reflect the need for the extra param.


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes


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

2017-07-27 Thread Matthew Jacobs (Code Review)
Matthew Jacobs has posted comments on this change.

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


Patch Set 26:

hm, the failures in the last two runs look different from IMPALA-5729

-- 
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: 26
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.

2017-07-27 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 26:

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

-- 
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: 26
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