catalog: improve how TableColMap handles system columns#72676
catalog: improve how TableColMap handles system columns#72676craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
RaduBerinde
left a comment
There was a problem hiding this comment.
Cool change!
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @jordanlewis, and @yuzefovich)
pkg/sql/catalog/table_col_map.go, line 30 at r1 (raw file):
type TableColMap struct { m util.FastIntMap // systemColMap maps all system columns to their values. Columns here are
I think a unit test for this structure is warranted now.
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously, `TableColMap` would very often incur an allocation when mapping from system columns are set (because system columns have very large IDs). This commit teaches `TableColMap` to handle the system columns separately which allows us to remove those allocations. Release note: None
147b4c9 to
812551a
Compare
yuzefovich
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/sql/catalog/table_col_map.go, line 30 at r1 (raw file):
Previously, RaduBerinde wrote…
I think a unit test for this structure is warranted now.
Good point, added. This actually revealed a minor bug in String implementation (forgetting to use spaces between mapping of system columns).
|
Build succeeded: |
Previously,
TableColMapwould very often incur an allocation whenmapping from system columns are set (because system columns have very
large IDs). This commit teaches
TableColMapto handle the systemcolumns separately which allows us to remove those allocations.
Release note: None