Skip to content
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

Closed
cockroach-teamcity opened this issue Sep 29, 2022 · 9 comments · Fixed by #89113
Assignees
Labels
branch-master Failures on the master branch. branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 C-technical-advisory Caused a technical advisory C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. S-1 High impact: many users impacted, serious risk of high unavailability or data loss T-sql-queries SQL Queries Team
Projects
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Sep 29, 2022

roachtest.tlp failed with artifacts on master @ 4780ee15194e7d22b661ac92254b13cd2b71dcb1:

test artifacts and logs in: /artifacts/tlp/run_1
	tlp.go:179,tlp.go:75,test_runner.go:928: expected unpartitioned and partitioned results to be equal
		(1) attached stack trace
		  -- stack trace:
		  | github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests.runTLPQuery.func2
		  | 	github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests/tlp.go:267
		  | github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests.runWithTimeout.func1
		  | 	github.com/cockroachdb/cockroach/pkg/cmd/roachtest/tests/tlp.go:290
		  | runtime.goexit
		  | 	GOROOT/src/runtime/asm_amd64.s:1594
		Wraps: (2) expected unpartitioned and partitioned results to be equal
		  |   []string{
		  | - 	"2466559136",
		  | + 	"416233601",
		  |   }
		  | sql: SELECT MIN(first) FROM (SELECT * FROM defaultdb.public.table3 AS tab_15580) tab_15580(first)
		  | SELECT MIN(agg) FROM (SELECT MIN(first) AS agg FROM (SELECT * FROM defaultdb.public.table3 AS tab_15580 WHERE (NULL OR EXISTS (WITH with_387 (col_1057) AS (SELECT * FROM (VALUES ((NULL, 0:::OID))) AS tab_15581 (col_1057)), with_388 (col_1058) AS (SELECT * FROM (VALUES (0.2983944561166533:::FLOAT8), ((-0.7761676685224073):::FLOAT8), ((-2.0824722316068613):::FLOAT8), (NULL), ((-0.820130187622559):::FLOAT8), (0.5816144538486323:::FLOAT8)) AS tab_15582 (col_1058)) SELECT CASE WHEN true THEN e'0b\x19x\x10.h':::STRING:::NAME ELSE e'Ay\x13':::STRING:::NAME END AS col_1059, 'k':::rand_typ_1 AS col_1060 FROM defaultdb.public.table2 AS tab_15583 JOIN defaultdb.public.table2 AS tab_15584 ON (tab_15583.col2_0) = (tab_15584.col2_0), defaultdb.public.table4@table4_expr_col4_3_idx AS tab_15585, with_388 AS cte_ref_125))) tab_15580(first) UNION ALL SELECT MIN(first) AS agg FROM (SELECT * FROM defaultdb.public.table3 AS tab_15580 WHERE NOT ((NULL OR EXISTS (WITH with_387 (col_1057) AS (SELECT * FROM (VALUES ((NULL, 0:::OID))) AS tab_15581 (col_1057)), with_388 (col_1058) AS (SELECT * FROM (VALUES (0.2983944561166533:::FLOAT8), ((-0.7761676685224073):::FLOAT8), ((-2.0824722316068613):::FLOAT8), (NULL), ((-0.820130187622559):::FLOAT8), (0.5816144538486323:::FLOAT8)) AS tab_15582 (col_1058)) SELECT CASE WHEN true THEN e'0b\x19x\x10.h':::STRING:::NAME ELSE e'Ay\x13':::STRING:::NAME END AS col_1059, 'k':::rand_typ_1 AS col_1060 FROM defaultdb.public.table2 AS tab_15583 JOIN defaultdb.public.table2 AS tab_15584 ON (tab_15583.col2_0) = (tab_15584.col2_0), defaultdb.public.table4@table4_expr_col4_3_idx AS tab_15585, with_388 AS cte_ref_125)))) tab_15580(first) UNION ALL SELECT MIN(first) AS agg FROM (SELECT * FROM defaultdb.public.table3 AS tab_15580 WHERE ((NULL OR EXISTS (WITH with_387 (col_1057) AS (SELECT * FROM (VALUES ((NULL, 0:::OID))) AS tab_15581 (col_1057)), with_388 (col_1058) AS (SELECT * FROM (VALUES (0.2983944561166533:::FLOAT8), ((-0.7761676685224073):::FLOAT8), ((-2.0824722316068613):::FLOAT8), (NULL), ((-0.820130187622559):::FLOAT8), (0.5816144538486323:::FLOAT8)) AS tab_15582 (col_1058)) SELECT CASE WHEN true THEN e'0b\x19x\x10.h':::STRING:::NAME ELSE e'Ay\x13':::STRING:::NAME END AS col_1059, 'k':::rand_typ_1 AS col_1060 FROM defaultdb.public.table2 AS tab_15583 JOIN defaultdb.public.table2 AS tab_15584 ON (tab_15583.col2_0) = (tab_15584.col2_0), defaultdb.public.table4@table4_expr_col4_3_idx AS tab_15585, with_388 AS cte_ref_125))) IS NULL) tab_15580(first))
		  | with args: []
		Error types: (1) *withstack.withStack (2) *errutil.leafError

