sql: add pg_lsn data type#105031
Conversation
|
This change is |
0b36605 to
ba8d78a
Compare
b057b4f to
385fa58
Compare
d36e852 to
bd0fd6e
Compare
pkg/ccl/changefeedccl/avro_test.go
Outdated
| switch typ.Family() { | ||
| case types.OidFamily: | ||
| continue | ||
| return |
There was a problem hiding this comment.
This diff looks like it short-circuits the test as soon as we hit an OID type?
There was a problem hiding this comment.
ah a bad edit - thanks!
mgartner
left a comment
There was a problem hiding this comment.
Reviewed 64 of 64 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @miretskiy, @otan, @rafiss, @renatolabs, @rharding6373, and @srosenberg)
pkg/sql/randgen/datum.go line 716 at r1 (raw file):
tree.DMaxIPAddr, }, types.PGLSNFamily: {
❤️
pkg/sql/logictest/testdata/logic_test/pg_lsn line 2 at r1 (raw file):
query T SELECT 'A01F0/1AAA'::pg_lsn
Can you test some invalid formats?
pkg/sql/logictest/testdata/logic_test/pg_lsn line 13 at r1 (raw file):
query TT SELECT * FROM pg_lsn_table ORDER BY id
A lookup by PK might be a good test to have too.
otan
left a comment
There was a problem hiding this comment.
thanks for the quick reviews!
Reviewable status:
pkg/sql/logictest/testdata/logic_test/pg_lsn line 2 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Can you test some invalid formats?
Done.
pkg/sql/logictest/testdata/logic_test/pg_lsn line 13 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
A lookup by PK might be a good test to have too.
Done.
pkg/ccl/changefeedccl/avro_test.go line 466 at r1 (raw file):
Previously, HonoreDB (Aaron Zinger) wrote…
Would you mind also adding an example to value_goldens?
Done.
This commit adds the `pg_lsn` data type from postgres. It is an uint64 integer but displays/parses as `%X/%X`, where the high 32 bits and low 32 bits are separated by a `/`. I believe using this data type is strictly superior to INT/STRING, as there are custom operations that can be performed on it which I don't think is appropriate for those types, hence the addition. We're mostly using the automated tests to cover any poignant issues, with some basic logic test operations added. Missing casts and operators with pg_lsn will be added in a later PR. Release note (sql change): Introduce the `pg_lsn` data type, which is used to store the `lsn` associated with replication.
rharding6373
left a comment
There was a problem hiding this comment.
Reviewable status:
|
bors r+ |
|
Timed out. |
|
bors r+ |
|
Build succeeded: |
|
i'm aware, was going to add it later (unless it's failing now): #105130 |
|
It is failing now #105324 (comment) |
This commit adds the
pg_lsndata type from postgres. It is an uint64integer but displays/parses as
%X/%X, where the high 32 bits and low32 bits are separated by a
/.I believe using this data type is strictly superior to INT/STRING, as
there are custom operations that can be performed on it which I don't
think is appropriate for those types, hence the addition.
We're mostly using the automated tests to cover any poignant issues,
with some basic logic test operations added. Missing casts and operators
with pg_lsn will be added in a later PR.
Release note (sql change): Introduce the
pg_lsndata type, which isused to store the
lsnassociated with replication.Epic: https://cockroachlabs.atlassian.net/browse/CRDB-26486