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

sql: prepared selects that filter a column with a constant casted to the wrong type fail to return the expected results #81315

Closed
rhu713 opened this issue May 16, 2022 · 1 comment · Fixed by #81331
Labels
branch-release-21.2 Used to mark technical advisories for 21.2 branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-technical-advisory Caused a technical advisory S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. T-sql-queries SQL Queries Team
Projects

Comments

@rhu713
Copy link
Contributor

rhu713 commented May 16, 2022

Describe the problem

After upgrading from 21.1.10 to 21.2.7, prepared statements on a table with a DECIMAL primary key do not return the matching rows if the WHERE clause casts the primary key parameter to INT::8. This statement previously worked on a 21.1.10 cluster.

To Reproduce
This small test case on a demo cluster reproduces the issue

root@127.0.0.1:26257/movr> create table t_dec (a decimal not null primary key, b int);
CREATE TABLE


Time: 11ms total (execution 11ms / network 0ms)

root@127.0.0.1:26257/movr>  prepare stmt1 AS SELECT * FROM t_dec WHERE a = $1::INT8;
PREPARE


Time: 8ms total (execution 8ms / network 0ms)

root@127.0.0.1:26257/movr> INSERT INTO t_dec VALUES (1, 100), (2, 200);                                                                       
INSERT 2


Time: 2ms total (execution 1ms / network 0ms)

root@127.0.0.1:26257/movr>  SELECT * FROM t_dec WHERE a = 1;
  a |  b
----+------
  1 | 100
(1 row)


Time: 1ms total (execution 1ms / network 0ms)

root@127.0.0.1:26257/movr>  EXECUTE stmt1 (1);
  a | b
----+----
(0 rows)

Expected behavior
EXECUTE stmt1 (1); is expected to return the (1, 100) row on 21.2. The row is returned if these steps are reproduced on a 21.1 cluster.

Additional data / screenshots

Environment:

  • CockroachDB version 21.2.7

Jira issue: CRDB-15244

@rhu713 rhu713 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 16, 2022
@stevendanna stevendanna added the T-sql-queries SQL Queries Team label May 16, 2022
@blathers-crl blathers-crl bot added this to Triage in SQL Queries May 16, 2022
@rafiss
Copy link
Collaborator

rafiss commented May 16, 2022

We saw this bug in a support case, and captured statement bundles of the working and non-working versions of the query.

In that example, the decimal value was 331, and when using the prepared statement, it was encoded as \367\016\217\212\367\001K\210; when used in a simple query it was \367\016\217\212+\007>\000\210. My understanding from reading reading https://github.com/cockroachdb/cockroach/blob/master/docs/tech-notes/encoding.md#composite-encoding) is that we are supposed to treat these as equivalent, but I’m not too familiar with the details.

@rytaft rytaft added the S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. label May 16, 2022
@jordanlewis jordanlewis changed the title Prepared statements on table with DECIMAL pk do not return matching rows sql: prepared selects that filter a column with a constant casted to the wrong type fail to return the expected results May 16, 2022
rytaft added a commit to rytaft/cockroach that referenced this issue May 16, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes cockroachdb#81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
rytaft added a commit to rytaft/cockroach that referenced this issue May 16, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes cockroachdb#81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
rytaft added a commit to rytaft/cockroach that referenced this issue May 16, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes cockroachdb#81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
craig bot pushed a commit that referenced this issue May 16, 2022
81331: opt: do not use placeholder fast path if types do not match r=rytaft a=rytaft

Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes #81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column `a` was of type
`DECIMAL`, the following prepared query could produce incorrect results
when executed:
```
SELECT * FROM t_dec WHERE a = $1::INT8;
```

Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
craig bot pushed a commit that referenced this issue May 17, 2022
81253: sql: inverted index accelerate json ? string, json ?& array, and json ?| array ops r=jordanlewis a=jordanlewis

Closes #81114

This commit adds support for accelerating the json ? string operator
when the json argument is a column that has an inverted index over it.

Release note (sql change): the json ? string operator is now index
accelerated if there is an inverted index over the json column referred
to on the left hand side of the expression.

81331: opt: do not use placeholder fast path if types do not match r=rytaft a=rytaft

Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes #81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column `a` was of type
`DECIMAL`, the following prepared query could produce incorrect results
when executed:
```
SELECT * FROM t_dec WHERE a = $1::INT8;
```

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
@craig craig bot closed this as completed in a863f6b May 17, 2022
SQL Queries automation moved this from Triage to Done May 17, 2022
blathers-crl bot pushed a commit that referenced this issue May 17, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes #81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
blathers-crl bot pushed a commit that referenced this issue May 17, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes #81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
rytaft added a commit to rytaft/cockroach that referenced this issue May 17, 2022
Fixed a bug where we were incorrectly using the placeholder fast
path if the type of the placholder did not match the type of the column.
We should disallow mixed-type comparisons for the placeholder fast path
because it results in incorrect encodings, and therefore incorrect results
from the scan. We now disable the placeholder fast path if the types do
not match.

Fixes cockroachdb#81315

Release note (bug fix): Fixed a bug in which some prepared statements
could result in incorrect results when executed. This could occur when
the prepared statement included an equality comparison between an index
column and a placeholder, and the placholder was cast to a type that was
different from the column type. For example, if column a was of type
DECIMAL, the following prepared query could produce incorrect results
when executed:
SELECT * FROM t_dec WHERE a = $1::INT8;
cucaroach added a commit to cucaroach/cockroach that referenced this issue Jul 29, 2022
Add a flag to disable this optimization in case it bites us again.

Informs cockroachdb#81315

Release note: none
@rytaft rytaft added the C-technical-advisory Caused a technical advisory label Dec 6, 2023
@rytaft rytaft added branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 branch-release-21.2 Used to mark technical advisories for 21.2 labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release-21.2 Used to mark technical advisories for 21.2 branch-release-22.1 Used to mark release blockers and technical advisories for 22.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-technical-advisory Caused a technical advisory S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants