[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: IMPALA-13051: Speed up, refactor query log tests .. Patch Set 6: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py File tests/util/workload_management.py: http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py@26 PS6, Line 26: from SystemTables.ttypes import TQueryTableColumn > The last addition (line 573) is there to ensure all columns are tested. ack -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 06:52:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13057: Incorporate tuple/slot information into tuple cache key
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21398 to look at the new patch set (#4). Change subject: IMPALA-13057: Incorporate tuple/slot information into tuple cache key .. IMPALA-13057: Incorporate tuple/slot information into tuple cache key The tuple cache keys currently do not include information about the tuples or slots, as that information is stored outside the PlanNode thrift structures. The tuple/slot information is critical to determining which columns are referenced and what data layout the result tuple has. This adds code to incorporate the TupleDescriptors and SlotDescriptors into the cache key. Since the tuple and slot ids are indexes into a global structure (the descriptor table), they hinder cache key matches across different queries. If a query has an extra filter, it can shift all the slot ids. If the query has an extra join, it can shift all the tuple ids. To eliminate this effect, this adds the ability to translate tuple and slot ids from global indices to local indices. The translation only contains information from the subtree below that point, so it is not influenced by unrelated parts of the query. When the code registers a tuple with the TupleCacheInfo, it also registers a translation from the global index to a local index. Any code that puts SlotIds or TupleIds into a Thrift data structure can use the translateTupleId() and translateSlotId() functions to get the local index. These are exposed on ThriftSerializationCtx by functions of the same name, but those functions apply the translation only when working for the tuple cache. This passes the ThriftSerializationCtx into Exprs that have TupleIds or SlotIds and applies the translation. It also passes the ThriftSerializationCtx into PlanNode::toThrift(), which is used to translate TupleIds in HdfsScanNode. This also adds a way to register a table with the tuple cache and incorporate information about it. This allows us to mask out additional fields in PlanNode and enable a test case that relies on matching with different table aliases. Testing: - This fixes some commented out test cases in TupleCacheTest (specifically telling columns apart) - This adds new test cases that match due to id translation (extra filters, extra joins) - This adds a unit test for the id translation to TupleCacheInfoTest Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c --- M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java M fe/src/main/java/org/apache/impala/common/ThriftSerializationCtx.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/TupleCacheInfo.java M fe/src/main/java/org/apache/impala/planner/TupleCachePlanner.java M fe/src/test/java/org/apache/impala/planner/TupleCacheInfoTest.java M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test 17 files changed, 442 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/21398/4 -- To view, visit http://gerrit.cloudera.org:8080/21398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c Gerrit-Change-Number: 21398 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 23: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16108/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 23 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 06:25:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 23: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 23 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 06:23:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 23: (1 comment) > Uploaded patch set 23. http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py File tests/query_test/test_insert.py: http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py@310 PS22, Line 310: \ > We need double back slashes to fix this, i.e. "\\" Done -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 23 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 06:00:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#23). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the "\u00FF" literal. Inclusion of "\u00FF" was likely a mistake from the beginning and it should have been '\x7F'. The patch makes three key changes: 1. Before the change, the set of characters that need to be escaped was stored as a string. The current patch uses an unordered_set instead. 2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. 3. The list of characters to be escaped is extended to match the current list in Hive. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test, coding-util-test.cc and test_insert.py. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test M tests/query_test/test_insert.py 5 files changed, 113 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/23 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 23 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 22: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16107/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 22 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 05:50:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 22:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py@310
PS22, Line 310: \
> flake8: W605 invalid escape sequence '\{'
We need double back slashes to fix this, i.e. "\\"
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 22
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 05:33:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 22: (1 comment) > Uploaded patch set 22. http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py File tests/query_test/test_insert.py: http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@299 PS20, Line 299: tb > Not done yet. In PS21, it's still using 4 spaces. Done -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 22 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 05:25:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#22). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the "\u00FF" literal. Inclusion of "\u00FF" was likely a mistake from the beginning and it should have been '\x7F'. The patch makes three key changes: 1. Before the change, the set of characters that need to be escaped was stored as a string. The current patch uses an unordered_set instead. 2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. 3. The list of characters to be escaped is extended to match the current list in Hive. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test, coding-util-test.cc and test_insert.py. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test M tests/query_test/test_insert.py 5 files changed, 113 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/22 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 22 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 22:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/22/tests/query_test/test_insert.py@310
PS22, Line 310: \
flake8: W605 invalid escape sequence '\{'
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 22
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 05:25:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 21: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16106/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 21 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 05:03:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 21: (1 comment) http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py File tests/query_test/test_insert.py: http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@299 PS20, Line 299: > Done Not done yet. In PS21, it's still using 4 spaces. -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 21 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 07 May 2024 04:49:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 21:
(5 comments)
> Patch Set 21:
>
> (11 comments)
>
> > Uploaded patch set 21.
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@96
PS20, Line 96: string test =
"SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r\x0E\x0F\x10"
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@301
PS21, Line 301: )
> flake8: E501 line too long (91 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@310
PS21, Line 310: \
> flake8: W605 invalid escape sequence '\{'
Done
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@311
PS21, Line 311: \
> flake8: E501 line too long (91 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@320
PS21, Line 320: e
> flake8: E501 line too long (94 > 90 characters)
Done
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 04:39:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 21:
(11 comments)
> Uploaded patch set 21.
http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc@97
PS19, Line 97:
"\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\x7F\"#%'"
> Why is the single quote escaped?
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@98
PS20, Line 98: "*/:=?\\{[]^";
> Please add '^'
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@104
PS20, Line 104: }
> nit: remove blank line
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@297
PS20, Line 297: def test_escaped_partition_values(self, unique_database):
> nit: move this inside the method, i.e.
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@299
PS20, Line 299:
> nit: use 2 spaces indention
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@300
PS20, Line 300:
> wrap this to be
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@301
PS20, Line 301: )
> flake8: E501 line too long (104 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@302
PS20, Line 302:
> Single quote is missing here. The two single quotes in the middle doesn't r
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@303
PS20, Line 303:
> wrap this to be
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@305
PS20, Line 305: special_characters =
"SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
> This just verifies the partition value. We still need to verify the partiti
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@306
PS20, Line 306:
"\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
> Can you add a comment about why splitting this out was necessary?
"" is used to represent a backslash as it gets escaped again during parsing
of the insert statement. It is no more needed as we introduce a part_value
variable to store the correct string.
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 04:38:56 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 21:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@301
PS21, Line 301: )
flake8: E501 line too long (91 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@310
PS21, Line 310: \
flake8: W605 invalid escape sequence '\{'
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@311
PS21, Line 311: \
flake8: E501 line too long (91 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/21131/21/tests/query_test/test_insert.py@320
PS21, Line 320: e
flake8: E501 line too long (94 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 21
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 04:38:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#21). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the "\u00FF" literal. Inclusion of "\u00FF" was likely a mistake from the beginning and it should have been '\x7F'. The patch makes three key changes: 1. Before the change, the set of characters that need to be escaped was stored as a string. The current patch uses an unordered_set instead. 2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. 3. The list of characters to be escaped is extended to match the current list in Hive. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test, coding-util-test.cc and test_insert.py. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test M tests/query_test/test_insert.py 5 files changed, 113 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/21 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 21 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/21357 )
Change subject: IMPALA-12935: First pass on Calcite planner functions
..
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java:
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java@61
PS7, Line 61: abstract public class FunctionSignature {
Impala has logic for this type of matching as part of its Function class. Is
there a way we could use that logic? Why or why not?
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java@210
PS7, Line 210: It is possible for signatures to be equal when they have
: // a different number of arguments. For this reason, we will
return the same
: // hashcode based only on the first argument, if it exists.
This will result
: // in a couple extra collisions, but this cost should be small.
Ok, I get this now. This is about vararg functions. It's ok to hash the first
arg, because every vararg function has at least one non-vararg argument.
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java:
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java@50
PS7, Line 50: public class ImpalaOperatorTable extends
ReflectiveSqlOperatorTable {
If I'm understanding the Calcite code, this is primarily for sets of functions
that rarely change. How do UDFs fit into that? How does the session's current
database interact with function lookups?
Have you considered reusing the existing Impala structures for looking up
functions? What does using the Calcite structure get us?
My mental model here is that function resolution should basically produce the
same results for Calcite vs regular Impala. Is that accurate or inaccurate?
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@125
PS7, Line 125: TPrimitiveType primitiveType =
impalaType.getPrimitiveType().toThrift();
: Type normalizedImpalaType = getImpalaType(primitiveType);
Nit: It may not be necessary to convert to TPrimitiveType. We have locations in
code that do a switch directly on PrimitiveType.
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@200
PS7, Line 200: -1
Nit: For these -1 precision values, can you use
RelDataType::PRECISION_NOT_SPECIFIED instead?
http://gerrit.cloudera.org:8080/#/c/21357/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@320
PS7, Line 320: public static RelDataType createRelDataType(Type impalaType) {
What is the distinction between getRelDataType(Type impalaType) and
createRelDataType(Type impalaType)?
It looks like one is normalized and the other isn't for decimal? Let's try to
express that in the names so we can tell them apart.
--
To view, visit http://gerrit.cloudera.org:8080/21357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88
Gerrit-Change-Number: 21357
Gerrit-PatchSet: 7
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Steve Carlin
Gerrit-Comment-Date: Tue, 07 May 2024 04:12:39 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. IMPALA-13054: Avoid revisiting children in QueryStateExpanded Avoids revisiting children in QueryStateExpanded due to duplicate recursion. Since process_exec_profile visits children recursively, we only want immediate children from GetChildren, not all children from GetAllChildren. Adjusts HDFS_SCAN_NODE test to look back at top of stack, rather than depend on a specific depth that was caused by calling GetAllChildren from the root. This was identified by running the Impala E2E test suite with workload management enabled, where test_nested_types.py::TestMaxNestingDepth::test_max_nesting_depth took multiple hours to run and blocked the FinishUnregisterQuery for other tests. Testing: - Manual test with enable_workload_mgmt=true running test_max_nesting_depth. Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Reviewed-on: http://gerrit.cloudera.org:8080/21392 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/query-state-record.cc 1 file changed, 23 insertions(+), 11 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 04:07:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc File be/src/rpc/thrift-util.cc: http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61 PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024, Yeah, it'd be perfect if we can split up them. We already have a pretty large limit. A query sent from the client shouldn't need that large. > Is 8GB reasonable? It's hard to estimate the ratio since it depends on many factors, e.g. number of partitions. However, it's reported in IMPALA-12350 that 800MB could grow to 3.4GB after upgrade. So 4x larger is possible in reality and 4GB x 4 = 16GB might be safer. -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 07 May 2024 04:02:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 20:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@98
PS20, Line 98: "*/:=?\\{[]";
Please add '^'
http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@104
PS20, Line 104:
nit: remove blank line
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@297
PS20, Line 297: """Test for special characters in partition values."""
nit: move this inside the method, i.e.
def test_escaped_partition_values(self, unique_database):
"""Test for special characters in partition values."""
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@299
PS20, Line 299:
nit: use 2 spaces indention
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@300
PS20, Line 300: e
> flake8: E501 line too long (106 > 90 characters)
wrap this to be
self.execute_query(
"create table {}(i int) partitioned by (p string) stored as
parquet".format(tbl))
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@302
PS20, Line 302: 7
> flake8: E501 line too long (108 > 90 characters)
Single quote is missing here. The two single quotes in the middle doesn't
represent a single quote. They become the end and start of two strings. We can
double quote the string and wrap it as
special_characters_ =
"SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
"\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
"\x1C\x1D\x1E\x1F\"\x7F'%*/:=?\\{[]#^"
Also add '^' and remove "()"
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@303
PS20, Line 303: _
> flake8: E501 line too long (104 > 90 characters)
wrap this to be
self.execute_query(
"insert into {} partition(p='{}') values (0)".format(tbl,
special_characters_))
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@305
PS20, Line 305: assert a.data[0] == special_characters_
This just verifies the partition value. We still need to verify the partition
dir created by Impala is correct. This seems better:
def test_escaped_partition_values(self, unique_database):
"""Test for special characters in partition values."""
tbl = unique_database + ".tbl"
self.execute_query(
"create table {}(i int) partitioned by (p string) stored as
parquet".format(tbl))
# Use "\\'" for single quote since the insert statement use single quote on
the
# partition value. Use "" for back slash since it will be escaped again
in
# parsing the insert statement
special_characters =
"SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
"\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
"\x1C\x1D\x1E\x1F\"\x7F\\'%*/:=?{[]#^"
part_value = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
"\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
"\x1C\x1D\x1E\x1F\"\x7F'%*/:=?\{[]#^"
part_dir =
"p=SpecialCharacters%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%10%11" \
"%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%22%7F%27%25%2A%2F%3A%3D%3F" \
"%5C%7B%5B%5D%23%5E"
res = self.execute_query(
"insert into {} partition(p='{}') values (0)".format(tbl,
special_characters))
assert res.data[0] == part_dir + ": 1"
res = self.client.execute("select p from {}".format(tbl))
assert res.data[0] == part_value
res = self.execute_query("show partitions " + tbl)
# There are "\t" in the partition value so we can't easily split the result
line.
# Just verify the values are inside it.
assert part_value in res.data[0]
assert part_dir in res.data[0]
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Tue, 07 May 2024 03:26:22 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12712: Invalidate metadata on table should set better createEventId
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21402 ) Change subject: IMPALA-12712: Invalidate metadata on table should set better createEventId .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16105/ : 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/21402 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iff6ac18fe8d9e7b25cc41c7e41eecde251fbccdd Gerrit-Change-Number: 21402 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 07 May 2024 02:58:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12712: Invalidate metadata on table should set better createEventId
Sai Hemanth Gantasala has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21402 Change subject: IMPALA-12712: Invalidate metadata on table should set better createEventId .. IMPALA-12712: Invalidate metadata on table should set better createEventId "INVALIDATE METADATA " can be used to bring up a table in Impala's catalog cache if the table exists in HMS. Currently, createEventId for such tables are always set as -1 which will lead to always removing the table. Sequence of drop table + create table + invalidate table can lead to flaky test failures like IMPALA-12266. Solution: When Invalidate metadata is fired, fetch the latest eventId from HMS and set it as createEventId for the table, so that drop table event happend before invalidate query will be ignored without removing the table from cache. Testing: - Added an end-to-end test to verify that drop table event happened before time shouldn't remove the metadata object from cache. Change-Id: Iff6ac18fe8d9e7b25cc41c7e41eecde251fbccdd --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_events_custom_configs.py 4 files changed, 34 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/21402/1 -- To view, visit http://gerrit.cloudera.org:8080/21402 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iff6ac18fe8d9e7b25cc41c7e41eecde251fbccdd Gerrit-Change-Number: 21402 Gerrit-PatchSet: 1 Gerrit-Owner: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 07 May 2024 01:56:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/21326 )
Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for
some dropped partitions
..
IMPALA-13009: Fix catalogd not sending deletion updates for some dropped
partitions
*Background*
Since IMPALA-3127, catalogd sends incremental partition updates based on
the last sent table snapshot ('maxSentPartitionId_' to be specific).
Dropped partitions since the last catalog update are tracked in
'droppedPartitions_' of HdfsTable. When catalogd collects the next
catalog update, they will be collected. HdfsTable then clears the set.
See details in CatalogServiceCatalog#addHdfsPartitionsToCatalogDelta().
If an HdfsTable is invalidated, it's replaced with an IncompleteTable
which doesn't track any partitions. The HdfsTable object is then added
to the deleteLog so catalogd can send deletion updates for all its
partitions. The same if the HdfsTable is dropped. However, the
previously dropped partitions are not collected in this case, which
results in a leak in the catalog topic if the partition name is not
reused anymore. Note that in the catalog topic, the key of a partition
update consists of the table name and the partition name. So if the
partition is added back to the table, the topic key will be reused then
resolves the leak.
The leak will be observed when a coordinator restarts. In the initial
catalog update sent from statestore, coordinator will find some
partition updates that are not referenced by the HdfsTable (assuming the
table is used again after the INVALIDATE). Then a Precondition check
fails and the table is not added to the coordinator.
*Overview of the patch*
This patch fixes the leak by also collecting the dropped partitions when
adding the HdfsTable to the deleteLog. A new field, dropped_partitions,
is added in THdfsTable to collect them. It's only used when catalogd
collects catalog updates.
Removes the Precondition check in coordinator and just reports the stale
partitions since IMPALA-12831 could also introduce them.
Also adds a log line in CatalogOpExecutor.alterTableDropPartition() to
show the dropped partition names for better diagnostics.
Tests
- Added e2e tests
Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21
Reviewed-on: http://gerrit.cloudera.org:8080/21326
Reviewed-by: Impala Public Jenkins
Tested-by: Impala Public Jenkins
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M tests/common/impala_test_suite.py
M tests/custom_cluster/test_partition.py
M tests/metadata/test_recover_partitions.py
8 files changed, 262 insertions(+), 54 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21326
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21
Gerrit-Change-Number: 21326
Gerrit-PatchSet: 9
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/21401 ) Change subject: IMPALA-13061: Set transactional=false on query live table .. Patch Set 1: (1 comment) Looks like a simple safe change http://gerrit.cloudera.org:8080/#/c/21401/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21401/1//COMMIT_MSG@10 PS1, Line 10: avoid creating a table that Impala can't read. Transactional tables are Is the point here is that if default_transactional_type=insert_only then the query live table can't be read? If so then that could be clearer here. -- To view, visit http://gerrit.cloudera.org:8080/21401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65 Gerrit-Change-Number: 21401 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 07 May 2024 00:32:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13038: Support profile tab for imported query profiles
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21400 ) Change subject: IMPALA-13038: Support profile tab for imported query profiles .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21400/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21400/1//COMMIT_MSG@14 PS1, Line 14: With the current patch "Query Profile" tab will also be supported. > Tried to run test with your patch on my local machine. But did not see the I messed up my local environment. It did show the "Query Profile" tab after clean build. Tried to switch tabs for an imported query. When switching from active tab "Query Profile" to other three tabs, it shows nothing with query-id as :. -- To view, visit http://gerrit.cloudera.org:8080/21400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc Gerrit-Change-Number: 21400 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 07 May 2024 00:30:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13038: Support profile tab for imported query profiles
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/21400 ) Change subject: IMPALA-13038: Support profile tab for imported query profiles .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/21400/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21400/1//COMMIT_MSG@14 PS1, Line 14: With the current patch "Query Profile" tab will also be supported. Tried to run test with your patch on my local machine. But did not see the new tab "Query Profile", still saw three tabs for imported queries. http://gerrit.cloudera.org:8080/#/c/21400/1//COMMIT_MSG@21 PS1, Line 21: converts nit: wrap around long line. -- To view, visit http://gerrit.cloudera.org:8080/21400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc Gerrit-Change-Number: 21400 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 07 May 2024 00:02:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/21194 )
Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..
Patch Set 9:
(2 comments)
So, I went down a rabbit hole trying to understand this. Basically, there are
two options. Option 1 is finding some way to pull in Calcite's
default_config.fmpp to our build and config.fmpp contains only the modified
values. Option 2 is to specify every value from default_config.fmpp in
config.fmpp and skip pulling in default_config.fmpp. This does option 2.
The way default_config.fmpp comes in is that Parser.jj has many references to
default.parser.X as the value to use if parser.X doesn't exist. i.e.
<#list (parser.imports!default.parser.imports) as importStr>
import ${importStr};
Calcite has a custom gradle plugin that incorporates default_config.fmpp into
the fmpp compilation. Basically, it is adding a link to a tdd file from the
data section as "default". So, it is equivalent to something like this:
data {
parser {
...
}
default: tdd("../default_config.fmpp")
}
Dremio does something similar with a Maven plugin. Everything in
default_config.fmpp is in the parser section, so it's values end up available
as default.parser.X.
In Option 2, parser.X always exists, so it doesn't matter that we never hooked
up default_config.fmpp to have the default.parser.X value available. I can live
with that, but see my comments about moving things to src/main/codegen and
using the Impala package.
All that said, I think Option 1 is easier to understand and I think it is
pretty easy. Basically, we pull in default_config.fmpp from Calcite, then add
that "default: tdd("../default_config.fmpp")" line to our config.fmpp after the
close of the parser section (the path has a .. because I guess it is evaluated
from the templates/ directory). After that, our config.fmpp only needs to
contain the things we modify. default_config.fmpp stays identical the Calcite
file. (Dremio does a slick thing where they pull default_config.fmpp out of the
Calcite jar. We could do that at some point, but I don't feel compelled to do
that immediately.)
http://gerrit.cloudera.org:8080/#/c/21194/9/java/calcite-planner/pom.xml
File java/calcite-planner/pom.xml:
http://gerrit.cloudera.org:8080/#/c/21194/9/java/calcite-planner/pom.xml@106
PS9, Line 106:
: org.apache.maven.plugins
: maven-resources-plugin
:
:
: copy-resources
: generate-sources
:
: copy-resources
:
:
:
${project.build.directory}/codegen
:
:
:
src/main/java/org/apache/impala/calcite/parserimpl/codegen
: false
:
:
:
:
:
:
After a second look, I think I would put the Parser.jj and config.fmpp in
src/main/codegen and skip this step. This is the organization that Calcite has,
and I think it makes sense.
http://gerrit.cloudera.org:8080/#/c/21194/9/java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp:
http://gerrit.cloudera.org:8080/#/c/21194/9/java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp@21
PS9, Line 21: package: "org.apache.calcite.sql.parser.impl",
Let's make this somewhere in org.apache.impala.
--
To view, visit http://gerrit.cloudera.org:8080/21194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Steve Carlin
Gerrit-Comment-Date: Mon, 06 May 2024 23:48:56 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21401 ) Change subject: IMPALA-13061: Set transactional=false on query live table .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16104/ : 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/21401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65 Gerrit-Change-Number: 21401 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 23:42:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13061: Set transactional�lse on query live table
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21401 Change subject: IMPALA-13061: Set transactional=false on query live table .. IMPALA-13061: Set transactional=false on query live table Sets 'transactional'='false' in tblproperties for query live table to avoid creating a table that Impala can't read. Transactional tables are assumed to return valid getValidWriteIds, but SystemTables don't. Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65 --- M be/src/service/workload-management.cc M tests/custom_cluster/test_query_live.py 2 files changed, 18 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21401/1 -- To view, visit http://gerrit.cloudera.org:8080/21401 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie60a2bd03fabc63c85bcd9fa2489e9d47cd2aa65 Gerrit-Change-Number: 21401 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: IMPALA-13051: Speed up, refactor query log tests .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py File tests/util/workload_management.py: http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py@26 PS6, Line 26: from SystemTables.ttypes import TQueryTableColumn > While these changes simplify the code quite a bit (20% or so), they also re The last addition (line 573) is there to ensure all columns are tested. I agree that duplicating the column names helps reinforce they should not be changed, but feels kind of excessive. Protocol definitions usually have the ability to break things if you change existing fields. -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 23:06:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: IMPALA-13051: Speed up, refactor query log tests .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py File tests/util/workload_management.py: http://gerrit.cloudera.org:8080/#/c/21358/6/tests/util/workload_management.py@26 PS6, Line 26: from SystemTables.ttypes import TQueryTableColumn While these changes simplify the code quite a bit (20% or so), they also remove a couple of important assertions for total number of columns and column names. Asserting the total number of columns against a test constant prompts anyone adding a column to also update the python tests. The column name assertions are important because they ensure columns are not inadvertently renamed. Highly unlikely, but possible where a character could be deleted or added accidentally. Even though the code review should catch such a mistake, it is still best for automated checks to ensure those mistakes never cause distractions in code reviews. I like the removal of the index variable. We should remove that in favor of using the indexes from TQueryTableColumn. Then we could keep the column_val function and use it to simplify the code. -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 23:02:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10611/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:59:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:59:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 2: Code-Review+2 LGTM -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:54:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 2: Code-Review+1 Much cleaner! Thanks for tackling this bug. -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:44:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Jason Fehr has removed a vote on this change. Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Removed Code-Review+1 by Jason Fehr -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-13054: Avoid revisiting children in QueryStateExpanded
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21392 ) Change subject: IMPALA-13054: Avoid revisiting children in QueryStateExpanded .. Patch Set 2: Code-Review+1 Much cleaner! -- To view, visit http://gerrit.cloudera.org:8080/21392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia36d53e52095d487ffb3a162fc723f1e815bec11 Gerrit-Change-Number: 21392 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:43:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: IMPALA-13051: Speed up, refactor query log tests .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16103/ : 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/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:33:06 + Gerrit-HasComments: No
[Impala-ASF-CR] Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21398 ) Change subject: Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c Gerrit-Change-Number: 21398 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 06 May 2024 22:31:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21358 ) Change subject: IMPALA-13051: Speed up, refactor query log tests .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py File tests/common/custom_cluster_test_suite.py: http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py@161 PS5, Line 161: # IMPALA-13051: Add faster default graceful shutdown options before processing > Can you mention the Jira? It may not be evident for the reader why this is Done http://gerrit.cloudera.org:8080/#/c/21358/5/tests/common/custom_cluster_test_suite.py@236 PS5, Line 236: > I'll probably back this change out, it's weird but doesn't cause issues rig Done http://gerrit.cloudera.org:8080/#/c/21358/5/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/21358/5/tests/custom_cluster/test_query_log.py@411 PS5, Line 411: """Tests to assert that query_log_table_name works with non-default value.""" > nit: I was confused at first that this means that table table can be rename Done http://gerrit.cloudera.org:8080/#/c/21358/5/tests/util/workload_management.py File tests/util/workload_management.py: http://gerrit.cloudera.org:8080/#/c/21358/5/tests/util/workload_management.py@93 PS5, Line 93: session_id = re.search(r'\n\s+Session ID:\s+(.*)\n', profile_text) > An idea to make these even more compact: Haven't had the bandwidth to look at this more. Seems reasonable, but also triggers a significant amount of other cleanup in this file that will take me a little while. -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 06 May 2024 22:09:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13051: Speed up, refactor query log tests
Hello Quanlong Huang, Andrew Sherman, Riza Suminto, Jason Fehr, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21358 to look at the new patch set (#6). Change subject: IMPALA-13051: Speed up, refactor query log tests .. IMPALA-13051: Speed up, refactor query log tests Sets faster default shutdown_grace_period_s and shutdown_deadline_s when impalad_graceful_shutdown=True in tests. Impala waits until grace period has passed and all queries are stopped (or deadline is exceeded) before flushing the query log, so grace period of 0 is sufficient. Adds them in setup_method to reduce duplication in test declarations. Re-uses TQueryTableColumn Thrift definitions for testing. Moves waiting for query log table to exist to setup_method rather than as a side-effect of get_client. Refactors workload management code to reduce if-clause nesting. Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 --- M be/src/service/workload-management.cc M tests/common/custom_cluster_test_suite.py M tests/custom_cluster/test_query_live.py M tests/custom_cluster/test_query_log.py M tests/util/workload_management.py 5 files changed, 332 insertions(+), 494 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/21358/6 -- To view, visit http://gerrit.cloudera.org:8080/21358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855 Gerrit-Change-Number: 21358 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 20: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16102/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 20 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 06 May 2024 21:46:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 19: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16101/ : 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/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 19 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 06 May 2024 21:44:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 20: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc@97
PS19, Line 97:
"\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\x7F\"#%'"
> Why is the single quote escaped?
Done
http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@302
PS19, Line 302:
'\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\"\x7F''()%*/:=?{[]#')
> This looks like you forgot to escape the double quote. The rest are omitted
Done
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py
File tests/query_test/test_insert.py:
http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@306
PS20, Line 306: self.execute_query("insert into {} partition(p='')
values (0)".format(tbl))
Can you add a comment about why splitting this out was necessary?
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 20
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Mon, 06 May 2024 21:21:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#20). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the "\u00FF" literal. Inclusion of "\u00FF" was likely a mistake from the beginning and it should have been '\x7F'. The patch makes three key changes: 1. Before the change, the set of characters that need to be escaped was stored as a string. The current patch uses an unordered_set instead. 2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. 3. The list of characters to be escaped is extended to match the current list in Hive. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test and coding-util-test.cc. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test M tests/query_test/test_insert.py 5 files changed, 100 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/20 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 20 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#19). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the "\u00FF" literal. Inclusion of "\u00FF" was likely a mistake from the beginning and it should have been '\x7F'. The patch makes three key changes: 1. Before the change, the set of characters that need to be escaped was stored as a string. The current patch uses an unordered_set instead. 2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. 3. The list of characters to be escaped is extended to match the current list in Hive. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test and coding-util-test.cc. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test M tests/query_test/test_insert.py 5 files changed, 96 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/19 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 19 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 20: (5 comments) http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc File be/src/util/coding-util-test.cc: http://gerrit.cloudera.org:8080/#/c/21131/20/be/src/util/coding-util-test.cc@96 PS20, Line 96: string test = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r\x0E\x0F\x10\x11" line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py File tests/query_test/test_insert.py: http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@300 PS20, Line 300: e flake8: E501 line too long (106 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@301 PS20, Line 301: E flake8: E501 line too long (104 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@302 PS20, Line 302: 7 flake8: E501 line too long (108 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/20/tests/query_test/test_insert.py@303 PS20, Line 303: _ flake8: E501 line too long (104 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 20 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 06 May 2024 21:21:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 19: (7 comments) http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc File be/src/util/coding-util-test.cc: http://gerrit.cloudera.org:8080/#/c/21131/19/be/src/util/coding-util-test.cc@96 PS19, Line 96: string test = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r\x0E\x0F\x10\x11" line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py File tests/query_test/test_insert.py: http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@300 PS19, Line 300: e flake8: E501 line too long (106 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@301 PS19, Line 301: E flake8: E501 line too long (104 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@302 PS19, Line 302: " flake8: E501 line too long (107 > 90 characters) http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@302 PS19, Line 302: # flake8: E261 at least two spaces before inline comment http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@302 PS19, Line 302: # flake8: E262 inline comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/21131/19/tests/query_test/test_insert.py@303 PS19, Line 303: _ flake8: E501 line too long (104 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 19 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 06 May 2024 21:19:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21376 ) Change subject: IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@1118 PS1, Line 1118: existing_cluster_size = cluster_ops.get_cluster().get_max_impalad_id() + 1 We currently don't have a way to remember the cluster size. So if the last impalad is killed, we will see the size shrink. I think that's ok. This check is added to avoid abusing this feature since I haven't test it with all combinations of other flags. This script is only used by developers (in test codes, we already have a way to restart any process). We can add support for more complex use cases if we really need. > For example start 3, kill number 2, then restart number 0, it runs on the > assertion again. In this case, when restarting number 0, we need "-s 2" as a workaround. > Isn't it possible to use the length of cluster_ops.get_cluster().__impalads? This list consists of the running impalads. So if some were killed, the length shrinks as well. -- To view, visit http://gerrit.cloudera.org:8080/21376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4a863853341959eb6870b662d82188e7d570971 Gerrit-Change-Number: 21376 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 20:56:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 7: > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10606/ The failure is due to IMPALA-9441. Retrigger the job. -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 20:43:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10610/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 20:44:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 20:44:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc File be/src/rpc/thrift-util.cc: http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61 PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024, > Is 8GB reasonable? Could we split up those limits? -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 18:42:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13020: Change thrift rpc max message size to int64 t
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc File be/src/rpc/thrift-util.cc: http://gerrit.cloudera.org:8080/#/c/21367/4/be/src/rpc/thrift-util.cc@61 PS4, Line 61: DEFINE_int64(thrift_rpc_max_message_size, 4L * 1024 * 1024 * 1024, > Can we set it to unlimited or a larger value? We have customers using impal Is 8GB reasonable? Setting it to unlimited means there is no protection from the CVE that prompted it. That is probably ok for messages that we send within the Impala cluster, but we use the same limit for external clients. -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 18:03:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21398 ) Change subject: Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16100/ : 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/21398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c Gerrit-Change-Number: 21398 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 06 May 2024 17:32:15 + Gerrit-HasComments: No
[Impala-ASF-CR] Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21398 ) Change subject: Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10609/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c Gerrit-Change-Number: 21398 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 06 May 2024 17:28:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21357 ) Change subject: IMPALA-12935: First pass on Calcite planner functions .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16099/ : 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/21357 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88 Gerrit-Change-Number: 21357 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Mon, 06 May 2024 17:12:49 + Gerrit-HasComments: No
[Impala-ASF-CR] Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21398 to look at the new patch set (#3). Change subject: Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key .. Prototype: IMPALA-13057: Incorporate tuple/slot information to tuple cache key Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c --- M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java M fe/src/main/java/org/apache/impala/analysis/SlotRef.java M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java M fe/src/main/java/org/apache/impala/common/ThriftSerializationCtx.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/TupleCacheInfo.java M fe/src/main/java/org/apache/impala/planner/TupleCachePlanner.java M fe/src/test/java/org/apache/impala/planner/TupleCacheInfoTest.java M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java M testdata/workloads/functional-query/queries/QueryTest/explain-level1.test M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test 17 files changed, 442 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/21398/3 -- To view, visit http://gerrit.cloudera.org:8080/21398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f5278e9dbb976cbebdc6a21a6e66bc90ce06c6c Gerrit-Change-Number: 21398 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Steve Carlin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21194 )
Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21194/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21194/8//COMMIT_MSG@18
PS8, Line 18: The config.fmpp file was grabbed from Calcite 1.36
default_config.fmpp. It does
: have two very small modifications from the original Calcite fmpp
file in order
: to fix compilation issues within Impala
:
: 1) the entire json is wrapped with a "data {}" tag
: 2) the class used is ImpalaSqlParserImpl as opposed to the
Calcite SqlParserImpl
:class to prevent naming collisions with Calcite.
> Let me check my understanding:
Ok, modified the message a bit. I hope I hit the points you mentioned. If
not, I can modify again.
--
To view, visit http://gerrit.cloudera.org:8080/21194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Steve Carlin
Gerrit-Comment-Date: Mon, 06 May 2024 16:49:30 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Hello Aman Sinha, Joe McDonnell, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21357 to look at the new patch set (#7). Change subject: IMPALA-12935: First pass on Calcite planner functions .. IMPALA-12935: First pass on Calcite planner functions This commit handles the first pass on getting functions to work through the Calcite planner. Only basic functions will work with this commit. Implicit conversions for parameters are not yet supported. Custom UDFs are also not supported yet. The functions are loaded in CalciteJniFrontend from the Impala "builtin" database. A "FunctionSignature" is created for each builtin function. Each function is loaded into Calcite's "ImpalaOperatorTable" object which is used in the validation process. Each Impala function maps to a Calcite "ImpalaOperator" object. For function names that are not an exact match with a Calcite operator, an entry is found in FunctionDetailStatics to do the conversion. When the Calcite validator tries to validate the function, it gets the return type through the "ImpalaOperator.inferReturnType()" method. In this method, a "FunctionSignatureForLookup" signature is created for looking up the stored signatures. The "Lookup" signature does not have a return type (yet), so the matching of signatures will be based on matching name and parameters. As mentioned above, implicit conversion is not yet supported in this commit; this will be added later. After validation is complete, the functions will be in a Calcite format. After the rest of compilation (relnode conversion, optimization) is complete, the function needs to be converted back into Impala form (the Expr object) to eventually get it into its thrift request. In this commit, all functions are converted into Expr starting in the ImpalaProjectRel, since this is the RelNode where functions do their thing. The RexCallConverter and RexLiteralConverter get called via the CreateExprVisitor for this conversion. Since Calcite is providing the analysis portion of the planning, there is no need to go through Impala's Analyzer object. However, the Impala planner requires Expr objects to be analyzed. To get around this, the AnalyzedFunctionCallExpr and AnalyzedNullLiteral objects exist which analyze the expression in the constructor. While this could potentially be combined with the existing FunctionCallExpr and NullLiteral objects, this fits in with the general plan to avoid changing "fe" Impala code as much as we can until much later in the commit cycle. Also, there will be other Analyzed*Expr classes created in the future, but this commit is intended for basic function call expressions only. One minor change to the parser is added with this commit. Calcite parser does not have acknowledge the "string" datatype, so this has been added here in Parser.jj and config.fmpp. Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88 --- A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionDetailStatics.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignatureForLookup.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignatureForStorage.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaAggregateOperator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaHelperOperator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaScalarOperator.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp M java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/templates/Parser.jj M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java M java/calcite-planner/src/main/java/org/apache/impala/calcit
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Hello Aman Sinha, Joe McDonnell, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21194 to look at the new patch set (#9). Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. IMPALA-12934: Added Calcite parsing files to Impala Adding the framework to create our own parsing syntax for Impala using the base Calcite Parser.jj file. The Parser.jj file here was grabbed from Calcite 1.36. So with this commit, we are using the same parsing analysis as Calcite 1.36. Any changes made on top of the Parser.jj file or the config.fmpp file in the future are Impala specific changes, so a diff can be done from this commit to see all the Impala parsing changes. The config.fmpp file was grabbed from Calcite 1.36 default_config.fmpp. The Calcite intention of the config.fmpp file is to allow markup of variables in the Parser.jj file. So it is always preferable to modify the default_config.fmpp file when possible. Our version is grabbed from https://github.com/apache/calcite/blob/main/core/src/main/codegen/config.fmpp and slightly modified with the class name to make it compile for Impala. There's no unit test needed since there is no functional change. The Calcite planner will eventually make changes in the ".jj" file to support the differences between the Impala parser and the Calcite parser. Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 --- M java/calcite-planner/pom.xml A java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp A java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/templates/Parser.jj M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java 4 files changed, 9,616 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/21194/9 -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 9 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10606/ -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 15:50:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21376 ) Change subject: IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@1118 PS1, Line 1118: existing_cluster_size = cluster_ops.get_cluster().get_max_impalad_id() + 1 > Yeah, that's an issue. Removed this check when using --kill. Also improved It's still not good if you get the existing cluster size from the max impalad id: then killing the last and the first impalas will cause different behaviour in the future. For example start 3, kill number 2, then restart number 0, it runs on the assertion again. Isn't it possible to use the length of cluster_ops.get_cluster().__impalads? -- To view, visit http://gerrit.cloudera.org:8080/21376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4a863853341959eb6870b662d82188e7d570971 Gerrit-Change-Number: 21376 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 15:21:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP)
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21388 ) Change subject: IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP) .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/16098/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/21388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfbb589513aacdb68a86c1aec4a0d39b12091820 Gerrit-Change-Number: 21388 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 06 May 2024 14:38:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP)
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21388 ) Change subject: IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP) .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10607/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfbb589513aacdb68a86c1aec4a0d39b12091820 Gerrit-Change-Number: 21388 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 06 May 2024 14:19:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP)
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/21388 ) Change subject: IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP) .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/21388/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java File fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java: http://gerrit.cloudera.org:8080/#/c/21388/1/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java@183 PS1, Line 183: dataFilesWithoutDeletes_ = > line too long (96 > 90) Done http://gerrit.cloudera.org:8080/#/c/21388/1/fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java File fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java: http://gerrit.cloudera.org:8080/#/c/21388/1/fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java@123 PS1, Line 123: // Add data files with deletes to check if they are considered in the 1 file per > line too long (100 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/21388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icfbb589513aacdb68a86c1aec4a0d39b12091820 Gerrit-Change-Number: 21388 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 06 May 2024 14:18:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP)
Noemi Pap-Takacs has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21388 ) Change subject: IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP) .. IMPALA-12867: Filter files to OPTIMIZE based on file size (WIP) OPTIMIZE TABLE statement is currently used to rewrite the entire Iceberg table. With 'FILE_SIZE_THRESHOLD' option, the user can specify a file size limit to rewrite only small files. Syntax: OPTIMIZE TABLE [(FILE_SIZE_THRESHOLD_MB=100)]; The value of the threshold is the file size in MBs. Data files larger than the given limit will only be rewritten if they are referenced from delete deltas. If only 1 file is selected in a partition, it will not be rewritten. IMPALA-12839: 'Optimizing empty table should be no-op' is also resolved in this patch. Testing: - Parser test - FE unit tests - TODO: E2E tests Change-Id: Icfbb589513aacdb68a86c1aec4a0d39b12091820 --- M be/src/service/client-request-state.cc M common/thrift/CatalogService.thrift M common/thrift/Query.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java M fe/src/main/java/org/apache/impala/analysis/TableRef.java M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java M fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java A fe/src/main/java/org/apache/impala/util/IcebergFileFilter.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java A fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test M testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test 17 files changed, 482 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/21388/2 -- To view, visit http://gerrit.cloudera.org:8080/21388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icfbb589513aacdb68a86c1aec4a0d39b12091820 Gerrit-Change-Number: 21388 Gerrit-PatchSet: 2 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-13055: Some Iceberg metadata table tests don't assert
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/21394 )
Change subject: IMPALA-13055: Some Iceberg metadata table tests don't assert
..
Patch Set 1:
(5 comments)
Thanks Gábor!
http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-13055: Some Iceberg metadata table tests don't assert
Do you think it is an error in the test framework? If so, could you open a
separate Jira for it with repro instructions?
http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG@11
PS1, Line 11: unconditionally passes
We should make it clear the it is not the actual result from Impala that can be
anything but the "expected" string.
http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:
http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@49
PS1, Line 49: : VERIFY_IS_SUBSET
Is adding VERIFY_IS_SUBSET needed? I ran the test locally without it and it
passed.
http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@365
PS1, Line 365: # Currently not supported, complex type slots are set to NULL
(IMPALA-12205)
This comment is stale, we should change it.
http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@371
PS1, Line 371: '{.*}'
As this test section deals with complex types, I think we shouldn't put a
wildcard for all the result. We should include details and use regexes for
easily changing values (file size etc.).
--
To view, visit http://gerrit.cloudera.org:8080/21394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie47093f25a70253b3e6faca27d466d7cf6999fad
Gerrit-Change-Number: 21394
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Mon, 06 May 2024 14:15:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 3 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal Gerrit-Comment-Date: Mon, 06 May 2024 12:30:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. IMPALA-12977: add search and pagination to /hadoop-varz Added search and pagination feature to /hadoop-varz Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Reviewed-on: http://gerrit.cloudera.org:8080/21329 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M www/hadoop-varz.tmpl 1 file changed, 25 insertions(+), 11 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 4 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal
[Impala-ASF-CR] IMPALA-13038: Support profile tab for imported query profiles
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21400 ) Change subject: IMPALA-13038: Support profile tab for imported query profiles .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16097/ : 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/21400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc Gerrit-Change-Number: 21400 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Comment-Date: Mon, 06 May 2024 11:15:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13038: Support profile tab for imported query profiles
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/21400 ) Change subject: IMPALA-13038: Support profile tab for imported query profiles .. Patch Set 1: An example profile and its indented text have been posted on the associated JIRA. -- To view, visit http://gerrit.cloudera.org:8080/21400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc Gerrit-Change-Number: 21400 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Surya Hebbar Gerrit-Comment-Date: Mon, 06 May 2024 10:53:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13038: Support profile tab for imported query profiles
Surya Hebbar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21400 Change subject: IMPALA-13038: Support profile tab for imported query profiles .. IMPALA-13038: Support profile tab for imported query profiles For query profile imports currently the following tabs are supported. - Query Statement - Query Timeline - Query Text Plan With the current patch "Query Profile" tab will also be supported. In the imported "Query Profile" page, query profile download section and server's non-existing query id alerts are removed on loading the page. All unsupported navbar tabs are removed and current tab is set to active. Query profile is retrieved from the indexedDB's "imported_queries" database. Profile is passed onto "profileToString" method, which converts the profile into indented text for displaying on the profile page. Each profile and its child profiles are printed in the following order with the right indentation(fields are skipped, if they do not exist). Profile name: - Info strings: - Event sequences: - Offset: - Events: - Child profile(recursive): - Counters: Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc --- M www/query_plan_text.tmpl M www/query_profile.tmpl M www/query_stmt.tmpl M www/query_timeline.tmpl 4 files changed, 76 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/21400/1 -- To view, visit http://gerrit.cloudera.org:8080/21400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iddcf2e285abbf42f97bde19014be076ccd6374bc Gerrit-Change-Number: 21400 Gerrit-PatchSet: 1 Gerrit-Owner: Surya Hebbar
[Impala-ASF-CR] IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21376 ) Change subject: IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16096/ : 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/21376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4a863853341959eb6870b662d82188e7d570971 Gerrit-Change-Number: 21376 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 10:47:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 10:41:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10606/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 10:43:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 06 May 2024 10:43:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21376 ) Change subject: IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@95 PS1, Line 95: Start > Nit: Starts. Done http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@260 PS1, Line 260: > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@275 PS1, Line 275: # find_user_processes() returns a generator. Consumes the tuples from it so > Could use "list(find_user_processes(binary_names))" instead of list compreh Good point! Done. http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@277 PS1, Line 277: kill_processes(list(find_user_processes(binary_names)), force) > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@833 PS1, Line 833: > Optional: could extract into a variable, also used on L840. Done http://gerrit.cloudera.org:8080/#/c/21376/1/bin/start-impala-cluster.py@1118 PS1, Line 1118: existing_cluster_size = cluster_ops.get_cluster().get_max_impalad_id() + 1 > If we kill some impalads, doesn't that change the cluster size? Yeah, that's an issue. Removed this check when using --kill. Also improved the code of getting the existing cluster size. -- To view, visit http://gerrit.cloudera.org:8080/21376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id4a863853341959eb6870b662d82188e7d570971 Gerrit-Change-Number: 21376 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 06 May 2024 10:25:34 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py
Hello Daniel Becker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21376 to look at the new patch set (#2). Change subject: IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py .. IMPALA-13047: Support restarting/killing specified impalads in start-impala-cluster.py This patch adds a new option, --impalad_ids, in start-impala-cluster.py to specify the impalads to restart/kill. It's a comma seperated list of the impalad index, starting from 0. Assuming the current cluster has 3 impalads, to restart the first impalad: bin/start-impala-cluster.py -r --impalad_ids=0 To restart the second and third impalad: bin/start-impala-cluster.py -r --impalad_ids=1,2 To kill the last impalad: bin/start-impala-cluster.py --kill --impalad_ids=2 To be simple, it's not allowed to change the cluster size (by -s) when restarting specified impalads. This patch also adds logs to show which process is killed, e.g. 17:33:33 MainThread: Killed statestored (pid 6618) 17:33:33 MainThread: Killed catalogd (pid 6676) 17:33:33 MainThread: Killed impalad (pid 6726) 17:33:33 MainThread: Killed impalad (pid 6729) Tests: - Verified the commands locally Change-Id: Id4a863853341959eb6870b662d82188e7d570971 --- M bin/start-impala-cluster.py M tests/common/impala_cluster.py 2 files changed, 90 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/21376/2 -- To view, visit http://gerrit.cloudera.org:8080/21376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id4a863853341959eb6870b662d82188e7d570971 Gerrit-Change-Number: 21376 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10605/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 3 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal Gerrit-Comment-Date: Mon, 06 May 2024 07:27:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 3 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal Gerrit-Comment-Date: Mon, 06 May 2024 07:27:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. Patch Set 2: Code-Review+2 That makes sense. Thanks for adding this! -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 2 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal Gerrit-Comment-Date: Mon, 06 May 2024 07:26:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 18:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21131/18/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/18/be/src/util/coding-util-test.cc@96
PS18, Line 96:
TestUrl("Impala\x01""dev\x02""\tenv\v\x14\x12""for\x1D\x05\b\x03\x04"
:
"\x05\x06\x07\x09\x0C\r\x0E""beginners\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18"
: "\x19\x10""Amuststart\x1B\x1C\x1D\x1E\x1F\x7F",
: "Impala%01dev%02%09env%0b%14%12for%1d%05"
:
"%08%03%04%05%06%07%09%0c%0d%0ebeginners%0f%10%11%12%13%14%15%16%17%18%19%10Amust"
: "start%1b%1c%1d%1e%1f%7f", false);
:
TestUrl("Impala\x01""dev\x02""\tenv\v\x14\x12""for\x1D\x05\b\x03\x04\x05\x06"
:
"\x07\x09\x0C\r\x0E""beginners\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19"
: "\x10""Amuststart\x1B\x1C\x1D\x1E\x1F\x7F",
: "Impala%01dev%02%09env%0b%14%12for%1d"
:
"%05%08%03%04%05%06%07%09%0c%0d%0ebeginners%0f%10%11%12%13%14%15%16%17%18%19%10"
: "Amuststart%1b%1c%1d%1e%1f%7f", true);
The tests look a bit messy for me. Not sure if all escaped characters are
covered. Can we add a simple case that the input string just consists of all
the escaped characters?
Please extract the strings into variables to avoid writing them twice.
If the following tests don't add more coverage, we can probably remove them.
Just keep this one and add the simple case before this.
If they do provide more coverage, please add some comments to explain it.
http://gerrit.cloudera.org:8080/#/c/21131/15/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/15/be/src/util/coding-util.cc@66
PS15, Line 66: !isalnum(static_cast(ch)) &&
!ShouldNotEscape(ch))) {
> Done.
We can write the test in python codes instead of using test files, e.g. add
this in tests/query_test/test_insert.py under class TestInsertPartKey:
def test_escaped_partition_values(self, unique_database):
tbl = unique_database + ".tbl"
self.execute_query("create table {}(i int) partitioned by(p
string)".format(tbl))
import struct
# TODO: add more values
for b in range(1, 32):
stmt = "insert into {} partition(p='{}') values (0)".format(tbl,
struct.pack("B", b))
self.execute_query(stmt)
res = self.execute_query("show partitions " + tbl)
# TODO: verify more values
for i in range(8):
columns = res.data[i].split("\t")
# Verify the partition value
assert columns[0] == struct.pack("B", i+1)
# Verify the partition location
assert "p=%0" + str(i+1) in columns[8]
assert len(res.data) == 32
Note that we still use python2 in tests so we need 'struct.pack("B", b)' to
create a byte string. In python3, we can use bytes directly.
The above test is incomplete. Please resolve the TODOs.
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 18
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Mon, 06 May 2024 07:15:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12977: add search and pagination to /hadoop-varz
Saurabh Katiyal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21329 ) Change subject: IMPALA-12977: add search and pagination to /hadoop-varz .. Patch Set 2: > Patch Set 2: > > (1 comment) "order" is the default sorting for DataTable. Since/hadoop-varz displays configuration from other services in cluster, For a better user experience, sorted table on configuration name in alphabetical order using "aaSorting" -- To view, visit http://gerrit.cloudera.org:8080/21329 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8cac23b655fa58ce12d9857649705574614a5f0 Gerrit-Change-Number: 21329 Gerrit-PatchSet: 2 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Saurabh Katiyal Gerrit-Comment-Date: Mon, 06 May 2024 06:58:19 + Gerrit-HasComments: No
