from:"anujphadke \(Code Review\)"

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

2017-11-14 Thread anujphadke (Code Review)

anujphadke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8548


Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..

IMPALA-5052: Read and write signed integer logical types in Parquet

This patch maps a signed integer logical type in parquet to a supported
Impala column type. This change introduces the following mapping -

  INT_8  -> TINYINT
  INT_16 -> SMALLINT
  INT_32 -> INT
  INT_64 -> BIGINT

Also, added a parquet file with the following schema for testing -

  schema {
optional int32 id;
optional int32 tinyint_col (INT_8);
optional int32 smallint_col (INT_16);
optional int32 int_col;
optional int64 bigint_col;
  }

Change-Id: I47a8371858c9597c6a440808cf6f933532468927
---
M be/src/exec/hdfs-parquet-table-writer.cc
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
A testdata/data/signed_integer_logical_types.parquet
M tests/query_test/test_insert_parquet.py
4 files changed, 63 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-6137: fix text scanner split delim mem mgmt

2017-11-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8438 )

Change subject: IMPALA-6137: fix text scanner split delim mem mgmt
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddbb5cf6acc8f0b0e0b4c205c334f21e03d06f1c
Gerrit-Change-Number: 8438
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 01 Nov 2017 17:29:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6137: fix text scanner split delim mem mgmt

2017-11-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8438 )

Change subject: IMPALA-6137: fix text scanner split delim mem mgmt
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8438/2//COMMIT_MSG@9
PS2, Line 9: The bug was that the buffer pointed to by byte_buffer_ could be 
freed by
Do you mean byte_buffer_ptr_?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iddbb5cf6acc8f0b0e0b4c205c334f21e03d06f1c
Gerrit-Change-Number: 8438
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 01 Nov 2017 08:52:37 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6080: clean up table descriptor handling

2017-11-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8330 )

Change subject: IMPALA-6080: clean up table descriptor handling
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8330/2/be/src/benchmarks/expr-benchmark.cc
File be/src/benchmarks/expr-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/8330/2/be/src/benchmarks/expr-benchmark.cc@57
PS2, Line 57: #include "service/frontend.h"
I don't think this include is needed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id427dab0c196b556bd8b2d64ec618403d5cbd4d6
Gerrit-Change-Number: 8330
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 01 Nov 2017 08:05:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1575: part 2: yield admission control resourcesa

2017-10-30 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8323 )

Change subject: IMPALA-1575: part 2: yield admission control resourcesa
..


Patch Set 4:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8323/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-1575: part 2: yield admission control resourcesa
typo.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80279eb2bda740d7f61420f52db3bfa42a6a51ac
Gerrit-Change-Number: 8323
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 31 Oct 2017 00:32:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5957: print memory address, not memory

2017-10-24 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8371 )

Change subject: IMPALA-5957: print memory address, not memory
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89250646bf683dd2d636dcb37a66ceb0428af8b2
Gerrit-Change-Number: 8371
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 24 Oct 2017 19:15:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4964: Fix Decimal modulo overflow

2017-10-23 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8329 )

Change subject: IMPALA-4964: Fix Decimal modulo overflow
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/8329/1/be/src/runtime/decimal-value.inline.h@499
PS1, Line 499: // Mod by 0.
Shouldn't we raise an error on decimal modulo 0 operations like you did in 
Impala-5018?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5420201d4440d421e33e443df005cdcc16b8a6cd
Gerrit-Change-Number: 8329
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 23 Oct 2017 21:49:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5018: Error on decimal modulo or divide by zero

2017-10-23 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8344 )

Change subject: IMPALA-5018: Error on decimal modulo or divide by zero
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8344/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/8344/3/be/src/exprs/expr-test.cc@2506
PS3, Line 2506: 0/0
> Seems like something we should fix, even if it's part of a different change
I think we should even change the math function fmod to return a consistent 
error? Maybe as part of the other change proposed above (Since fmod casted this 
to float or double) -

[localhost:21000] > select fmod(cast(9 as decimal(38, 7)), 0);
Query: select fmod(cast(9 as decimal(38, 7)), 0)
Query submitted at: 2017-10-23 13:03:29 (Coordinator: 
http://anuj-OptiPlex-9020:25000)
Query progress can be monitored at: 
http://anuj-OptiPlex-9020:25000/query_plan?query_id=9a48d96079161183:fdd6aed3
+---+
| fmod(cast(9 as decimal(38,7)), 0) |
+---+
| NULL  |
+---+
Fetched 1 row(s) in 0.01s


[localhost:21000] > select cast(9 as decimal(38,7)) % 0;
Query: select cast(9 as decimal(38,7)) % 0
Query submitted at: 2017-10-23 13:13:12 (Coordinator: 
http://anuj-OptiPlex-9020:25000)
Query progress can be monitored at: 
http://anuj-OptiPlex-9020:25000/query_plan?query_id=bc4c087b22f4e1f7:1302ad6a
ERROR: UDF ERROR: Cannot divide decimal by zero



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7a7131e657fcdd293ade78d62f851dac0f1e3eb
Gerrit-Change-Number: 8344
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 23 Oct 2017 20:18:24 +
Gerrit-HasComments: Yes

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

2017-10-12 Thread anujphadke (Code Review)

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

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


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8038/6/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/8038/6/shell/impala_shell.py@1332
PS6, Line 1332:   There are two types of options: shell options and 
query_options. Both can be set be set
typo. "be set be set"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26a3b67230c80a99bd246b6af205d558fec9a986
Gerrit-Change-Number: 8038
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 12 Oct 2017 23:29:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker errors

2017-10-11 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8233 )

Change subject: IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker 
errors
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8233/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8233/2/be/src/codegen/llvm-codegen.cc@1621
PS2, Line 1621: DiagnosticHandler* diagnostic_handler = 
reinterpret_cast(context);
> I dont think there is any scenario as far as how we are using this handler,
You are right. I confused this with something I read about LLVMContext usage 
online. I don't think we need to check for a null here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I907ff1f8975c02d422b4a669afbfc2e536ddf1ff
Gerrit-Change-Number: 8233
Gerrit-PatchSet: 3
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 11 Oct 2017 21:43:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker errors

2017-10-10 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8233 )

Change subject: IMPALA-6002: Add a LLVM diagnostic handler for LLVM linker 
errors
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8233/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/8233/2/be/src/codegen/llvm-codegen.cc@1621
PS2, Line 1621: DiagnosticHandler* diagnostic_handler = 
reinterpret_cast(context);
Can context be a nullptr in any scenarios? We should check for that before 
dereferencing it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I907ff1f8975c02d422b4a669afbfc2e536ddf1ff
Gerrit-Change-Number: 8233
Gerrit-PatchSet: 2
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 10 Oct 2017 19:02:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Test cleanup related to the old join node.

2017-09-27 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8153 )

Change subject: Test cleanup related to the old join node.
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idda4b34b5e6e9b5012b177a4c00077aa7fec394c
Gerrit-Change-Number: 8153
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 27 Sep 2017 15:07:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4682 Fix IllegalStateException issue

2017-09-26 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8143 )

Change subject: IMPALA-4682 Fix IllegalStateException issue
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/8143/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-4682 Fix IllegalStateException issue
Can you state the issue in short?


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

http://gerrit.cloudera.org:8080/#/c/8143/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@731
PS1, Line 731: + selectListItem.toSql());
Can  we add a test  for this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57c20aeed401275d45913fedfd61c206c38641b7
Gerrit-Change-Number: 8143
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 26 Sep 2017 21:44:03 +
Gerrit-HasComments: Yes

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

2017-09-20 Thread anujphadke (Code Review)

Hello Impala Public Jenkins, Tim Armstrong,

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

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

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

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

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

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

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

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
M tests/query_test/test_scanners.py
7 files changed, 71 insertions(+), 20 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

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

2017-09-20 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: Remove unused MemPool::peak_allocated_bytes_
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99eba01869914c1d1e0a6ed0cab039d904fecc02
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

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

2017-09-20 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 8:

Ran the core tests and the exhaustive tests with the change and they passed.

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

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

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

2017-09-20 Thread anujphadke (Code Review)

Hello Impala Public Jenkins, Tim Armstrong,

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

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

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

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

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

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

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

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
M tests/query_test/test_scanners.py
7 files changed, 71 insertions(+), 20 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-09-11 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#7).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 169 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-3897 Codegen null-aware constant in PHJ::ProcessBuildBatch()

2017-09-11 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-3897 Codegen null-aware constant in 
PHJ::ProcessBuildBatch()
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7849/1/be/src/exec/partitioned-hash-join-builder.h
File be/src/exec/partitioned-hash-join-builder.h:

Line 280:   /// Free local allocations made from expr evaluators during hash 
table construction.
> Can you add a short description of the parameter?
Done


PS1, Line 477: 
> nit: parameter name doesn't match other functions
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06acbebc9d2d23bef4734b480a5d3ce41680ea70
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3897 Codegen null-aware constant in PHJ::ProcessBuildBatch()

2017-09-11 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

Change subject: IMPALA-3897 Codegen null-aware constant in 
PHJ::ProcessBuildBatch()
..

IMPALA-3897 Codegen null-aware constant in PHJ::ProcessBuildBatch()

This change codegen outs a branch in ProcessBuildBatch(). This branch never gets
executed for most of the join types except NULL_AWARE_LEFT_ANTI_JOIN.
The branch itself is not expensive to execute, but it will reduce codegen
time by removing the dead code inside the branch for almost all join
modes.

Change-Id: I06acbebc9d2d23bef4734b480a5d3ce41680ea70
---
M be/src/codegen/gen_ir_descriptions.py
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
4 files changed, 20 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06acbebc9d2d23bef4734b480a5d3ce41680ea70
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong

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

2017-09-11 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 7:

I tested this locally on my machine with a cluster size = 1.
I was able to recreate this locally on starting the impala minicluster with 
size=3.
I have a fix for this which I am testing right now.
Spoke to Tim about it. Sorry forgot to update it here.

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

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

[Impala-ASF-CR] IMPALA-3897 Codegen null-aware constant in PHJ::ProcessBuildBatch()

2017-08-25 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: IMPALA-3897 Codegen null-aware constant in 
PHJ::ProcessBuildBatch()
..

IMPALA-3897 Codegen null-aware constant in PHJ::ProcessBuildBatch()

This change codegen outs a branch in ProcessBuildBatch(). This branch never gets
executed for most of the join types except NULL_AWARE_LEFT_ANTI_JOIN.
The branch itself is not expensive to execute, but it will reduce codegen
time by removing the dead code inside the branch for almost all join
modes.

Change-Id: I06acbebc9d2d23bef4734b480a5d3ce41680ea70
---
M be/src/codegen/gen_ir_descriptions.py
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
4 files changed, 18 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I06acbebc9d2d23bef4734b480a5d3ce41680ea70
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

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

2017-08-16 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7245/5/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

Line 885: THdfsCompression::type compression_type = 
std::get<2>(it->first);
> nit: missing space before <<
Done


PS5, Line 888: fi
> nit: align <<'s vertically (it's in the style guide somewhere).
Done


http://gerrit.cloudera.org:8080/#/c/7245/5/testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
File 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test:

Line 9: 
> I missed this in an earlier pass but we don't have a regression test for IM
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

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

2017-08-16 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#6).

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

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

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

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

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
M tests/query_test/test_scanners.py
7 files changed, 71 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

2017-08-11 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-2689: Log why codegen is disabled in 
TextConverter::CodegenWriteSlot()
..


Patch Set 3:

