catalog: improve how TableColMap handles system columns

[yuzefovich]

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

[cockroach-teamcity]

This change is 

[yuzefovich]

name                                           old time/op    new time/op    delta
FlowSetup/vectorize=true/distribute=true-24       220µs ± 2%     214µs ± 4%  -2.56%  (p=0.023 n=10+10)
FlowSetup/vectorize=true/distribute=false-24      212µs ± 2%     210µs ± 3%    ~     (p=0.143 n=10+10)
FlowSetup/vectorize=false/distribute=true-24      214µs ± 5%     208µs ± 4%  -2.62%  (p=0.035 n=10+10)
FlowSetup/vectorize=false/distribute=false-24     203µs ± 3%     202µs ± 3%    ~     (p=0.720 n=9+10)

name                                           old alloc/op   new alloc/op   delta
FlowSetup/vectorize=true/distribute=true-24      33.3kB ± 2%    32.1kB ± 3%  -3.59%  (p=0.001 n=9+9)
FlowSetup/vectorize=true/distribute=false-24     31.3kB ± 1%    30.1kB ± 0%  -3.69%  (p=0.000 n=8+9)
FlowSetup/vectorize=false/distribute=true-24     38.2kB ± 0%    37.1kB ± 1%  -3.04%  (p=0.000 n=8+8)
FlowSetup/vectorize=false/distribute=false-24    36.5kB ± 0%    35.3kB ± 0%  -3.18%  (p=0.000 n=9+9)

name                                           old allocs/op  new allocs/op  delta
FlowSetup/vectorize=true/distribute=true-24         270 ± 4%       267 ± 1%  -1.14%  (p=0.008 n=10+9)
FlowSetup/vectorize=true/distribute=false-24        254 ± 0%       252 ± 0%  -0.79%  (p=0.001 n=8+9)
FlowSetup/vectorize=false/distribute=true-24        259 ± 0%       257 ± 0%  -0.69%  (p=0.000 n=8+9)
FlowSetup/vectorize=false/distribute=false-24       245 ± 0%       243 ± 0%  -0.82%  (p=0.000 n=8+8)

[RaduBerinde]

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]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @jordanlewis, and @yuzefovich)

[yuzefovich]

TFTR!

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner, @jordanlewis, and @RaduBerinde)

---

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).

[craig]

Build succeeded:

• GitHub CI (Cockroach)