Skip to content

Commit

Permalink
Merge #67994
Browse files Browse the repository at this point in the history
67994: importccl: fail PGDUMP IMPORT if we see a UDT r=pbardea a=adityamaru

In 21.1 we added `ignore_unsupported_statements` that ignored
all statements that we didnt support while parsing the PGDUMP
file. We don't support `CREATE TYPE` and its subsequent usage,
and this currently causes a NPE.

While we work on adding UDT support to PGDUMP we should make
an exception to the ignore option and fail the import instead.

Informs: #67983

Release note (bug fix): IMPORT PGDUMP with a UDT would result
in a nil pointer exception. This change makes it fail gracefully.

Co-authored-by: Aditya Maru <[email protected]>
  • Loading branch information
craig[bot] and adityamaru committed Aug 5, 2021
2 parents 6d4639a + 01b50fa commit 7285f58
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 2 deletions.
6 changes: 5 additions & 1 deletion pkg/ccl/importccl/import_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,12 @@ func importPlanHook(
"IMPORT to REGIONAL BY ROW table not supported",
)
}
// IMPORT TABLE do not support user defined types, and so we nil out the
// type resolver to protect against unexpected behavior on UDT
// resolution.
semaCtxPtr := makeSemaCtxWithoutTypeResolver(p.SemaCtx())
tbl, err := MakeSimpleTableDescriptor(
ctx, p.SemaCtx(), p.ExecCfg().Settings, create, db, sc, defaultCSVTableID, NoFKs, walltime)
ctx, semaCtxPtr, p.ExecCfg().Settings, create, db, sc, defaultCSVTableID, NoFKs, walltime)
if err != nil {
return err
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/ccl/importccl/import_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,30 @@ CREATE INDEX i ON t USING btree (a) WHERE (b > 10);
`,
err: "cannot import a table with partial indexes",
},
{
name: "user defined type",
typ: "PGDUMP",
data: `
CREATE TYPE duration AS ENUM (
'YESTERDAY',
'LAST_7_DAYS',
'LAST_28_DAYS',
'LAST_90_DAYS',
'LAST_365_DAYS',
'LIFE_TIME'
);
CREATE TABLE t (a duration);
`,
err: "IMPORT PGDUMP does not support user defined types",
},
{
name: "user defined type without create",
typ: "PGDUMP",
data: `
CREATE TABLE t (a duration);
`,
err: "type \"duration\" does not exist",
},

// Error
{
Expand Down
6 changes: 6 additions & 0 deletions pkg/ccl/importccl/import_table_creation.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ func MakeTestingSimpleTableDescriptor(
return MakeSimpleTableDescriptor(ctx, semaCtx, st, create, db, sc, tableID, fks, walltime)
}

func makeSemaCtxWithoutTypeResolver(semaCtx *tree.SemaContext) *tree.SemaContext {
semaCtxCopy := *semaCtx
semaCtxCopy.TypeResolver = nil
return &semaCtxCopy
}

// MakeSimpleTableDescriptor creates a tabledesc.Mutable from a CreateTable
// parse node without the full machinery. Many parts of the syntax are
// unsupported (see the implementation and TestMakeSimpleTableDescriptorErrors
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/importccl/read_import_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,10 @@ func mysqlTableToCockroach(
if p != nil {
semaCtxPtr = p.SemaCtx()
}

// Bundle imports do not support user defined types, and so we nil out the
// type resolver to protect against unexpected behavior on UDT resolution.
semaCtxPtr = makeSemaCtxWithoutTypeResolver(semaCtxPtr)
desc, err := MakeSimpleTableDescriptor(
ctx, semaCtxPtr, evalCtx.Settings, stmt, parentDB,
schemadesc.GetPublicSchema(), id, fks, time.WallTime,
Expand Down
8 changes: 7 additions & 1 deletion pkg/ccl/importccl/read_import_pgdump.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,10 @@ func createPostgresTables(
return nil, err
}
removeDefaultRegclass(create)
desc, err := MakeSimpleTableDescriptor(evalCtx.Ctx(), p.SemaCtx(), p.ExecCfg().Settings,
// Bundle imports do not support user defined types, and so we nil out the
// type resolver to protect against unexpected behavior on UDT resolution.
semaCtxPtr := makeSemaCtxWithoutTypeResolver(p.SemaCtx())
desc, err := MakeSimpleTableDescriptor(evalCtx.Ctx(), semaCtxPtr, p.ExecCfg().Settings,
create, parentDB, schema, getNextPlaceholderDescID(), fks, walltime)
if err != nil {
return nil, err
Expand Down Expand Up @@ -858,6 +861,9 @@ func readPostgresStmt(
return unsupportedStmtLogger.log(fmt.Sprintf("%s", stmt), false /* isParseError */)
}
return wrapErrorWithUnsupportedHint(errors.Errorf("unsupported %T statement: %s", stmt, stmt))
case *tree.CreateType:
return errors.New("IMPORT PGDUMP does not support user defined types; please" +
" remove all CREATE TYPE statements and their usages from the dump file")
case error:
if !errors.Is(stmt, errCopyDone) {
return stmt
Expand Down

0 comments on commit 7285f58

Please sign in to comment.