(13 comments)

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

PS2, Line 7: why codegen is disabled in
> How about "Log errors in"? "Disabled" reads as if it was done by a user on 
Tried to keep it consistent with the older fixes -
http://gerrit.cloudera.org:8080/#/c/2048/

Let me know if this is fine. I will change it if sounds misleading.


PS2, Line 10: propagate
> typo
Done


http://gerrit.cloudera.org:8080/#/c/7574/2/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

PS2, Line 326: tabl
> Switch to nullptr
Done


http://gerrit.cloudera.org:8080/#/c/7574/1/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:

Line 280: // Parse failed, set slot to null and return false
> ?
Done


http://gerrit.cloudera.org:8080/#/c/7574/2/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:

Line 76: //   %1 = bitcast <{ i32, i8 }>* %tuple_arg to i8*
> Same question. I'm guessing that the old IR was just stale, in which case -
This patch should not change the IR. 
This is just the updated IR.


Line 111:   DCHECK(fn != nullptr);
> You could add a DCHECK(fn != nullptr)
Done


Line 125:   }
> This could be a DCHECK. We shouldn't be missing builtin functions.
Done


PS2, Line 126: 
> Yes, that sounds reasonable. Please add () to the function names to follow 
Added a DCHECK instead.


PS2, Line 127: is_null_strin
> From the code and this error message it's not clear to me what went wrong. 
Added a DCHECK.


Line 232:   default:
> What does NYI mean? Maybe make this "Unsupported type". It should never be 
Not yet implemented. We have used this acronym elsewhere while fixing this JIRA 
in other places. 
This message gets propagated to the runtime profile.


Line 280: // Parse failed, set slot to null and return false
> This doesn't have any effect, did you forget a "return"?
Forgot to git add. Had this change locally.


Line 281: builder.SetInsertPoint(parse_failed_block);
> Following the same convention as elsewhere in the code makes sense to me.
Done


http://gerrit.cloudera.org:8080/#/c/7574/2/be/src/exec/text-converter.h
File be/src/exec/text-converter.h:

Line 83:   /// strict_mode: If set, numerical overflow/underflow are considered 
to be parse
> Can you add a comment for the new output parameter?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

2017-08-11 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

Change subject: IMPALA-2689: Log why codegen is disabled in 
TextConverter::CodegenWriteSlot()
..

IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

Plumbing statuses through TextConverter::CodegenWriteSlot(), which
eventually propagate to the runtime profile.

Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
---
M be/src/exec/hdfs-scanner.cc
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
3 files changed, 53 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

2017-08-03 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-2869: Log why codegen is disabled in
TextConverter::CodegenWriteSlot()
..

Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7574/2/be/src/exec/text-converter.cc
File be/src/exec/text-converter.cc:

PS2, Line 126: TextConverter::CodegenWriteSlot
> Do you need this part? If the error gets logged, won't it include where it
oops it should be Impala-2689. I will fix the commit message.
This shows up in runtime profile when codegen is disabled.

Ex: from the profile -
ExecOption: TEXT Codegen Disabled: TextConverter::CodegenWriteSlot: Char isn't
supported for CodegenWriteSlot

I think it would be useful to print this.
Let me know what you think? I will fix this in the next patch.

Line 281:"WriteSlot function failed verification");
> Maybe call it "FinalizeFunction() failed"? The error message seems differen
oops it should be Impala-2689. I will fix the commit message in the next patch.

FinalizeFunction returns NULL when verification fails. We have a similar log
message's elsewhere too -
https://github.com/cloudera/Impala/commit/2be7a1723f0340367e050f7cafd8a65fab55e37e#diff-1a1c603752b59b3305d47ba0805f1ca2R597
I ll also append the "check logs" to the message.

Let me know what do you think? I will change it accordingly in the next patch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: anujphadke
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

2017-08-03 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

Change subject: IMPALA-2869: Log why codegen is disabled in 
TextConverter::CodegenWriteSlot()
..

IMPALA-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

Plumbing statuses through TextConverter::CodegenWriteSlot(), which
eventually propogate to the runtime profile.

Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
---
M be/src/exec/hdfs-scanner.cc
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
3 files changed, 53 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] Impala-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

2017-08-03 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: Impala-2869: Log why codegen is disabled in 
TextConverter::CodegenWriteSlot()
..

Impala-2869: Log why codegen is disabled in TextConverter::CodegenWriteSlot()

Plumbing statuses through TextConverter::CodegenWriteSlot(), which
eventually propogate to the runtime profile.

Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
---
M be/src/exec/hdfs-scanner.cc
M be/src/exec/text-converter.cc
M be/src/exec/text-converter.h
3 files changed, 52 insertions(+), 40 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

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

2017-08-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7245/4/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:

PS4, Line 259: 
 :   /// 2. scan range 
> This part isn't quite right, since we do know the compression type and runt
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

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

2017-08-01 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#5).

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

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

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

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

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
6 files changed, 48 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

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

2017-07-31 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7245/3/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

Line 27: #include 
> Shouldn't this be in the header instead of the .cc?
Done


Line 776:   vector types;
> Parameter names don't match header
Done


Line 883: // If a scan range stored as parquet is skipped, its 
compression type
> One line comment explaining why Parquet is a special case.
Done


