[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1: Code-Review-1

Tamas just submitted a patch to fix whatever went wrong by IMPALA-8980. Can we 
give it a chance and let's see if this fixes the build?

Btw, if you go for a revert, these are the follow up changes that went in for 
IMPALA-8980:
https://gerrit.cloudera.org/#/c/15766/
https://gerrit.cloudera.org/#/c/15816/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Tue, 12 May 2020 05:13:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9645 Port LLVM codegen to adapt aarch64

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15718 )

Change subject: IMPALA-9645 Port LLVM codegen to adapt aarch64
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6036/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11
Gerrit-Change-Number: 15718
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 May 2020 04:38:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9630 Keep blocking queue cache line aligned on aarch64

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15705 )

Change subject: IMPALA-9630 Keep blocking queue cache line aligned on aarch64
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6037/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia169e15d3a225f1e4780e671b8cce680b176c171
Gerrit-Change-Number: 15705
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 May 2020 04:38:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9676 Add aarch64 compile options for clang

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15755 )

Change subject: IMPALA-9676 Add aarch64 compile options for clang
..


Patch Set 8:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I69a5ff64bbd4427dd87ec6e884251e76d6a73122
Gerrit-Change-Number: 15755
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 12 May 2020 04:15:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON instructions

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15531 )

Change subject: IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON 
instructions
..


Patch Set 31:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7dfe17125b2910ece54e7dd18b4e4b25d7de8b9
Gerrit-Change-Number: 15531
Gerrit-PatchSet: 31
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 12 May 2020 04:14:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9688: Support create iceberg table by impala

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15797 )

Change subject: IMPALA-9688: Support create iceberg table by impala
..


Patch Set 18:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6034/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d85db4c904a8c758c4cfb4f19cfbdab7e6ea284
Gerrit-Change-Number: 15797
Gerrit-PatchSet: 18
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Tue, 12 May 2020 04:04:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9645 Port LLVM codegen to adapt aarch64

2020-05-11 Thread Anonymous Coward (Code Review)
zhaoren...@hotmail.com has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15718 )

Change subject: IMPALA-9645 Port LLVM codegen to adapt aarch64
..


Patch Set 10:

Hi, Tamas, these have been fixed as your request.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11
Gerrit-Change-Number: 15718
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 May 2020 03:48:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON instructions

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15531 )

Change subject: IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON 
instructions
..


Patch Set 31:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h
File be/src/util/sse2neon.h:

http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@213
PS31, Line 213: // 
https://msdn.microsoft.com/en-us/library/bb514059%28v=vs.120%29.aspx?f=255=-2147217396
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@406
PS31, Line 406: // 
https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/whtfzhzk(v=vs.100)
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@413
PS31, Line 413: // 
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_set1_epi64x=4961
line too long (97 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@1054
PS31, Line 1054: // 
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_shuffle_epi8=5146
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@1199
PS31, Line 1199: // 
https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2010/y41dkk37(v=vs.100)
line too long (104 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@1645
PS31, Line 1645: // 
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_test_all_zeros=5871
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@3581
PS31, Line 3581: // 
https://github.com/ColinIanKing/linux-next-mirror/blob/b5f466091e130caaf0735976648f72bd5e09aa84/crypto/aegis128-neon-inner.c#L52
line too long (131 > 90)


http://gerrit.cloudera.org:8080/#/c/15531/31/be/src/util/sse2neon.h@3681
PS31, Line 3681: // 
cpp-compiler-developer-guide-and-reference-allocating-and-freeing-aligned-memory-blocks
line too long (98 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7dfe17125b2910ece54e7dd18b4e4b25d7de8b9
Gerrit-Change-Number: 15531
Gerrit-PatchSet: 31
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 12 May 2020 03:48:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9630 Keep blocking queue cache line aligned on aarch64

2020-05-11 Thread Anonymous Coward (Code Review)
zhaoren...@hotmail.com has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/15705 )

Change subject: IMPALA-9630 Keep blocking queue cache line aligned on aarch64
..

IMPALA-9630 Keep blocking queue cache line aligned on aarch64

On aarch64, the DNCHECK_NE in BlockingQueue construct function
 will fail. So here use a different method to keep the 'put'
 class members and 'get' class members cache line aligned.

Change-Id: Ia169e15d3a225f1e4780e671b8cce680b176c171
---
M be/src/common/compiler-util.h
M be/src/exec/hdfs-scanner.h
M be/src/util/aligned-new.h
M be/src/util/blocking-queue.h
4 files changed, 13 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia169e15d3a225f1e4780e671b8cce680b176c171
Gerrit-Change-Number: 15705
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON instructions

2020-05-11 Thread Anonymous Coward (Code Review)
zhaoren...@hotmail.com has uploaded a new patch set (#31). ( 
http://gerrit.cloudera.org:8080/15531 )

Change subject: IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON 
instructions
..

IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON instructions

Replace Intel's SSE instructions with ARM's NEON instructions
Replace Intel's crc32 instructions with ARM's instructions
Replace Intel's popcntq instruction with ARM's mechanism
Replace Intel's pcmpestri and pcmpestrm instructions
with ARM mechanism

Change-Id: Id7dfe17125b2910ece54e7dd18b4e4b25d7de8b9
---
M CMakeLists.txt
M be/CMakeLists.txt
M be/src/benchmarks/bswap-benchmark.cc
M be/src/benchmarks/int-hash-benchmark.cc
M be/src/codegen/CMakeLists.txt
M be/src/codegen/gen_ir_descriptions.py
M be/src/exec/delimited-text-parser.inline.h
M be/src/exec/filter-context.cc
M be/src/kudu/util/block_bloom_filter.cc
M be/src/kudu/util/group_varint-inl.h
M be/src/kudu/util/group_varint-test.cc
A be/src/kudu/util/sse2neon.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.cc
M be/src/util/bit-util.h
M be/src/util/bloom-filter-ir.cc
M be/src/util/bloom-filter.cc
M be/src/util/bloom-filter.h
M be/src/util/cpu-info.cc
M be/src/util/hash-util-ir.cc
M be/src/util/hash-util.h
M be/src/util/sse-util.h
A be/src/util/sse2neon.h
23 files changed, 4,004 insertions(+), 24 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id7dfe17125b2910ece54e7dd18b4e4b25d7de8b9
Gerrit-Change-Number: 15531
Gerrit-PatchSet: 31
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9645 Port LLVM codegen to adapt aarch64

2020-05-11 Thread Anonymous Coward (Code Review)
zhaoren...@hotmail.com has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/15718 )

Change subject: IMPALA-9645 Port LLVM codegen to adapt aarch64
..

IMPALA-9645 Port LLVM codegen to adapt aarch64

On aarch64, the Lowered type  of  struct {bool, int128} is form
{ {i8}, {i128} }. No padding add. This is different with x86-64,
which is { {i8}, {15*i8}, {i128} } with padding add automatically.

And here also add some type conversion between x86 and aarch64 data types.

And also add some aarch64 cpu's feature.

Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11
---
M be/src/codegen/codegen-anyval.cc
M be/src/codegen/codegen-anyval.h
M be/src/codegen/llvm-codegen.cc
M be/src/exec/text-converter.cc
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-fn-call.cc
6 files changed, 144 insertions(+), 6 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11
Gerrit-Change-Number: 15718
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9676 Add aarch64 compile options for clang

2020-05-11 Thread Anonymous Coward (Code Review)
zhaoren...@hotmail.com has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/15755 )

Change subject: IMPALA-9676 Add aarch64 compile options for clang
..

IMPALA-9676 Add aarch64 compile options for clang

Add signed-char and armv8a and crc compile options to clang

Change-Id: I69a5ff64bbd4427dd87ec6e884251e76d6a73122
---
M be/CMakeLists.txt
1 file changed, 7 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69a5ff64bbd4427dd87ec6e884251e76d6a73122
Gerrit-Change-Number: 15755
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9688: Support create iceberg table by impala

2020-05-11 Thread wangsheng (Code Review)
wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15797 )

