roachtest/sqlsmith: projected column has inconsistent type #91817
Assignees
Labels
A-sql-optimizer
SQL logical planning and optimizations.
branch-release-22.1
Used to mark release blockers and technical advisories for 22.1
C-test-failure
Broken test (automatically or manually discovered).
O-roachtest
O-robot
Originated from a bot.
S-3
Medium-low impact: incurs increased costs for some users (incl lower avail, recoverable bad data)
T-sql-queries
SQL Queries Team
Projects
Milestone
Comments
cockroach-teamcity
added
branch-release-22.1
|
Stack trace: |
|
Snippet from the Notice how in the |
DrewKimball
changed the title
|
[triage] we think this is in the optimizer |
|
@DrewKimball were you able to reduce a reproduction for this? |
|
Reproduces with this: |
|
Reduced: |
mgartner
added
the
S-3
|
I'm going to look into this one. |
|
Reduced further: CREATE TABLE t (
k INT2 PRIMARY KEY,
v INT2 GENERATED ALWAYS AS (k + 1) VIRTUAL
);
INSERT INTO t VALUES (0);
SELECT var_pop(v::INT8) OVER ()
FROM t
GROUP BY k, v
HAVING every(true); |
|
I believe the fix is to add an assigment cast when projecting the virtual column expression to ensure that the type of the projected expression matches the column type of the virtual column. That's a fairly easy change to make. However, I'm worried about the fallout of such a change without also fixing #81698. I believe that would make it possible to add a virtual column to a table without error, but then all subsequent SELECTs could fail if the existing data in the table cannot be assignment cast to the column type. For example: CREATE TABLE t2 (s STRING);
INSERT INTO t2 VALUES ('foo');
-- This should error, but does not due to #81698.
ALTER TABLE t2 ADD COLUMN v CHAR AS (s) VIRTUAL;
-- This will error with "value too long for type CHAR".
SELECT * FROM t2;So I think we'll need to also fix #81698 to avoid collateral damage. |
|
Here's an example showing the root problem in this issue: CREATE TABLE t (
s STRING,
comp_s "char" AS (s) STORED,
comp_v "char" AS (s) VIRTUAL
);
INSERT INTO t VALUES ('foo');
SELECT * FROM t;
-- s | comp_s | comp_v
-- ------+--------+---------
-- foo | f | foo
-- (1 row)The values of both computed columns should be the same, but they are different because the STORED column value undergoes an assignment cast when it is written, while the VIRTUAL column value doesn't undergo the same assignment cast when it is read. |
mgartner
added a commit
to mgartner/cockroach
that referenced
this issue
Jun 28, 2023
optbuilder wraps an assignment cast around a virtual computed column expression when writing to the table if the expression does not have the same type as the virtual column. This matches the behavior of all writes to columns of expressions that don't match the target column type. However, the same cast was not applied to the projection of the virtual column expression when reading from the table. This cast is necessary in order to produce the correct the value of the column - the projection must produce the same value that would have been written to the table if the column was a stored computed column. This commit corrects optbuilder so that the cast is correctly added to the projection. Note that this commit adds a standard cast, not an assignment cast, as a temporary workaround until cockroachdb#81698 is addressed. This is because an assignment cast can fail in cases when a regular cast will not. Because we don't validate that all existing rows can successfully produce a new virtual column expression during a backfill, adding an assignment cast to the projection of the virtual column could cause future reads to error. Once cockroachdb#81698 is addressed, we can change these casts to assignment casts so that they directly match the values that would be written to the same column if it were stored. Fixes cockroachdb#91817 Informs cockroachdb#81698 Release note (bug fix): A bug has been fixed that could produce incorrect values for virtual computed columns in rare cases. The bug only presented when the virtual column expression's type did not match the type of the virtual column.
mgartner
added a commit
to mgartner/cockroach
that referenced
this issue
Jun 29, 2023
optbuilder wraps an assignment cast around a virtual computed column expression when writing to the table if the expression does not have the same type as the virtual column. This matches the behavior of all writes to columns of expressions that don't match the target column type. However, the same cast was not applied to the projection of the virtual column expression when reading from the table. This cast is necessary in order to produce the correct the value of the column - the projection must produce the same value that would have been written to the table if the column was a stored computed column. This commit corrects optbuilder so that the cast is correctly added to the projection. Note that this commit adds a standard cast, not an assignment cast, as a temporary workaround until cockroachdb#81698 is addressed. This is because an assignment cast can fail in cases when a regular cast will not. Because we don't validate that all existing rows can successfully produce a new virtual column expression during a backfill, adding an assignment cast to the projection of the virtual column could cause future reads to error. Once cockroachdb#81698 is addressed, we can change these casts to assignment casts so that they directly match the values that would be written to the same column if it were stored. Fixes cockroachdb#91817 Informs cockroachdb#81698 Release note (bug fix): A bug has been fixed that could produce incorrect values for virtual computed columns in rare cases. The bug only presented when the virtual column expression's type did not match the type of the virtual column.
craig bot
pushed a commit
that referenced
this issue
Jul 5, 2023
105736: opt: wrap virtual computed column projections in a cast r=mgartner a=mgartner #### sql/logictest: remove assignment cast TODO This commit removes a TODO that was partially addressed by #82022. Informs #73743 Release note: None #### sql/types: fix T.Identical This commit fixes a bug in `types.T.Identical` which caused it to return false for collated string types with a different string-representation of locales that represents the same locale logically. For example, collated string types with locales `en_US` and `en_us` would not be identical, even though these are both valid representations of the same locale. There is no release note because this has not caused any known bugs. Release note: None #### opt: wrap virtual computed column projections in a cast optbuilder wraps an assignment cast around a virtual computed column expression when writing to the table if the expression does not have the same type as the virtual column. This matches the behavior of all writes to columns of expressions that don't match the target column type. However, the same cast was not applied to the projection of the virtual column expression when reading from the table. This cast is necessary in order to produce the correct value of the column - the projection must produce the same value that would have been written to the table if the column was a stored computed column. This commit corrects optbuilder so that the cast is correctly added to the projection. Note that this commit adds a standard cast, not an assignment cast, as a temporary workaround until #81698 is addressed. This is because an assignment cast can fail in cases when a regular cast will not. Because we don't validate that all existing rows can successfully produce a new virtual column expression during a backfill, adding an assignment cast to the projection of the virtual column could cause future reads to error. Once #81698 is addressed, we can change these casts to assignment casts so that they directly match the values that would be written to the same column if it were stored. Fixes #91817 Informs #81698 Release note (bug fix): A bug has been fixed that could produce incorrect values for virtual computed columns in rare cases. The bug only presented when the virtual column expression's type did not match the type of the virtual column. 105932: opt/exec: add passthrough cols to DELETE USING result cols in explain r=yuzefovich,DrewKimball a=michae2 Now that we support `DELETE USING`, delete nodes can have passthrough columns. Add these to the result columns used by `EXPLAIN (TYPES)`. Fixes: #105803 Release note (sql change): Fix an internal error when using `EXPLAIN (TYPES)` on a `DELETE FROM ... USING ... RETURNING` statement. Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com> Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
roachtest.sqlsmith/setup=rand-tables/setting=no-ddl failed with artifacts on release-22.1 @ 4ca19806493494cfbe881cb86685079fe15a9f0d:
Help
See: roachtest README
See: How To Investigate (internal)
This test on roachdash | Improve this report!
Jira issue: CRDB-21441
The text was updated successfully, but these errors were encountered: