colinfo: add version gate for storing pg_lsn types#105391
colinfo: add version gate for storing pg_lsn types#105391craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
62c8e6e to
0643f6f
Compare
There was a problem hiding this comment.
nit: no need to make a new cluster version. you can just use clusterversion.V23_2
There was a problem hiding this comment.
there is no V23_2 though, only V23_2Start.
There was a problem hiding this comment.
oh it's not in the const
b7a921f to
cd928c1
Compare
cd928c1 to
aad4c19
Compare
rafiss
left a comment
There was a problem hiding this comment.
not gonna block this, but see my comment about cast expressions
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @otan, @renatolabs, and @smg260)
pkg/sql/logictest/testdata/logic_test/pg_lsn_mixed line 2 at r1 (raw file):
# LogicTest: local-mixed-22.2-23.1 # TODO(otan): add tests for mixed 23.1-23.2.
i've got this PR brewing #104994
pkg/sql/logictest/testdata/logic_test/pg_lsn_mixed line 5 at r1 (raw file):
statement error pg_lsn not supported until version 23.2 CREATE TABLE pg_lsn_table(id pg_lsn, val pg_lsn)
i feel like we also need logic for blocking pg_lsn in cast expressions (and a test), otherwise won't distsql get confused in a mixed version cluster?
|
Previously, rafiss (Rafi Shamim) wrote…
i'll add a test; i've never had to add a mixed version test for casts before - not sure there's a great place to do that either |
We don't want mixed version clusters storing pg_lsn types, in case they need to rollback / older versions do not understand the type. Release note: None
aad4c19 to
ff12251
Compare
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/pg_lsn_mixed line 5 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
i'll add a test; i've never had to add a mixed version test for casts before - not sure there's a great place to do that either
you can use this same logic test file. what would be the problem with that?
|
Previously, rafiss (Rafi Shamim) wrote…
sorry, i meant adding a version gate for "check you can't ever instantiate a pg_lsn instance anywhere distsql is used". |
|
bors r=rafiss mergin' for now, will update mixed-version tests later |
you don't need to do this. all we'd need is a gate for using i've added two new items to your tracking issue in #105130
|
|
Build succeeded: |
already there! |
|
oh wait, i changed the json but forgot to commit the generate-binary changes |
We don't want mixed version clusters storing pg_lsn types, in case they need to rollback / older versions do not understand the type.
Informs #105130
Release note: None