Change subject: IMPALA-9688: Support create iceberg table by impala
..


Patch Set 18:

(5 comments)

Hi Zoltan, I'm preparing to implement the function of query iceberg table by 
impala recently, and I will update here if there is any progess: IMPALA-9741, 
including my design or solution.

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@569
PS17, Line 569: a managed Icebe
> "a managed Iceberg"
Done


http://gerrit.cloudera.org:8080/#/c/15797/11/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/15797/11/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@236
PS11, Line 236: return true;
> Do you have a design doc about it? E.g. a google doc that you could share w
Thanks for your patient review again, Zoltan. I didn't have a design doc about 
query iceberg table by impala now. But I am considering how to implement this 
recently, I will discussed with my colleague who research the iceberg. And I've 
already create a jira: IMPALA-9741, I will update here if there is progress.


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@64
PS17, Line 64: icebergTableLocat
> Shouldn't this be renamed to icebergTableLocation_ here and in the thrift s
Done, yes, you are right, I will use table location to replace table name here, 
including thrift file to identify iceberg file system table.


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@24
PS17, Line 24: import org.apache.iceberg.hadoop.HadoopTableOperations;
 : import org.apache.iceberg.hadoop.HadoopTables;
> So initially it'll only support "Files System Tables", and not "Metastore t
Iceberg indeed support HiveCatalog and HadoopTables to create 'Metastore 
tables' and 'Files System Tables' as you mentioned above, and you can find a 
quick start here: 
https://iceberg.apache.org/api-quickstart/#using-a-hive-catalog.

I use HadoopTables in this patch to implement this function: impala create hms 
table and iceberg create file system table, just like impala supporting kudu. 
Thus impala can manage hms table and iceberg manage file system table. I think 
HiveCatalog is not very suitable for impala due to HiveCatalog creating both 
table in hms and hdfs. We cannot use this in external iceberg table situation: 
if we use HiveCatalog to create a metastore table, we cannot create external 
table by impala, because there is already a table in hms created by iceberg. 
But if we use HadoopTables to create a file system table, we can use create 
external table by impala through a table location.


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@66
PS17, Line 66: tableLoca
> nit: tableLocation?
Done, I already modify this as tableLocation to identity the iceberg file 
system table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d85db4c904a8c758c4cfb4f19cfbdab7e6ea284
Gerrit-Change-Number: 15797
Gerrit-PatchSet: 18
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Tue, 12 May 2020 03:16:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9688: Support create iceberg table by impala

2020-05-11 Thread wangsheng (Code Review)
wangsheng has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/15797 )

Change subject: IMPALA-9688: Support create iceberg table by impala
..

IMPALA-9688: Support create iceberg table by impala

This patch mainly realizes the creation of iceberg table through impala,
we can use the following sql to create a new iceberg table:
create table iceberg_test(
level string,
event_time timestamp,
message string,
register_time date,
telephone array 
)
partition by spec(
level identity,
event_time identity,
event_time hour,
register_time day
)
stored as iceberg;
'identity' is one of Iceberg's Partition Transforms. 'identity' means that
the source data values are used to create partitions, and other partition
transfroms would be supported in the future, such as BUCKET/TRUNCATE. We
can alse use 'show create table iceberg_test' to dispaly table schema, and
use 'show partitions iceberg_test' to display partition column info. By the
way, parititon column must be the source column.

Testing:
- Add test cases in metadata/test_show_create_table.py.
- Add custom cluster test test_iceberg.py.

Change-Id: I8d85db4c904a8c758c4cfb4f19cfbdab7e6ea284
---
M be/src/service/query-options-test.cc
M bin/impala-config.sh
M common/thrift/CatalogObjects.thrift
M common/thrift/Descriptors.thrift
M common/thrift/JniCatalog.thrift
M fe/pom.xml
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
A fe/src/main/java/org/apache/impala/analysis/IcebergPartitionField.java
A fe/src/main/java/org/apache/impala/analysis/IcebergPartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
A fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
M fe/src/main/java/org/apache/impala/catalog/HdfsStorageDescriptor.java
A fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
A fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
A fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
A fe/src/main/java/org/apache/impala/util/IcebergUtil.java
M fe/src/main/jflex/sql-scanner.flex
M impala-parent/pom.xml
A testdata/workloads/functional-query/queries/QueryTest/iceberg_create.test
M testdata/workloads/functional-query/queries/QueryTest/set.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_iceberg.py
31 files changed, 1,544 insertions(+), 14 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8d85db4c904a8c758c4cfb4f19cfbdab7e6ea284
Gerrit-Change-Number: 15797
Gerrit-PatchSet: 18
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 21: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Tue, 12 May 2020 03:01:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6984: coordinator cancels backends on EOS

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15840 )

Change subject: IMPALA-6984: coordinator cancels backends on EOS
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6033/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I966eceaafdc18a019708b780aee4ee9d70fd3a47
Gerrit-Change-Number: 15840
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 May 2020 02:37:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6:

Thanks for fixing this. I don't plan to review so will remove myself as a 
reviewer.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Tue, 12 May 2020 01:48:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6984: coordinator cancels backends on EOS

2020-05-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15840 )

Change subject: IMPALA-6984: coordinator cancels backends on EOS
..


Patch Set 4:

I tried to think through what queries might be vulnerable to profile counter 
values being non-deterministic because of early cancellation. Dividing queries 
into categories...

Frst, queries that return non-deterministic results, e.g. select * from table 
limit 10 from a table with more than 10 rows. These would, in most cases, 
already be flaky, because they do not return consistent results.

Second, a special case is queries that are not guaranteed to be deterministic, 
but happen to return deterministic results on the particular input data. E.g. 
my primitive_cancel_union query, or "select * from table where  
limit 10" where the predicate returns exactly 10 rows. The case when the rows 
returned is exactly the limit is particularly flaky, because cancellation will 
race. These could definitely get flakier. Hopefully we don't have many goofy 
test queries that do things like this and also have RUNTIME_PROFILE checks. I 
guess the various runtime filter tests *do* weird things like this

Test case 12 in runtime_filters.test looks like an example of this. I could 
tweak this to increase the limits and avoid the non-determinism. I looked at 
the limit queries in bloom_filter.test and I think they're benign, because the 
RUNTIME_PROFILE checks are not testing counters in the plan subtrees that would 
be short-circuited.

Third, the plan tree of a deterministic query will generally process the whole 
input without cancelling any fragments early. E.g. queries without limits, a 
limit with an ORDER BY or an aggregation at the top of the query. We can only 
start cancelling the query once eos has been sent through the exchange. The 
profiles for the ExecNode tree should (as far as I can see) be deterministic, 
because exec_tree_->Close() is called before 
KrpcDataStreamSender::FlushFinal(), so I don't think the ExecNode tree profiles 
can change after eos is sent through the exchange.

I guess one weird thing that can happen is that the sender fragment gets 
cancelled before the EOS RPC gets acked, and FlushFinal() exits with CANCELLED 
status. As far as I can tell the code flow doesn't really change in that case, 
just the state of the fragment is CANCELLED instead of OK.

I *think* that might cover all queries. I'm going to loop the runtime profile 
tests for a while to see if anything is flaky, but I'm thinking that maybe this 
is fairly safe.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I966eceaafdc18a019708b780aee4ee9d70fd3a47
Gerrit-Change-Number: 15840
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 12 May 2020 01:46:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6984: coordinator cancels backends on EOS

2020-05-11 Thread Tim Armstrong (Code Review)
Hello Thomas Tauber-Marshall, Sahil Takiar, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-6984: coordinator cancels backends on EOS
..

IMPALA-6984: coordinator cancels backends on EOS

Before this patch, when the coordinator returned the last row,
it waited for backends to finish of their own accord, which
could happen indirectly as exchanges got closed.

