Skip to content

Commit

Permalink
sql: fix CREATE MATERIALIZED VIEW AS schema change job description
Browse files Browse the repository at this point in the history
Fixes #107445

This changes the CREATE MATERIALIZED VIEW AS schema change job description SQL
syntax.
For example
```
CREATE VIEW "v" AS "SELECT t.id FROM movr.public.t";
```
becomes
```
CREATE MATERIALIZED VIEW defaultdb.public.v AS SELECT t.id FROM defaultdb.public.t WITH DATA;
```

Release note (bug fix): Fix CREATE MATERIALIZED VIEW AS schema change job description SQL syntax.
  • Loading branch information
ecwall committed Jul 24, 2023
1 parent bebdf99 commit b59fc36
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 94 deletions.
16 changes: 15 additions & 1 deletion pkg/sql/create_as_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,16 @@ func TestFormat(t *testing.T) {
setup: "CREATE TABLE ctas_explicit_columns_source_tbl (id int PRIMARY KEY)",
expectedFormat: "CREATE TABLE defaultdb.public.ctas_explicit_columns_tbl (id) AS SELECT * FROM defaultdb.public.ctas_explicit_columns_source_tbl",
},
{
sql: "CREATE MATERIALIZED VIEW cmvas_implicit_columns_tbl AS SELECT * FROM cmvas_implicit_columns_source_tbl",
setup: "CREATE TABLE cmvas_implicit_columns_source_tbl (id int PRIMARY KEY)",
expectedFormat: "CREATE MATERIALIZED VIEW defaultdb.public.cmvas_implicit_columns_tbl AS SELECT cmvas_implicit_columns_source_tbl.id FROM defaultdb.public.cmvas_implicit_columns_source_tbl WITH DATA",
},
{
sql: "CREATE MATERIALIZED VIEW cmvas_explicit_columns_tbl (id2) AS SELECT * FROM cmvas_explicit_columns_source_tbl",
setup: "CREATE TABLE cmvas_explicit_columns_source_tbl (id int PRIMARY KEY)",
expectedFormat: "CREATE MATERIALIZED VIEW defaultdb.public.cmvas_explicit_columns_tbl (id2) AS SELECT cmvas_explicit_columns_source_tbl.id FROM defaultdb.public.cmvas_explicit_columns_source_tbl WITH DATA",
},
}

