Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: improved diff tool for any namespace #60283

Merged
merged 1 commit into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")

go_library(
name = "generate-pg-catalog_lib",
name = "generate-postgres-metadata-tables_lib",
srcs = ["main.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/cmd/generate-pg-catalog",
importpath = "github.com/cockroachdb/cockroach/pkg/cmd/generate-postgres-metadata-tables",
visibility = ["//visibility:private"],
deps = [
"//pkg/sql",
Expand All @@ -12,7 +12,7 @@ go_library(
)

go_binary(
name = "generate-pg-catalog",
embed = [":generate-pg-catalog_lib"],
name = "generate-postgres-metadata-tables",
embed = [":generate-postgres-metadata-tables_lib"],
visibility = ["//visibility:public"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,18 @@ import (
const getServerVersion = `SELECT current_setting('server_version');`

var (
postgresAddr = flag.String("addr", "localhost:5432", "Postgres server address")
postgresUser = flag.String("user", "postgres", "Postgres user")
postgresAddr = flag.String("addr", "localhost:5432", "Postgres server address")
postgresUser = flag.String("user", "postgres", "Postgres user")
postgresSchema = flag.String("catalog", "pg_catalog", "Catalog or namespace, default: pg_catalog")
)

func main() {
flag.Parse()
db := connect()
defer closeDB(db)
pgCatalogFile := &sql.PGCatalogFile{
PgVersion: getPGVersion(db),
PgCatalog: sql.PGCatalogTables{},
pgCatalogFile := &sql.PGMetadataFile{
PGVersion: getPGVersion(db),
PGMetadata: sql.PGMetadataTables{},
}

rows := describePgCatalog(db)
Expand All @@ -55,14 +56,14 @@ func main() {
if err := rows.Scan(&table, &column, &dataType, &dataTypeOid); err != nil {
panic(err)
}
pgCatalogFile.PgCatalog.AddColumnMetadata(table, column, dataType, dataTypeOid)
pgCatalogFile.PGMetadata.AddColumnMetadata(table, column, dataType, dataTypeOid)
}

pgCatalogFile.Save(os.Stdout)
}

func describePgCatalog(conn *pgx.Conn) *pgx.Rows {
rows, err := conn.Query(sql.GetPGCatalogSQL)
rows, err := conn.Query(sql.GetPGMetadataSQL, *postgresSchema)
if err != nil {
panic(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ go_library(
"partition.go",
"partition_utils.go",
"pg_catalog.go",
"pg_catalog_diff.go",
"pg_extension.go",
"pg_metadata_diff.go",
"plan.go",
"plan_batch.go",
"plan_columns.go",
Expand Down Expand Up @@ -449,7 +449,7 @@ go_test(
"namespace_test.go",
"old_foreign_key_desc_test.go",
"partition_test.go",
"pg_catalog_test.go",
"pg_metadata_test.go",
"pg_oid_test.go",
"pgwire_internal_test.go",
"plan_opt_test.go",
Expand Down
70 changes: 35 additions & 35 deletions pkg/sql/pg_catalog_diff.go → pkg/sql/pg_metadata_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// licenses/APL.txt.

// This file contains constants and types used by pg_catalog diff tool
// that are also re-used in /pkg/cmd/generate-pg-catalog
// that are also re-used in /pkg/cmd/generate-postgres-metadata-tables

package sql

Expand All @@ -19,9 +19,9 @@ import (
"os"
)

// GetPGCatalogSQL is a query uses udt_name::regtype instead of data_type column because
// GetPGMetadataSQL is a query uses udt_name::regtype instead of data_type column because
// data_type only says "ARRAY" but does not say which kind of array it is.
const GetPGCatalogSQL = `
const GetPGMetadataSQL = `
SELECT
c.relname AS table_name,
a.attname AS column_name,
Expand All @@ -31,58 +31,58 @@ const GetPGCatalogSQL = `
JOIN pg_attribute a ON a.attrelid = c.oid
JOIN pg_type t ON t.oid = a.atttypid
JOIN pg_namespace n ON n.oid = c.relnamespace
WHERE n.nspname = 'pg_catalog'
WHERE n.nspname = $1
AND a.attnum > 0
ORDER BY 1, 2;
`

// PGCatalogColumn is a structure which contains a small description about the datatype of a column, but this can also be
// PGMetadataColumnType is a structure which contains a small description about the datatype of a column, but this can also be
// used as a diff information if populating ExpectedOid. Fields are exported for Marshaling purposes.
type PGCatalogColumn struct {
type PGMetadataColumnType struct {
Oid uint32 `json:"oid"`
DataType string `json:"dataType"`
ExpectedOid *uint32 `json:"expectedOid"`
ExpectedDataType *string `json:"expectedDataType"`
}

// PGCatalogColumns maps column names to datatype description
type PGCatalogColumns map[string]*PGCatalogColumn
// PGMetadataColumns maps column names to datatype description
type PGMetadataColumns map[string]*PGMetadataColumnType

// PGCatalogTables have 2 use cases:
// PGMetadataTables have 2 use cases:
// First: This is used to model pg_schema for postgres and cockroach db for comparison purposes by mapping tableNames
// to columns.
// Second: This is used to store and load expected diffs:
// - Using it this way, a table name pointing to a zero length PGCatalogColumns means that we expect this table to be missing
// - Using it this way, a table name pointing to a zero length PGMetadataColumns means that we expect this table to be missing
// in cockroach db
// - If PGCatalogColumns is not empty but columnName points to null, we expect that column to be missing in that table in
// - If PGMetadataColumns is not empty but columnName points to null, we expect that column to be missing in that table in
// cockroach db
// - If column Name points to a not null PGCatalogColumn, the test column describes how we expect that data type to be
// - If column Name points to a not null PGMetadataColumnType, the test column describes how we expect that data type to be
// different between cockroach db and postgres
type PGCatalogTables map[string]PGCatalogColumns
type PGMetadataTables map[string]PGMetadataColumns

// PGCatalogFile is used to export pg_catalog from postgres and store the representation of this structure as a
// PGMetadataFile is used to export pg_catalog from postgres and store the representation of this structure as a
// json file
type PGCatalogFile struct {
PgVersion string `json:"pgVersion"`
PgCatalog PGCatalogTables `json:"pgCatalog"`
type PGMetadataFile struct {
PGVersion string `json:"pgVersion"`
PGMetadata PGMetadataTables `json:"pgMetadata"`
}

func (p PGCatalogTables) addColumn(tableName, columnName string, column *PGCatalogColumn) {
func (p PGMetadataTables) addColumn(tableName, columnName string, column *PGMetadataColumnType) {
columns, ok := p[tableName]

if !ok {
columns = make(PGCatalogColumns)
columns = make(PGMetadataColumns)
p[tableName] = columns
}

columns[columnName] = column
}

// AddColumnMetadata is used to load data from postgres or cockroach pg_catalog schema
func (p PGCatalogTables) AddColumnMetadata(
func (p PGMetadataTables) AddColumnMetadata(
tableName string, columnName string, dataType string, dataTypeOid uint32,
) {
p.addColumn(tableName, columnName, &PGCatalogColumn{
p.addColumn(tableName, columnName, &PGMetadataColumnType{
dataTypeOid,
dataType,
nil,
Expand All @@ -91,10 +91,10 @@ func (p PGCatalogTables) AddColumnMetadata(
}

// addDiff is for the second use case for pgTables which objective is create a datatype diff
func (p PGCatalogTables) addDiff(
tableName string, columnName string, expected *PGCatalogColumn, actual *PGCatalogColumn,
func (p PGMetadataTables) addDiff(
tableName string, columnName string, expected *PGMetadataColumnType, actual *PGMetadataColumnType,
) {
p.addColumn(tableName, columnName, &PGCatalogColumn{
p.addColumn(tableName, columnName, &PGMetadataColumnType{
actual.Oid,
actual.DataType,
&expected.Oid,
Expand All @@ -103,8 +103,8 @@ func (p PGCatalogTables) addDiff(
}

// isDiffOid verifies if there is a datatype mismatch or if the diff is an expected diff
func (p PGCatalogTables) isDiffOid(
tableName string, columnName string, expected *PGCatalogColumn, actual *PGCatalogColumn,
func (p PGMetadataTables) isDiffOid(
tableName string, columnName string, expected *PGMetadataColumnType, actual *PGMetadataColumnType,
) bool {
if expected.Oid == actual.Oid {
return false
Expand All @@ -125,9 +125,9 @@ func (p PGCatalogTables) isDiffOid(
return !(diff.Oid == actual.Oid && *diff.ExpectedOid == expected.Oid)
}

// isExpectedMissingTable is used by the diff PGCatalogTables to verify whether missing a table in cockroach is expected
// isExpectedMissingTable is used by the diff PGMetadataTables to verify whether missing a table in cockroach is expected
// or not
func (p PGCatalogTables) isExpectedMissingTable(tableName string) bool {
func (p PGMetadataTables) isExpectedMissingTable(tableName string) bool {
if columns, ok := p[tableName]; !ok || len(columns) > 0 {
return false
}
Expand All @@ -136,7 +136,7 @@ func (p PGCatalogTables) isExpectedMissingTable(tableName string) bool {
}

// isExpectedMissingColumn is similar to isExpectedMissingTable to verify column expected misses
func (p PGCatalogTables) isExpectedMissingColumn(tableName string, columnName string) bool {
func (p PGMetadataTables) isExpectedMissingColumn(tableName string, columnName string) bool {
columns, ok := p[tableName]
if !ok {
return false
Expand All @@ -151,24 +151,24 @@ func (p PGCatalogTables) isExpectedMissingColumn(tableName string, columnName st
}

// addMissingTable adds a tablename when it is not found in cockroach db
func (p PGCatalogTables) addMissingTable(tableName string) {
p[tableName] = make(PGCatalogColumns)
func (p PGMetadataTables) addMissingTable(tableName string) {
p[tableName] = make(PGMetadataColumns)
}

// addMissingColumn adds a column when it is not found in cockroach db
func (p PGCatalogTables) addMissingColumn(tableName string, columnName string) {
func (p PGMetadataTables) addMissingColumn(tableName string, columnName string) {
columns, ok := p[tableName]

if !ok {
columns = make(PGCatalogColumns)
columns = make(PGMetadataColumns)
p[tableName] = columns
}

columns[columnName] = nil
}

// rewriteDiffs creates pg_catalog_test-diffs.json
func (p PGCatalogTables) rewriteDiffs(diffFile string) error {
func (p PGMetadataTables) rewriteDiffs(diffFile string) error {
f, err := os.OpenFile(diffFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
if err != nil {
return err
Expand All @@ -188,7 +188,7 @@ func (p PGCatalogTables) rewriteDiffs(diffFile string) error {
}

// Save have the purpose of storing all the data retrieved from postgres and useful information as postgres version
func (f *PGCatalogFile) Save(writer io.Writer) {
func (f *PGMetadataFile) Save(writer io.Writer) {
byteArray, err := json.MarshalIndent(f, "", " ")
if err != nil {
panic(err)
Expand Down
35 changes: 19 additions & 16 deletions pkg/sql/pg_catalog_test.go → pkg/sql/pg_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,16 @@ import (

// Test data files
const (
pgCatalogDump = "pg_catalog_tables.json" // PostgreSQL pg_catalog schema
expectedDiffs = "pg_catalog_test_expected_diffs.json" // Contains expected difference between postgres and cockroach
testdata = "testdata" // testdata directory
catalogDump = "%s_tables.json" // PostgreSQL pg_catalog schema
expectedDiffs = "%s_test_expected_diffs.json" // Contains expected difference between postgres and cockroach
testdata = "testdata" // testdata directory
)

// When running test with -rewrite-diffs test will pass and re-create pg_catalog_test-diffs.json
var rewriteFlag = flag.Bool("rewrite-diffs", false, "This will re-create the expected diffs file")
var (
rewriteFlag = flag.Bool("rewrite-diffs", false, "This will re-create the expected diffs file")
catalogName = flag.String("catalog", "pg_catalog", "Catalog or namespace, default: pg_catalog")
)

// summary will keep accountability for any unexpected difference and report it in the log
type summary struct {
Expand All @@ -77,9 +80,9 @@ func (sum *summary) report(t *testing.T) {
}

// loadTestData retrieves the pg_catalog from the dumpfile generated from Postgres
func loadTestData(t testing.TB) PGCatalogTables {
var pgCatalogFile PGCatalogFile
testdataFile := filepath.Join(testdata, pgCatalogDump)
func loadTestData(t testing.TB) PGMetadataTables {
var pgCatalogFile PGMetadataFile
testdataFile := filepath.Join(testdata, fmt.Sprintf(catalogDump, *catalogName))
f, err := os.Open(testdataFile)
if err != nil {
t.Fatal(err)
Expand All @@ -95,17 +98,17 @@ func loadTestData(t testing.TB) PGCatalogTables {
t.Fatal(err)
}

return pgCatalogFile.PgCatalog
return pgCatalogFile.PGMetadata
}

// loadCockroachPgCatalog retrieves pg_catalog schema from cockroach db
func loadCockroachPgCatalog(t testing.TB) PGCatalogTables {
crdbTables := make(PGCatalogTables)
func loadCockroachPgCatalog(t testing.TB) PGMetadataTables {
crdbTables := make(PGMetadataTables)
ctx := context.Background()
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(ctx)
sqlRunner := sqlutils.MakeSQLRunner(db)
rows := sqlRunner.Query(t, GetPGCatalogSQL)
rows := sqlRunner.Query(t, GetPGMetadataSQL, *catalogName)
defer rows.Close()

for rows.Next() {
Expand All @@ -120,15 +123,15 @@ func loadCockroachPgCatalog(t testing.TB) PGCatalogTables {
}

// loadExpectedDiffs get all differences that will be skipped by the this test
func loadExpectedDiffs(t *testing.T) (diffs PGCatalogTables) {
diffs = PGCatalogTables{}
func loadExpectedDiffs(t *testing.T) (diffs PGMetadataTables) {
diffs = PGMetadataTables{}

if *rewriteFlag {
// For rewrite we want this to be empty and get populated
return
}

diffFile := filepath.Join(testdata, expectedDiffs)
diffFile := filepath.Join(testdata, fmt.Sprintf(expectedDiffs, *catalogName))
if _, err := os.Stat(diffFile); err != nil {
if oserror.IsNotExist(err) {
// File does not exists it means diffs are not expected
Expand Down Expand Up @@ -160,7 +163,7 @@ func errorf(t *testing.T, format string, args ...interface{}) {
}
}

func rewriteDiffs(t *testing.T, diffs PGCatalogTables, diffsFile string) {
func rewriteDiffs(t *testing.T, diffs PGMetadataTables, diffsFile string) {
if !*rewriteFlag {
return
}
Expand Down Expand Up @@ -217,5 +220,5 @@ func TestPGCatalog(t *testing.T) {
}

sum.report(t)
rewriteDiffs(t, diffs, filepath.Join(testdata, expectedDiffs))
rewriteDiffs(t, diffs, filepath.Join(testdata, fmt.Sprintf(expectedDiffs, *catalogName)))
}
Loading