sql: Define more columns in pg_catalog.pg_statistic_ext#93274
sql: Define more columns in pg_catalog.pg_statistic_ext#93274craig[bot] merged 1 commit intocockroachdb:masterfrom
pg_catalog.pg_statistic_ext#93274Conversation
|
This change is |
knz
left a comment
There was a problem hiding this comment.
pls also take a review from @rafiss
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @e-mbrown)
-- commits line 7 at r1:
looks to me like a bug fix worthy of a release note.
pkg/sql/pg_catalog.go line 3465 at r1 (raw file):
for _, row := range rows { tableID := tree.MustBeDInt(row[0]) name := tree.MustBeDString(row[1])
Using MustBeDString for the name forces a heap allocation when you convert it back to a Datum below.
You could simply refer to row[1] in the addRow call below instead.
pkg/sql/pg_catalog.go line 3488 at r1 (raw file):
schemaOid(statSchema), // stxnamespace tree.DNull, // stxowner tree.NewDInt(-1), // stxstattarget
You can allocate this DInt just once out of the loop.
6058fc2 to
0ad6ab3
Compare
e-mbrown
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously, knz (Raphael 'kena' Poss) wrote…
looks to me like a bug fix worthy of a release note.
Done.
pkg/sql/pg_catalog.go line 3465 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
Using
MustBeDStringfor the name forces a heap allocation when you convert it back to a Datum below.
You could simply refer torow[1]in the addRow call below instead.
Done.
pkg/sql/pg_catalog.go line 3488 at r1 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
You can allocate this DInt just once out of the loop.
Done.
rafiss
left a comment
There was a problem hiding this comment.
lgtm, with one nit!
Reviewable status:
pkg/sql/pg_catalog.go line 3453 at r2 (raw file):
schema: vtable.PgCatalogStatisticExt, populate: func(ctx context.Context, p *planner, db catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { query := `SELECT "tableID", name, "columnIDs", "statisticID", '{d}'::CHAR[] FROM system.table_statistics;`
nit: the {d} value should be cast to "char"[]
also, can you add a comment explaining what d means here (based on the PG docs)
`pg_catalog.pg_statistic_ext` is now populated with more data. Release note (bug fix): The `stxnamespace`, `stxkind` and `stxstattarget` columns are now defined in `pg_statistics_ext`.
rafiss
left a comment
There was a problem hiding this comment.
nice fix! i'm adding a backport label as well
Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status:
|
bors r=rafiss |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 5fb4c60 to blathers/backport-release-22.1-93274: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. error creating merge commit from 5fb4c60 to blathers/backport-release-22.2-93274: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Epic: CRDB-23454
Fixes: #88108
Fixes: #93430
This commit makes sure the
stxnamespace,stxkindandstxstattargetcolumns are defined inpg_statistics_ext.Release note: The
stxnamespace,stxkindandstxstattargetcolumns are now defined inpg_statistics_ext. Alsopg_statistics_extno longer crashes when stxname is null.