Repository navigation
Honor null-on-empty for-each quantifiers outside select boxes - #4489
robert-brunel wants to merge 1 commit into
Conversation
alecgrieser
left a comment
There was a problem hiding this comment.
A few minor things, but this mostly makes sense to me, though I would defer to @normen662. I think the new sanity check does help, but I am a bit worried that there's a bigger laundry list of places where we aren't quite handling forEachWithDefaultOnNull expressions correctly, though I'm not certain about what a better system for dealing with them would be. I guess I could see removing the default-on-empty facility from Quantifier.ForEach all together, and instead introducing a special logical expression here. But that would be a bigger change, and maybe there's a reason that would be a bad idea that I'm not aware of.
0cf625b to
8380a3a
Compare
This worry is now mostly addressed (and the sanity check gone) by the |
2bb4ec6 to
c6fe92d
Compare
ad75bf3 to
45ac76d
Compare
d6f2a07 to
c20e372
Compare
c20e372 to
d44e94f
Compare
4bec3cf to
4f64101
Compare
3d4ab09 to
a5b0bc6
Compare
a5b0bc6 to
4f45af0
Compare
📊 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:
|
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:
Quantifier.applyGlue().SELECTnever has partial matches.plainForEachQuantifier*(). Such a rule now simply does not fire on a NoE quantifier. SplitSelectExtractIndependentQuantifiersRule and InComparisonToExplodeRule keep matching any for-each quantifier, since their rewrites preserve the semantics.FDBSimpleQueryGraphTestthat were disabled pending For-each quantifiers with null-on-empty semantics only work as input toSelectExpressions#3431, and add tests there that cover the glue in the distinct, union, unique and sort rules (over both empty and non-empty inputs), and that a rule matching only plain for-each quantifiers refuses a NoE quantifier. Add outer-join queries withOR,INandUNION ALLtojoin-tests-outer.yamsql.Resolves #3431.