PS3, Line 886:   } else {
 : ss << file_format << "/
> AVRO/SNAPPY(Skipped) I think reads better
Done. Changed it everywhere.


http://gerrit.cloudera.org:8080/#/c/7245/3/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:

Line 257:   /// in the file. The metrics are incremented for each 
compression_type.
> Parameter names are inconsistent - skipped vs filtered. We should also docu
Done


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

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

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

2017-07-31 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#4).

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

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

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

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

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
6 files changed, 49 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[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-5688: Reduce run-time of a couple of expr-test heavy-hitters

2017-07-20 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5688: Reduce run-time of a couple of expr-test 
heavy-hitters
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2962115734aff8dcaae0cc405274765105e31572
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-20 Thread anujphadke (Code Review)

Hello Impala Public Jenkins, Michael Brown, Bharath Vissapragada, Matthew 
Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 188 insertions(+), 93 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7393/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 1023: for (int j = 0; j < build_rows->num_rows(); ++j) {
> We still need the check in the outer loop in case there are many matched ro
I thought you implied to move this inside. 
Makes sense we could be  stuck in the outer loop as well as the inner one.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-19 Thread anujphadke (Code Review)

Hello Thomas Tauber-Marshall,

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

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

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

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..

IMPALA-5586: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe() and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 16 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7332/7/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS7, Line 105: " 
> technically this is still mixed case, no?
Done.


Line 168: try:
> This line should be outside of the try loop, otherwise role_name isn't in s
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-19 Thread anujphadke (Code Review)

Hello Bharath Vissapragada, Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 187 insertions(+), 93 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7332/6/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS6, Line 104:   db_name = "test_role_privilege_case_x_" + ge
> Why use a random suffix here but not in test_grant_revoke?
Done.
I  did not look much into that test since this patch only adds 
test_role_privilege_case.
Thanks for pointing that out.


PS6, Line 105:   db_name_upper_case = "TEST_ROLE_PRIVILEGE_CASE_y_" + 
get_random_id(5)
 :   db_name_mixed_case = "TesT_Role_PRIVIlege_case
> Maybe use TEST_GRANT_REVOKE and Test_GraNt_revoke or similar for prefixes?
Done


Line 138:   assert any(db_name_mixed_case.lower() in x for x in result.data)
> Yeah, most of these fit within 90 chars on 1 line. For the ones that don't,
Done


PS6, Line 195: 
> Not your change, but could you remove this semi-colon?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-19 Thread anujphadke (Code Review)

Hello Bharath Vissapragada, Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 187 insertions(+), 93 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-18 Thread anujphadke (Code Review)

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 171 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-17 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

Patch Set 5:

(5 comments)

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

PS4, Line 9: Privileges granted to a role assigned
> Privileges are granted "to" roles "on" objects. I think this might need a l
Done

http://gerrit.cloudera.org:8080/#/c/7332/4/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
File fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java:

PS4, Line 76: // (IMPALA-2695) URIs are case sensitive
: authorizable.add(KV_JOINER.join("uri",
privilege.getUri()
> // (IMPALA-2695) URIs are case sensitive.
Done

http://gerrit.cloudera.org:8080/#/c/7332/4/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS4, Line 106: dom_id(5)
> Could you randomize it as discussed in the other thread? Look for usages of
Done.
I thought we should fix this after fixing IMPALA-5660

PS4, Line 127: self.cli
> you tested that this fails without your change? I'm just wondering if 2sec
yes, It failed without this change if statestore_update_frequency_ms is set to
a high value. I had removed it in the last patch. Setting it back to 300ms.

I did try running this test on asf master
but with a sleep > 1 min since sentry_catalog_polling_frequency_s was not part
of it.

PS4, Line 135: grant all on d
> drop database if exists
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: anujphadke
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-17 Thread anujphadke (Code Review)

Hello Matthew Jacobs,

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

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

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 171 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-17 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7393/3/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 1018:   // For each row, iterate over all rows in the build table.
> The need for RETURN_IF_CANCELLED is a little subtle. A one-line comment wou
Done


Line 1023:   // This loop may run for a long time. Periodically check for 
cancellation.
> This loop can also potentially run for a long time if build_rows is large. 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-17 Thread anujphadke (Code Review)

Hello Thomas Tauber-Marshall,

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

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

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

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..

IMPALA-5586: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe() and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 13 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-17 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 4:

(10 comments)

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

PS3, Line 16: n update of the catalog object followed by a
: removal of the old object. Since they both use the same key
> qq, Isn't this order always the same?
Verified that the order is always the same. Removal followed by additions.
Removed this comment.


http://gerrit.cloudera.org:8080/#/c/7332/3/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:

PS3, Line 42:  
> nit space
Done


http://gerrit.cloudera.org:8080/#/c/7332/3/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
File fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java:

PS3, Line 336: trings.nullToEmpty(privilege.getUri()));
> s/ URIs are case sensitive ?
Done


http://gerrit.cloudera.org:8080/#/c/7332/3/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
File fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java:

Line 75:   toLowerCase()));
> nit: 4 space formatting everywhere
Done


http://gerrit.cloudera.org:8080/#/c/7332/3/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

Line 25: 
> It shouldn't be unless any is shadowed, which it ought not be. If it is, th
Done


PS3, Line 129: 
> why not just 1 second?
setting this 1 and the sleep below to 2.


PS3, Line 130: assert any('test2' in x for x in result.data)
 :   assert any('test3' in x for x in result.data)
> Is this required?
Removed.


Line 149: """
> Could you also add some grants on camel case DB names?
Done


PS3, Line 156: grp.getgrnam(getuser()).gr_name))
 : result = self.client.execute("show tables in functional")
 : assert 'alltypes' in result.data
 : privileges_before = self.client.execute("show grant role 
test_role")
 : # Wait a 
> I wonder if we can lower this if we reduce the sentry polling freq. Even a 
Done


PS3, Line 165: verifier.wait_for_metric("catalog.ready", 
> How about adding it to a finally block incase the test fails, also clean up
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-17 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#4).

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a db/table whose name contains upper case
characters can disappear after a few seconds. A privilege
is inserted into the catalogObjectCache using a key that
uses the db/table name. The key gets converted to a lower case
before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
9 files changed, 145 insertions(+), 71 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7332/3/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:

PS3, Line 41: DEFINE_int64
> int32 should be fine.
int64_t maps to a long?

scheduleAtFixedRate(Runnable command,
 long initialDelay,
 long period,
 TimeUnit unit)


http://gerrit.cloudera.org:8080/#/c/7332/3/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

PS3, Line 143: grant_rev_db
> We looked at that, but it actually doesn't work for these custom cluster te
def test_role_privilege_case(self, vector, unique_database):

This is the error I got on doing it. -
ERROR at setup of TestGrantRevoke.test_role_privilege_case[exec_option: 
{'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 
'disable_codegen': False, 'abort_on_error': 1, 
'exec_single_node_rows_threshold': 0} | table_format: text/none] 
conftest.py:294: in unique_database
E   ImpalaBeeswaxException: ImpalaBeeswaxException:
EINNER EXCEPTION: 
EMESSAGE: AuthorizationException: User 'anuj' does not have privileges to 
execute 'DROP' on: test_role_privilege_case_2af3a508


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..


Patch Set 3:

(2 comments)

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

PS2, Line 15: PartitionedHashJoinNode::EvaluateNullProbe
> Add '()' after like you have above.
Done


http://gerrit.cloudera.org:8080/#/c/7393/2/be/src/exec/partitioned-hash-join-node.h
File be/src/exec/partitioned-hash-join-node.h:

PS2, Line 350: u
> extra space
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-13 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..

IMPALA-5586: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe() and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 12 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Thomas Tauber-Marshall

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5582: Store sentry privileges in lower case
..


Patch Set 3:

Discussed this issue with MJ. This issue does occur due to inconsistent 
handling of casing.
Changed buildRolePrivilegeName to return the privilege names in lower case 
instead of changing  it in privilegeSpec.
 Also changed getRolePrivileges to return the output of show grant roles in 
lower case since we are not lower casing privilegeSpec object anymore.

Added a test in test_grant_revoke.py and made sentryProxy configurable.

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

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

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-07-13 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a db/table whose name contains upper case
characters can disappear after a few seconds. A privilege
is inserted into the catalogObjectCache using a key that
uses the db/table name. The key gets converted to lower case
before inserting. The db/table name is stored in upper case in
the rolePrivilege object.
The sentryProxy thread on the other hand returns the db/table name
in lower case. If the catalogObjectCache on a update adds the new
rolePrivilege object first and removes the old object later, it
ends up deleting the newer verison of the rolePrivilege since they
both use the same key to add/remove from the cache.
This change sets the privilege name in lower case which does not
trigger these chain of events on a sentryProxy thread update.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables  specified in
lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M be/src/catalog/catalog.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M tests/authorization/test_grant_revoke.py
8 files changed, 90 insertions(+), 21 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5586: Null-aware anti-join can take a long time to cancel

2017-07-11 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

Change subject: IMPALA-5586: Null-aware anti-join can take a long time to cancel
..

IMPALA-5586: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 12 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-5582: Null-aware anti-join can take a long time to cancel

2017-07-10 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: IMPALA-5582: Null-aware anti-join can take a long time to cancel
..

IMPALA-5582: Null-aware anti-join can take a long time to cancel

Queries with a null-aware anti-join joining on a large number of NULLs
can take a long time to cancel if threads are stuck in
PartitionedHashJoinNode::EvaluateNullProbe(). This change adds the
RETURN_IF_CANCELLED macro to the function.

Testing:
Added logs to PartitionedHashJoinNode::EvaluateNullProbe and made sure
that the function returns right away on cancellation.

Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
---
M be/src/exec/partitioned-hash-join-node.cc
M be/src/exec/partitioned-hash-join-node.h
2 files changed, 12 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0800754d4ad31cbadbdfadc630c640963f3f6053
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

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

2017-07-10 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 3:

(5 comments)

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

Line 7: IMPALA-4863/IMPALA-5311: Correctly account the file type and 
compression codec
> It would be easier to review if we included the IMPALA-5311 fix in this, si
Done


http://gerrit.cloudera.org:8080/#/c/7245/2/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

PS2, Line 553: 
> It seems like this will remove useful information for file formats where we
Done


Line 771: 
> missing space - maybe run it it through clang-format before posting?
Done


http://gerrit.cloudera.org:8080/#/c/7245/2/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:

PS2, Line 250: t 
> should have spaces around " = "
Done


PS2, Line 462: 
> don't need space between >> in C++11
Not needed anymore.
Moved std::pair to a tuple


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

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

2017-07-10 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

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

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

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

e.g. - File Formats: PARQUET/SNAPPY:364 PARQUET(Skipped):1460

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M 
testdata/workloads/functional-query/queries/QueryTest/hdfs_scanner_profile.test
6 files changed, 42 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-06-29 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/5/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

Line 452: // We can store the results in a decimal8Value. But this 
change hard codes to
> It would be good to summarize the calculations and comment on the chosen ty
Done. 
Do you think it would useful to move the comments at relevant places inside the 
functions instead of adding it before the definition?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-06-29 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#6).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 169 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-06-29 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a db whose name contains upper case
characters can disappear after a few seconds. A privilege
is inserted into the catalogObjectCache using a key that
uses the db name. The key gets converted to lower case
before inserting. The sentryProxy thread on the other hand
returns the db name in lower case. When the catalogObjectCache
gets updated and the old catalog object is removed from the
cache it ends up deleting the new object since they both use
the same key. This change stores the privileges in lower case
which does not trigger these chain on events on a sentryProxy
thread update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
1 file changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-5582: Store sentry privileges in lower case

2017-06-29 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: IMPALA-5582: Store sentry privileges in lower case
..

IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a db whose name contains upper case characters can 
disappear
after a few seconds. A privilege is inserted into the catalogObjectCache using 
a key
that uses the db name. The key gets converted to lower case before inserting.
The sentryProxy thread on the other hand returns the db name in lower case. 
When the
catalogObjectCache gets updated and the old catalog object is removed from the 
cache
it ends up deleting the new object since they both use the same key. This 
change stores
the privileges in lower case  which does not trigger these chain on events on a
sentryProxy thread update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
1 file changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-06-22 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..


Patch Set 5:

Rebased. 
Also, corrected the result of related query in nested-types-subplan.test.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-06-22 Thread anujphadke (Code Review)

Hello Dan Hecht, Tim Armstrong,

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

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

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

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..

IMPALA-4866: Hash join node does not apply limits correctly

Hash join node currently does not apply the limits correctly.
This issue gets masked most of the times since the planner sticks
an exchange node on top of most of the joins. This issue gets
exposed when NUM_NODES=1.

Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
---
M be/src/exec/partitioned-hash-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
A 
testdata/workloads/functional-query/queries/QueryTest/single-node-joins-with-limits-exhaustive.test
M tests/query_test/test_join_queries.py
4 files changed, 69 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

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

2017-06-21 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

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


Patch Set 2:

Defaulting filtered to false in the function definition instead of passing 
false at every function invocation.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

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

2017-06-21 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

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

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

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

e.g. - File Formats: PARQUET/SNAPPY:364 PARQUET(Filtered):1460

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
4 files changed, 21 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong

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

2017-06-21 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

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

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

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

e.g. - File Formats: PARQUET/SNAPPY:364 PARQUET(Filtered):1460

Change-Id: I797916505f62e568f4159e07099481b8ff571da2
---
M be/src/exec/base-sequence-scanner.cc
M be/src/exec/hdfs-parquet-scanner.cc
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scan-node.h
M be/src/exec/hdfs-text-scanner.cc
7 files changed, 27 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I797916505f62e568f4159e07099481b8ff571da2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-06-21 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6778/3/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

PS3, Line 502: rue) {
> We don't need this now since we don't update num_rows_returned_ inside the 
Done


Line 508: 
> Remove the extra line - don't need that much vertical whitespace.
Done


PS3, Line 577: 
> This is also unnecessary.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-06-21 Thread anujphadke (Code Review)

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..

IMPALA-4866: Hash join node does not apply limits correctly

Hash join node currently does not apply the limits correctly.
This issue gets masked most of the times since the planner sticks
an exchange node on top of most of the joins. This issue gets
exposed when NUM_NODES=1.

Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
---
M be/src/exec/partitioned-hash-join-node.cc
A 
testdata/workloads/functional-query/queries/QueryTest/single-node-joins-with-limits-exhaustive.test
M tests/query_test/test_join_queries.py
3 files changed, 68 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-06-20 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..

IMPALA-4866: Hash join node does not apply limits correctly

Hash join node currently does not apply the limits correctly.
This issue gets masked most of the times since the planner sticks
an exchange node on top of most of the joins. This issue gets
exposed when NUM_NODES=1.

Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
---
M be/src/exec/partitioned-hash-join-node.cc
A 
testdata/workloads/functional-query/queries/QueryTest/single-node-joins-with-limits-exhaustive.test
M tests/query_test/test_join_queries.py
3 files changed, 67 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5497: spilling hash joins that output build rows hit OOM

2017-06-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5497: spilling hash joins that output build rows hit OOM
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7180/4/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 510:   OutputUnmatchedBuild(out_batch);
I think we should check the status returned by this function for any error 
cases (maybe just RETURN_IF_ERROR).
If this function returns early on an error, output_batch might not be at 
capacity , and the assumption we make on line-515 might not be true?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I075388d348499c5692d044ac1bc38dd8dd0b10c7
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5524: Fixes NPE during planning with DISABLE UNFASE SPILLS=1

2017-06-18 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5524: Fixes NPE during planning with 
DISABLE_UNFASE_SPILLS=1
..


Patch Set 1:

(1 comment)

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

PS1, Line 7: DISABLE_UNFASE_SPILLS
typo


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccae7fdeaf0ade0b8728a7d249981c8270e8c327
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Vincent Tran 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5497: spilling hash joins that output build rows hit OOM

2017-06-16 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5497: spilling hash joins that output build rows hit OOM
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7180/4/testdata/workloads/functional-query/queries/QueryTest/spilling.test
File testdata/workloads/functional-query/queries/QueryTest/spilling.test:

Line 694: # IMPALA-5173: spilling hash join feeding into right side of nested 
loop join.
This query failed  locally on my desktop with the following error.


Query progress can be monitored at: 
http://anuj-OptiPlex-9020:25000/query_plan?query_id=d94cf9b183dae78b:7f8ff8e4
WARNINGS: Memory limit exceeded: The memory limit is set too low to initialize 
spilling operator (id=3). The minimum required memory to spill this operator is 
136.00 MB.
Error occurred on backend anuj-OptiPlex-9020:22000 by fragment 
d94cf9b183dae78b:7f8ff8e4


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I075388d348499c5692d044ac1bc38dd8dd0b10c7
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5499: avoid ephemeral port conflicts

2017-06-14 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5499: avoid ephemeral port conflicts
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id02c83e6f946a14b83f5e6561957d7ad81442835
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5499: avoid ephemeral port conflicts

2017-06-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5499: avoid ephemeral port conflicts
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7171/1/be/src/util/network-util.cc
File be/src/util/network-util.cc:

Line 197:   continue;
Should'nt we increment tries here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id02c83e6f946a14b83f5e6561957d7ad81442835
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5495: Improve error message if no impalad role configured

2017-06-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5495: Improve error message if no impalad role configured
..


Patch Set 1: Code-Review+1

(1 comment)

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

Line 7: IMPALA-5495: Improve error message if no impalad role configured
role is configured.


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

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

[Impala-ASF-CR] IMPALA-5223: Add waiting for HBase Zookeeper nodes to retry loop

2017-06-12 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5223: Add waiting for HBase Zookeeper nodes to retry loop
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8dbdff4ad02cac1322e7d580e0a6971daf6ea28
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-07 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5400: Execute tests in subplans.test
..


Patch Set 5:

Removing the test that failed.
Raised a JIRA to track it -
https://issues.apache.org/jira/browse/IMPALA-5438

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-06-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 5:

(13 comments)

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

Line 31: #include "exprs/decimal-operators.h"
> order alphabetically
Done


Line 430: const T1& max_range, const IntVal& num_buckets) {
> move declarations closer to where they are used
Done


Line 432:   bool overflow = false;
> // FE casts all the arguments to the same type.
Done


Line 433:   // FE casts all the arguments to the same type.
> formatting, space after ','
Done


Line 434:   int input_scale = 
ctx->impl()->GetConstFnAttr(FunctionContextImpl::ARG_TYPE_SCALE, 1);
> formatting, space after ','
Done


Line 442: 
> single line and formatting (please fix formatting everywhere)
Done


Line 447: result.val = num_buckets.val;
> How about we make the return value a BIGINT and simplify this code (no need
Done


Line 457:   input_scale, input_precision, input_scale, false, );
> use int128_t consistently
Done


Line 460: error_msg << "Overflow while evaluating the difference between 
min_range: " <<
> Please make the error more specific, so we can see where the overflow happe
Done


Line 472: ctx->SetError(error_msg.str().c_str());
> Don't you need to convert the arguments before the multiplication?
Done


Line 475: 
> typo: resulting
Done


Line 476:   // resulting scale should be 2 * input_scale as per multiplication 
rules
> I think we'll need to be more stingy with the types. For example, the input
Discussed this with Alex.

For this  particular scenario where we might need to store results with 
decimal8Values inputs into int256_t.

In this particular case- where expr and min_range are decimal8Values. This 
subtraction can overflow
into a decimal16Values 

ex:
expr = max(decimal8Value) 
min_range = -1 (or any negative value)


width_size = expr.template Subtract<__int128_t>(input_scale, min_range, 
input_scale,
   input_precision, input_scale, false, );

width_size has to be decimal16values (to store this overflowed value) even for 
decimal8values.

Should we templetize  this function? Since we convert intermediate results to 
decimal16value .


http://gerrit.cloudera.org:8080/#/c/6023/4/be/src/exprs/math-functions.h
File be/src/exprs/math-functions.h:

Line 111:   const IntVal& num_buckets);
> weird indentation
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5400: Execute tests in subplans.test
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7038/3/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 264: bigint, string
> nit: lower case types as well
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

Hello Alex Behm, Tim Armstrong,

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

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

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

Change subject: IMPALA-5400: Execute tests in subplans.test
..

IMPALA-5400: Execute tests in subplans.test

This change executes the tests added to subplans.test and removes
a test which incorrectly references subplannull_data.test (a file
which does not exist)

Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
D testdata/workloads/functional-query/queries/QueryTest/subplans.test
M tests/query_test/test_queries.py
3 files changed, 47 insertions(+), 239 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-06-06 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#5).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 130 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5400: Execute tests in subplans.test
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7038/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 257: select count(okey), opriority
> please make the casing of these new tests consistent with the others in thi
Done


Line 331:  RESULTS:
> remove the "VERIFY_IS_EQUAL_SORTED" part since it's not needed here
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5400: Execute tests in subplans.test
..

IMPALA-5400: Execute tests in subplans.test

This change executes the tests added to subplans.test and removes
a test which incorrectly references subplannull_data.test (a file
which does not exist)

Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
D testdata/workloads/functional-query/queries/QueryTest/subplans.test
M tests/query_test/test_queries.py
3 files changed, 44 insertions(+), 236 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5400: Execute tests in subplans.test
..


Patch Set 2:

(14 comments)

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

Line 1
> Let's merge these tests into nested-types-subplan.test. I'll add comments w
Done


Line 3
> Merge into nested-types-subplan.test
Impala-5438  will add a new test.


Line 36
> Remove. Already covered in nested-types-subplan.test
Done


Line 50
> Remove. Already covered in nested-types-subplan.test
Done


Line 63
> Remove. Already covered in nested-types-subplan.test
Done


Line 87
> Remove. Already covered in nested-types-subplan.test
Done


Line 110
> Remove. Already covered in nested-types-subplan.test
Done


Line 124
> Remove. Already covered in nested-types-subplan.test
Done


Line 137
> Remove. Already covered in nested-types-subplan.test
Done


Line 158
> Remove. Already covered in nested-types-subplan.test
Done


Line 171
> Remove. Already covered in nested-types-subplan.test
Done


Line 188
> Merge into nested-types-subplan.test
Done


Line 205
> Merge into nested-types-subplan.test
Done


Line 219
> Merge into nested-types-subplan.test
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-06 Thread anujphadke (Code Review)

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-5400: Execute tests in subplans.test
..

IMPALA-5400: Execute tests in subplans.test

This change executes the tests added to subplans.test and removes
a test which incorrectly references subplannull_data.test (a file
which does not exist)

Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
---
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
D testdata/workloads/functional-query/queries/QueryTest/subplans.test
M tests/query_test/test_queries.py
3 files changed, 44 insertions(+), 236 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5438: Always eval union const exprs in subplan.

2017-06-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5438: Always eval union const exprs in subplan.
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icd2f21f0213188e2304f8e9536019c7940c07768
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5438: Always eval union const exprs in subplan.

2017-06-05 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5438: Always eval union const exprs in subplan.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7091/1/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 592:  QUERY
I think we should  validate the results  of this query. Maybe check the number 
of rows returned. 
Won't this test work with/without this change?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icd2f21f0213188e2304f8e9536019c7940c07768
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-06-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5400: Execute tests in subplans.test
..


Patch Set 1:

(1 comment)

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

Line 229: 16
These tests have not been running for a while.
I am not sure if this test has regressed or the data has changed?
Does anyone how do I check what the correct result is?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5400: Execute tests in subplans.test

2017-05-31 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: IMPALA-5400: Execute tests in subplans.test
..

IMPALA-5400: Execute tests in subplans.test

This change executes the tests added to subplans.test and removes
a test which incorrectly references subplannull_data.test (a file
which does not exist)

Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
---
M testdata/workloads/functional-query/queries/QueryTest/subplans.test
M tests/query_test/test_nested_types.py
M tests/query_test/test_queries.py
3 files changed, 33 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit

2017-05-31 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7014/4/testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test:

Line 598:  RESULTS
> Just realised these were missing a RESULTS section - could you add one. Sor
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit

2017-05-31 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#4).

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit
..

IMPALA-5363: Reset probe_batch_ after reaching limit

For every new iteration of a subplan there are leftover
rows from the previous iteration of a subplan. This change
transfers the ownership from the probe_batch_ to output_batch_
and resets the probe_batch_ on hitting the limit.

Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
---
M be/src/exec/partitioned-hash-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/nested-types-subplan.test
2 files changed, 24 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit

2017-05-30 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#3).

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit
..

IMPALA-5363: Reset probe_batch_ after reaching limit

For every new iteration of a subplan there are leftover
rows from the previous iteration of a subplan. This change
transfers the ownership from the probe_batch_ to output_batch_
and resets the probe_batch_ on hitting the limit.

Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
---
M be/src/exec/partitioned-hash-join-node.cc
M testdata/workloads/functional-query/queries/QueryTest/joins.test
2 files changed, 24 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit

2017-05-30 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit
..


Patch Set 2:

(3 comments)

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

> I think this patch needs a test case (I think you already had a query, righ
Done. 
I had tried running similar query to produce a nested loop join. Looks like it 
is handled correctly there since impala does not crash in that case.

I have added the query which crashed impala to the test cases.


http://gerrit.cloudera.org:8080/#/c/7014/1/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 498:   while (!ReachedLimit()) {
> It looks like TransferResourceOwnership() already calls Reset(), so we can 
Done


Line 506:   DCHECK(NeedToProcessUnmatchedBuildRows());
> How about we avoid the duplicate ReachedLimit() return path by changing thi
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit

2017-05-30 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2).

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit
..

IMPALA-5363: Reset probe_batch_ after reaching limit

For every new iteration of a subplan there are leftover
rows from the previous iteration of a subplan. This change
transfers the ownership from the probe_batch_ to output_batch_
and resets the probe_batch_ on hitting the limit.

Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
---
M be/src/exec/partitioned-hash-join-node.cc
M testdata/workloads/functional-query/queries/QueryTest/joins.test
2 files changed, 14 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit.

2017-05-29 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit.
..


Patch Set 1:

Ran exhaustive tests and it passed.
Ran this particular query which crashed impala before this change. 
select * FROM tpch_nested_parquet.customer c, (SELECT ca.o_orderkey okey, 
ca.o_orderpriority opriority FROM c.c_orders ca, c.c_orders cb WHERE 
ca.o_orderkey = cb.o_orderkey limit 2) v limit 51;
Initial draft patch. Will add a test.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5363: Reset probe batch after reaching limit.

2017-05-29 Thread anujphadke (Code Review)

anujphadke has uploaded a new change for review.

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

Change subject: IMPALA-5363: Reset probe_batch_ after reaching limit.
..

IMPALA-5363: Reset probe_batch_ after reaching limit.

For every new iteration of a subplan there are leftover
rows from the previous iteration of a subplan. This change
transfers the ownership from the probe_batch_ to output_batch_
and resets the probe_batch_ on hitting the limit.

Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
---
M be/src/exec/partitioned-hash-join-node.cc
1 file changed, 7 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-5347: Parquet scanner microoptimizations

2017-05-22 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-5347: Parquet scanner microoptimizations
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6950/2/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

Line 426: *num_values = (curr_tuple - tuple_mem) / tuple_size;
Should we add a DCHECK to make sure that tuple_size is non-zero?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I49ec523a65542fdbabd53fbcc4a8901d769e5cd5
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.

2017-05-10 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
..


Patch Set 2:

(2 comments)

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

PS2, Line 12: ProcessSpit
nit: typo


http://gerrit.cloudera.org:8080/#/c/6527/2/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

Line 34: #include "util/pretty-printer.h"
unused.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4866: Hash join node does not apply limits correctly

2017-05-08 Thread anujphadke (Code Review)

anujphadke has posted comments on this change.

Change subject: IMPALA-4866: Hash join node does not apply limits correctly
..


Patch Set 2:

(5 comments)

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

PS1, Line 11: exchange
> typo
Done


http://gerrit.cloudera.org:8080/#/c/6778/1/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 506: DCHECK(status.ok());
> there are a lot of other places that this loop can exit. Can't the out_batc
I have moved the check outside the while loop, so that this gets invoked 
everytime we break out of it. 
Ran a bunch of queries and tried to verify that the limit gets applied 
correctly Found a few cases where num_rows_returned was not incremented and the 
ReachedLimit check was not correctly applied.


PS1, Line 580: 
> >= ?
Removed this check now. Please  ignore.


Line 581:   DCHECK(current_probe_row_ == NULL);
> I think you need to update num_rows_returned_ as well
Removed this check. Please  ignore.


http://gerrit.cloudera.org:8080/#/c/6778/1/tests/common/test_dimensions.py
File tests/common/test_dimensions.py:

PS1, Line 114: # Don't run with NUM_NODES=1 due to IMPALA-561
 : # ALL_CLUSTER_SIZES = [0, 1]
> update comment
Ran the private tests by changing this value ( but without the fix to correctly 
apply the limits). Current tests did not catch this. 
I am reverting it back and adding a few tests which can catch this issue.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: anujphadke 
Gerrit-HasComments: Yes

1 2 >

1 - 100 of 156 matches

Mail list logo