Joe McDonnell 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: Code-Review+1 (4 comments) This looks right to me. 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(); : > Any ideas for tests covering it? This seems complicated to test. Maybe something like this: 1. create external table with a partition column 2. add partition part3 with some data 3. add partition part2 with some data 4. add partition part1 with some data 5. Use with the tuple cache 6. drop table (which doesn't remove the data) 7. create external table with same name / schema 8. recover partitions 9. Use with the tuple cache again. It seems like the partitions would have different ids. I'm not sure if there is a simpler way to do this with invalidate metadata. 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@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 To get a bushy plan like that with straight_join, we can use a subquery: select straight_join a.id from functional.alltypes c, (select a.id from functional.alltypestiny a, functional.alltypes b where a.id = b.id) a where a.id = c.id; I checked and it has the same keys as: select a.id from functional.alltypestiny a, functional.alltypes b where a.id = b.id Using straight_join in the subquery as well would be: select straight_join a.id from functional.alltypes c, (select straight_join a.id from functional.alltypes b, functional.alltypestiny a where a.id = b.id) a where a.id = c.id; http://gerrit.cloudera.org:8080/#/c/21729/11/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/11/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@70 PS11, Line 70: String select = "select straight_join"; : String alltypes = "functional.alltypes"; : String alltypestiny = "functional.alltypestiny"; : String basicJoin = select + " probe.id from " + alltypes + " probe, " + : alltypestiny + " build where probe.id = build.id"; What do you think about using String.format()? i.e. String basicJoinTemplate = "select straight_join probe.id from functional.alltypes probe, %s build where %s"; This can handle differences in the where clause or build table name. verifyIdenticalCacheKeys( String.format(basicJoinTemplate, "functional.alltypestiny", "a.id = b.id"), String.format(basicJoinTemplate, "functional.alltypestiny", "b.id = a.id")); That could cover most of the test cases. The remaining test cases use an extra join and we could just hard-code those. 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): : """ > Done. I could probably simplify this since it's primarily testing that a sc I'm ok with having the more complicated query. It verifies that things work with multiple layers of runtime filters. -- 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: Wed, 02 Oct 2024 17:47:31 +0000 Gerrit-HasComments: Yes
