[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13710/ : 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/20294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Fri, 04 Aug 2023 05:14:36 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Yifan Zhang (Code Review)
Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh
File bin/bootstrap_build.sh:

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh@64
PS3, Line 64: ./buildall.sh -notests -so
> Ah, create-load-data calls copy-udfs-udas, which makes a few specific targe
This change was intended to fix the failure in copy-udfs-udas. But it turns out 
that 'bootstrap_build.sh' is called in 'jenkins/build-only.sh', which is not 
used to run all tests.

It seems that we run tests using 'jenkins/dockerized-impala-run-tests.sh', 
which calls './buildall.sh -format -testdata -notests' to build Impala and load 
data. Considering that building all backend tests is not necessary, I fixed the 
issue in copy-udfs-udas by manually running cmake again before building tests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Fri, 04 Aug 2023 04:53:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Yifan Zhang (Code Review)
Hello Michael Smith, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..

IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test targets

This patch adds a new option 'BUILD_WITH_NO_TESTS' to tell CMake not to
generate test targets. The option is set ON when building impala using
the command 'buildall.sh -notests'. This should be useful for a packing
build because cpack built all targets prior to this change.

Testing:
  - Ran 'buildall.sh -release -package' with and without '-notests'
flag and verified generated executables.

Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
---
M be/CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/catalog/CMakeLists.txt
M be/src/codegen/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/avro/CMakeLists.txt
M be/src/exec/parquet/CMakeLists.txt
M be/src/experiments/CMakeLists.txt
M be/src/exprs/CMakeLists.txt
M be/src/gutil/CMakeLists.txt
M be/src/rpc/CMakeLists.txt
M be/src/runtime/CMakeLists.txt
M be/src/runtime/bufferpool/CMakeLists.txt
M be/src/runtime/io/CMakeLists.txt
M be/src/scheduling/CMakeLists.txt
M be/src/service/CMakeLists.txt
M be/src/statestore/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/udf/CMakeLists.txt
M be/src/udf_samples/CMakeLists.txt
M be/src/util/CMakeLists.txt
M be/src/util/cache/CMakeLists.txt
M buildall.sh
M testdata/bin/copy-udfs-udas.sh
25 files changed, 174 insertions(+), 66 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 5
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13709/ : 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/20294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Fri, 04 Aug 2023 04:07:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Yifan Zhang (Code Review)
Hello Michael Smith, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..

IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test targets

This patch adds a new option 'BUILD_WITH_NO_TESTS' to tell CMake not to
generate test targets. The option is set ON when building impala using
the command 'buildall.sh -notests'. This should be useful for a packing
build because cpack built all targets prior to this change.

Testing:
  - Ran 'buildall.sh -release -package' with and without '-notests'
flag and verified generated executables.

Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
---
M be/CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/catalog/CMakeLists.txt
M be/src/codegen/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/avro/CMakeLists.txt
M be/src/exec/parquet/CMakeLists.txt
M be/src/experiments/CMakeLists.txt
M be/src/exprs/CMakeLists.txt
M be/src/gutil/CMakeLists.txt
M be/src/rpc/CMakeLists.txt
M be/src/runtime/CMakeLists.txt
M be/src/runtime/bufferpool/CMakeLists.txt
M be/src/runtime/io/CMakeLists.txt
M be/src/scheduling/CMakeLists.txt
M be/src/service/CMakeLists.txt
M be/src/statestore/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/udf/CMakeLists.txt
M be/src/udf_samples/CMakeLists.txt
M be/src/util/CMakeLists.txt
M be/src/util/cache/CMakeLists.txt
M buildall.sh
M testdata/bin/copy-udfs-udas.sh
25 files changed, 174 insertions(+), 66 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 


[Impala-ASF-CR] IMPALA-5741: Support reading tiny RDBMS tables

2023-08-03 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17842 )

Change subject: IMPALA-5741: Support reading tiny RDBMS tables
..


Patch Set 9:

(1 comment)

Fucun Chu, do you plan to continue working on this patch?

http://gerrit.cloudera.org:8080/#/c/17842/9/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
File 
java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java:

