sql/schemachanger: block data type alter of col used in computed cols#125870
Conversation
|
This change is |
f94cca5 to
9a921b7
Compare
rafiss
left a comment
There was a problem hiding this comment.
i just had one optional suggestion to possible make the function simpler
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @spilchen and @wenyihu6)
pkg/sql/catalog/schemaexpr/expr.go line 644 at r1 (raw file):
} if referencedCols.Contains(dependentColID) { return errCallback()
i think we could keep this simpler by having this function return a (bool, error) pair. then rather than calling the error callback here, we just leave it to the caller to return the error it wants to.
Previously, altering the data type of a column used in a generated column was allowed, causing downstream issues when querying the generated column. To be consistent with postgres, we will now block any attempt to alter the data type of a column used in a computed column. The update to event_test.go was needed because it was doing an ALTER that is now blocked. The test was changed to not reference a column in the computed expression that we intend to change the data type on. Epic: CRDB-25314 Closes: cockroachdb#72457 Release note (bug fix): Attempts to alter the data type of a column used in a computed column expression are now blocked. Signed-off-by: Matt Spilchen <matt.spilchen@cockroachlabs.com>
9a921b7 to
c0a3044
Compare
spilchen
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/catalog/schemaexpr/expr.go line 644 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
i think we could keep this simpler by having this function return a
(bool, error)pair. then rather than calling the error callback here, we just leave it to the caller to return the error it wants to.
Good suggestion, this is much simpler.
|
tftr! bors r+ |
Previously, altering the data type of a column used in a generated column was allowed, causing downstream issues when querying the generated column.
To be consistent with postgres, we will now block any attempt to alter the data type of a column used in a computed column.
Epic: CRDB-25314
Closes: #72457
Release note (bug fix): Attempts to alter the data type of a column used in a computed column expression are now blocked.