Repository navigation
fix: reject correlated IN/NOT IN over a groupless aggregate - #26677
mohammadnaqvi04 wants to merge 3 commits into
Conversation
|
@jayzhan211 this addresses your comment (#24960 (comment)) similar to how #26544 handles its no row for an unmatched outer row case. |
| if !pull_up.can_pull_up { | ||
| return Ok(None); | ||
| } | ||
| // Stopgap until a follow-up adds count-bug compensation for `IN`/`NOT IN`. |
There was a problem hiding this comment.
nit: could this point at the tracking issue, so whoever adds the IN count-bug compensation knows to remove the block?
- // Stopgap until a follow-up adds count-bug compensation for `IN`/`NOT IN`.
+ // TODO(#24960): stopgap until count-bug compensation covers `IN`/`NOT IN`.| // Stopgap until a follow-up adds count-bug compensation for `IN`/`NOT IN`. | ||
| // The subquery stays correlated unless an outer row with no match gets the | ||
| // right answer anyway. | ||
| if !exists_sub_query { |
There was a problem hiding this comment.
nit: the extra rewrite(true) only finds something when a groupless aggregate was pulled up, and the first rewrite already records that. Gating on it skips the second pass for every other IN subquery. Optimizer tests and subquery.slt still pass with this:
- if !exists_sub_query {
+ if !exists_sub_query && pull_up.pulled_up_scalar_agg {| @@ -2124,14 +2124,35 @@ WHERE t1_int + 2 IN (SELECT max(t2_int) FROM t2 WHERE t2.t2_id = t1.t1_id) | |||
| ---- | |||
| 11 | |||
There was a problem hiding this comment.
nit: the LeftMark branch that is allowed through (an IN under OR in a WHERE) has no test. Something like this would pin it down:
SELECT t1_id FROM t1
WHERE t1_int + 2 IN (SELECT max(t2_int) FROM t2 WHERE t2.t2_id = t1.t1_id) OR t1_id = 33
ORDER BY t1_id;
-- 11
-- 33|
@mohammadnaqvi04 Thanks for the fix! I left a few nits inline that are still open, so please take a look when you get a chance. |
Which issue does this PR close?
Part of #24960. This PR adds a safe rejection until
IN/NOT INcount-bug compensation is implemented.Rationale for this change
A correlated
IN/NOT INover an aggregate withoutGROUP BYreturns wrong rows when an outer row has no match (see #24960 (comment)). #25391 fixedEXISTS/NOT EXISTS.IN/NOT INwill be covered in a follow-up, so until then these queries should error instead of returning wrong results.What changes are included in this PR?
build_joinindecorrelate_predicate_subquery.rsnow keeps these subqueries correlated, so they fail to plan with a not-implemented error. The exception is anINin aWHEREover an aggregate that is NULL on empty input (e.g.max,sum), which the current rewrite already answers correctly.What is the testing strategy for this PR?
Are there any user-facing changes?
These queries now return a not-implemented error instead of wrong results.