The idea of this change is to send out cancellation RPCs to
expedite cancellation, then wait for the final exec status
reports to come in. Those reports will be included in the
final profile because the backend is *not* marked as done
when sending out the cancellation RPCs.

The bulk of this change is modifying the cancellation code
path to allow sending the cancel RPCs but *not* consider
the backend done until it gets back the final status report.
The old "fire and forget" mode of cancellation is still used
for explicit cancellation and errors.

Testing:
Ran exhaustive tests.

Ran cancellation tests under TSAN, checked for errors.

Manually inspected logs of some queries with limit,
saw that it sent cancellation then waited for backends
as expected.

Added a functional perf test that goes from ~5s down to < ~1s
on my system.

Change-Id: I966eceaafdc18a019708b780aee4ee9d70fd3a47
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/util/counting-barrier.h
M testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test
M 
testdata/workloads/functional-query/queries/QueryTest/runtime_filters_mt_dop.test
A testdata/workloads/targeted-perf/queries/primitive_cancel_union.test
M tests/experiments/test_targeted_perf.py
9 files changed, 124 insertions(+), 40 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I966eceaafdc18a019708b780aee4ee9d70fd3a47
Gerrit-Change-Number: 15840
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9727: Fix HBaseScanNode explain formatting

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15749 )

Change subject: IMPALA-9727: Fix HBaseScanNode explain formatting
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30fad791408a1f7e35e9b3f2e6cb4958952dd567
Gerrit-Change-Number: 15749
Gerrit-PatchSet: 5
Gerrit-Owner: Shant Hovsepian 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 12 May 2020 01:18:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9727: Fix HBaseScanNode explain formatting

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15749 )

Change subject: IMPALA-9727: Fix HBaseScanNode explain formatting
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30fad791408a1f7e35e9b3f2e6cb4958952dd567
Gerrit-Change-Number: 15749
Gerrit-PatchSet: 5
Gerrit-Owner: Shant Hovsepian 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 12 May 2020 01:18:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9597: Eliminate redundant Ranger audits for column masking

2020-05-11 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15854 )

Change subject: IMPALA-9597: Eliminate redundant Ranger audits for column 
masking
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15854/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
File 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java:

http://gerrit.cloudera.org:8080/#/c/15854/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@40
PS3, Line 40:   private List stashedAuditEvents_;
> Thanks Quanlong! If we use a Map to stash the even
Looks like we never use stashedAuditEvents_ if the authorization failed. Is it 
just for performance reason to keep the stashedAuditEvent list? But appending 
to the list is nearly the same perf as inserting to the hashmap...


http://gerrit.cloudera.org:8080/#/c/15854/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
File 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java:

http://gerrit.cloudera.org:8080/#/c/15854/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@48
PS5, Line 48: HashMap
I think we should use LinkedHashMap to keep the insert order.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42d60130fba93d63fbc36949f2bf746b7ae2497d
Gerrit-Change-Number: 15854
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Tue, 12 May 2020 01:16:32 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9714: Fix edge cases in SimpleLogger and add test

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15861 )

Change subject: IMPALA-9714: Fix edge cases in SimpleLogger and add test
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6032/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
Gerrit-Change-Number: 15861
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Tue, 12 May 2020 00:45:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9714: Fix edge cases in SimpleLogger and add test

2020-05-11 Thread Joe McDonnell (Code Review)
Hello Thomas Tauber-Marshall, Sahil Takiar, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9714: Fix edge cases in SimpleLogger and add test
..

IMPALA-9714: Fix edge cases in SimpleLogger and add test

SimpleLogger is used for several existing log types and a change
to use it for the data cache access trace is underway. Since this
is commonly used, it is useful to nail down specific semantics and
test them.

This fixes the following edge cases:
1. LoggingSupport::DeleteOldLogs() currently maintains a map from mtime
   to the filename in order to decide which files need to be deleted.
   This stops working when there are fast updates to the log, because
   mtime has seconds resolution and DeleteOldLogs() is only able to
   recognize a single file per mtime with the current map. This changes
   the map to a set of pairs of mtime + filename. The behavior is
   identical except that if there are multiple files with the same
   mtime, they each get their own entry in the set. This allows
   DeleteOldLogs() to more accurately maintain the maximum log files.
2. SimpleLogger::Init() now enforces the limit on the maximum number
   of log files. This provides a clear semantic when dealing with
   preexisting files from a previous incarnation of the same logger.
3. SimpleLogger will now create any intermediate directories when
   creating the logging directory (i.e. existingdir/a/b/c works).
4. This changes the enforcement moves enforcement max_audit_event_log_files
   to use the limits provided by SimpleLogger rather than a background
   thread calling DeleteOldLogs() periodically.

This also introduces SimpleLogger::GetLogFiles(), which is a static
function to get the log files given a directory and prefix. This
is necessary for testing, but it also will be useful for code that
wants to process logs from SimpleLogger.

Testing:
 - Added a new simple-logger-test that codifies the expected behavior
 - Ran core tests

Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
---
M be/src/common/init.cc
M be/src/common/logging.cc
M be/src/common/logging.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/util/CMakeLists.txt
M be/src/util/filesystem-util-test.cc
M be/src/util/filesystem-util.cc
M be/src/util/filesystem-util.h
M be/src/util/logging-support.cc
A be/src/util/simple-logger-test.cc
M be/src/util/simple-logger.cc
M be/src/util/simple-logger.h
13 files changed, 419 insertions(+), 49 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
Gerrit-Change-Number: 15861
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-9714: Fix edge cases in SimpleLogger and add test

2020-05-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15861 )

Change subject: IMPALA-9714: Fix edge cases in SimpleLogger and add test
..


Patch Set 2:

(3 comments)

I addressed some comments and then also moved the enforcement of 
max_audit_event_log_files to use the enforcement from SimpleLogger rather than 
its own code.

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.h
File be/src/util/simple-logger.h:

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.h@60
PS2, Line 60:   bool initialized_ = false;
> nit: docs
Done


http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc
File be/src/util/simple-logger.cc:

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc@31
PS2, Line 31: #include "common/names.h"
> nit: why the include ordering change?
common/names.h is special in that it only is defining aliases, and it looks at 
which compile guards are defined to determine which aliases to add. Best 
practice is for it to go last so that all the compile guards from the include 
files are defined.
https://github.com/apache/impala/blob/master/be/src/common/names.h#L28-L29

In this case, nothing changes, and this isn't really needed for this change.


http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc@141
PS2, Line 141:   FileSystemUtil::Directory dir(log_dir);
> The difference in pattern I think is probably a good enough reason to leave
I'm going to leave this as-is for this change. I will file a followup JIRA. I 
got it partially working, but I would need more tests for the glog tests. I 
moved the regex filtering to FileSystemUtil::GetEntryNames() and simplified 
this function to use it. If we do the further change, it won't be that big.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
Gerrit-Change-Number: 15861
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 11 May 2020 23:52:08 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 21:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 23:38:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-3741 [part 2]: Push runtime bloom filter to Kudu

2020-05-11 Thread Bankim Bhavsar (Code Review)
Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15683 )

Change subject: IMPALA-3741 [part 2]: Push runtime bloom filter to Kudu
..


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15683/12/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/15683/12/be/src/exec/kudu-scanner.cc@252
PS12, Line 252: ImpalaBloomFilterBufferAllocator* allocator =
  : new ImpalaBloomFilterBufferAllocator(
  : state_->filter_bank()->buffer_pool_client());
This doesn't look right.
The allocator that is used to create BBF must be passed down to kudu.
In current impl, the one stored in impala BF must be passed down. 
https://gerrit.cloudera.org/c/15683/12/be/src/util/bloom-filter.h#214



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9100076f68ea299ddb6ec8bc027cac7a47f5d754
Gerrit-Change-Number: 15683
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 11 May 2020 23:15:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 21:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 22:47:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 21: Code-Review+2

