Honor null-on-empty for-each quantifiers outside select boxes - #4489
Open
robert-brunel wants to merge 1 commit into
Open
Honor null-on-empty for-each quantifiers outside select boxes#4489robert-brunel wants to merge 1 commit into
robert-brunel wants to merge 1 commit into
Conversation
A `ForEach` quantifier with the `isNullOnEmpty()` flag flows a single `NULL` row when the sub-graph it ranges over is empty. However, so far, the flag is only honored if it sits directly under a `SelectExpression`, since only `ImplementSimpleSelectRule` and `ImplementNestedLoopJoinRule` ever inspect it. Anywhere else in a query graph it is silently ignored, which would lead to incorrect results if NoE ever makes it there. To address this issue: * Change **RemoveSortRule**, **ImplementDistinctRule**, **ImplementUniqueRule**, **ImplementDistinctUnionRule** and **ImplementUnorderedUnionRule** to correctly honor NoE quantifiers. * Prevent **PredicateToLogicalUnionRule** from firing on a NoE quantifier, since the rewrite wouldn’t correctly preserve the semantics. * Add helpers to facilitate the handling of NoE quantifiers; notably, `forNullOnEmpty()` utility in `RecordQueryDefaultOnEmptyPlan` to construct the `ON EMPTY NULL` wrapper node from a given quantifier, and `Quantifiers.implementNullOnEmptyIfPresent()` for rules that need to establish those semantics. * Add a **validation pass**, `Quantifiers.verifyNullOnEmptyAwareness()`, that runs from `Reference.insertUnchecked()` and `Traversal.addExpression()` if sanity checks are enabled. This pass ensures that expressions can only have a NoE quantifier if they are known to honor it. Resolves #3431.
📊 Metrics Diff Analysis ReportSummary
ℹ️ About this analysisThis automated analysis compares query planner metrics between the base branch and this PR. It categorizes changes into:
The last category in particular may indicate planner regressions that should be investigated. New QueriesCount of new queries by file:
|
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
1 participant
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.
A
ForEachquantifier with theisNullOnEmpty()flag flows a singleNULLrow when the sub-graph it ranges over is empty. However, so far, the flag is only honored if it sits directly under aSelectExpression, since onlyImplementSimpleSelectRuleandImplementNestedLoopJoinRuleever inspect it. Anywhere else in a query graph it is silently ignored, which would lead to incorrect results if NoE ever makes it there.To address this issue:
forNullOnEmpty()utility inRecordQueryDefaultOnEmptyPlanto construct theON EMPTY NULLwrapper node from a given quantifier, andQuantifiers.implementNullOnEmptyIfPresent()for rules that need to establish those semantics.Quantifiers.verifyNullOnEmptyAwareness(), that runs fromReference.insertUnchecked()andTraversal.addExpression()if sanity checks are enabled. This pass ensures that expressions can only have a NoE quantifier if they are known to honor it.Resolves #3431.