Parameters: ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , ROACHTEST_encrypted=false , ROACHTEST_ssd=0

Help

See: roachtest README

See: How To Investigate (internal)

/cc @cockroachdb/sql-queries

This test on roachdash | Improve this report!

Jira issue: CRDB-20069

@cockroach-teamcity cockroach-teamcity added branch-master Failures on the master branch. C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Sep 29, 2022
@cockroach-teamcity cockroach-teamcity added this to the 22.2 milestone Sep 29, 2022
@cockroach-teamcity cockroach-teamcity added this to Triage in SQL Queries Sep 29, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Sep 29, 2022
@cockroach-teamcity

This comment was marked as off-topic.

@mgartner
Copy link
Collaborator

mgartner commented Sep 30, 2022

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 a=0. Maybe the scan is incorrectly produce a=NULL for that row.

EXPLAIN ANALYZE (DISTSQL)
SELECT min(a) FROM t;
----
planning time: 10µs
execution time: 100µs
distribution: <hidden>
vectorized: <hidden>
rows read from KV: 2 (16 B, 2 gRPC calls)
maximum memory usage: <hidden>
network usage: <hidden>
regions: <hidden>
·
• group (scalar)
│ nodes: <hidden>
│ regions: <hidden>
│ actual row count: 1
│
└── • limit
    │ count: 1
    │
    └── • union all
        │ nodes: <hidden>
        │ regions: <hidden>
        │ actual row count: 1
        │
        ├── • scan
        │     nodes: <hidden>
        │     regions: <hidden>
        │     actual row count: 0
        │     KV time: 0µs
        │     KV contention time: 0µs
        │     KV rows read: 0
        │     KV bytes read: 0 B
        │     KV gRPC calls: 0
        │     estimated max memory allocated: 0 B
        │     missing stats
        │     table: t@t_b_c_a_idx
        │     spans: (/11/50/NULL - /11/50]
        │     limit: 1
        │
        └── • limit
            │ count: 1
            │
            └── • filter
                │ nodes: <hidden>
                │ regions: <hidden>
                │ actual row count: 1
                │ filter: a IS NOT NULL
                │
                └── • scan
                      nodes: <hidden>
                      regions: <hidden>
                      actual row count: 2
                      KV time: 0µs
                      KV contention time: 0µs
                      KV rows read: 2
                      KV bytes read: 16 B
                      KV gRPC calls: 2
                      estimated max memory allocated: 0 B
                      missing stats
                      table: t@t_b_c_a_idx
                      spans: [/NULL - /11/49] (/11/51/NULL - ]
·
Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJzEUtFO2zAUfd9X3N0n0IISt2UPfiqrQEIrLSod0jRVlRtfhWiJndk3WhHqZ-0H9mVT4iEI0G4wob3lHp8TH99zbtB_K1DixfH4eDSHMjd7ah9OZtMzYIzQWE0TVZJH-QUFLiKsnE3Je-sa6KYlnOo1yiTC3FQ1N_AiwtQ6QnmDnHNBKHGuVgXNSGlycYIRamKVF-1vecjL1TJdqmWu1xjhyBZ1abwEhRFeVKr5jCefxuODWIj4MAFlNAiwfEUOI_x4CZyXJCH5-cOHObWGyXBuzaMjZ797cKS0hF5AVtdMt5B4Dx8Cms3OR5CqovCBeHY5GoFnqiC1tWHYozXHueF9CUn7oEAg-rqNUKo1lFRadw2qKGyqmLSEpL1wpTi9Ig-25qpmCQ2_dXoL9HCxiTBMvzfsWWWEUmyiv0_hJC-YHLlYdCMIuIShgNMLmEzn0OwbIxznZc7QsGlNaf14pX90LrY67z3H-f3-9F7Qn7Y58du7GonO615couRxiZKnOpT8_w4lW5Pob03iLgDrNDnS3dUPxTtcbJ6Ia2IPbBX3u-zXLFPynDIdZZmjTLF18aBr8WjyeTmZzpdNT_aGYv9VvA6e43VGvrLGU8fn9i0sIiSdUcjO29qldO5s2l4TxmmrawFNnsOpCMOpCUeNwftisVPc74jFQ3HvX8T9neLBbtuDneLDB-LF5s2vAAAA__-le0bE

@rytaft
Copy link
Collaborator

rytaft commented Sep 30, 2022

I've opened an issue for the second failure here: #89101. (I've hidden the failure to minimize distraction here)

@mgartner mgartner changed the title roachtest: tlp failed roachtest: tlp failed: incorrect column value fetched from partitioned index Sep 30, 2022
@mgartner mgartner changed the title roachtest: tlp failed: incorrect column value fetched from partitioned index roachtest: tlp failed: SplitLimitedSelectIntoUnionSelects makes incorrect transformation Sep 30, 2022
@mgartner
Copy link
Collaborator

The plan looks ok to me, but it seems like the filter is incorrectly filtering the row where a=0. Maybe the scan is incorrectly produce a=NULL for that row.

The plan is not correct, actually. We're building a LIMIT 1 below the union all in SplitLimitedSelectIntoUnionSelects that should not be there.

@DrewKimball
Copy link
Collaborator

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.

@mgartner
Copy link
Collaborator

Here's a reproduction that does not require partitions:

statement ok
CREATE TABLE t2 (
  a INT,
  b INT,
  c INT,
  INDEX (b, c, a)
);

statement ok
INSERT INTO t2 (a, b, c) VALUES (1, 10, 20), (0, 11, 100);

# This should return 0, but returns 1.
query I
SELECT min(a) FROM t2 WHERE (b <= 11 AND c < 50) OR (b = 11 AND c = 50) OR (b >= 11 AND c > 50)
----
1

You can see in the plan that there is a limit incorrectly constructed below the UnionAll:

query T
EXPLAIN (OPT, VERBOSE)
SELECT min(a) FROM t2 WHERE (b <= 11 AND c < 50) OR (b = 11 AND c = 50) OR (b >= 11 AND c > 50)
----
scalar-group-by
 ├── columns: min:7
 ├── cardinality: [1 - 1]
 ├── stats: [rows=1]
 ├── cost: 43.5286371
 ├── key: ()
 ├── fd: ()-->(7)
 ├── distribution: test
 ├── prune: (7)
 ├── limit
 │    ├── columns: a:1 b:2 c:3
 │    ├── internal-ordering: +1
 │    ├── cardinality: [0 - 1]
 │    ├── stats: [rows=1]
 │    ├── cost: 43.4886371
 │    ├── key: ()
 │    ├── fd: ()-->(1-3)
 │    ├── distribution: test
 │    ├── union-all
 │    │    ├── columns: a:1 b:2 c:3
 │    │    ├── left columns: a:9 b:10 c:11
 │    │    ├── right columns: a:15 b:16 c:17
 │    │    ├── cardinality: [0 - 2]
 │    │    ├── stats: [rows=1.273811]
 │    │    ├── cost: 43.4686371
 │    │    ├── ordering: +1
 │    │    ├── limit hint: 1.00
 │    │    ├── distribution: test
 │    │    ├── limit
 │    │    │    ├── columns: a:9 b:10 c:11
 │    │    │    ├── cardinality: [0 - 1]
 │    │    │    ├── stats: [rows=0.2738108]
 │    │    │    ├── cost: 15.0073854
 │    │    │    ├── key: ()
 │    │    │    ├── fd: ()-->(9-11)
 │    │    │    ├── limit hint: 1.00
 │    │    │    ├── distribution: test
 │    │    │    ├── interesting orderings: (+9 opt(10,11))
 │    │    │    ├── select
 │    │    │    │    ├── columns: a:9 b:10 c:11
 │    │    │    │    ├── stats: [rows=0.2738108, distinct(10)=0.273811, null(10)=0, distinct(11)=0.273811, null(11)=0]
 │    │    │    │    ├── cost: 14.9946473
 │    │    │    │    ├── fd: ()-->(10,11)
 │    │    │    │    ├── limit hint: 1.00
 │    │    │    │    ├── distribution: test
 │    │    │    │    ├── interesting orderings: (+9 opt(10,11))
 │    │    │    │    ├── scan t2@t2_b_c_a_idx
 │    │    │    │    │    ├── columns: a:9 b:10 c:11
 │    │    │    │    │    ├── constraint: /10/11/9/12: (/11/50/NULL - /11/50]
 │    │    │    │    │    ├── stats: [rows=0.8214324, distinct(9)=0.821432, null(9)=0, distinct(10)=0.821432, null(10)=0, distinct(11)=0.821432, null(11)=0, distinct(10,11)=0.821432, null(10,11)=0, distinct(9-11)=0.821432, null(9-11)=0]
 │    │    │    │    │    ├── cost: 14.956433
 │    │    │    │    │    ├── fd: ()-->(10,11)
 │    │    │    │    │    ├── distribution: test
 │    │    │    │    │    └── interesting orderings: (+9 opt(10,11))
 │    │    │    │    └── filters
 │    │    │    │         └── (((b:10 <= 11) AND (c:11 < 50)) OR ((b:10 = 11) AND (c:11 = 50))) OR ((b:10 >= 11) AND (c:11 > 50)) [outer=(10,11), constraints=(/10: (/NULL - ]; /11: (/NULL - /49] [/50 - /50] [/51 - ])]
 │    │    │    └── 1
 │    │    └── limit
 │    │         ├── columns: a:15 b:16 c:17
 │    │         ├── cardinality: [0 - 1]
 │    │         ├── stats: [rows=1]
 │    │         ├── cost: 28.4385135
 │    │         ├── key: ()
 │    │         ├── fd: ()-->(15-17)
 │    │         ├── limit hint: 1.00
 │    │         ├── distribution: test
 │    │         ├── interesting orderings: (+16,+17,+15)
 │    │         ├── select
 │    │         │    ├── columns: a:15 b:16 c:17
 │    │         │    ├── stats: [rows=37.0066, distinct(15)=37.0066, null(15)=0, distinct(16)=33.3333, null(16)=0, distinct(17)=32.7553, null(17)=0, distinct(15,17)=37.0066, null(15,17)=0]
 │    │         │    ├── cost: 28.4185135
 │    │         │    ├── limit hint: 1.00
 │    │         │    ├── distribution: test
 │    │         │    ├── interesting orderings: (+16,+17,+15)
 │    │         │    ├── scan t2@t2_b_c_a_idx
 │    │         │    │    ├── columns: a:15 b:16 c:17
 │    │         │    │    ├── constraint: /16/17/15/18
 │    │         │    │    │    ├── (/NULL - /11/49]
 │    │         │    │    │    └── (/11/51/NULL - ]
 │    │         │    │    ├── stats: [rows=333.3333, distinct(15)=98.2658, null(15)=3.33333, distinct(16)=33.3333, null(16)=0, distinct(17)=98.2658, null(17)=3.33333, distinct(15,17)=333.333, null(15,17)=0.0333333]
 │    │         │    │    ├── cost: 28.2884395
 │    │         │    │    ├── limit hint: 9.01
 │    │         │    │    ├── distribution: test
 │    │         │    │    └── interesting orderings: (+16,+17,+15)
 │    │         │    └── filters
 │    │         │         ├── (((b:16 <= 11) AND (c:17 < 50)) OR ((b:16 = 11) AND (c:17 = 50))) OR ((b:16 >= 11) AND (c:17 > 50)) [outer=(16,17), constraints=(/16: (/NULL - ]; /17: (/NULL - /49] [/50 - /50] [/51 - ])]
 │    │         │         └── a:15 IS NOT NULL [outer=(15), constraints=(/15: (/NULL - ]; tight)]
 │    │         └── 1
 │    └── 1
 └── aggregations
      └── const-agg [as=min:7, outer=(1)]
           └── a:1

