Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13051 )
Change subject: IMPALA-7957: Fix slot equivalences may be enforced multiple
times
......................................................................
Patch Set 3:
(4 comments)
I'm afraid I found a case that this doesn't handle, i.e. the solution is
incomplete. I think the issue with the current approach is that it tracks the
exact set of predicates that were placed, but createEquivConjuncts() can
enumerate predicates derived from any pair of slots in the equivalence class.
I think this is on the right track, but I think maybe we need to take the set
of predicates that were already assigned for pairs of slots within the tuple,
then union the sets in partialEquivSlots() based on those predicates.
SELECT t.id, t2.id
FROM functional.alltypestiny t
LEFT JOIN
(SELECT id, int_col, smallint_col
FROM functional.alltypestiny
WHERE int_col = id and smallint_col = id and tinyint_col = id) t2
ON (t.id = t2.id)
UNION ALL
VALUES (NULL, NULL)
http://gerrit.cloudera.org:8080/#/c/13051/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13051/3//COMMIT_MSG@43
PS3, Line 43: * Run all tests locally in CORE exploration strategy
Can you also add the repro query to an end-to-end inline-view test, just to
confirm that it produces the expected results.
http://gerrit.cloudera.org:8080/#/c/13051/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/13051/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@220
PS3, Line 220: infereed
inferred
http://gerrit.cloudera.org:8080/#/c/13051/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@221
PS3, Line 221: public final Map<Pair<SlotId, SlotId>, ExprId>
inferredConjunctsBySlots =
Can you document the invariant that both orders of every pair must be added,
i.e.
inferredConjunctsBySlots[(a, b)] = c <=> inferredConjunctsBySlots[(b, a)] = c?
Or actually, maybe it's better to normalise the pairs, e.g. so that the lower
SlotId is always the first one in the pair.
http://gerrit.cloudera.org:8080/#/c/13051/3/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/13051/3/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@1833
PS3, Line 1833: ====
Some more test cases:
# Add the same predicate *outside* the left join, it should still be enforced.
SELECT t.id
FROM functional.alltypestiny t
LEFT JOIN
(SELECT id, int_col
FROM functional.alltypestiny
WHERE int_col = id) t2
ON (t.id = t2.id) where t2.id = t2.int_col
UNION ALL
VALUES (NULL);
# Same thing except with a predicate on a different column (i.e. adding it to
the equivalence class)
SELECT t.id, t2.id
FROM functional.alltypestiny t
LEFT JOIN
(SELECT id, int_col, smallint_col
FROM functional.alltypestiny
WHERE int_col = id) t2
ON (t.id = t2.id) where t2.int_col = t2.smallint_col
UNION ALL
VALUES (NULL, NULL)
# Multiple predicates that must not be placed above the join
SELECT t.id, t2.id
FROM functional.alltypestiny t
LEFT JOIN
(SELECT id, int_col, smallint_col
FROM functional.alltypestiny
WHERE int_col = id and smallint_col = id) t2
ON (t.id = t2.id)
UNION ALL
VALUES (NULL, NULL);
--
To view, visit http://gerrit.cloudera.org:8080/13051
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida2d5d8149b217e18ebae61e136848162503653e
Gerrit-Change-Number: 13051
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 26 Apr 2019 16:49:29 +0000
Gerrit-HasComments: Yes
