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: serialized expressions containing enums should use the physical representation #49379

Closed
rohany opened this issue May 21, 2020 · 0 comments · Fixed by #49565
Closed

sql: serialized expressions containing enums should use the physical representation #49379

rohany opened this issue May 21, 2020 · 0 comments · Fixed by #49565
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@rohany
Copy link
Contributor

rohany commented May 21, 2020

We serialize expressions in a variety of places in order to parse them back and use, like in expressions for default and computed columns, or in check constraints. In order to be resilient to operations that rename enum members, our serialized enums should contain the unchanging physical representation of the enum, rather than its logical string representation.
@rohany rohany added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label May 21, 2020
@rohany rohany mentioned this issue May 27, 2020
Closed
35 tasks
@rohany rohany mentioned this issue May 27, 2020
Merged
rohany added a commit to rohany/cockroach that referenced this issue May 28, 2020
@rohany
sql: serialize UDTs in expressions in a stable way
a1161c3 
Fixes cockroachdb#49379.

This PR ensures that serialized expressions stored durably in table
descriptors are serialized in a format that is stable across changes to
user defined types present in those expressions. An effect of this
change is that these expressions must be reparsed and formatted in a
human readable way before display in statements like `SHOW CREATE
TABLE`. That work will be done in a follow up PR.

Release note: None
rohany added a commit to rohany/cockroach that referenced this issue Jun 2, 2020
@rohany
sql: serialize UDTs in expressions in a stable way
acbd408 
Fixes cockroachdb#49379.

This PR ensures that serialized expressions stored durably in table
descriptors are serialized in a format that is stable across changes to
user defined types present in those expressions. An effect of this
change is that these expressions must be reparsed and formatted in a
human readable way before display in statements like `SHOW CREATE
TABLE`. That work will be done in a follow up PR.

Release note: None
rohany added a commit to rohany/cockroach that referenced this issue Jun 2, 2020
@rohany
sql: serialize UDTs in expressions in a stable way
b736656 
Fixes cockroachdb#49379.

This PR ensures that serialized expressions stored durably in table
descriptors are serialized in a format that is stable across changes to
user defined types present in those expressions. An effect of this
change is that these expressions must be reparsed and formatted in a
human readable way before display in statements like `SHOW CREATE
TABLE`. That work will be done in a follow up PR.

Release note: None
craig bot pushed a commit that referenced this issue Jun 3, 2020
@rohany @itsbilal @andreimatei
Merge #49565 #49721 #49815
c8808ad 
49565: sql: serialize UDTs in expressions in a stable way r=otan,jordanlewis a=rohany

Fixes #49379.

This PR ensures that serialized expressions stored durably in table
descriptors are serialized in a format that is stable across changes to
user defined types present in those expressions. An effect of this
change is that these expressions must be reparsed and formatted in a
human readable way before display in statements like `SHOW CREATE
TABLE`. 

Release note: None

49721: storage: Add rocksdb-vs-pebble benchmark for ExportToSst r=itsbilal a=itsbilal

As part of the investigation into #49710, this change adds a
benchmark for ExportToSst that tests both RocksDB and Pebble.

Here are some example runs without contention (old = rocksdb,
new = pebble):

	name                                                   old time/op  new time/op  delta
	ExportToSst/rocksdb/numKeys=64/numRevisions=1-12       43.9µs ± 3%  34.5µs ± 4%  -21.33%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=64/numRevisions=10-12       281µs ± 3%   169µs ± 6%  -39.89%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=64/numRevisions=100-12     1.82ms ±22%  1.17ms ± 1%  -35.73%  (p=0.000 n=10+9)
	ExportToSst/rocksdb/numKeys=512/numRevisions=1-12       212µs ± 6%   111µs ± 3%  -47.77%  (p=0.000 n=10+9)
	ExportToSst/rocksdb/numKeys=512/numRevisions=10-12     1.91ms ± 1%  1.19ms ± 8%  -37.65%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=512/numRevisions=100-12    13.7ms ± 3%  10.1ms ±12%  -26.21%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=1024/numRevisions=1-12      390µs ± 1%   215µs ±12%  -44.94%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=1024/numRevisions=10-12    4.01ms ± 6%  2.40ms ±16%  -40.13%  (p=0.000 n=10+9)
	ExportToSst/rocksdb/numKeys=1024/numRevisions=100-12   27.9ms ± 2%  20.8ms ± 2%  -25.48%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=8192/numRevisions=1-12     2.97ms ± 2%  1.42ms ± 5%  -52.24%  (p=0.000 n=9+10)
	ExportToSst/rocksdb/numKeys=8192/numRevisions=10-12    32.8ms ± 7%  19.1ms ± 3%  -41.59%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=8192/numRevisions=100-12    224ms ± 3%   169ms ±25%  -24.64%  (p=0.000 n=9+10)
	ExportToSst/rocksdb/numKeys=65536/numRevisions=1-12    23.7ms ± 4%  13.4ms ±20%  -43.65%  (p=0.000 n=9+10)
	ExportToSst/rocksdb/numKeys=65536/numRevisions=10-12    264ms ± 4%   201ms ±24%  -23.92%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=65536/numRevisions=100-12   1.88s ± 6%   1.23s ± 8%  -34.70%  (p=0.000 n=10+8)

And some with contention=true:

	name                                                                   old time/op  new time/op  delta
	ExportToSst/rocksdb/numKeys=65536/numRevisions=10/contention=true-12    362ms ± 7%   168ms ± 3%  -53.60%  (p=0.000 n=10+10)
	ExportToSst/rocksdb/numKeys=65536/numRevisions=100/contention=true-12   2.24s ± 6%   1.24s ±10%  -44.50%  (p=0.000 n=10+10)

Release note: None.

49815: roachpb: refuse nil desc in NewRangeKeyMismatchError r=andreimatei a=andreimatei

Since recently RangeKeyMismatchError does not support nil descriptors,
but it still had code that pretended to deal with nils (even though a
nil would have exploded a bit later). Only one test caller was passing a
nil, and it turns out that was dead code.

Release note: None

Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
@craig craig bot closed this as completed in b527d66 Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: serialize UDTs in expressions in a stable way rohany/cockroach
1 participant
@rohany