Carrying +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 22:46:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has uploaded a new patch set (#21) to the change originally 
created by 583424...@qq.com. ( http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..

IMPALA-8205: Support number of true and false statistics for boolean column

This change compute the real number of true and false statistics
information for boolean columns. Before this, impala used to set
numTrues and numFalses to hardcoded -1 to indicate that its
statistics is missing.

Test Done:
Append the numTrue and numFalse test for all the statistics-related
test cases including the non-incremental, incremental and other test
cases.

Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
---
M be/src/exec/catalog-op-executor.cc
M be/src/exec/incr-stats-util-test.cc
M be/src/exec/incr-stats-util.cc
M be/src/exec/incr-stats-util.h
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M testdata/workloads/functional-query/queries/QueryTest/acid-compute-stats.test
M 
testdata/workloads/functional-query/queries/QueryTest/alter-table-set-column-stats.test
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-avro-catalog-v2.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats-avro.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats-date.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-decimal.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
M 
testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats.test
M testdata/workloads/functional-query/queries/QueryTest/hbase-show-stats.test
M testdata/workloads/functional-query/queries/QueryTest/show-stats.test
M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test
25 files changed, 1,981 insertions(+), 1,867 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 


[Impala-ASF-CR] Bump Kudu to 389d4f1e1, fix toolchain Python

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15845 )

Change subject: Bump Kudu to 389d4f1e1, fix toolchain Python
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6030/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc2d95e2ca4d9819a6cc0b4eb2865d60e21f153
Gerrit-Change-Number: 15845
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 11 May 2020 22:41:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] Asynchronous code generation

2020-05-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15105 )

Change subject: Asynchronous code generation
..


Patch Set 33:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15105/35//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15105/35//COMMIT_MSG@7
PS35, Line 7: Asynchronous code generation
IMPALA-5444



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7cbfa7c6734dcf03641629429057d6a4194aa6b
Gerrit-Change-Number: 15105
Gerrit-PatchSet: 33
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 22:24:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 22:15:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9199: Add support for single query retries on cluster membership changes

2020-05-11 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14824 )

Change subject: IMPALA-9199: Add support for single query retries on cluster 
membership changes
..


Patch Set 26:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/14824/26/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/14824/26/be/src/runtime/coordinator.cc@511
PS26, Line 511: if (!failed_backend_states.empty()) {
Seems like it shouldn't be possible for this to ever be false. Maybe add a 
DCHECK?


http://gerrit.cloudera.org:8080/#/c/14824/26/be/src/runtime/coordinator.cc@512
PS26, Line 512: RETURN_IF_ERROR
I don't think its possible for HandleFailedExecRpcs() to return an error, and 
even if it did it probably wouldn't be correct to just return like this here 
since we still need to call UpdateExecState() below to update the query to an 
error status.


http://gerrit.cloudera.org:8080/#/c/14824/26/be/src/runtime/coordinator.cc@982
PS26, Line 982:   
parent_query_driver_->TryQueryRetry(parent_request_state_, _status);
Probably fine to leave as is for now since currently any query that hits this 
is going to end up failing, but just wanted to point out that this code path 
means that we could potentially cancel the query without having actually 
received an error status back yet.

As noted elsewhere, this makes it more difficult to use the AddDetail() on the 
Status out parameter in TryQueryRetry.


http://gerrit.cloudera.org:8080/#/c/14824/26/be/src/runtime/coordinator.cc@1079
PS26, Line 1079:   parent_query_driver_->TryQueryRetry(parent_request_state_, 
_status);
Wanted to point out that there's an unfortunate situation here and elsewhere 
TryQueryRetry is called where we end up with two slightly different versions 
for the overall query status - the one that gets set on Coordinator (in this 
case, exec_rpc_status, which represents the error from the first backend to 
fail in Exec()) and 'retryable_status' which gets set on ClientRequestState in 
MarkAsRetrying().

Normally, those two statuses would be the same, because ClientRequestState 
would get Coordinator's state returned to it from Exec(), which it would then 
set on itself, it won't set it anymore as we only preserve the first error 
message. I think that's messy and has the potential to be confusing for 
developers.

Personally, I still really think it would be worth it to do the work to at 
least somewhat remove the circular dependency here, eg. you could move the call 
to TryQueryRetry to ClientRequestState::UpdateQueryStatus() along with defining 
a Coordinator::BlacklistedExecutors() or something like that, It wouldn't be 
much work, it would solve this problem, and it would make the control flow a 
lot cleaner, make Coordinator better encapsulated since it wouldn't need to 
know anything about retries, etc.


http://gerrit.cloudera.org:8080/#/c/14824/24/be/src/runtime/query-driver.h
File be/src/runtime/query-driver.h:

http://gerrit.cloudera.org:8080/#/c/14824/24/be/src/runtime/query-driver.h@136
PS24, Line 136:   /// query option 'retry_failed_queries') and if the query can 
be retried. Queries can
> Doesn't return a Status anymore.
Sure, 'status' is used in one code path that calls TryQueryRetry (the code path 
in Coordinator where we get an error in a status report, which 
test_multiple_retries is testing), but its not used in any of the other code 
paths that call TryQueryRetry so it gets lost in those cases (eg. 
HandleFailedExecRpcs)

One option would be to just fix those code paths to actually set the status 
they passed in as the out parameter to the overall error status, but that's not 
always going to be easy (eg. the path in Coordinator where we get an 
AuxErrorInfo but no overall query error status in the report, so we have no 
error status to add the details to).

Instead, maybe it would be better to just add an info string to the profile 
that says "This query was not retried because..."

One disadvantage of that is that it means you can't immediately tell from the 
overall error why the query wasn't retried and have to dig down into the 
profile/logs, but that's already the case since you're only adding the detail 
if we hit the max retries and not for other cases such as if rows had already 
been fetched, though maybe you want to do an AddDetail for those cases too.


http://gerrit.cloudera.org:8080/#/c/14824/24/be/src/runtime/query-driver.h@204
PS24, Line 204:   /// 'RunFrontendPlanner'.
> Do you mean that CreateRetriedClientRequestState could just pass in a point
Personally, I think the code is way more confusing this way.

You're not "moving" the TExecRequest from the original ClientRequestState to 
the new one - the TExecRequest is just owned by the QueryDriver, so you're 
moving it from one place in the QueryDriver to a different place in the 
QueryDriver.

It doesn't change at any point, 

[Impala-ASF-CR] Bump Kudu to 389d4f1e1, fix toolchain Python

2020-05-11 Thread Laszlo Gaal (Code Review)
Hello Thomas Tauber-Marshall, Wenzhe Zhou, Impala Public Jenkins,

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

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

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

Change subject: Bump Kudu to 389d4f1e1, fix toolchain Python
..

Bump Kudu to 389d4f1e1, fix toolchain Python

Pick up the rebuilt toolchain Python, now with readline support.
The new toolchain version contains a newer Kudu version, so bump
the Kudu versions to the one too.

Change-Id: Idcc2d95e2ca4d9819a6cc0b4eb2865d60e21f153
---
M bin/impala-config.sh
1 file changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idcc2d95e2ca4d9819a6cc0b4eb2865d60e21f153
Gerrit-Change-Number: 15845
Gerrit-PatchSet: 3
Gerrit-Owner: Laszlo Gaal 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 


[Impala-ASF-CR] IMPALA-9382: part 1: transposed profile prototype

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15798 )

Change subject: IMPALA-9382: part 1: transposed profile prototype
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6029/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0838c6a0872f57c696267ff4e92d29c08748eb7a
Gerrit-Change-Number: 15798
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 11 May 2020 21:11:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 20: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 20:44:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6984: coordinator cancels backends on EOS

2020-05-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15840 )

Change subject: IMPALA-6984: coordinator cancels backends on EOS
..


Patch Set 4:

That is a very good point. I don't think this change is that pressing - I had 
originally thought we were seeing this in some perf runs but it turned out to 
be other issues. So I don't want to destabilise any tests for the sake of 
getting it in.

I hadn't thought about the cancellation from the coordinator racing with the 
normal fragment teardown when it notices eos, but that makes some sense.