ctx := context.Background()
Expand All @@ -342,14 +352,18 @@ func TestFormat(t *testing.T) {
switch stmt := statements[0].AST.(type) {
case *tree.CreateTable:
name = stmt.Table.Table()
case *tree.CreateView:
name = stmt.Name.Table()
default:
require.Failf(t, "missing case", "unexpected type %T", stmt)
}
// Filter description starting with CREATE to filter out CMVAS
// "updating view reference" job.
query := fmt.Sprintf(
`SELECT description
FROM [SHOW JOBS]
WHERE job_type IN ('SCHEMA CHANGE', 'NEW SCHEMA CHANGE')
AND description LIKE '%%%s%%'`,
AND description LIKE 'CREATE%%%s%%'`,
name,
)
sqlRunner.CheckQueryResults(t, query, [][]string{{tc.expectedFormat}})
Expand Down
57 changes: 24 additions & 33 deletions pkg/sql/create_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,12 @@ import (

// createViewNode represents a CREATE VIEW statement.
type createViewNode struct {
// viewName is the fully qualified name of the new view.
viewName *tree.TableName
createView *tree.CreateView
// viewQuery contains the view definition, with all table names fully
// qualified.
viewQuery string
ifNotExists bool
replace bool
persistence tree.Persistence
materialized bool
dbDesc catalog.DatabaseDescriptor
columns colinfo.ResultColumns
viewQuery string
dbDesc catalog.DatabaseDescriptor
columns colinfo.ResultColumns

// planDeps tracks which tables and views the view being created
// depends on. This is collected during the construction of
Expand All @@ -65,9 +60,6 @@ type createViewNode struct {
// depends on. This is collected during the construction of
// the view query's logical plan.
typeDeps typeDependencies
// withData indicates if a materialized view should be populated
// with data by executing the underlying query.
withData bool
}

// ReadingOwnWrites implements the planNodeReadingOwnWrites interface.
Expand All @@ -76,16 +68,17 @@ type createViewNode struct {
func (n *createViewNode) ReadingOwnWrites() {}

func (n *createViewNode) startExec(params runParams) error {
createView := n.createView
tableType := tree.GetTableType(
false /* isSequence */, true /* isView */, n.materialized,
false /* isSequence */, true /* isView */, createView.Materialized,
)
if n.replace {
if createView.Replace {
telemetry.Inc(sqltelemetry.SchemaChangeCreateCounter(fmt.Sprintf("or_replace_%s", tableType)))
} else {
telemetry.Inc(sqltelemetry.SchemaChangeCreateCounter(tableType))
}

viewName := n.viewName.Object()
viewName := createView.Name.Object()
log.VEventf(params.ctx, 2, "dependencies for view %s:\n%s", viewName, n.planDeps.String())

// Check that the view does not contain references to other databases.
Expand Down Expand Up @@ -120,14 +113,14 @@ func (n *createViewNode) startExec(params runParams) error {
if err != nil {
return err
}
if !n.persistence.IsTemporary() && backRefMutable.Temporary {
if !createView.Persistence.IsTemporary() && backRefMutable.Temporary {
hasTempBackref = true
}
backRefMutables[id] = backRefMutable
}
}
if hasTempBackref {
n.persistence = tree.PersistenceTemporary
createView.Persistence = tree.PersistenceTemporary
// This notice is sent from pg, let's imitate.
params.p.BufferClientNotice(
params.ctx,
Expand All @@ -136,24 +129,24 @@ func (n *createViewNode) startExec(params runParams) error {
}

var replacingDesc *tabledesc.Mutable
schema, err := getSchemaForCreateTable(params, n.dbDesc, n.persistence, n.viewName,
tree.ResolveRequireViewDesc, n.ifNotExists)
schema, err := getSchemaForCreateTable(params, n.dbDesc, createView.Persistence, &createView.Name,
tree.ResolveRequireViewDesc, createView.IfNotExists)
if err != nil && !sqlerrors.IsRelationAlreadyExistsError(err) {
return err
}
if err != nil {
switch {
case n.ifNotExists:
case createView.IfNotExists:
return nil
case n.replace:
case createView.Replace:
// If we are replacing an existing view see if what we are
// replacing is actually a view.
id, err := params.p.Descriptors().LookupObjectID(
params.ctx,
params.p.txn,
n.dbDesc.GetID(),
schema.GetID(),
n.viewName.Table(),
createView.Name.Table(),
)
if err != nil {
return err
Expand All @@ -174,7 +167,7 @@ func (n *createViewNode) startExec(params runParams) error {
}
}

if n.persistence.IsTemporary() {
if createView.Persistence.IsTemporary() {
telemetry.Inc(sqltelemetry.CreateTempViewCounter)
}

Expand Down Expand Up @@ -235,14 +228,14 @@ func (n *createViewNode) startExec(params runParams) error {
&params.p.semaCtx,
params.p.EvalContext(),
params.p.EvalContext().Settings,
n.persistence,
createView.Persistence,
n.dbDesc.IsMultiRegion(),
params.p)
if err != nil {
return err
}

if n.materialized {
if createView.Materialized {
// If the view is materialized, set up some more state on the view descriptor.
// In particular,
// * mark the descriptor as a materialized view
Expand All @@ -254,7 +247,7 @@ func (n *createViewNode) startExec(params runParams) error {
// the table descriptor as requiring a REFRESH VIEW to indicate the view
// should only be accessed after a REFRESH VIEW operation has been called
// on it.
desc.RefreshViewRequired = !n.withData
desc.RefreshViewRequired = !createView.WithData
desc.State = descpb.DescriptorState_ADD
version := params.ExecCfg().Settings.Version.ActiveVersion(params.ctx)
if err := desc.AllocateIDs(params.ctx, version); err != nil {
Expand Down Expand Up @@ -282,12 +275,10 @@ func (n *createViewNode) startExec(params runParams) error {
desc.DependsOnTypes = append(desc.DependsOnTypes, orderedTypeDeps.Ordered()...)
newDesc = &desc

// TODO (lucy): I think this needs a NodeFormatter implementation. For now,
// do some basic string formatting (not accurate in the general case).
if err = params.p.createDescriptor(
params.ctx,
newDesc,
fmt.Sprintf("CREATE VIEW %q AS %q", n.viewName, n.viewQuery),
tree.AsStringWithFQNames(n.createView, params.Ann()),
); err != nil {
return err
}
Expand Down Expand Up @@ -317,7 +308,7 @@ func (n *createViewNode) startExec(params runParams) error {
params.ctx,
backRefMutable,
descpb.InvalidMutationID,
fmt.Sprintf("updating view reference %q in table %s(%d)", n.viewName,
fmt.Sprintf("updating view reference %q in table %s(%d)", &createView.Name,
updated.desc.GetName(), updated.desc.GetID(),
),
); err != nil {
Expand Down Expand Up @@ -362,7 +353,7 @@ func (n *createViewNode) startExec(params runParams) error {
return params.p.logEvent(params.ctx,
newDesc.ID,
&eventpb.CreateView{
ViewName: n.viewName.FQString(),
ViewName: createView.Name.FQString(),
ViewQuery: n.viewQuery,
})
}()
Expand Down Expand Up @@ -634,7 +625,7 @@ func (p *planner) replaceViewDesc(
ctx,
desc,
descpb.InvalidMutationID,
fmt.Sprintf("removing view reference for %q from %s(%d)", n.viewName,
fmt.Sprintf("removing view reference for %q from %s(%d)", &n.createView.Name,
desc.Name, desc.ID,
),
); err != nil {
Expand Down Expand Up @@ -670,7 +661,7 @@ func (p *planner) replaceViewDesc(
// Since we are replacing an existing view here, we need to write the new
// descriptor into place.
if err := p.writeSchemaChange(ctx, toReplace, descpb.InvalidMutationID,
fmt.Sprintf("CREATE OR REPLACE VIEW %q AS %q", n.viewName, n.viewQuery),
fmt.Sprintf("CREATE OR REPLACE VIEW %q AS %q", &n.createView.Name, n.viewQuery),
); err != nil {
return nil, err
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/sql/distsql_spec_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,17 +1042,12 @@ func (e *distSQLSpecExecFactory) ConstructCreateTableAs(
}

func (e *distSQLSpecExecFactory) ConstructCreateView(
createView *tree.CreateView,
schema cat.Schema,
viewName *cat.DataSourceName,
ifNotExists bool,
replace bool,
persistence tree.Persistence,
materialized bool,
viewQuery string,
columns colinfo.ResultColumns,
deps opt.SchemaDeps,
typeDeps opt.SchemaTypeDeps,
withData bool,
) (exec.Node, error) {
return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: create view")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/event_log
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ SELECT "eventType", "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID'
WHERE "eventType" in ('create_view', 'drop_view')
ORDER BY "timestamp", info
----
create_view 1 {"EventType": "create_view", "Statement": "CREATE VIEW \"\".\"\".v AS SELECT 1", "Tag": "CREATE VIEW", "User": "root", "ViewName": "test.public.v", "ViewQuery": "SELECT 1"}
create_view 1 {"EventType": "create_view", "Statement": "CREATE VIEW test.public.v AS SELECT 1", "Tag": "CREATE VIEW", "User": "root", "ViewName": "test.public.v", "ViewQuery": "SELECT 1"}
drop_view 1 {"EventType": "drop_view", "Statement": "DROP VIEW test.public.v", "Tag": "DROP VIEW", "User": "root", "ViewName": "test.public.v"}


Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/event_log_legacy
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ SELECT "eventType", "reportingID", info::JSONB - 'Timestamp' - 'DescriptorID'
WHERE "eventType" in ('create_view', 'drop_view')
ORDER BY "timestamp", info
----
create_view 1 {"EventType": "create_view", "Statement": "CREATE VIEW \"\".\"\".v AS SELECT 1", "Tag": "CREATE VIEW", "User": "root", "ViewName": "test.public.v", "ViewQuery": "SELECT 1"}
create_view 1 {"EventType": "create_view", "Statement": "CREATE VIEW test.public.v AS SELECT 1", "Tag": "CREATE VIEW", "User": "root", "ViewName": "test.public.v", "ViewQuery": "SELECT 1"}
drop_view 1 {"EventType": "drop_view", "Statement": "DROP VIEW test.public.v", "Tag": "DROP VIEW", "User": "root", "ViewName": "test.public.v"}


Expand Down
7 changes: 1 addition & 6 deletions pkg/sql/opt/exec/execbuilder/statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,12 @@ func (b *Builder) buildCreateView(cv *memo.CreateViewExpr) (execPlan, error) {
cols[i].Typ = md.ColumnMeta(cv.Columns[i].ID).Type
}
root, err := b.factory.ConstructCreateView(
cv.Syntax,
schema,
cv.ViewName,
cv.IfNotExists,
cv.Replace,
cv.Persistence,
cv.Materialized,
cv.ViewQuery,
cols,
cv.Deps,
cv.TypeDeps,
cv.WithData,
)
return execPlan{root: root}, err
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/sql/opt/exec/factory.opt
Original file line number Diff line number Diff line change
Expand Up @@ -621,17 +621,12 @@ define CreateTableAs {

# CreateView implements a CREATE VIEW statement.
define CreateView {
CreateView *tree.CreateView
Schema cat.Schema
ViewName *cat.DataSourceName
IfNotExists bool
Replace bool
Persistence tree.Persistence
Materialized bool
ViewQuery string
Columns colinfo.ResultColumns
deps opt.SchemaDeps
typeDeps opt.SchemaTypeDeps
withData bool
}

# SequenceSelect implements a scan of a sequence as a data source.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/memo/expr_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,7 @@ func FormatPrivate(f *ExprFmtCtx, private interface{}, physProps *physical.Requi

case *CreateViewPrivate:
schema := f.Memo.Metadata().Schema(t.Schema)
fmt.Fprintf(f.Buffer, " %s.%s", schema.Name(), t.ViewName)
fmt.Fprintf(f.Buffer, " %s.%s", schema.Name(), t.Syntax.Name.Table())

case *JoinPrivate:
// Nothing to show; flags are shown separately.
Expand Down
8 changes: 3 additions & 5 deletions pkg/sql/opt/ops/statement.opt
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,11 @@ define CreateView {

[Private]
define CreateViewPrivate {
# Syntax is the CREATE VIEW AST node.
Syntax TreeCreateView

# Schema is the ID of the catalog schema into which the new table goes.
Schema SchemaID
ViewName TableName
Persistence Persistence
IfNotExists bool
Replace bool
Materialized bool

# ViewQuery contains the query for the view; data sources are always fully
# qualified.
Expand Down
32 changes: 15 additions & 17 deletions pkg/sql/opt/optbuilder/create_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ import (

func (b *Builder) buildCreateView(cv *tree.CreateView, inScope *scope) (outScope *scope) {
b.DisableMemoReuse = true
sch, resName := b.resolveSchemaForCreateTable(&cv.Name)
schID := b.factory.Metadata().AddSchema(sch)
viewName := tree.MakeTableNameFromPrefix(resName, tree.Name(cv.Name.Object()))

preFuncResolver := b.semaCtx.FunctionResolver
b.semaCtx.FunctionResolver = nil

Expand All @@ -39,14 +35,21 @@ func (b *Builder) buildCreateView(cv *tree.CreateView, inScope *scope) (outScope
if b.sourceViews == nil {
b.sourceViews = make(map[string]struct{})
}
b.sourceViews[viewName.FQString()] = struct{}{}

viewName := &cv.Name
sch, resName := b.resolveSchemaForCreateTable(viewName)
viewName.ObjectNamePrefix = resName
schID := b.factory.Metadata().AddSchema(sch)

viewFQString := viewName.FQString()
b.sourceViews[viewFQString] = struct{}{}
defer func() {
b.insideViewDef = false
b.trackSchemaDeps = false
b.schemaDeps = nil
b.schemaTypeDeps = intsets.Fast{}
b.qualifyDataSourceNamesInAST = false
delete(b.sourceViews, viewName.FQString())
delete(b.sourceViews, viewFQString)

b.semaCtx.FunctionResolver = preFuncResolver
switch recErr := recover().(type) {
Expand Down Expand Up @@ -106,17 +109,12 @@ func (b *Builder) buildCreateView(cv *tree.CreateView, inScope *scope) (outScope
outScope = b.allocScope()
outScope.expr = b.factory.ConstructCreateView(
&memo.CreateViewPrivate{
Schema: schID,
ViewName: &viewName,
IfNotExists: cv.IfNotExists,
Replace: cv.Replace,
Persistence: cv.Persistence,
Materialized: cv.Materialized,
ViewQuery: tree.AsStringWithFlags(cv.AsSource, tree.FmtParsable),
Columns: p,
Deps: b.schemaDeps,
TypeDeps: b.schemaTypeDeps,
WithData: cv.WithData,
Syntax: cv,
Schema: schID,
ViewQuery: tree.AsStringWithFlags(cv.AsSource, tree.FmtParsable),
Columns: p,
Deps: b.schemaDeps,
TypeDeps: b.schemaTypeDeps,
},
)
return outScope
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/opt/optgen/cmd/optgen/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ func newMetadata(compiled *lang.CompiledExpr, pkg string) *metadata {
"Volatility": {fullName: "volatility.V", passByVal: true},
"LiteralRows": {fullName: "opt.LiteralRows", isExpr: true, isPointer: true},
"Distribution": {fullName: "physical.Distribution", passByVal: true},
"TreeCreateView": {fullName: "tree.CreateView", isPointer: true, usePointerIntern: true},
}

// Add types of generated op and private structs.
Expand Down
Loading

0 comments on commit b59fc36

Please sign in to comment.