http://gerrit.cloudera.org:8080/#/c/17842/9/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java@131
PS9, Line 131:  dbAccessor = DatabaseAccessorFactory.getAccessor(tableConfig);
Is it possible to create dbAccessor in prepare() API and get 
totalNumberOfRecords so that we can return totalNumberOfRecords as 
num_rows_estimate in prepare()?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8244e978c7717c6f1452f66f1630b6441392e7d2
Gerrit-Change-Number: 17842
Gerrit-PatchSet: 9
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Fri, 04 Aug 2023 00:03:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp@150
PS10, Line 150:   && THRIFT_strncasecmp(header, TRANSFER_ENCODING, sz) == 
0) {
> Based on my reading of the docs and some experimentation with code, we shou
Unfortunately, when parsing the following header
"Transfer-Encoding-the: fake",
the condition that does not use the 'sz' variable will be true, because the 
first 17 characters will match:
THRIFT_strncasecmp(header, TRANSFER_ENCODING, strlen(TRANSFER_ENCODING)) == 0



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Comment-Date: Thu, 03 Aug 2023 20:03:46 +
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-11603: (Addendum) fix cloudflarezlib noop build

2023-08-03 Thread Laszlo Gaal (Code Review)
Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20312 )

Change subject: IMPALA-11603: (Addendum) fix cloudflarezlib noop build
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbc5d774ca871d7410025d25690d7f96893d2f37
Gerrit-Change-Number: 20312
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Thu, 03 Aug 2023 19:19:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Jason Fehr (Code Review)
Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

(1 comment)

Looks good, one small nit.

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp@150
PS10, Line 150:   && THRIFT_strncasecmp(header, TRANSFER_ENCODING, sz) == 
0) {
Based on my reading of the docs and some experimentation with code, we should 
be able to remove the `sz` variable completely and use the length of the 
expected header:
if (THRIFT_strncasecmp(header, TRANSFER_ENCODING, strlen(TRANSFER_ENCODING))

or use the `strcasecmp` function instead and remove all the string length 
checks.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Comment-Date: Thu, 03 Aug 2023 19:05:39 +
Gerrit-HasComments: Yes


[native-toolchain-CR] IMPALA-12334: Add debug symbols to LLVM assert build

2023-08-03 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20311 )

Change subject: IMPALA-12334: Add debug symbols to LLVM assert build
..


Patch Set 3: Code-Review+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 18:31:38 +
Gerrit-HasComments: No


[native-toolchain-CR] IMPALA-12334: Add debug symbols to LLVM assert build

2023-08-03 Thread Michael Smith (Code Review)
Hello Yida Wu, Joe McDonnell,

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

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

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

Change subject: IMPALA-12334: Add debug symbols to LLVM assert build
..

IMPALA-12334: Add debug symbols to LLVM assert build

Builds the LLVM debug build (-asserts) with RelWithDebInfo so we can step
into LLVM code while debugging Impala.

Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
---
M source/llvm/build-source-tarball.sh
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/11/20311/3
--
To view, visit http://gerrit.cloudera.org:8080/20311
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 


[native-toolchain-CR] IMPALA-12334: Add debug symbols to LLVM assert build

2023-08-03 Thread Yida Wu (Code Review)
Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20311 )

Change subject: IMPALA-12334: Add debug symbols to LLVM assert build
..


Patch Set 2:

Paste a link of related doc: https://llvm.org/docs/CMake.html#cmake-build-type


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 18:05:50 +
Gerrit-HasComments: No


[native-toolchain-CR] IMPALA-12334: Add debug symbols to LLVM assert build

2023-08-03 Thread Yida Wu (Code Review)
Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20311 )

Change subject: IMPALA-12334: Add debug symbols to LLVM assert build
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 18:04:42 +
Gerrit-HasComments: No


[native-toolchain-CR] IMPALA-12334: Add debug symbols to LLVM assert build

2023-08-03 Thread Michael Smith (Code Review)
Hello Yida Wu, Joe McDonnell,

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

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

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

Change subject: IMPALA-12334: Add debug symbols to LLVM assert build
..

IMPALA-12334: Add debug symbols to LLVM assert build