It seems like that problem could still exist before this patch if a status 
report noticed cancellation on the coordinator, right? But would only happen if 
the query finished around the same time as the status report was sent out (i.e. 
every 5 seconds).

I'll have a think about what to do here.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I966eceaafdc18a019708b780aee4ee9d70fd3a47
Gerrit-Change-Number: 15840
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 11 May 2020 20:22:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9382: part 1: transposed profile prototype

2020-05-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/15798 )

Change subject: IMPALA-9382: part 1: transposed profile prototype
..

IMPALA-9382: part 1: transposed profile prototype

This adds an experimental profile representation
that is denser than the traditional representation.
Counters, info strings and other information for all
instances of a fragment are merged into a single
tree. Descriptive stats (min, max, mean) are shown for
each counter, along with the values for each instance. It
can be enabled by setting --gen_experimental_profile=true.
The default behaviour is unchanged, aside from including
a few extra counters in existing profiles.

An example of the pretty-printed profile is attached
to the JIRA.

The thrift representation of the profile is extended
so that all instances of a fragment can be merged
together into a single "aggregated" fragment, with
vectors of counters.

The in-memory representation is transformed in
a similar way. The RuntimeProfile class is
restructured so that there is a common RuntimeProfileBase
class, with RuntimeProfile and AggregatedRuntimeProfile
base classes. Execution fills in counters in RuntimeProfile
for each instances, then these are aggregated together into
an AggregatedRuntimeProfile on the coordinator. This replaces
the "averaged" profile concept with an abstraction that
more clearly distinguishes what operations apply to aggregated
and unaggregated profiles.

In a future change, we could use AggregatedRuntimeProfile
for status reports so that less data needs to be sent to
the coordinator, and the coordinator needs to do less
processing.

The new profile removes the bad practice of including aggregated
stats as strings from the new profile. These stats can now be
automatically as aggregations of counters. The legacy uses of
InfoString are preserved so as to not lose information but
can be removed when we switch to the transposed profile.

Also make TotalTime and InactiveTime behave like other counters -
they are pretty-printed the same as other counters. Inactive time
is also included in the local time calculation, fixing IMPALA-2794.

TODO in later patches for IMPALA-9382:
These will need to be fixed before this can be considered production
ready.
* The JSON profile generation is not fully implemented for aggregated
  profiles.
* Not all counter times are included in aggregated profile, e.g. time
  series counters.
* The pretty-printing of the various profile counters will need to be
  improved to be more readable, e.g. grouping by host, improving
  formatting.
* The aggregated profile is only updated at the end of the query.
  We need to support live updating.
* Consider how to show local time per instance - make it a first-class
  counter?

Possible extensions:
* We could included more general summary stats for each counter, e.g.
  median, p95, etc.
* We could better highlight outliers

Testing:
* I diffed the text profile of TPC-DS Q1 to make sure there were no
  unexpected changes.

TODO
* exhaustive tests
* TSAN/ASAN

Change-Id: I0838c6a0872f57c696267ff4e92d29c08748eb7a
---
M be/src/runtime/bufferpool/buffer-pool-test.cc
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/util/dummy-runtime-profile.h
M be/src/util/pretty-printer-test.cc
M be/src/util/pretty-printer.h
M be/src/util/runtime-profile-counters.h
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M common/thrift/RuntimeProfile.thrift
12 files changed, 1,383 insertions(+), 607 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0838c6a0872f57c696267ff4e92d29c08748eb7a
Gerrit-Change-Number: 15798
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9680: Fixed compressed inserts failing

2020-05-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15816 )

Change subject: IMPALA-9680: Fixed compressed inserts failing
..

IMPALA-9680: Fixed compressed inserts failing

Modified the insert testfiles to get which database they need to
use for 'CREATE TABLE LIKE' dynamically.

Tests:
Did targeted exhaustive testruns in test_insert.py and
test_mt_dop.py and did a full exhaustive testrun.

Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Reviewed-on: http://gerrit.cloudera.org:8080/15816
Tested-by: Impala Public Jenkins 
Reviewed-by: Csaba Ringhofer 
---
M testdata/workloads/functional-query/queries/QueryTest/insert-mem-limit.test
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/insert_bad_expr.test
M testdata/workloads/functional-query/queries/QueryTest/insert_null.test
M testdata/workloads/functional-query/queries/QueryTest/insert_overwrite.test
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_insert.py
M tests/query_test/test_mt_dop.py
9 files changed, 49 insertions(+), 28 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Csaba Ringhofer: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Gerrit-Change-Number: 15816
Gerrit-PatchSet: 9
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9680: Fixed compressed inserts failing

2020-05-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15816 )

Change subject: IMPALA-9680: Fixed compressed inserts failing
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Gerrit-Change-Number: 15816
Gerrit-PatchSet: 8
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 19:32:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9708: Remove Sentry support

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15833 )

Change subject: IMPALA-9708: Remove Sentry support
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6028/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e99c15936d6d250cf258e3a1dcba11d3eb4661e
Gerrit-Change-Number: 15833
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 19:28:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9708: Remove Sentry support

2020-05-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15833


Change subject: IMPALA-9708: Remove Sentry support
..

IMPALA-9708: Remove Sentry support

Impala 4 decided to drop Sentry support in favor of Ranger. This
removes Sentry support and related tests. It retires startup
flags related to Sentry and removes as much code as possible.
Documentation will still need to be adjusted to remove
references to Sentry.

Some issues came up when implementing this. Here is a summary
of how this patch resolves them:
1. authorization_provider currently defaults to "sentry", but
   "ranger" requires extra parameters to be set. This changes the
   default value of authorization_provider to "", which translates
   internally to the noop policy that does no authorization.
2. These flags are Sentry specific and are now retired:
 - authorization_policy_provider_class
 - sentry_catalog_polling_frequency_s
 - sentry_config
3. The authorization_factory_class may be obsolete now that
   there is only one authorization policy, but this leaves it
   in place.
4. Sentry is the last component using CDH_COMPONENTS_HOME, so
   that is removed. There are still Maven dependencies coming
   from the CDH_BUILD_NUMBER repository, so that is not removed.
5. To make the transition easier, testdata/bin/kill-sentry-service.sh
   is not removed and it is still called from testdata/bin/kill-all.sh.

Testing:
 - Core job passes

Change-Id: I8e99c15936d6d250cf258e3a1dcba11d3eb4661e
---
M README-build.md
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-service-client-wrapper.h
M be/src/catalog/catalog.cc
M be/src/catalog/catalog.h
M be/src/common/global-flags.cc
M be/src/exec/catalog-op-executor.cc
M be/src/exec/catalog-op-executor.h
M be/src/service/fe-support.cc
M be/src/service/frontend.cc
M be/src/transport/TSasl.cpp
M be/src/util/backend-gflag-util.cc
M bin/bootstrap_toolchain.py
M bin/create-test-configuration.sh
M bin/impala-config.sh
M buildall.sh
M common/thrift/BackendGflags.thrift
M common/thrift/CatalogService.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
D fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaAction.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaActionFactory.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaPrivilegeModel.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizable.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableColumn.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableDb.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFn.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableServer.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableTable.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableUri.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
D fe/src/main/java/org/apache/impala/authorization/sentry/SentryConfig.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyReaderException.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
D fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java
D 
fe/src/main/java/org/apache/impala/authorization/sentry/SentryUnavailableException.java
D fe/src/main/java/org/apache/impala/authorization/sentry/SentryUtil.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/FeSupport.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M fe/src/main/java/org/apache/impala/util/AuthorizationUtil.java
M 

[Impala-ASF-CR] IMPALA-9680: Fixed compressed inserts failing

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15816 )

Change subject: IMPALA-9680: Fixed compressed inserts failing
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Gerrit-Change-Number: 15816
Gerrit-PatchSet: 8
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 18:02:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9628: Refactor bootstrap system.sh

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15817 )

