Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24123 )
Change subject: IMPALA-14796: Show effective runtime filter targets in profile ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/24123/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24123/4//COMMIT_MSG@25 PS4, Line 25: Addi > Consider rewording the "BTW..." sentence in the commit message to be slight Done http://gerrit.cloudera.org:8080/#/c/24123/4/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/24123/4/be/src/runtime/coordinator.cc@684 PS4, Line 684: } > nit: Using "N/A" or "-" instead of "N" for empty effective targets maybe be I prefer "N" because "N/A" means "Not Available" and "N" means "None". http://gerrit.cloudera.org:8080/#/c/24123/4/be/src/runtime/coordinator.cc@794 PS4, Line 794: rack the current scan node id as we traverse the > The `current_node_id` is captured by reference here. We are actually tracking the scan node ids since Runtime Filters are only applied in ScanNodes which are leafs of the plan tree, i.e. won't have any child nodes. So the above problem won't happen. http://gerrit.cloudera.org:8080/#/c/24123/4/be/src/runtime/coordinator.cc@797 PS4, Line 797: curr_scan_node_id, &find_effective_filters, > nit: Using boost::split to populate a vector<string> allocates memory for b Good point. Changed to use std::string_view. http://gerrit.cloudera.org:8080/#/c/24123/4/be/src/runtime/coordinator.cc@815 PS4, Line 815: d = 0; > std::stoi throws exceptions (std::invalid_argument, std::out_of_range) if p Good point. Changed to use std::from_chars. http://gerrit.cloudera.org:8080/#/c/24123/4/tests/custom_cluster/test_runtime_profile.py File tests/custom_cluster/test_runtime_profile.py: http://gerrit.cloudera.org:8080/#/c/24123/4/tests/custom_cluster/test_runtime_profile.py@54 PS4, Line 54: test_tpch_q5_final_filter_table > The test_runtime_profile.py seems to be exclusively to verify the runtime p tests/query_test/test_runtime_filters.py is in e2e tests where we can't customized the startup flags like gen_experimental_profile. test_query_log.py itself is in custom-cluster test so it's able to add tests for both profile modes. For running the test only on parquet, it's already guarenteed: * custom-cluster test will only have one table format in the vector: https://github.com/apache/impala/blob/bacd1a561c89465a15d78c9d220e758443e53790/tests/common/custom_cluster_test_suite.py#L138-L140 * the test file explicitly uses tpch_parquet -- To view, visit http://gerrit.cloudera.org:8080/24123 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iccf4b87ac4579a70273f3306ec7b58850f06b17c Gerrit-Change-Number: 24123 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Mon, 30 Mar 2026 03:49:01 +0000 Gerrit-HasComments: Yes