Builds the LLVM debug build (-assert) with RelWithDebInfo so we can step
into LLVM code while debugging Impala.

Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
---
M source/llvm/build-source-tarball.sh
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/native-toolchain 
refs/changes/11/20311/2
--
To view, visit http://gerrit.cloudera.org:8080/20311
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yida Wu 


[native-toolchain-CR] IMPALA-11603: (Addendum) fix cloudflarezlib noop build

2023-08-03 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20312


Change subject: IMPALA-11603: (Addendum) fix cloudflarezlib noop build
..

IMPALA-11603: (Addendum) fix cloudflarezlib noop build

Addresses failure when no change is necessary to cloudflarezlib
build.sh: line 38: return: can only `return' from a function or sourced script

Change-Id: Icbc5d774ca871d7410025d25690d7f96893d2f37
---
M source/cloudflarezlib/build.sh
1 file changed, 1 insertion(+), 1 deletion(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icbc5d774ca871d7410025d25690d7f96893d2f37
Gerrit-Change-Number: 20312
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[native-toolchain-CR] Build LLVM assert build with debug symbols

2023-08-03 Thread Michael Smith (Code Review)
Michael Smith has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20311


Change subject: Build LLVM assert build with debug symbols
..

Build LLVM assert build with debug symbols

Builds the LLVM debug build (-assert) with RelWithDebInfo so we can step
into LLVM code while debugging Impala.

Addresses failure when no change is necessary to cloudflarezlib
build.sh: line 38: return: can only `return' from a function or sourced script

Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
---
M source/cloudflarezlib/build.sh
M source/llvm/build-source-tarball.sh
2 files changed, 2 insertions(+), 2 deletions(-)



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

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c58bfe2233dcec39fcad92fc52ce372165229d8
Gerrit-Change-Number: 20311
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 17:33:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13708/ : 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/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:35:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 9:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13707/ : 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/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 9
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:33:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp@139
PS8, Line 139:
> Okay!
Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:14:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 117 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 9
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh
File bin/bootstrap_build.sh:

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh@64
PS3, Line 64: ./buildall.sh -skiptests -so
> What led you to this change?
Ah, create-load-data calls copy-udfs-udas, which makes a few specific targets. 
Loading data into a cluster now requires -skiptests and not -notests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:13:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 118 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Michael Smith (Code Review)
Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 3:

(1 comment)

A rebase would fix the clang-tidy failure I saw.

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh
File bin/bootstrap_build.sh:

http://gerrit.cloudera.org:8080/#/c/20294/3/bin/bootstrap_build.sh@64
PS3, Line 64: ./buildall.sh -skiptests -so
What led you to this change?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:06:04 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12332: Undo IMPALA-8615 as the corresponding configs have been removed

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20308 )

Change subject: IMPALA-12332: Undo IMPALA-8615 as the corresponding configs 
have been removed
..

IMPALA-12332: Undo IMPALA-8615 as the corresponding configs have been removed

IMPALA-8615 documented the changes made in IMPALA-8536, but the configs
were subsequently removed in IMPALA-9077. Rollback IMPALA-8615 to bring
the docs up to date.

Revert "IMPALA-8615: [DOCS] Document the scalable admission control parameters"
This was a clean revert, and there were no overlapping changes to this file.

TESTING:
- built docs and reviewed the file.

This reverts commit b2136c39fcafacec308dc9dd13ad13133596d05d.

Change-Id: Ibc856c62babb4b305b6a7c286a0f4c86e6e418cc
Reviewed-on: http://gerrit.cloudera.org:8080/20308
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M docs/topics/impala_admission.xml
1 file changed, 6 insertions(+), 36 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc856c62babb4b305b6a7c286a0f4c86e6e418cc
Gerrit-Change-Number: 20308
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 


[Impala-ASF-CR] IMPALA-12332: Undo IMPALA-8615 as the corresponding configs have been removed

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20308 )

Change subject: IMPALA-12332: Undo IMPALA-8615 as the corresponding configs 
have been removed
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc856c62babb4b305b6a7c286a0f4c86e6e418cc
Gerrit-Change-Number: 20308
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:08:03 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12332: Undo IMPALA-8615 as the corresponding configs have been removed

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20308 )

Change subject: IMPALA-12332: Undo IMPALA-8615 as the corresponding configs 
have been removed
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc856c62babb4b305b6a7c286a0f4c86e6e418cc
Gerrit-Change-Number: 20308
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 03 Aug 2023 15:59:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12332: Undo IMPALA-8615 as the corresponding configs have been removed

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20308 )

Change subject: IMPALA-12332: Undo IMPALA-8615 as the corresponding configs 
have been removed
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/381/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc856c62babb4b305b6a7c286a0f4c86e6e418cc
Gerrit-Change-Number: 20308
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 03 Aug 2023 15:59:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12269: Codegen cache false negative because of function names hash

2023-08-03 Thread Yida Wu (Code Review)
Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20168 )

Change subject: IMPALA-12269: Codegen cache false negative because of function 
names hash
..


Patch Set 6: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/20168/5/be/src/codegen/llvm-codegen.cc@1438
PS5, Line 1438:   for (auto& entry : fns_to_jit_compile_) {
> It's already checked by the DCHECKS on L1448 and L1469.
Ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
Gerrit-Change-Number: 20168
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 15:02:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG@10
PS8, Line 10: This change introduces a new optional impala-shell option
: (send_auth_header_for_hiveserver2) to add an 'auth' header to http
: requests, which is expected in latest hiveserver2 builds
: when supporting multiple authentication modes at once
: (see HIVE-27352).
> There is already an option called strict_hs2_protocol which is used when we
I didn't dare to put it under strict_hs2_protocol, because then if someone uses 
a new impala-shell with an old impala build and sets strict_hs2_protocol when 
connecting to the coordinator, then the impala-shell will send the 'auth' 
header and authentication in hs2-http requests will not work because of the old 
impala build in the coordinator. I don't know much about impala users, if this 
is not a real use-case, then this can of course be added under 
strict_hs2_protocol.


http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp@139
PS8, Line 139: _
> Thanks for fixing this bug!
Okay!
This length check was my original idea too, but the conditions seemed pretty 
ugly to me, so I moved to the current one. I'll revert back to that solution.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 14:59:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12269: Codegen cache false negative because of function names hash

2023-08-03 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20168 )

Change subject: IMPALA-12269: Codegen cache false negative because of function 
names hash
..


Patch Set 6: Code-Review+2

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/20168/6/be/src/codegen/llvm-codegen.cc@1196
PS6, Line 1196:   //  The function names are sorted in 'fns_to_jit_compile_'.
nit: extra space?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
Gerrit-Change-Number: 20168
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 14:43:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8:

(3 comments)

Some high level, comments, I haven't checked all the tests yet.

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG@7
PS8, Line 7: impala-shell
The change also affects the server side, so I would change this to something 
like "Support "auth" header in hs2-http requests"


http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG@10
PS8, Line 10: This change introduces a new optional impala-shell option
: (send_auth_header_for_hiveserver2) to add an 'auth' header to http
: requests, which is expected in latest hiveserver2 builds
: when supporting multiple authentication modes at once
: (see HIVE-27352).
There is already an option called strict_hs2_protocol which is used when we 
connect to hiveserver2 instead of Impala so impala-shell tries to do things the 
Hive way. Do you think that we could use that option to decide whether to add 
"auth" header instead of adding a new option?


http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp@139
PS8, Line 139: _
Thanks for fixing this bug!
I would prefer a solution though that doesn't need a malloc here. For example 
THRIFT_strcasecmp could also check that  the "sz" and length of the right 
string are equal or check if "sz" char of the right string is \0.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 14:38:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 13:13:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13706/ : 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/20294
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 12:50:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 12:34:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Yifan Zhang (Code Review)
Hello Michael Smith, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..

IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test targets

This patch adds a new option 'BUILD_WITH_NO_TESTS' to tell CMake not to
generate test targets. The option is set ON when building impala using
the command 'buildall.sh -notests'. This should be useful for a packing
build because cpack built all targets prior to this change.

Testing:
  - Ran 'buildall.sh -release -package' with and without '-notests'
flag and verified generated executables.

Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
---
M be/CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/catalog/CMakeLists.txt
M be/src/codegen/CMakeLists.txt
M be/src/common/CMakeLists.txt
M be/src/exec/CMakeLists.txt
M be/src/exec/avro/CMakeLists.txt
M be/src/exec/parquet/CMakeLists.txt
M be/src/experiments/CMakeLists.txt
M be/src/exprs/CMakeLists.txt
M be/src/gutil/CMakeLists.txt
M be/src/rpc/CMakeLists.txt
M be/src/runtime/CMakeLists.txt
M be/src/runtime/bufferpool/CMakeLists.txt
M be/src/runtime/io/CMakeLists.txt
M be/src/scheduling/CMakeLists.txt
M be/src/service/CMakeLists.txt
M be/src/statestore/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/udf/CMakeLists.txt
M be/src/udf_samples/CMakeLists.txt
M be/src/util/CMakeLists.txt
M be/src/util/cache/CMakeLists.txt
M bin/bootstrap_build.sh
M buildall.sh
25 files changed, 168 insertions(+), 67 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 11:54:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12049: Deflake test drop corrupt table

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20289 )

Change subject: IMPALA-12049: Deflake test_drop_corrupt_table
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cbdf5646ed20bb8333e5557ed43226de993b7dd
Gerrit-Change-Number: 20289
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 11:41:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12049: Deflake test drop corrupt table

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/20289 )

Change subject: IMPALA-12049: Deflake test_drop_corrupt_table
..

IMPALA-12049: Deflake test_drop_corrupt_table

Originally there were two flavors of the test_drop_corrupt_table test.
One version (test_drop_corrupt_table_with_invalidate) did
an 'invalidate metadata' as part of the test after deleting metadata
files, and the other version did not. The version without invalidation
was depending on the catalog update resulting from adding the iceberg
table arriving at the coordinator before the deletion. This happens a
lot of the time but it can't be guaranteed. Fix the test by removing
test_drop_corrupt_table. Simplify the code a little by inlining a
method, and rename the remaining test to be test_drop_corrupt_table.

Change-Id: I4cbdf5646ed20bb8333e5557ed43226de993b7dd
Reviewed-on: http://gerrit.cloudera.org:8080/20289
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M tests/query_test/test_iceberg.py
1 file changed, 2 insertions(+), 10 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4cbdf5646ed20bb8333e5557ed43226de993b7dd
Gerrit-Change-Number: 20289
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 


[Impala-ASF-CR] IMPALA-12269: Codegen cache false negative because of function names hash

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20168 )

Change subject: IMPALA-12269: Codegen cache false negative because of function 
names hash
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13705/ : 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/20168
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
Gerrit-Change-Number: 20168
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 10:17:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12306: (Part 1) Make codegen cache tests with symbol emitter more robust

2023-08-03 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20304 )

Change subject: IMPALA-12306: (Part 1) Make codegen cache tests with symbol 
emitter more robust
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/20304/2//COMMIT_MSG@24
PS2, Line 24: Instead of using a fixed codegen
: cache size we use binary search to find a good value
I was thinking about a potentially simpler solution: add a flag to BE that 
increases the size used for eviction calculation.
https://github.com/apache/impala/blob/8638255e5074f1342dfc452bca39f649a76612d6/be/src/codegen/llvm-codegen-cache.h#L174

total_bytes_charge is only used for the eviction decision, so it shouldn't have 
any side effect. A large enough "codegen_cache_entry_bytes_charge_overhead" 
could ensure that the effect of the actual bytecode is negligible so reliable 
tests could be written for size based eviction.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If801ae6d3d9f5286ed886b1d06c37a32bc1d2c54
Gerrit-Change-Number: 20304
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 10:06:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12269: Codegen cache false negative because of function names hash

2023-08-03 Thread Daniel Becker (Code Review)
Daniel Becker has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/20168 )

Change subject: IMPALA-12269: Codegen cache false negative because of function 
names hash
..

IMPALA-12269: Codegen cache false negative because of function names hash

Codegen cache entries (execution engines holding an LLVM code module)
are stored by keys derived from the unoptimised llvm modules: the key is
either the whole unoptimised module (normal mode) or its hash (optimal
mode). Because hash collisions are possible (in optimal mode), as an
extra precaution we also compare the hashes of the function names in the
current and the cached module. However, when assembling the function
name list we do not filter out duplicate function names, which may
result in cases where the unoptimised llvm modules are identical but the
function name hashes do not match.

Example:
First query:
  select int_col, tinyint_col
  from alltypessmall
  order by int_col desc
  limit 20;

Second query:
  select tinyint_col
  from alltypessmall
  order by int_col desc
  limit 20;

In the first query, there are two 'SlotRef' objects referencing
'tinyint_col' which want to codegen a 'GetSlotRef()' function. The
second invokation of 'SlotRef::GetCodegendComputeFnImpl()' checks the
already codegen'd functions, finds the function created by its first
invokation and returns that. The two 'SlotRef' objects will use the same
'llvm::Function' and there will be only one copy of it in the module,
but both 'SlotRef's will call 'LlvmCodeGen::AddFunctionToJit()' with
this function in order for their respective function pointers to be set
after JIT-compilation.

'LlvmCodeGen::GetAllFunctionNames()' will return the names of all
functions with which 'LlvmCodeGen::AddFunctionToJit()' has been called,
including duplicates.

The second query generates the same unoptimised module as the first
query (for the corresponding fragment), but does not have a duplicated
'GetSlotRef()' function in its function name list, so the cached module
is rejected.

Note that this also results in the cached module being evicted when the
new module from the second query is inserted into the cache because the
new module will have the same key as the cached one (the modules are
identical).

This change fixes this problem by using a de-duplicated and sorted
function name list.

Testing:
  - Added a test in test_codegen_cache.py that asserts that there is a
cache hit and no eviction in the above example.

Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
---
M be/src/codegen/llvm-codegen-cache-test.cc
M be/src/codegen/llvm-codegen.cc
M be/src/codegen/llvm-codegen.h
M tests/custom_cluster/test_codegen_cache.py
4 files changed, 125 insertions(+), 61 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
Gerrit-Change-Number: 20168
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 


[Impala-ASF-CR] IMPALA-12269: Codegen cache false negative because of function names hash

2023-08-03 Thread Daniel Becker (Code Review)
Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20168 )

Change subject: IMPALA-12269: Codegen cache false negative because of function 
names hash
..


Patch Set 6:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/20168/5/be/src/codegen/llvm-codegen.cc@1202
PS5, Line 1202:   return result.str();
> nit. unnecessary newline
Done


http://gerrit.cloudera.org:8080/#/c/20168/5/be/src/codegen/llvm-codegen.cc@1438
PS5, Line 1438:   for (auto& entry : fns_to_jit_compile_) {
> Because for SetFunctionPointers() "either both or none of 'cache' and 'cach
It's already checked by the DCHECKS on L1448 and L1469.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf1d2b424c969fbba181ab90bf9c7bf22355f139
Gerrit-Change-Number: 20168
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Daniel Becker 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Yida Wu 
Gerrit-Comment-Date: Thu, 03 Aug 2023 09:52:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 08:16:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/13704/ : 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/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 08:05:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 106 insertions(+), 38 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12288: Add BUILD WITH NO TESTS option to remove test targets

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20294 )

Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test 
targets
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a
Gerrit-Change-Number: 20294
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 07:41:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12049: Deflake test drop corrupt table

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20289 )

Change subject: IMPALA-12049: Deflake test_drop_corrupt_table
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cbdf5646ed20bb8333e5557ed43226de993b7dd
Gerrit-Change-Number: 20289
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 07:22:30 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-12049: Deflake test drop corrupt table

2023-08-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20289 )

Change subject: IMPALA-12049: Deflake test_drop_corrupt_table
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cbdf5646ed20bb8333e5557ed43226de993b7dd
Gerrit-Change-Number: 20289
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Thu, 03 Aug 2023 07:22:29 +
Gerrit-HasComments: No