-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
roachtest: tlp failed: SplitLimitedSelectIntoUnionSelects makes incorrect transformation #88993
Comments
This comment was marked as off-topic.
This comment was marked as off-topic.
Partially reduced SQL from the first failure: https://gist.github.com/mgartner/1ea50716d6b1e61a1d955172d3d4cfe8. Still working on reducing more. Update: further reduced: https://gist.github.com/mgartner/549fa5a709ded4ba92f50b31d0ad57c0 Fully reduced example: CREATE TABLE t (
a INT,
b INT,
c INT,
INDEX (b, c, a) PARTITION BY LIST (b, c) (
PARTITION p1 VALUES IN ((11, 50))
)
);
INSERT INTO t (a, b, c) VALUES (1, 10, 150), (0, 11, 100);
-- The min should be 0, but 1 is returned.
SELECT min(a) FROM t; This doesn't reproduce in v21.2, so I believe it was introduced in v22.1.0. The plan looks ok to me, but it seems like the filter is incorrectly filtering the row where
|
I've opened an issue for the second failure here: #89101. (I've hidden the failure to minimize distraction here) |
The plan is not correct, actually. We're building a LIMIT 1 below the union all in |
I think the limits below the union all are expected; we only need one min row from each input in order to get the min row for the whole query. |
Here's a reproduction that does not require partitions:
You can see in the plan that there is a limit incorrectly constructed below the UnionAll:
|
How can that be? If the scan has multiple spans, then we aren't guaranteed to get rows the original Limit's ordering. |
I think this is the problem: cockroach/pkg/sql/opt/xform/general_funcs.go Line 731 in 8389ac4
We construct a non-ordered limit when it should actually be ordered. |
Or the problem is that we are using cockroach/pkg/sql/opt/xform/general_funcs.go Line 577 in 8389ac4
cockroach/pkg/sql/opt/xform/general_funcs.go Line 657 in 8389ac4
|
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes cockroachdb#88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. This bug was present since version 22.1.0.
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes cockroachdb#88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. This bug was present since version 22.1.0.
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes cockroachdb#88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0.
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes cockroachdb#88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0.
89003: storage: CheckSSTConflicts performance fixes r=erikgrinaker a=itsbilal storage: Use range key masking in CheckSSTConflicts This change uses range key masking in case of import cancellation/restart inside CheckSSTConflicts. This prevents point keys shadowed by range tombstones from appearing to CheckSSTConflicts, significantly speeding up its runtime and allowing import restarts to succeed without getting wedged. Unblocks #87309. Release note: None. --- storage: Move no-overlap fast path up in CheckSSTConflicts This change moves up the fast path in CheckSSTConflicts where we check for any engine keys within the overlapping SST keys. If no engine keys are found, we can return from the function immediately. Previously we'd open a range-key sst iterator before doing this check, which has a pretty significant construction cost. Fixes #88702. Release note: None 89088: reduce: remove and simplify index PARTITION BY clauses r=mgartner a=mgartner Release note: None 89113: opt: fix invalid transformation in SplitLimitedSelectIntoUnionSelects r=mgartner a=mgartner #### opt: fix invalid transformation in SplitLimitedSelectIntoUnionSelects This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes #88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0. 89166: storage: improve `BenchmarkUpdateSSTTimestamps` r=erikgrinaker a=erikgrinaker This patch improves `BenchmarkUpdateSSTTimestamps`, by running separate benchmarks varying the number of keys and concurrency. In particular, this exercises the `sstable.RewriteKeySuffixes` fast path, while the old benchmark only used the naïve read/write slow path. Touches #88723. Release note: None Co-authored-by: Bilal Akhtar <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Erik Grinaker <[email protected]>
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes #88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0.
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes #88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0.
This commit removes some untested logic in `SplitLimitedSelectIntoUnionSelects` that created invalid expression transformations. With this logic, this rule could construct an unordered limit below the `UnionAll` which is incorrect. The bug could cause incorrect query results. Fixes #88993 Release note (bug fix): A bug has been fixed that could cause incorrect results in rare cases. The bug could only present if the following conditions were true: 1. A query with `ORDER BY` and `LIMIT` was executed. 2. The table containing the `ORDER BY` columns had an index containing those columns. 3. The index in (2) contained a prefix of columns held to a fixed number of values by the query filter (e.g., `WHERE a IN (1, 3)`), a `CHECK` constraint (e.g., `CHECK (a IN (1, 3))`), inferred by a computed column expression (e.g. `WHERE a IN (1, 3)` and a column `b INT AS (a + 10) STORED`), or inferred by a `PARTITION BY` clause (e.g. `INDEX (a, ...) PARTITION BY LIST (a) (PARTITION p VALUES ((1), (3)))`). This bug was present since version 22.1.0.
roachtest.tlp failed with artifacts on master @ 4780ee15194e7d22b661ac92254b13cd2b71dcb1:
Parameters:
ROACHTEST_cloud=gce
,ROACHTEST_cpu=4
,ROACHTEST_encrypted=false
,ROACHTEST_ssd=0
Help
See: roachtest README
See: How To Investigate (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-20069
The text was updated successfully, but these errors were encountered: