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.

Sign up for GitHub

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

Jump to bottom

sql: CBO fails to validate/enforce schema constraints prior to running CHECK constraints #35364

Closed
knz opened this issue Mar 4, 2019 · 1 comment
Closed

sql: CBO fails to validate/enforce schema constraints prior to running CHECK constraints #35364

knz opened this issue Mar 4, 2019 · 1 comment
Assignees
@knz @andy-kimball
Labels
A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.

Comments

@knz
Copy link
Contributor

knz commented Mar 4, 2019 •
edited
Loading

This is a regression from the HP code. Found while investigating #35040.

CREATE TABLE d(x DECIMAL(1,0) CHECK (x = 0));
INSERT INTO d(x) VALUES (0); -- ok
UPDATE d SET x = 0.1; -- OK in HP and PostgreSQL, fails with CBO enabled

The problem is that column type constraints (width precision etc) must be applied (and the results truncated apropriately) prior to running the CHECK constraints.

This is non-trivial to fix in CBO yet as it requires a SQL built-in function able to apply column restrictions (this is not the same as performing a cast) and we don't have code to that effect yet.

My recommendation would be to temporarily disable CHECK planning in the CBO for 19.1, and improve the code towards a better fix post-release.
@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases. labels Mar 4, 2019
@knz
Copy link
Contributor Author

knz commented Mar 4, 2019

Discussed offline: this may be best served by a dedicated SQL scalar operator akin to CAST but with the separate semantics. Maybe call it "FIT".

Would also help with solving #35040 and later ALTER COLUMN SET TYPE.

Needs both SQL syntax (even though it would not be user-facing, it needs to be representable in SQL because of distsql planning) and support during mutation planning.

@knz knz mentioned this issue Mar 4, 2019
Open
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 22, 2019
@andy-kimball
opt: Round decimal values before check constraints
80ec518 
The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes cockroachdb#35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.
@andy-kimball andy-kimball mentioned this issue Mar 22, 2019
Merged
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 25, 2019
@andy-kimball
opt: Round decimal values before check constraints
06026cc 
The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes cockroachdb#35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 25, 2019
@andy-kimball
opt: Round decimal values before check constraints
0f080fd 
The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes cockroachdb#35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 25, 2019
@andy-kimball
opt: Round decimal values before check constraints
95030f2 
The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes cockroachdb#35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.
craig bot pushed a commit that referenced this issue Mar 25, 2019
@andy-kimball
Merge #36066
96dc666 
36066: opt: Round decimal values before check constraints r=andy-kimball a=andy-kimball

opt: Round decimal values before check constraints

The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes #35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.

Co-authored-by: Andrew Kimball <[email protected]>
@craig craig bot closed this as completed in #36066 Mar 25, 2019
andy-kimball added a commit to andy-kimball/cockroach that referenced this issue Mar 25, 2019
@andy-kimball
opt: Round decimal values before check constraints
8dcefed 
The PG spec requires the following DECIMAL type behavior:

  If the scale of a value to be stored is greater than the declared scale
  of the column, the system will round the value to the specified number
  of fractional digits.

This is currently happening at the very end of the mutation operator, after
any check constraints. This commit moves the rounding to occur before check
constraints. Rounding must be performed on inserted and updated values before
computed columns are evaluated as well, since computed columns should run on
the final values to be inserted/updated.

Fixes cockroachdb#35364

Release note (sql change): Computed columns are now evaluated after rounding
any decimal values in input columns. Previously, computed columns used input
values before rounding.
@andy-kimball andy-kimball mentioned this issue Mar 25, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

@andy-kimball andy-kimball

Labels
A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. A-sql-pgcompat Semantic compatibility with PostgreSQL C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@knz @andy-kimball