[Impala-ASF-CR] IMPALA-3942: Fix wronly escaped string literal in front-end
Hello Gabor Kaszab, Jim Apple, Tim Armstrong, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8818 to look at the new patch set (#2). Change subject: IMPALA-3942: Fix wronly escaped string literal in front-end .. IMPALA-3942: Fix wronly escaped string literal in front-end String literal can be wrapped by either single or double quotes. There are some holes in escaping the string literal. The solution is to normalize any string which comes from user's given string or a generated string (e.g. constant fold by the rewritter rule). Testing: Add some test cases to TestEscapingStringLiteral Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec --- M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java M tests/query_test/test_queries.py 2 files changed, 112 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/18/8818/2 -- To view, visit http://gerrit.cloudera.org:8080/8818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec Gerrit-Change-Number: 8818 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3942: Fix wronly escaped string literal in front-end
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8818 ) Change subject: IMPALA-3942: Fix wronly escaped string literal in front-end .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8818/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java: http://gerrit.cloudera.org:8080/#/c/8818/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@49 PS1, Line 49: boolean useSingleQuote = true; > Another, umh, "pragmatic" solution might be to just try both quotes and see This approach can be implemented easily, but I would prefer the above than this: 1. To avoid unnecessary overhead from try and error method. 2. More elaborative and robust code. 3. Fix for the problem is not urgent. http://gerrit.cloudera.org:8080/#/c/8818/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@49 PS1, Line 49: boolean useSingleQuote = true; > I think all proposed approaches do not consider that string literals can al Thanks a lot for the information. I didn't aware of the string literal can come from the rewritter rule. The normalization stage should be required, so I introduced getEscapedValue() method in the recent patch set. -- To view, visit http://gerrit.cloudera.org:8080/8818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec Gerrit-Change-Number: 8818 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 18 Dec 2017 08:32:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3942: Fix wronly escaped string literal in front-end
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8818 ) Change subject: IMPALA-3942: Fix wronly escaped string literal in front-end .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8818/2/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java: http://gerrit.cloudera.org:8080/#/c/8818/2/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@104 PS2, Line 104: getEscapedValue Escaping and unescaping the value happen here. Do you have more intuitive name for this? (e.g. getNormalizedValue) http://gerrit.cloudera.org:8080/#/c/8818/2/tests/query_test/test_queries.py File tests/query_test/test_queries.py: http://gerrit.cloudera.org:8080/#/c/8818/2/tests/query_test/test_queries.py@276 PS2, Line 276: def test_simple(self): Please let me know if there is any missing tests. -- To view, visit http://gerrit.cloudera.org:8080/8818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc4b5f5d8ffaa8feb96a466959427a04b3b06fec Gerrit-Change-Number: 8818 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 18 Dec 2017 08:35:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Hello Lars Volker, Gabor Kaszab, Philip Zeyliger, Tim Armstrong, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8621 to look at the new patch set (#14). Change subject: IMPALA-3703: Store query context in thread-local variables .. IMPALA-3703: Store query context in thread-local variables This commit introduces the ThreadDebugInfo class which can hold information about the current thread that can be useful in debug sessions. It needs to be allocated on the stack of each thread in order to include it to minidumps. Currently a ThreadDebugInfo object is created in Thread::SuperviseThread. This object is available in all the child stack frames through the global function GetThreadDebugInfo(). ThreadDebugInfo has members for the thread name and instance id. These are fixed size char buffers. If you have a core dump written by Impala, you can locate the ThreadDebugInfo for the current thread through the global pointer impala::thread_debug_info. In a core file that has been created from a minidump, we need to select the stack frame that allocated the ThreadDebugInfo object in order to inspect it. It is currently allocated in Thread::SuperviseThread(). We can use printf in gdb to print the members, e.g.: printf "%s\n" thread_debug_info.instance_id Currently the thread name and instance id is stored. I created some tests in thread-debug-info-test.cc Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 --- M be/src/common/CMakeLists.txt A be/src/common/thread-debug-info-test.cc A be/src/common/thread-debug-info.cc A be/src/common/thread-debug-info.h M be/src/exec/blocking-join-node.cc M be/src/exec/hdfs-scan-node.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-state.cc M be/src/util/thread.cc 9 files changed, 241 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/8621/14 -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. Patch Set 14: If I'm not mistaken the Jenkins verify job failed because of the warnings caused by the DCHECK_NOTNULL macros. I replaced DCHECK_NOTNULL() with DCHECK(ptr != nullptr) -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 11:43:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. Patch Set 14: Code-Review+2 This seems to be a known issue: https://github.com/google/glog/issues/172 Carrying Tim's +2. -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 11:54:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1634/ -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 11:54:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6190/6246: Add instances tab and event sequence
Hello Michael Ho, Joe McDonnell, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8758 to look at the new patch set (#8). Change subject: IMPALA-6190/6246: Add instances tab and event sequence .. IMPALA-6190/6246: Add instances tab and event sequence This change adds tracking of the current state during the execution of a fragment instance. The current state is then reported back to the coordinator and exposed to users via a new tab in the query detail debug webpage. This change also adds an event timeline to fragment instances in the query profile. The timeline measures the time since fragment start at which particular events complete. Events are derived from the current state of the execution of a fragment instance. For example: - Codegen Finished: 79.312ms (79.312ms) - Prepare Finished: 79.454ms (141.949us) - Open Finished: 93.287ms (13.832ms) - First Batch Received: 259.751ms (166.464ms) - First Batch Sent: 260.219ms (467.917us) - ExecInternal Finished: 2s733ms (2s473ms) I added automated tests for both extensions and additionally verified the change by manual inspection. Here are the TPCH performance comparison results between this change and the previous commit on a 16 node cluster. ++---+-++++ | Workload | File Format | Avg (s) | Delta(Avg) | GeoMean(s) | Delta(GeoMean) | ++---+-++++ | TPCH(_300) | parquet / none / none | 18.47 | -0.94% | 9.72 | -1.08% | ++---+-++++ ++--+---++-++++-+---+ | Workload | Query| File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Num Clients | Iters | ++--+---++-++++-+---+ | TPCH(_300) | TPCH-Q5 | parquet / none / none | 48.88 | 46.93 | +4.15% | 0.14%| 3.61%| 1 | 3 | | TPCH(_300) | TPCH-Q13 | parquet / none / none | 21.64 | 21.15 | +2.29% | 2.06%| 1.84%| 1 | 3 | | TPCH(_300) | TPCH-Q11 | parquet / none / none | 1.71 | 1.70| +1.12% | 0.54%| 2.51%| 1 | 3 | | TPCH(_300) | TPCH-Q18 | parquet / none / none | 33.15 | 32.79 | +1.09% | 0.13%| 2.03%| 1 | 3 | | TPCH(_300) | TPCH-Q14 | parquet / none / none | 5.95 | 5.90| +0.82% | 2.19%| 0.49%| 1 | 3 | | TPCH(_300) | TPCH-Q1 | parquet / none / none | 13.99 | 13.90 | +0.63% | 0.25%| 1.39%| 1 | 3 | | TPCH(_300) | TPCH-Q2 | parquet / none / none | 3.44 | 3.44| +0.00% | * 20.29% * | * 20.76% * | 1 | 3 | | TPCH(_300) | TPCH-Q6 | parquet / none / none | 1.21 | 1.22| -0.01% | 0.06%| 0.06%| 1 | 3 | | TPCH(_300) | TPCH-Q20 | parquet / none / none | 3.51 | 3.51| -0.11% | 7.15%| 7.30%| 1 | 3 | | TPCH(_300) | TPCH-Q16 | parquet / none / none | 6.89 | 6.91| -0.21% | 0.65%| 0.55%| 1 | 3 | | TPCH(_300) | TPCH-Q4 | parquet / none / none | 4.78 | 4.80| -0.38% | 0.06%| 0.59%| 1 | 3 | | TPCH(_300) | TPCH-Q19 | parquet / none / none | 30.78 | 31.04 | -0.83% | 0.45%| 1.03%| 1 | 3 | | TPCH(_300) | TPCH-Q22 | parquet / none / none | 6.06 | 6.12| -1.02% | 1.51%| 2.12%| 1 | 3 | | TPCH(_300) | TPCH-Q10 | parquet / none / none | 9.43 | 9.58| -1.54% | 0.69%| 3.30%| 1 | 3 | | TPCH(_300) | TPCH-Q21 | parquet / none / none | 93.41 | 95.18 | -1.86% | 0.08%| 0.81%| 1 | 3 | | TPCH(_300) | TPCH-Q15 | parquet / none / none | 3.40 | 3.47| -1.99% | 0.72%| 1.27%| 1 | 3 | | TPCH(_300) | TPCH-Q7 | parquet / none / none | 44.98 | 46.24 | -2.71% | 1.83%| 1.27%| 1 | 3 | | TPCH(_300) | TPCH-Q3 | parquet / none / none | 28.06 | 29.11 | -3.61% | 1.62%| 1.23%| 1 | 3 | | TPCH(_300) | TPCH-Q12 | parquet / none / none | 3.15 | 3.28| -3.80% | 0.96%| 1.32%| 1 | 3 | | TPCH(_300) | TPCH-Q9 | parquet / none / none | 29.47 | 30.80 | -4.30% | 0.29%| 0.34%| 1
[Impala-ASF-CR] IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow CFB mode is stream cipher and is secure when use different nonce/IV for every message. However it would be a performance
Xianda Ke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8861 Change subject: IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow CFB mode is stream cipher and is secure when use different nonce/IV for every message. However it would be a performance bottleneck. CTR mode is also stream cipher and is secure, 4~6x faster .. IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow CFB mode is stream cipher and is secure when use different nonce/IV for every message. However it would be a performance bottleneck. CTR mode is also stream cipher and is secure, 4~6x faster than CFB mode in OpenSSL. AES-CTR+SHA256 is about 40~70% faster than AES-CFB+SHA256. CTR mode is used if OpenSSL version>=1.0.1 at runtime, otherwise fall back to CFB mode. Testing: run runtime tmp-file-mgr-test, openssl-util-test, buffer-pool-test and buffered-tuple-stream-test Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff --- M be/src/runtime/tmp-file-mgr.cc M be/src/util/openssl-util-test.cc M be/src/util/openssl-util.cc M be/src/util/openssl-util.h 4 files changed, 70 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/8861/2 -- To view, visit http://gerrit.cloudera.org:8080/8861 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff Gerrit-Change-Number: 8861 Gerrit-PatchSet: 2 Gerrit-Owner: Xianda Ke
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1635/ -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 14:57:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 14:57:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. IMPALA-3703: Store query context in thread-local variables This commit introduces the ThreadDebugInfo class which can hold information about the current thread that can be useful in debug sessions. It needs to be allocated on the stack of each thread in order to include it to minidumps. Currently a ThreadDebugInfo object is created in Thread::SuperviseThread. This object is available in all the child stack frames through the global function GetThreadDebugInfo(). ThreadDebugInfo has members for the thread name and instance id. These are fixed size char buffers. If you have a core dump written by Impala, you can locate the ThreadDebugInfo for the current thread through the global pointer impala::thread_debug_info. In a core file that has been created from a minidump, we need to select the stack frame that allocated the ThreadDebugInfo object in order to inspect it. It is currently allocated in Thread::SuperviseThread(). We can use printf in gdb to print the members, e.g.: printf "%s\n" thread_debug_info.instance_id Currently the thread name and instance id is stored. I created some tests in thread-debug-info-test.cc Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Reviewed-on: http://gerrit.cloudera.org:8080/8621 Reviewed-by: Lars Volker Tested-by: Impala Public Jenkins --- M be/src/common/CMakeLists.txt A be/src/common/thread-debug-info-test.cc A be/src/common/thread-debug-info.cc A be/src/common/thread-debug-info.h M be/src/exec/blocking-join-node.cc M be/src/exec/hdfs-scan-node.cc M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-state.cc M be/src/util/thread.cc 9 files changed, 241 insertions(+), 2 deletions(-) Approvals: Lars Volker: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 15 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. Patch Set 14: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 15:32:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3703: Store query context in thread-local variables
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8621 ) Change subject: IMPALA-3703: Store query context in thread-local variables .. Patch Set 15: Thank everyone for the comments! -- To view, visit http://gerrit.cloudera.org:8080/8621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609 Gerrit-Change-Number: 8621 Gerrit-PatchSet: 15 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 15:34:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior
Hello Taras Bobrovytsky, Tim Armstrong, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8801 to look at the new patch set (#8). Change subject: IMPALA-5191: Standardize column alias behavior .. IMPALA-5191: Standardize column alias behavior We should not perform alias substitution in the subexpressions of GROUP BY, HAVING, and ORDER BY to be more standard conformant. === Allowed === SELECT int_col / 2 AS x FROM functional.alltypes GROUP BY x; SELECT int_col / 2 AS x FROM functional.alltypes ORDER BY x; SELECT NOT bool_col AS nb FROM functional.alltypes GROUP BY nb HAVING nb; === Not allowed === SELECT int_col / 2 AS x FROM functional.alltypes GROUP BY x / 2; SELECT int_col / 2 AS x FROM functional.alltypes ORDER BY -x; SELECT int_col / 2 AS x FROM functional.alltypes GROUP BY x HAVING x > 3; Some extra checks were added to AnalyzeExprsTest.java. I had to update other tests to make them pass since the new behavior is more restrictive. I added alias.test to the end-to-end tests. Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8 --- M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test M testdata/workloads/functional-planner/queries/PlannerTest/predicate-propagation.test A testdata/workloads/functional-query/queries/QueryTest/alias.test M tests/query_test/test_queries.py 8 files changed, 203 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/8801/8 -- To view, visit http://gerrit.cloudera.org:8080/8801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8 Gerrit-Change-Number: 8801 Gerrit-PatchSet: 8 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-5191: Standardize column alias behavior
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8801 ) Change subject: IMPALA-5191: Standardize column alias behavior .. Patch Set 7: (6 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java: http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@282 PS7, Line 282:* Substitute exprs of the form "" with the corresponding > Update this comment to describe our new alias/ordinal substitution behavior Updated the comment. http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java: http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@548 PS7, Line 548: havingClause_; > To simplify and be more internally consistent I think we should use substit substituteOrdinalsAliases() takes a list of Exprs as param, while we only have a single expr here. I introduced the substituteOrdinalAlias() method, which takes a single expr, and now substituteOrdinalsAliases() calls this method in a loop. I can confirm the bug, and added a new check to TestAliasesAndOrdinals(). http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@985 PS7, Line 985: // IMPALA-5191: In GROUP BY, HAVING, ORDER BY, aliases and ordinals must only be > Now that aliases and ordinals are treated consistently, can you combine the I found it cumbersome to combine the tests, because some of them pass, some of them fails with different error messages. But added more tests by hand. ORDER BY ordinal works a bit funny currently. If the expression is an integer and can be interpreted as an ordinal, then the corresponding column is used. If the expression is a floating point number, then this float value is used in the order by, which basically means no sorting. E.g.: select int_col / 2 as x, max(int_col) from functional.alltypes group by 1 order by 1 * 1; // order by first col select int_col / 2 as x, max(int_col) from functional.alltypes group by 1 order by 1 * 3; // error, no 3rd col select int_col / 2 as x, max(int_col) from functional.alltypes group by 1 order by 1 * 3.0; // order by 3.0 => no sorting (PostgreSQL allows writing constant arithmetic expressions in the ORDER BY clause, and an expression like 'ORDER BY 1 * 2' has no effect on the sorting.) The bug hides in AnalysisContext.analyze(): In the first round of analysis ORDER BY gets the ArithmeticExpr '1 * 1'. Then the statement rewriter rewrites it to 1. After that the whole select statement is re-analyzed, so ORDER BY gets NumericLiteral 1 in the second round, which will be interpreted as an ordinal. GROUP BY and HAVING generate error for 1 * 1, but accept 1. It's because GROUP BY and HAVING throw exception during the first round of the analysis. What solution do you suggest for this particular bug? Should I track if the statement is re-analyzed and not substitute ordinals? Should I not rewrite the expressions of ORDER BY? http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@996 PS7, Line 996: + "group by nb having nb"); > you can leave out group by Done http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@1007 PS7, Line 1007: // select count(*) a ... GROUP BY, ORDER BY, HAVING > Comment doesn't add much since we can see that from the test query test. A Done http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@1019 PS7, Line 1019: // select sum(id) over(order by id) a ... GROUP BY, ORDER BY, HAVING > Alias referring to analytic output in GROUP BY ... Done -- To view, visit http://gerrit.cloudera.org:8080/8801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8 Gerrit-Change-Number: 8801 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 16:19:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila Jeges, Tim Armstrong, Csaba Ringhofer, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8490 to look at the new patch set (#16). Change subject: IMPALA-2248: Make idle_session_timeout a query option .. IMPALA-2248: Make idle_session_timeout a query option This commit makes idle_session_timeout a query option. idle_session_timeout currently can be set as a command line option, which will be the default timeout for sessions. HS2 sessions can override it with a smaller value by setting it in the configuration overlay of HS2 OpenSession(). However, we can't override idle_session_timeout for JDBC/ODBC connections, because we cannot put this in the connection string. This commit is a workaround for this problem, it allows JDBC/ODBC connections to set the session timeout as a query option with the SET statement. After this commit, the session timeout can be overridden to any value, i.e. the command line flag idle_session_timeout doesn't limit this option anymore. I created an automated test case in JdbcTest.java based on test_hs2.py::test_concurrent_session_mixed_idle_timeout. I also extended the test_session_expiration and test_set_and_unset test suites. Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/util/Metrics.java M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_set_and_unset.py M tests/hs2/hs2_test_suite.py 14 files changed, 397 insertions(+), 53 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/16 -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 16 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8490 ) Change subject: IMPALA-2248: Make idle_session_timeout a query option .. Patch Set 15: After an offline discussion with Greg and Michael, the following conclusion was reached: FLAGS_idle_session_timeout should not limit the value of query option idle_session_timeout. From now on, any timeout value is allowed to be set by this query option. I updated my commit with this in mind. -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 15 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 18 Dec 2017 17:31:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 18 Dec 2017 18:37:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp In the commit 4feb4f3a, the third party library pcg-cpp was excluded from the clang-tidy check. It could make unexpected side effect, so fixing some warnings from clang-tidy is better than avoidance of the check. Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Reviewed-on: http://gerrit.cloudera.org:8080/8829 Reviewed-by: Jim Apple Tested-by: Impala Public Jenkins --- M be/src/thirdparty/pcg-cpp-0.98/include/pcg_extras.hpp M bin/run_clang_tidy.sh 2 files changed, 13 insertions(+), 2 deletions(-) Approvals: Jim Apple: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-6295: Fix mix/max handling of 'nan' and 'inf'
Thomas Tauber-Marshall has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/8854 ) Change subject: IMPALA-6295: Fix mix/max handling of 'nan' and 'inf' .. IMPALA-6295: Fix mix/max handling of 'nan' and 'inf' This patch fixes several issues related to the min/max aggregate functions and their handling of 'nan' and 'inf': - Previously, if 'inf' or '-inf' was the only value for the min/max and codegen was being used, the result would be incorrect. This occurred, for example in the case of 'inf' and 'min', because we set an initial value of numeric_limits::max, which is less than 'inf', so the returned min was numeric_limits::max when it should be 'inf'. The fix is to set the initial value to numeric_limits::infinity. - Previously, if one of the values was 'nan', the result of min/max was non-deterministic depending on the order the values were evaluated in. This occurs because 'nan' < or > 'any value' is always false, so if the first value added was 'nan', all other comparisons would be false and 'nan' would be returned, whereas if the first value wasn't 'nan' then the 'nan' wouldn't be returned. The fix is to treat 'nan' specially and to always return 'nan' if there is a single 'nan' value. Testing: - Added e2e tests for both scenarios. Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81 --- M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/partitioned-aggregation-node.cc M be/src/exprs/aggregate-functions-ir.cc M be/src/exprs/expr-value.h M testdata/workloads/functional-query/queries/QueryTest/aggregation.test 6 files changed, 76 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/8854/2 -- To view, visit http://gerrit.cloudera.org:8080/8854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81 Gerrit-Change-Number: 8854 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-2181: [DOCS] Document changes to SET output
John Russell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8865 Change subject: IMPALA-2181: [DOCS] Document changes to SET output .. IMPALA-2181: [DOCS] Document changes to SET output Change-Id: Iade7cb326715ebbb8518230d518d05601d615f61 --- M docs/topics/impala_set.xml 1 file changed, 73 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/8865/1 -- To view, visit http://gerrit.cloudera.org:8080/8865 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iade7cb326715ebbb8518230d518d05601d615f61 Gerrit-Change-Number: 8865 Gerrit-PatchSet: 1 Gerrit-Owner: John Russell
[Impala-ASF-CR] IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8829 ) Change subject: IMPALA-5754: Rollback the exclusion of clang-tidy check for pcg-cpp .. Patch Set 5: Jim, thanks for your review! -- To view, visit http://gerrit.cloudera.org:8080/8829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365 Gerrit-Change-Number: 8829 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Tue, 19 Dec 2017 00:03:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6177: Cleanup incomplete handcrafted IRs before finalizing module
Hello Michael Ho, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8541 to look at the new patch set (#11). Change subject: IMPALA-6177: Cleanup incomplete handcrafted IRs before finalizing module .. IMPALA-6177: Cleanup incomplete handcrafted IRs before finalizing module Currently, if an error is encountered during the creation of a handcrafted codegen method, then the resulting IR is left in an incomplete state. This patch ensures that all such IRs are cleaned up (method is deleted from the module) before the llvm module is finalized. Testing: - added a backend test to exercise the added code path. - tested manually by executing the following query: select * from charTable A, charTable B where A.charColumn = B.charColumn and A.charColumn = 'foo'; and looking at the logs to verify that 'InsertRuntimeFilters' and 'FilterContextInsert' methods have been removed. Change-Id: If975cfb3906482b36dd6ede32ca81de6fcee1d7f --- M be/src/codegen/llvm-codegen-test.cc M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/hdfs-text-scanner.cc 4 files changed, 98 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/8541/11 -- To view, visit http://gerrit.cloudera.org:8080/8541 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If975cfb3906482b36dd6ede32ca81de6fcee1d7f Gerrit-Change-Number: 8541 Gerrit-PatchSet: 11 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6177: Cleanup incomplete handcrafted IRs before finalizing module
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/8541 ) Change subject: IMPALA-6177: Cleanup incomplete handcrafted IRs before finalizing module .. Patch Set 10: (7 comments) http://gerrit.cloudera.org:8080/#/c/8541/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8541/10//COMMIT_MSG@12 PS10, Line 12: (method body is truncated) > deleted from module ? Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.h File be/src/codegen/llvm-codegen.h: http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.h@374 PS10, Line 374: removed > deleted from the module Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.h@374 PS10, Line 374: their function body > the functions Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.cc@658 PS10, Line 658: DCHECK( > nit: DCHECK_EQ() Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.cc@771 PS10, Line 771: codegen_->handcrafted_functions_.push_back(fn); > Mind adding a comment on why we only add function to "handcrafted_functions Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.cc@1059 PS10, Line 1059: deleting the function's body. > Is this still true ? Done http://gerrit.cloudera.org:8080/#/c/8541/10/be/src/codegen/llvm-codegen.cc@1062 PS10, Line 1062: fn->eraseFromParent(); > I assume this should not happen very often so is it worth logging to aid de Done -- To view, visit http://gerrit.cloudera.org:8080/8541 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If975cfb3906482b36dd6ede32ca81de6fcee1d7f Gerrit-Change-Number: 8541 Gerrit-PatchSet: 10 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 19 Dec 2017 00:54:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6225: Part 2: Query profile date-time strings should have ns precision.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/8784 ) Change subject: IMPALA-6225: Part 2: Query profile date-time strings should have ns precision. .. Patch Set 6: Hi PhilZ, are you ok with the latest patch? -- To view, visit http://gerrit.cloudera.org:8080/8784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3421a34cc029ebca551730084c7cbd402d5c109 Gerrit-Change-Number: 8784 Gerrit-PatchSet: 6 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 19 Dec 2017 01:01:37 + Gerrit-HasComments: No
[Impala-ASF-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8875 to review the following change. Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping .. KUDU-2198. Allow disregarding system-wide auth-to-local mapping This adds a workaround for an issue reported on the user mailing list. Some systems are configured such that the auth_to_local mapping provided by the krb5 library doesn't work properly for service accounts. This patch adds a new configuration which allows Kudu to disregard the system auth_to_local rules and instead just map kerberos principals to their first component, which is typically the username. Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Reviewed-on: http://gerrit.cloudera.org:8080/8373 Reviewed-by: Alexey Serbin Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins --- M be/src/kudu/security/init.cc 1 file changed, 24 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/8875/1 -- To view, visit http://gerrit.cloudera.org:8080/8875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Gerrit-Change-Number: 8875 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8878 ) Change subject: KUDU-2228: Make Messenger options configurable .. Patch Set 1: (13 comments) http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/client_negotiation.h File be/src/kudu/rpc/client_negotiation.h: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/client_negotiation.h@65 PS1, Line 65: security:: Added security:: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/client_negotiation.h@224 PS1, Line 224: security:: Added security:: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/messenger.cc File be/src/kudu/rpc/messenger.cc: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/messenger.cc@a30 PS1, Line 30: : Some conflicts here. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/messenger.cc@a50 PS1, Line 50: : : Some conflicts here. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/sasl_common.cc File be/src/kudu/rpc/sasl_common.cc: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/sasl_common.cc@a28 PS1, Line 28: Conflict here. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/server_negotiation.h File be/src/kudu/rpc/server_negotiation.h: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/server_negotiation.h@62 PS1, Line 62: security:: Added security:: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/rpc/server_negotiation.h@224 PS1, Line 224: security:: Added security:: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/init.cc File be/src/kudu/security/init.cc: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/init.cc@a33 PS1, Line 33: : : : : : : Some conflicts in the header files here. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/init.cc@a45 PS1, Line 45: Some conflicts here due to missing fix of KUDU-1955, which was mostly undone by this change anyway. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/test/mini_kdc-test.cc File be/src/kudu/security/test/mini_kdc-test.cc: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/test/mini_kdc-test.cc@a21 PS1, Line 21: Conflicts here due to difference in header file names. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/security/test/mini_kdc-test.cc@20 PS1, Line 20: #include Conflicts here. Changed from to . http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/util/flags.h File be/src/kudu/util/flags.h: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/kudu/util/flags.h@a24 PS1, Line 24: Conflicts here. http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: http://gerrit.cloudera.org:8080/#/c/8878/1/be/src/rpc/authentication.cc@847 PS1, Line 847: InitKerberosForServer(principal_, keytab_file_ Change in interface here. -- To view, visit http://gerrit.cloudera.org:8080/8878 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Gerrit-Change-Number: 8878 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Tue, 19 Dec 2017 03:39:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable
Hello Dan Burkert, Kudu Jenkins, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/8878 to review the following change. Change subject: KUDU-2228: Make Messenger options configurable .. KUDU-2228: Make Messenger options configurable Currently, the RPC layer accesses many gflags directly to take certain decisions, eg. whether to turn on encryption, authentication, etc. Since the RPC layer is to be used more like a library, these should be configurable options that are passed to the Messenger (which is the API endpoint for the application using the RPC layer), instead of the RPC layer itself directly accessing these flags. This patch converts the following flags to Messenger options and moves the flag definitions to server_base.cc which is the "application" in Kudu that uses the Messenger: FLAGS_rpc_default_keepalive_time_ms FLAGS_rpc_negotiation_timeout_ms FLAGS_rpc_authentication FLAGS_rpc_encryption FLAGS_rpc_tls_ciphers FLAGS_rpc_tls_min_protocol FLAGS_rpc_certificate_file FLAGS_rpc_private_key_file FLAGS_rpc_ca_certificate_file FLAGS_rpc_private_key_password_cmd FLAGS_keytab_file Most of the remaining flags are test or benchmark related flags. There may be a few more flags that can be moved out and converted to options, but we can leave that as future work if we decide to move them. Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Reviewed-on: http://gerrit.cloudera.org:8080/8789 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M be/src/kudu/rpc/client_negotiation.cc M be/src/kudu/rpc/client_negotiation.h M be/src/kudu/rpc/messenger.cc M be/src/kudu/rpc/messenger.h M be/src/kudu/rpc/negotiation.cc M be/src/kudu/rpc/negotiation.h M be/src/kudu/rpc/reactor.cc M be/src/kudu/rpc/rpc-test-base.h M be/src/kudu/rpc/rpc-test.cc M be/src/kudu/rpc/sasl_common.cc M be/src/kudu/rpc/sasl_common.h M be/src/kudu/rpc/server_negotiation.cc M be/src/kudu/rpc/server_negotiation.h M be/src/kudu/security/CMakeLists.txt M be/src/kudu/security/init.cc M be/src/kudu/security/init.h A be/src/kudu/security/security_flags.cc A be/src/kudu/security/security_flags.h M be/src/kudu/security/test/mini_kdc-test.cc M be/src/kudu/security/tls_context.cc M be/src/kudu/security/tls_context.h M be/src/kudu/util/flags.cc M be/src/kudu/util/flags.h M be/src/rpc/authentication.cc 24 files changed, 381 insertions(+), 283 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/8878/1 -- To view, visit http://gerrit.cloudera.org:8080/8878 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Gerrit-Change-Number: 8878 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[Impala-ASF-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping
Michael Ho has removed Dan Burkert from this change. ( http://gerrit.cloudera.org:8080/8875 ) Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping .. Removed reviewer Dan Burkert. -- To view, visit http://gerrit.cloudera.org:8080/8875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Gerrit-Change-Number: 8875 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8875 ) Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping .. Patch Set 1: (1 comment) Mostly a clean cherry-pick albeit hiding the newly added flags for now. http://gerrit.cloudera.org:8080/#/c/8875/1/be/src/kudu/security/init.cc File be/src/kudu/security/init.cc: http://gerrit.cloudera.org:8080/#/c/8875/1/be/src/kudu/security/init.cc@54 PS1, Line 54: DEFINE_bool_hidden Switched to using DEFINE_bool_hidden here. -- To view, visit http://gerrit.cloudera.org:8080/8875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Gerrit-Change-Number: 8875 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 19 Dec 2017 03:42:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping
Michael Ho has removed Alexey Serbin from this change. ( http://gerrit.cloudera.org:8080/8875 ) Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping .. Removed reviewer Alexey Serbin. -- To view, visit http://gerrit.cloudera.org:8080/8875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Gerrit-Change-Number: 8875 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] KUDU-2198. Allow disregarding system-wide auth-to-local mapping
Michael Ho has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8875 ) Change subject: KUDU-2198. Allow disregarding system-wide auth-to-local mapping .. Removed reviewer Kudu Jenkins. -- To view, visit http://gerrit.cloudera.org:8080/8875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486 Gerrit-Change-Number: 8875 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable
Michael Ho has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8878 ) Change subject: KUDU-2228: Make Messenger options configurable .. Removed reviewer Kudu Jenkins. -- To view, visit http://gerrit.cloudera.org:8080/8878 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Gerrit-Change-Number: 8878 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8878 ) Change subject: KUDU-2228: Make Messenger options configurable .. Patch Set 1: Some number of conflicts in the #include files. Conflicts have been annotated in the code review below. -- To view, visit http://gerrit.cloudera.org:8080/8878 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Gerrit-Change-Number: 8878 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Tue, 19 Dec 2017 03:44:33 + Gerrit-HasComments: No
[Impala-ASF-CR] KUDU-2228: Make Messenger options configurable
Hello Lars Volker, Dan Burkert, Kudu Jenkins, Sailesh Mukil, Joe McDonnell, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8878 to look at the new patch set (#2). Change subject: KUDU-2228: Make Messenger options configurable .. KUDU-2228: Make Messenger options configurable Currently, the RPC layer accesses many gflags directly to take certain decisions, eg. whether to turn on encryption, authentication, etc. Since the RPC layer is to be used more like a library, these should be configurable options that are passed to the Messenger (which is the API endpoint for the application using the RPC layer), instead of the RPC layer itself directly accessing these flags. This patch converts the following flags to Messenger options and moves the flag definitions to server_base.cc which is the "application" in Kudu that uses the Messenger: FLAGS_rpc_default_keepalive_time_ms FLAGS_rpc_negotiation_timeout_ms FLAGS_rpc_authentication FLAGS_rpc_encryption FLAGS_rpc_tls_ciphers FLAGS_rpc_tls_min_protocol FLAGS_rpc_certificate_file FLAGS_rpc_private_key_file FLAGS_rpc_ca_certificate_file FLAGS_rpc_private_key_password_cmd FLAGS_keytab_file Most of the remaining flags are test or benchmark related flags. There may be a few more flags that can be moved out and converted to options, but we can leave that as future work if we decide to move them. Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Reviewed-on: http://gerrit.cloudera.org:8080/8789 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert --- M be/src/kudu/rpc/client_negotiation.cc M be/src/kudu/rpc/client_negotiation.h M be/src/kudu/rpc/messenger.cc M be/src/kudu/rpc/messenger.h M be/src/kudu/rpc/negotiation.cc M be/src/kudu/rpc/negotiation.h M be/src/kudu/rpc/reactor.cc M be/src/kudu/rpc/rpc-test-base.h M be/src/kudu/rpc/rpc-test.cc M be/src/kudu/rpc/sasl_common.cc M be/src/kudu/rpc/sasl_common.h M be/src/kudu/rpc/server_negotiation.cc M be/src/kudu/rpc/server_negotiation.h M be/src/kudu/security/CMakeLists.txt M be/src/kudu/security/init.cc M be/src/kudu/security/init.h A be/src/kudu/security/security_flags.cc A be/src/kudu/security/security_flags.h M be/src/kudu/security/test/mini_kdc-test.cc M be/src/kudu/security/tls_context.cc M be/src/kudu/security/tls_context.h M be/src/kudu/util/flags.cc M be/src/kudu/util/flags.h M be/src/rpc/authentication.cc 24 files changed, 381 insertions(+), 282 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/8878/2 -- To view, visit http://gerrit.cloudera.org:8080/8878 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba Gerrit-Change-Number: 8878 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-6225: Part 2: Query profile date-time strings should have ns precision.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8784 ) Change subject: IMPALA-6225: Part 2: Query profile date-time strings should have ns precision. .. Patch Set 6: Code-Review+1 Thanks! -- To view, visit http://gerrit.cloudera.org:8080/8784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3421a34cc029ebca551730084c7cbd402d5c109 Gerrit-Change-Number: 8784 Gerrit-PatchSet: 6 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 19 Dec 2017 05:27:02 + Gerrit-HasComments: No