Change subject: IMPALA-9628: Refactor bootstrap_system.sh
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9e4309d6648a160b58ebc1e18043afe4110789b
Gerrit-Change-Number: 15817
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Garaguly 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Comment-Date: Mon, 11 May 2020 18:00:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9680: Fixed compressed inserts failing

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15816 )

Change subject: IMPALA-9680: Fixed compressed inserts failing
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6027/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Gerrit-Change-Number: 15816
Gerrit-PatchSet: 8
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 17:36:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog

2020-05-11 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15887 )

Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in 
LocalCatalog
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@416
PS1, Line 416: Table msTbl = table.getMetaStoreTable();
Is msTbl always guaranteed to be not null?


http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@419
PS1, Line 419: getOrDefault(tableTypeStr, TABLE_TYPE_TABLE)
It is concerning that we are defaulting to TABLE type here when in fact we are 
not sure. Is it possible to saying TableType is "UNKNOWN" in such a case?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633
Gerrit-Change-Number: 15887
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 11 May 2020 17:33:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6026/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 17:25:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] Revert "IMPALA-9665: Fixed database not found errors in query test.test insert"

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15901 )

Change subject: Revert "IMPALA-9665: Fixed database not found errors in 
query_test.test_insert"
..


Patch Set 1:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6025/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17607897916b452c2b7354f3940449b2f6acd811
Gerrit-Change-Number: 15901
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 17:23:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9680: Fixed compressed inserts failing

2020-05-11 Thread Adam Tamas (Code Review)
Adam Tamas has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/15816 )

Change subject: IMPALA-9680: Fixed compressed inserts failing
..

IMPALA-9680: Fixed compressed inserts failing

Modified the insert testfiles to get which database they need to
use for 'CREATE TABLE LIKE' dynamically.

Tests:
Did targeted exhaustive testruns in test_insert.py and
test_mt_dop.py and did a full exhaustive testrun.

Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
---
M testdata/workloads/functional-query/queries/QueryTest/insert-mem-limit.test
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/insert_bad_expr.test
M testdata/workloads/functional-query/queries/QueryTest/insert_null.test
M testdata/workloads/functional-query/queries/QueryTest/insert_overwrite.test
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_insert.py
M tests/query_test/test_mt_dop.py
9 files changed, 49 insertions(+), 28 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib3c7ba02190f57a7ed40311c95a3dd9eca9b474d
Gerrit-Change-Number: 15816
Gerrit-PatchSet: 8
Gerrit-Owner: Adam Tamas 
Gerrit-Reviewer: Adam Tamas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9714: Fix edge cases in SimpleLogger and add test

2020-05-11 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15861 )

Change subject: IMPALA-9714: Fix edge cases in SimpleLogger and add test
..


Patch Set 2:

(1 comment)

Not your change, but I happened to notice that the audit logs use SimpleLogger 
but for some reason don't use its built in log rotation functionality and 
instead gets rotated on the same thread as the glog files.

I couldn't figure out from 'git blame' if this was intentional, but it seems 
wrong so we might want to go ahead and fix it.

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc
File be/src/util/simple-logger.cc:

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc@141
PS2, Line 141:   FileSystemUtil::Directory dir(log_dir);
> I see what you mean. I think we would be able to switch to using a delete f
The difference in pattern I think is probably a good enough reason to leave 
this as-is for now if you prefer.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
Gerrit-Change-Number: 15861
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 11 May 2020 17:11:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Revert "IMPALA-9665: Fixed database not found errors in query test.test insert"

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15901 )

Change subject: Revert "IMPALA-9665: Fixed database not found errors in 
query_test.test_insert"
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17607897916b452c2b7354f3940449b2f6acd811
Gerrit-Change-Number: 15901
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 16:36:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 16:35:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15902/1/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/15902/1/tests/common/impala_test_suite.py@784
PS1, Line 784: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/15902/1/tests/common/impala_test_suite.py@789
PS1, Line 789: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/15902/1/tests/common/impala_test_suite.py@927
PS1, Line 927: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/15902/1/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

http://gerrit.cloudera.org:8080/#/c/15902/1/tests/query_test/test_mt_dop.py@179
PS1, Line 179:
flake8: W391 blank line at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 16:33:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15902 )

Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 16:32:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

2020-05-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15902


Change subject: Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE 
tests"
..

Revert "IMPALA-8980: Remove functional*.alltypesinsert from EE tests"

Exhaustive tests fail, so reverting this.

This reverts commit c32849a3912a6fe295e74269bf0c1f350962d57f.

Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
---
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/insert-mem-limit.test
M testdata/workloads/functional-query/queries/QueryTest/insert.test
M testdata/workloads/functional-query/queries/QueryTest/insert_bad_expr.test
M testdata/workloads/functional-query/queries/QueryTest/insert_null.test
M testdata/workloads/functional-query/queries/QueryTest/insert_overwrite.test
M testdata/workloads/functional-query/queries/QueryTest/insert_permutation.test
M testdata/workloads/functional-query/queries/QueryTest/load.test
M testdata/workloads/functional-query/queries/QueryTest/show.test
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_mt_dop.py
M tests/query_test/test_insert.py
M tests/query_test/test_mt_dop.py
13 files changed, 266 insertions(+), 147 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d9acba85b1082502b2bc62acc76116de5750d30
Gerrit-Change-Number: 15902
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 


[Impala-ASF-CR] Revert "IMPALA-9665: Fixed database not found errors in query test.test insert"

2020-05-11 Thread Joe McDonnell (Code Review)
Joe McDonnell has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15901


Change subject: Revert "IMPALA-9665: Fixed database not found errors in 
query_test.test_insert"
..

Revert "IMPALA-9665: Fixed database not found errors in query_test.test_insert"

This is related to IMPALA-8980, which introduces exhaustive test failures.

This reverts commit 02d84dcf502afbb610eb1f911b3a832adfbf5fa9.

Change-Id: I17607897916b452c2b7354f3940449b2f6acd811
---
M tests/query_test/test_insert.py
1 file changed, 0 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I17607897916b452c2b7354f3940449b2f6acd811
Gerrit-Change-Number: 15901
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell 


[Impala-ASF-CR] IMPALA-9714: Fix edge cases in SimpleLogger and add test

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15861 )

Change subject: IMPALA-9714: Fix edge cases in SimpleLogger and add test
..


Patch Set 2: Code-Review+1

(2 comments)

some nits, overall LGTM pending Thomas' comment

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.h
File be/src/util/simple-logger.h:

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.h@60
PS2, Line 60:   bool initialized_ = false;
nit: docs


http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc
File be/src/util/simple-logger.cc:

http://gerrit.cloudera.org:8080/#/c/15861/2/be/src/util/simple-logger.cc@31
PS2, Line 31: #include "common/names.h"
nit: why the include ordering change?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd092a65b31d34f40a660cab7b5e0695a3627c78
Gerrit-Change-Number: 15861
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Comment-Date: Mon, 11 May 2020 16:17:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 20:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6024/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 16:16:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9688: Support create iceberg table by impala

2020-05-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15797 )

Change subject: IMPALA-9688: Support create iceberg table by impala
..


Patch Set 11:

(5 comments)

Thanks a ton for the changes and tests!

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@569
PS17, Line 569:
"a managed Iceberg"


http://gerrit.cloudera.org:8080/#/c/15797/11/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java:

http://gerrit.cloudera.org:8080/#/c/15797/11/fe/src/main/java/org/apache/impala/catalog/HdfsFileFormat.java@236
PS11, Line 236:   case ICEBERG:
> Yes, Avro, ORC, or Parquet are all splittable. But we found that it's diffi
Do you have a design doc about it? E.g. a google doc that you could share with 
the community via an email to the dev@impala channel? We'd be happy to review 
it.

I thought we'd just use Iceberg as a library to process the metadata, prune 
partitions, and select the data files that Impala needs to scan. But probably 
I'm missing something since I don't really know Iceberg.


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@64
PS17, Line 64: icebergTableName_
Shouldn't this be renamed to icebergTableLocation_ here and in the thrift 
structure as well?

