Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/23074 )
Change subject: IMPALA-13898: Incorporate partition information into tuple cache keys ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/23074/10/be/src/exec/tuple-cache-node.cc File be/src/exec/tuple-cache-node.cc: http://gerrit.cloudera.org:8080/#/c/23074/10/be/src/exec/tuple-cache-node.cc@503 PS10, Line 503: id_to_scan_node_map[exec_node->plan_node().tnode_->node_id] = scan_node; > It seems the exec_node->plan_node().tnode_->node_id are unique here, would Done http://gerrit.cloudera.org:8080/#/c/23074/10/be/src/exec/tuple-cache-node.cc@521 PS10, Line 521: if (partition_desc != nullptr) { > I think this is only used for release version when the previous DCHECK does Done http://gerrit.cloudera.org:8080/#/c/23074/10/tests/custom_cluster/test_tuple_cache.py File tests/custom_cluster/test_tuple_cache.py: http://gerrit.cloudera.org:8080/#/c/23074/10/tests/custom_cluster/test_tuple_cache.py@422 PS10, Line 422: assert result1.data[0].split("\t") == ["1", "1"] > Is it good if we also check the length? "assert len(result1.data) == 1" Done http://gerrit.cloudera.org:8080/#/c/23074/10/tests/custom_cluster/test_tuple_cache.py@430 PS10, Line 430: assert result2.data[0].split("\t") == ["1", "2"] > Is it good if we also check the length? "assert len(result2.data) == 1" Done -- To view, visit http://gerrit.cloudera.org:8080/23074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a7109fcf8a30bf915bb566f7d642f8037793a8c Gerrit-Change-Number: 23074 Gerrit-PatchSet: 10 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 15 Jul 2025 23:24:57 +0000 Gerrit-HasComments: Yes
