Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21729 )
Change subject: IMPALA-13185: Include runtime filter source in key ...................................................................... Patch Set 11: (7 comments) http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@386 PS9, Line 386: if (serialCtx.isTupleCache()) { : // Required property; RuntimeFilterId is irrelevant to tuple cache results. : tFilter.setFilter_id(0); : // The target plan is already serialized as part of the calling context. targets_ : // may reference multiple target nodes; the others are irrelevent for tuple : // caching. : tFilter.setTargets(new ArrayList<>()); : // Target PlanNodeId is global and not useful for tuple caching. : tFilter.setPlanid_to_target_ndx(new HashMap<>()); > For this case, we no longer include the TRuntimeFilterTargetDesc. I think t Done http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/main/java/org/apache/impala/planner/TupleCacheInfo.java File fe/src/main/java/org/apache/impala/planner/TupleCacheInfo.java: http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/main/java/org/apache/impala/planner/TupleCacheInfo.java@228 PS9, Line 228: for (TFileSplitGeneratorSpec origSplitSpec: orig.split_specs) { : TFileSplitGeneratorSpec splitSpec = origSplitSpec.deepCopy(); : > TFileSplitGeneratorSpec also has partition_id, so we can zero that out here Any ideas for tests covering it? http://gerrit.cloudera.org:8080/#/c/21729/5/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java File fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java: http://gerrit.cloudera.org:8080/#/c/21729/5/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@69 PS5, Line 69: public void testRuntimeFilterCacheKeys() { > I think using the aliases to annotate which is the probe side vs build side Done http://gerrit.cloudera.org:8080/#/c/21729/5/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@81 PS5, Line 81: // the build side is too complex. : verifyIdenticalCacheKeys(basicJoin, "select a.i > What I mean here is a predicate that gets assigned to the build side's scan Done http://gerrit.cloudera.org:8080/#/c/21729/5/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@85 PS5, Line 85: verifyOverlappingCacheKeys(basicJoin, select + " probe.id from " + alltypes + I don't see how to rewrite this as a straight_join because it relies on the subquery getting put on the build side of the join. http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java File fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java: http://gerrit.cloudera.org:8080/#/c/21729/9/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@78 PS9, Line 78: verifyIdenticalCacheKeys(basicJoin, select + " a.id from " + alltypes + : " a, " + alltypestiny + " b where b.id = a.id"); > The source expression / target expression piece of runtime filters is intri Ack http://gerrit.cloudera.org:8080/#/c/21729/9/tests/custom_cluster/test_tuple_cache.py File tests/custom_cluster/test_tuple_cache.py: http://gerrit.cloudera.org:8080/#/c/21729/9/tests/custom_cluster/test_tuple_cache.py@237 PS9, Line 237: def test_runtime_filters(self, vector, unique_database): : """ > Nit: Can we add a comment here describing the runtime filters that are in t Done. I could probably simplify this since it's primarily testing that a scan depending on a runtime filter correctly incorporates changes to the source of the runtime filter. -- To view, visit http://gerrit.cloudera.org:8080/21729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0077964be5acdb588d76251a6a39e57a0f42bb5a Gerrit-Change-Number: 21729 Gerrit-PatchSet: 11 Gerrit-Owner: Michael Smith <[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: Steve Carlin <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 01 Oct 2024 23:44:10 +0000 Gerrit-HasComments: Yes