Or is it an hdfs location for "File System Tables" and table name for 
"Metastore Tables"?


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
File fe/src/main/java/org/apache/impala/util/IcebergUtil.java:

http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@24
PS17, Line 24: import org.apache.iceberg.hadoop.HadoopTables;
 : import org.apache.iceberg.PartitionField;
So initially it'll only support "Files System Tables", and not "Metastore 
tables"?

https://iceberg.apache.org/spec/#file-system-tables
https://iceberg.apache.org/spec/#metastore-tables


http://gerrit.cloudera.org:8080/#/c/15797/17/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@66
PS17, Line 66:
nit: tableLocation?

Or "tableIdentifier" if later we'll support "Metastore tables"? But as I look 
at Iceberg's code "Metastore tables" maybe not even a thing yet.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d85db4c904a8c758c4cfb4f19cfbdab7e6ea284
Gerrit-Change-Number: 15797
Gerrit-PatchSet: 11
Gerrit-Owner: wangsheng 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: wangsheng 
Gerrit-Comment-Date: Mon, 11 May 2020 15:34:40 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 20:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 15:25:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..


Patch Set 20: Code-Review+2

Carrying +2. Kicking off pre-commit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 15:25:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8205: Support number of true and false statistics for boolean column

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has uploaded a new patch set (#20) to the change originally 
created by 583424...@qq.com. ( http://gerrit.cloudera.org:8080/14666 )

Change subject: IMPALA-8205: Support number of true and false statistics for 
boolean column
..

IMPALA-8205: Support number of true and false statistics for boolean column

This change compute the real number of true and false statistics
information for boolean columns. Before this, impala used to set
numTrues and numFalses to hardcoded -1 to indicate that its
statistics is missing.

Test Done:
Append the numTrue and numFalse test for all the statistics-related
test cases including the non-incremental, incremental and other test
cases.

Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
---
M be/src/exec/catalog-op-executor.cc
M be/src/exec/incr-stats-util-test.cc
M be/src/exec/incr-stats-util.cc
M be/src/exec/incr-stats-util.h
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M testdata/workloads/functional-query/queries/QueryTest/acid-compute-stats.test
M 
testdata/workloads/functional-query/queries/QueryTest/alter-table-set-column-stats.test
M testdata/workloads/functional-query/queries/QueryTest/alter-table.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-avro-catalog-v2.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats-avro.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats-date.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-decimal.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
M 
testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/hbase-compute-stats.test
M testdata/workloads/functional-query/queries/QueryTest/hbase-show-stats.test
M testdata/workloads/functional-query/queries/QueryTest/show-stats.test
M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test
25 files changed, 1,981 insertions(+), 1,867 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I991bee8e7fdc644d908289f5fe2ee8032cc2c431
Gerrit-Change-Number: 14666
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Anonymous Coward <583424...@qq.com>
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Sahil Takiar 


[Impala-ASF-CR] IMPALA-9692 (part 2): Refactor parts of TExecPlanFragmentInfo to protobuf

2020-05-11 Thread Sahil Takiar (Code Review)
Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15844 )

Change subject: IMPALA-9692 (part 2): Refactor parts of TExecPlanFragmentInfo 
to protobuf
..


Patch Set 3:

(6 comments)

only looked through the proto and thrift file changes so far

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

http://gerrit.cloudera.org:8080/#/c/15844/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-9692 (part 2): Refactor parts of TExecPlanFragmentInfo to 
protobuf
is there a list of all the Thrift structs that need to be converted to 
protobuf? listing them out either in the JIRA or some type doc would be great, 
a long with an explanation of why thrift --> protobuf conversion is necessary


http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/common.proto
File common/protobuf/common.proto:

http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/common.proto@44
PS3, Line 44: enum CompressionType {
is this a mirror of THdfsCompression in CatalogObjects.thrift? Can we include 
comments in both this file and CatalogObjects.thrift stating that.


http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/control_service.proto
File common/protobuf/control_service.proto:

http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/control_service.proto@292
PS3, Line 292: // Execution parameters of a single fragment instance. 
Corresponds to a
 : // TPlanFragmentInstanceCtx with the same fragment_idx.
 : message PlanFragmentInstanceCtxPB {
this seems a bit confusing to me. this basically splits up 
TPlanFragmentInstanceCtx into two structs, some of the struct is defined in 
Thrift and some in Protobuf. I guess it is probably fine to re-factor it that 
way, but we should update the comments of both this struct and the Thrift 
version to reflect that. for example, the docs of each struct state that it is 
the "Execution parameters of a single fragment instance.", however, that is not 
really true anymore, it is just the partial set of execution parameters.
we could consider renaming this and TPlanFragmentInstanceCtx altogether to 
encapsulate the exact parameters that each one captures, however, not sure if 
that is possible.


http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/control_service.proto@335
PS3, Line 335:
 :   // General execution parameters for different fragments. 
Corresponds to 'fragments' in
 :   // the TExecPlanFragmentInfo sidecar.
 :   repeated PlanFragmentCtxPB fragment_ctxs = 7;
 :
 :   // Execution parameters for specific fragment instances. 
Corresponds to
 :   // 'fragment_instance_ctxs' in the TExecPlanFragmentInfo 
sidecar.
 :   repeated PlanFragmentInstanceCtxPB fragment_instance_ctxs = 8;
I'm still trying to understand the code, but it seems some of the info in these 
fields were being sent in the TExecPlanFragmentInfo sidecar, right? and now we 
are moving them out of the sidecar into protobuf fields? is it possible that 
moving things out of the sidecar can affect the perf of the Exec RPCs?
if the issue exists, might be a minor one; but just thought I'd bring it up to 
ensure we have considered it.


http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/planner.proto
File common/protobuf/planner.proto:

http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/planner.proto@1
PS3, Line 1: // Licensed to the Apache Software Foundation (ASF) under one
why name this planner.proto? looks like most of these structs come from 
PlanNodes.thrift


http://gerrit.cloudera.org:8080/#/c/15844/3/common/protobuf/planner.proto@25
PS3, Line 25: message HdfsFileSplitPB {
Is it a lot of effort to delete the corresponding THdfsFileSplit in 
PlanNodes.thrift? if we plan to keep these duplicated across Thrift and 
Protobuf, would be good to include a note in PlanNodes.thrift saying any 
changes to THdfsFileSplit, should be made to HdfsFileSplitPB in this file.
Same applies for all the other structs in this file.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a8e46767b257bbf677171ac2f4efb1b623ba41b
Gerrit-Change-Number: 15844
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Comment-Date: Mon, 11 May 2020 15:23:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6: Code-Review+1

(2 comments)

Found a couple of nits, but other than that LGTM.

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/aggregate-functions-ir.cc@758
PS6, Line 758: "Concatenated string length larger than allowed limit of "
 : + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES)
 : + " character data.").c_str()
> nit: Impala's convention is to use the Substitute() function from "gutil/st
Also, the format string could be stored in a global constant, so you wouldn't  
repeat it multiple times.


http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/string-functions-ir.cc
File be/src/exprs/string-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/string-functions-ir.cc@172
PS6, Line 172: "rpad() result is larger than allowed limit of "
nit: you could have a global format string like:

"$0 result is larger than allowed limit of $1 character data."

and you'd just pass 'space()/repeat()/lpad()/rpad()' and the limit to 
Substitute().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 14:23:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Zoltan Borok-Nagy (Code Review)
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/aggregate-functions-ir.cc
File be/src/exprs/aggregate-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/aggregate-functions-ir.cc@758
PS6, Line 758: "Concatenated string length larger than allowed limit of "
 : + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES)
 : + " character data.").c_str()
nit: Impala's convention is to use the Substitute() function from 
"gutil/strings/substitute.h"

E.g.:

  ctx->SetError(Substitute(
  "Concatenated string length larger than allowed limit of $0 character 
data.",
  PrettyPrinter::Print(StringVal::MAX_LENGTH, TUnit::BYTES)).c_str())


