Skip to content

Commit

Permalink
sql: refactoring virtualSchemaTable.allTableNames into unimplementedT…
Browse files Browse the repository at this point in the history 
…ableNames

Previously, allTableNames have implemented and unimplemented tableNames
This was inadequate because the duplicity makes difficult to maintain
To address this, this patch refactors allTableNames into
unimplementedTableNames to keep only the tables that are not in tableDefs

Release note: None

Fixes cockroachdb#61801
  • Loading branch information
MiguelNovelo committed Apr 6, 2021
1 parent 2a16884 commit cc0c22b
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 152 deletions.
25 changes: 1 addition & 24 deletions pkg/sql/information_schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ var pgCatalogNameDString = tree.NewDString(pgCatalogName)
// information_schema.
var informationSchema = virtualSchema{
name: sessiondata.InformationSchemaName,
allTableNames: buildStringSet(
unimplementedTableNames: buildStringSet(
// Generated with:
// select distinct '"'||table_name||'",' from information_schema.tables
// where table_schema='information_schema' order by table_name;
Expand All @@ -57,60 +57,38 @@ var informationSchema = virtualSchema{
"_pg_foreign_table_columns",
"_pg_foreign_tables",
"_pg_user_mappings",
"administrable_role_authorizations",
"applicable_roles",
"attributes",
"character_sets",
"check_constraint_routine_usage",
"check_constraints",
"collation_character_set_applicability",
"collations",
"column_domain_usage",
"column_options",
"column_privileges",
"column_udt_usage",
"columns",
"constraint_column_usage",
"constraint_table_usage",
"data_type_privileges",
"domain_constraints",
"domain_udt_usage",
"domains",
"element_types",
"enabled_roles",
"foreign_data_wrapper_options",
"foreign_data_wrappers",
"foreign_server_options",
"foreign_servers",
"foreign_table_options",
"foreign_tables",
"information_schema_catalog_name",
"key_column_usage",
"parameters",
"referential_constraints",
"role_column_grants",
"role_routine_grants",
"role_table_grants",
"role_udt_grants",
"role_usage_grants",
"routine_privileges",
"routines",
"schemata",
"sequences",
"sql_features",
"sql_implementation_info",
"sql_languages",
"sql_packages",
"sql_parts",
"sql_sizing",
"sql_sizing_profiles",
"table_constraints",
"table_privileges",
"tables",
"transforms",
"triggered_update_columns",
"triggers",
"type_privileges",
"udt_privileges",
"usage_privileges",
"user_defined_types",
Expand All @@ -119,7 +97,6 @@ var informationSchema = virtualSchema{
"view_column_usage",
"view_routine_usage",
"view_table_usage",
"views",
),
tableDefs: map[descpb.ID]virtualSchemaDef{
catconstants.InformationSchemaAdministrableRoleAuthorizationsID: informationSchemaAdministrableRoleAuthorizations,
Expand Down
84 changes: 1 addition & 83 deletions pkg/sql/pg_catalog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,82 +79,18 @@ var invertedIndexOid = stringOid(indexTypeInvertedIndex)
// in https://www.postgresql.org/docs/9.6/static/catalogs.html.
var pgCatalog = virtualSchema{
name: pgCatalogName,
allTableNames: buildStringSet(
unimplementedTableNames: buildStringSet(
// Generated with:
// select distinct '"'||table_name||'",' from information_schema.tables
// where table_schema='pg_catalog' order by table_name;
"pg_aggregate",
"pg_am",
"pg_amop",
"pg_amproc",
"pg_attrdef",
"pg_attribute",
"pg_auth_members",
"pg_authid",
"pg_available_extension_versions",
"pg_available_extensions",
"pg_cast",
"pg_class",
"pg_collation",
"pg_config",
"pg_constraint",
"pg_conversion",
"pg_cursors",
"pg_database",
"pg_db_role_setting",
"pg_default_acl",
"pg_depend",
"pg_description",
"pg_enum",
"pg_event_trigger",
"pg_extension",
"pg_file_settings",
"pg_foreign_data_wrapper",
"pg_foreign_server",
"pg_foreign_table",
"pg_group",
"pg_hba_file_rules",
"pg_index",
"pg_indexes",
"pg_inherits",
"pg_init_privs",
"pg_language",
"pg_largeobject",
"pg_largeobject_metadata",
"pg_locks",
"pg_matviews",
"pg_namespace",
"pg_opclass",
"pg_operator",
"pg_opfamily",
"pg_partitioned_table",
"pg_pltemplate",
"pg_policies",
"pg_policy",
"pg_prepared_statements",
"pg_prepared_xacts",
"pg_proc",
"pg_publication",
"pg_publication_rel",
"pg_publication_tables",
"pg_range",
"pg_replication_origin",
"pg_replication_origin_status",
"pg_replication_slots",
"pg_rewrite",
"pg_roles",
"pg_rules",
"pg_seclabel",
"pg_seclabels",
"pg_sequence",
"pg_sequences",
"pg_settings",
"pg_shadow",
"pg_shdepend",
"pg_shdescription",
"pg_shmem_allocations",
"pg_shseclabel",
"pg_stat_activity",
"pg_stat_all_indexes",
"pg_stat_all_tables",
"pg_stat_archiver",
Expand Down Expand Up @@ -185,26 +121,8 @@ var pgCatalog = virtualSchema{
"pg_statio_user_sequences",
"pg_statio_user_tables",
"pg_statistic",
"pg_statistic_ext",
"pg_stats",
"pg_subscription",
"pg_subscription_rel",
"pg_tables",
"pg_tablespace",
"pg_timezone_abbrevs",
"pg_timezone_names",
"pg_transform",
"pg_trigger",
"pg_ts_config",
"pg_ts_config_map",
"pg_ts_dict",
"pg_ts_parser",
"pg_ts_template",
"pg_type",
"pg_user",
"pg_user_mapping",
"pg_user_mappings",
"pg_views",
),
tableDefs: map[descpb.ID]virtualSchemaDef{
catconstants.PgCatalogAggregateTableID: pgCatalogAggregateTable,
Expand Down
97 changes: 68 additions & 29 deletions pkg/sql/pg_metadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
Expand Down Expand Up @@ -72,15 +74,15 @@ var (

// strings used on constants creations and text manipulation.
const (
pgCatalogPrefix = "PgCatalog"
pgCatalogIDConstant = "PgCatalogID"
tableIDSuffix = "TableID"
tableDefsDeclaration = `tableDefs: map[descpb.ID]virtualSchemaDef{`
tableDefsTerminal = `},`
allTableNamesDeclaration = `allTableNames: buildStringSet(`
allTableNamesTerminal = `),`
virtualTablePosition = `// typOid is the only OID generation approach that does not use oidHasher, because`
virtualTableTemplate = `var %s = virtualSchemaTable{
pgCatalogPrefix = "PgCatalog"
pgCatalogIDConstant = "PgCatalogID"
tableIDSuffix = "TableID"
tableDefsDeclaration = `tableDefs: map[descpb.ID]virtualSchemaDef{`
tableDefsTerminal = `},`
unimplementedTableNamesDeclaration = `unimplementedTableNames: buildStringSet(`
unimplementedTableNamesTerminal = `),`
virtualTablePosition = `// typOid is the only OID generation approach that does not use oidHasher, because`
virtualTableTemplate = `var %s = virtualSchemaTable{
comment: "%s was created for compatibility and is currently unimplemented",
schema: vtable.%s,
populate: func(ctx context.Context, p *planner, _ *dbdesc.Immutable, addRow func(...tree.Datum) error) error {
Expand Down Expand Up @@ -288,10 +290,13 @@ func fixVtable(t *testing.T, notImplemented PGMetadataTables) {
})
}

// fixPgCatalogGo will update pgCatalog.allTableNames, pgCatalog.tableDefs and
// fixPgCatalogGo will update pgCatalog.unimplementedTableNames, pgCatalog.tableDefs and
// will add needed virtualSchemas.
func fixPgCatalogGo(notImplemented PGMetadataTables) {
allTableNamesText := getAllTableNamesText(notImplemented)
func fixPgCatalogGo(t *testing.T, notImplemented PGMetadataTables) {
unimplementedTableNamesText, err := getUnimplementedTableNamesText(notImplemented, pgCatalog)
if err != nil {
t.Fatal(err)
}
tableDefinitionText := getTableDefinitionsText(pgCatalogGo, notImplemented)

rewriteFile(pgCatalogGo, func(input *os.File, output outputFile) {
Expand All @@ -309,8 +314,8 @@ func fixPgCatalogGo(notImplemented PGMetadataTables) {
switch trimText {
case tableDefsDeclaration:
printBeforeTerminalString(reader, output, tableDefsTerminal, tableDefinitionText)
case allTableNamesDeclaration:
printBeforeTerminalString(reader, output, allTableNamesTerminal, allTableNamesText)
case unimplementedTableNamesDeclaration:
printBeforeTerminalString(reader, output, unimplementedTableNamesTerminal, unimplementedTableNamesText)
}
}
})
Expand Down Expand Up @@ -410,7 +415,7 @@ func updateFile(inputFileName, outputFileName string, f func(input *os.File, out
}
defer dClose(input)

output, err := os.OpenFile(outputFileName, os.O_CREATE|os.O_WRONLY, 0644)
output, err := os.OpenFile(outputFileName, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
if err != nil {
panic(fmt.Errorf("error opening file %s: %v", outputFileName, err))
}
Expand Down Expand Up @@ -506,25 +511,59 @@ func printVirtualSchemas(newTableNameList PGMetadataTables) string {
return sb.String()
}

// getAllTableNamesText retrieves pgCatalog.allTableNames, then it merges the
// getTableNameFromCreateTable uses pkg/sql/parser to analize CREATE TABLE
// statement to retrieve table name
func getTableNameFromCreateTable(createTableText string) (string, error) {
stmt, err := parser.ParseOne(createTableText)
if err != nil {
return "", err
}

create := stmt.AST.(*tree.CreateTable)
return create.Table.Table(), nil
}

// getUnimplementedTableNamesText retrieves pgCatalog.unimplementedTableNames, then it merges the
// new table names and formats the replacement text.
func getAllTableNamesText(notImplemented PGMetadataTables) string {
newTableNameSet := make(map[string]struct{})
for tableName := range pgCatalog.allTableNames {
newTableNameSet[tableName] = none
func getUnimplementedTableNamesText(
notImplemented PGMetadataTables, vs virtualSchema,
) (string, error) {
newTableList, err := getUnimplementedTableNamesList(notImplemented, vs)
if err != nil {
return "", err
}
for tableName := range notImplemented {
newTableNameSet[tableName] = none
return formatUnimplementedTableNamesText(newTableList), nil
}

func getUnimplementedTableNamesList(
newTables PGMetadataTables, vs virtualSchema,
) ([]string, error) {
var unimplementedTableList []string
removeTables := make(map[string]struct{})
for _, table := range vs.tableDefs {
tableName, err := getTableNameFromCreateTable(table.getSchema())
if err != nil {
return nil, err
}

removeTables[tableName] = struct{}{}
}
newTableList := make([]string, 0, len(newTableNameSet))
for tableName := range newTableNameSet {
newTableList = append(newTableList, tableName)

for tableName := range newTables {
removeTables[tableName] = struct{}{}
}
sort.Strings(newTableList)
return formatAllTableNamesText(newTableList)

for tableName := range vs.unimplementedTableNames {
if _, ok := removeTables[tableName]; !ok {
unimplementedTableList = append(unimplementedTableList, tableName)
}
}

sort.Strings(unimplementedTableList)
return unimplementedTableList, nil
}

func formatAllTableNamesText(newTableNameList []string) string {
func formatUnimplementedTableNamesText(newTableNameList []string) string {
var sb strings.Builder
for _, tableName := range newTableNameList {
sb.WriteString("\t\t\"")
Expand Down Expand Up @@ -678,6 +717,6 @@ func TestPGCatalog(t *testing.T) {
unimplemented := diffs.getUnimplementedTables(pgTables)
fixConstants(t, unimplemented)
fixVtable(t, unimplemented)
fixPgCatalogGo(unimplemented)
fixPgCatalogGo(t, unimplemented)
}
}
Loading

0 comments on commit cc0c22b

Please sign in to comment.