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

release-23.1: sql: fix CREATE MATERIALIZED VIEW AS schema change job description #107471

Merged
merged 1 commit into from
Jul 25, 2023
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
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