http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/string-functions-ir.cc
File be/src/exprs/string-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/15864/6/be/src/exprs/string-functions-ir.cc@106
PS6, Line 106: +
The clang-tidy job failed on this addition operator.

You can separate string literals with white spaces in C++, the compiler will 
concatenate them.

E.g.:
  cout << "Hello" "World"; // or
  cout << "Hello"
  "World";

Also, don't forget to use Substitute() instead.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 14:14:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 14:05:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6:

Build Failed

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 13:53:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 13:27:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Akos Kovacs (Code Review)
Hello Zoltan Borok-Nagy, Tim Armstrong, Csaba Ringhofer, Bikramjeet Vig, Impala 
Public Jenkins,

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

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

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

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..

IMPALA-7833 Audit and fix string builtins for long string handling

Some string built-in functions could crash impalad,
in case the result was longer than 1 gig max size.
Added some overflow checks.
Overflow error messages modified not to hard code max size.

Testing:
* Added some backend tests to cover overflow check
* Ran core tests

Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/large_strings.test
4 files changed, 104 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 6
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-9645 Port LLVM codegen to adapt aarch64

2020-05-11 Thread Tamas Mate (Code Review)
Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15718 )

Change subject: IMPALA-9645 Port LLVM codegen to adapt aarch64
..


Patch Set 9:

(3 comments)

Hi Zhao, thank you for the update.
I had some questions on the whitelisted CPU attributes.

http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/codegen-anyval.cc
File be/src/codegen/codegen-anyval.cc:

http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/codegen-anyval.cc@416
PS9, Line 416:   // Lowered type is of form { {i8}, [15 x i8], {i128} }. 
Get the i128 value and
 :   // truncate it to the correct size. (The {i128} 
corresponds to the union of the
 :   // different width int types.)
 :   // On aarch64, the Lowered type is of form { {i8}, {i128} 
}. No padding add.
 : #ifdef __aarch64__
 :   uint32_t idxs[] = {1, 0};
 : #else
 :   uint32_t idxs[] = {2, 0};
 : #endif
Tim mentioned earlier that the comment could be next to the relevant code like 
this:

#ifdef __aarch64__
  // On aarch64, the Lowered type is of form { {i8}, {i128} }. No padding 
add.
  uint32_t idxs[] = {1, 0};
#else
  // Lowered type is of form { {i8}, [15 x i8], {i128} }. Get the i128 
value and
  // truncate it to the correct size. (The {i128} corresponds to the union 
of the
  // different width int types.)
  uint32_t idxs[] = {2, 0};
#endif

Same later from line #470.


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

http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/llvm-codegen.cc@113
PS9, Line 113: adx,aes,avx,avx2,bmi,bmi2,cmov,cx16,f16c,"
 : 
"fma,fsgsbase,hle,invpcid,lzcnt,mmx,movbe,pclmul,popcnt,prfchw,rdrnd,rdseed,rtm,smap,"
 : "sse,sse2,sse3,sse4.1,sse4.2,ssse3,xsave,xsaveopt,
Are these cpu attributes relevant on aarch64?


http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/llvm-codegen.cc@117
PS9, Line 117: The default flags are a known-good set that are "
 : "routinely tested.
Does this sentence remain true in aarch64 context?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11
Gerrit-Change-Number: 15718
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward 
Gerrit-Reviewer: Anonymous Coward 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 11 May 2020 12:58:33 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9628: Refactor bootstrap system.sh

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15817 )

Change subject: IMPALA-9628: Refactor bootstrap_system.sh
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9e4309d6648a160b58ebc1e18043afe4110789b
Gerrit-Change-Number: 15817
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Garaguly 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Comment-Date: Mon, 11 May 2020 12:45:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 5
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 11:53:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9380: addendum - remove redundant assert

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15876 )

Change subject: IMPALA-9380: addendum - remove redundant assert
..

IMPALA-9380: addendum - remove redundant assert

Change-Id: I4ff4ff0a92f928314064b492c13904bec5ec6d5e
Reviewed-on: http://gerrit.cloudera.org:8080/15876
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M tests/hs2/test_json_endpoints.py
1 file changed, 1 insertion(+), 6 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4ff4ff0a92f928314064b492c13904bec5ec6d5e
Gerrit-Change-Number: 15876
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9380: addendum - remove redundant assert

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15876 )

Change subject: IMPALA-9380: addendum - remove redundant assert
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ff4ff0a92f928314064b492c13904bec5ec6d5e
Gerrit-Change-Number: 15876
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 11 May 2020 10:36:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9722: Consolidate avg size calculation in PerColumnStats

2020-05-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15878 )

Change subject: IMPALA-9722: Consolidate avg_size calculation in PerColumnStats
..

IMPALA-9722: Consolidate avg_size calculation in PerColumnStats

This change refactors the 'avg_size' calculation, 'total_width' will be
storing the sum of row widths till a Finalize call calculates the
average column size.

Testing:
 - Added unit test to verify the aggregation result

Change-Id: Iae1efb1c568c67dff6f25887c2ea2b8b249eea4b
Reviewed-on: http://gerrit.cloudera.org:8080/15878
Tested-by: Impala Public Jenkins 
Reviewed-by: Csaba Ringhofer 
---
M be/src/exec/incr-stats-util-test.cc
M be/src/exec/incr-stats-util.cc
M be/src/exec/incr-stats-util.h
3 files changed, 27 insertions(+), 5 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Csaba Ringhofer: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae1efb1c568c67dff6f25887c2ea2b8b249eea4b
Gerrit-Change-Number: 15878
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-9722: Consolidate avg size calculation in PerColumnStats

2020-05-11 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15878 )

Change subject: IMPALA-9722: Consolidate avg_size calculation in PerColumnStats
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae1efb1c568c67dff6f25887c2ea2b8b249eea4b
Gerrit-Change-Number: 15878
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 11 May 2020 09:42:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6022/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 5
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 07:28:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9380: addendum - remove redundant assert

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15876 )

Change subject: IMPALA-9380: addendum - remove redundant assert
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6021/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ff4ff0a92f928314064b492c13904bec5ec6d5e
Gerrit-Change-Number: 15876
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sahil Takiar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 11 May 2020 06:52:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 5
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 06:28:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Akos Kovacs (Code Review)
Akos Kovacs has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..

IMPALA-7833 Audit and fix string builtins for long string handling

Some string built-in functions could crash impalad,
in case the result was longer than 1 gig max size.
Added some overflow checks.
Overflow error messages modified not to hard code max size.

Testing:
* Added some backend tests to cover overflow check
* Ran core tests

Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
---
M be/src/exprs/aggregate-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M testdata/workloads/functional-query/queries/QueryTest/large_strings.test
4 files changed, 94 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 5
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7833 Audit and fix string builtins for long string handling

2020-05-11 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15864 )

Change subject: IMPALA-7833 Audit and fix string builtins for long string 
handling
..


Patch Set 5:

(10 comments)

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

http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/aggregate-functions-ir.cc@759
PS5, Line 759: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/aggregate-functions-ir.cc@791
PS5, Line 791: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


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

http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@90
PS5, Line 90: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@104
PS5, Line 104: context->SetError(("Number of repeats in repeat() call is 
larger than allowed limit of "
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@105
PS5, Line 105: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@114
PS5, Line 114: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@136
PS5, Line 136: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@168
PS5, Line 168: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@892
PS5, Line 892: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/15864/5/be/src/exprs/string-functions-ir.cc@939
PS5, Line 939: + PrettyPrinter::Print(StringVal::MAX_LENGTH, 
TUnit::BYTES) + " character data.").c_str());
line too long (99 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93a53845f04e61ff446b363c78db1e49cbd5dc49
Gerrit-Change-Number: 15864
Gerrit-PatchSet: 5
Gerrit-Owner: Akos Kovacs 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 11 May 2020 06:28:57 +
Gerrit-HasComments: Yes