sql: enum introspection improvements#57197
Conversation
Previously, `SHOW ENUMS` was restricted to all schemas in the current database. Other analogous `SHOW` commands, such as `SHOW TABLES`, can optionally take a schema_name or a db_name.schema_name and only show objects under that hierarchy. This patch brings this functionality to the `SHOW ENUMS` statement as well. Closes cockroachdb#57196 Release note (sql change): `SHOW ENUMS` is now extended to take an optional "FROM" clause. The user can specify either the schema name or both the db name and schema name separated by a '.'. If a hierarchy is specified so, the statement returns ENUMS falling in that hierarchy rather than all of the enums in the current database.
|
This change is |
otan
left a comment
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, 3 of 3 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @arulajmani)
pkg/sql/pg_catalog.go, line 1303 at r2 (raw file):
return forEachTypeDesc(ctx, p, dbContext, func(_ *dbdesc.Immutable, _ string, typDesc *typedesc.Immutable) error { // We only want to iterate over ENUM types and multi-region enums. if typDesc.Kind != descpb.TypeDescriptor_ENUM &&
nit: nicer as a switch statement:
switch typDesc.Kind {
case ENUM,
MULTIREGION_ENUM:
return nil
}
pkg/sql/delegate/show_enums.go, line 63 at r1 (raw file):
WHERE types.typtype = 'e' %[2]s ORDER BY (nsp.nspname, types.typname) `, &name.CatalogName, schemaClause)
should this be lex.EscapeSQLString(name.CatalogName) ?
After this patch, multi-region enums can be introspected using the `pg_catalog.pg_enum` virtual table. As a consequence, they can also be viewed by issuing the `SHOW ENUMS` query or one of its variants. Unlike user defined enums, multi-region enums don't show `CREATE` statements, as they are constructed implicitly. Closes cockroachdb#57087 Release note (sql change): The multi-region enum, created implicitly for all multiregion databases, can be introspected using the `pg_catalog.pg_enum` table. It is also displayed in `SHOW ENUMS`.
c4c7078 to
a23d529
Compare
arulajmani
left a comment
There was a problem hiding this comment.
RFAL
Reviewable status:
pkg/sql/pg_catalog.go, line 1303 at r2 (raw file):
Previously, otan (Oliver Tan) wrote…
nit: nicer as a switch statement:
switch typDesc.Kind { case ENUM, MULTIREGION_ENUM: return nil }
Done
pkg/sql/delegate/show_enums.go, line 63 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
should this be lex.EscapeSQLString(name.CatalogName) ?
Naa. We're using this to construct a FQN of the pg_enum table, which won't work if we put quotes around the db name.
jordanlewis
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/delegate/show_enums.go, line 63 at r1 (raw file):
Previously, arulajmani (Arul Ajmani) wrote…
Naa. We're using this to construct a FQN of the pg_enum table, which won't work if we put quotes around the db name.
Try this out:
CREATE DATABASE "I am tricksy";
SHOW ENUMS FROM "I am tricksy";
|
pkg/sql/delegate/show_enums.go, line 63 at r1 (raw file): Previously, jordanlewis (Jordan Lewis) wrote…
yeah and use |
arulajmani
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/delegate/show_enums.go, line 63 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
yeah and use
EncodeRestrictedSQLIdentin that case.
Talked w @otan offline -- the case above works because the x.Name.String() escapes correctly, so we don't need anything special here.
otan
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:
|
TFTR! bors r=otan |
|
Build failed: |
|
bors r=otan |
|
Build succeeded: |
See individual commit messages for details.