GH-4160: Apply the filter-disjunction rewrite only when the disjuncts are mutually exclusive - #4161
Open
faubulous wants to merge 1 commit into
Open
GH-4160: Apply the filter-disjunction rewrite only when the disjuncts are mutually exclusive#4161faubulous wants to merge 1 commit into
faubulous wants to merge 1 commit into
Conversation
…juncts are mutually exclusive TransformFilterDisjunction rewrites filter(e1 || e2, P) into a disjunction that evaluates P once per disjunct, so a solution satisfying k disjuncts is returned k times where the filter returns it once. FILTER(?x = :c || ?x = :c) returns every matching row twice under the default optimizer, and FILTER(?x = :c || ?x != :d) returns rows with ?x = :c twice; disabling optFilterDisjunction restores the correct multiset. Several existing algebra tests pinned the unsound expansions; their expectation is now that the filter is left alone. Two changes together restore filter semantics: Repeated disjuncts are dropped first — (A || A) is A — so the degenerate duplicate that generated queries really contain (LDBC SPB emits FILTER(?pf = :c || ?pf = :c)) collapses to a single equality that grounds the pattern, rather than being either doubled (before) or left unoptimized (declined). The expansion is then applied only when every remaining disjunct tests one and the same variable against a constant (= or sameTerm) and the constants are pairwise known to be different values (NodeValue.notSameValueAs, with an indeterminate comparison treated as possibly equal), which makes the disjuncts mutually exclusive and the rewrite an exact partition. The motivating case — ?x IN (...) over distinct constants, including mixed IRI/literal lists — keeps its expansion; every other disjunction is now evaluated as the filter it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub issue resolved #4160
Pull request Description:
TransformFilterDisjunctionrewritesfilter(e1 || e2, P)into a disjunction that evaluatesPonce per disjunct, so a solution satisfying k disjuncts is returned k times where the filter returns it once.FILTER(?x = :c || ?x = :c)returns every matching row twice under the default optimizer, andFILTER(?x = :c || ?x != :d)returns rows with?x = :ctwice; disablingoptFilterDisjunctionrestores the correct multiset.Two changes together restore filter semantics:
(A || A)isA— so the degenerate duplicate that generated queries really contain (LDBC SPB emitsFILTER(?pf = :c || ?pf = :c)) collapses to a single equality that grounds the pattern, rather than being either doubled (before) or left unoptimized (declined).=orsameTerm) and the constants must be pairwise known to be different values (NodeValue.notSameValueAs, with an indeterminate comparison treated as possibly equal). That makes the rewrite an exact partition. The motivating case —?x IN (...)over distinct constants, including mixed IRI/literal lists — keeps its expansion; every other disjunction is now evaluated as the filter it is.Several existing algebra tests pinned the unsound expansions; their expectation is now that the filter is left alone.
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.