@mgartner
Copy link
Collaborator

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.

How can that be? If the scan has multiple spans, then we aren't guaranteed to get rows the original Limit's ordering.

@mgartner mgartner added the S-1 High impact: many users impacted, serious risk of high unavailability or data loss label Sep 30, 2022
@mgartner
Copy link
Collaborator

I think this is the problem:

c.EmptyOrdering(),

We construct a non-ordered limit when it should actually be ordered.

@mgartner
Copy link
Collaborator

Or the problem is that we are using limit in both these places instead of newHardLimit:

newScanOrSelect = c.wrapScanInLimitedSelect(newScanOrSelect, sp, newScanPrivate, filters, limit)

@mgartner mgartner self-assigned this Sep 30, 2022
@mgartner mgartner moved this from Triage to Active in SQL Queries Sep 30, 2022
mgartner added a commit to mgartner/cockroach that referenced this issue Sep 30, 2022
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.
mgartner added a commit to mgartner/cockroach that referenced this issue Sep 30, 2022
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.
mgartner added a commit to mgartner/cockroach that referenced this issue Sep 30, 2022
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.
mgartner added a commit to mgartner/cockroach that referenced this issue Sep 30, 2022
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.
craig bot pushed a commit that referenced this issue Oct 3, 2022
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 <bilal@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com>
@craig craig bot closed this as completed in d6ccd10 Oct 3, 2022
SQL Queries automation moved this from Active to Done Oct 3, 2022
blathers-crl bot pushed a commit that referenced this issue Oct 3, 2022
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.
blathers-crl bot pushed a commit that referenced this issue Oct 4, 2022
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.
mgartner added a commit that referenced this issue Oct 5, 2022
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.
@rytaft rytaft added C-technical-advisory Caused a technical advisory branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures on the master branch. branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 C-technical-advisory Caused a technical advisory C-test-failure Broken test (automatically or manually discovered). O-roachtest O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. S-1 High impact: many users impacted, serious risk of high unavailability or data loss T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants