Merge cockroachdb#32423

32423: opt: Add optimizer support for new Insert operator r=andy-kimball a=andy-kimball

Add Insert operator to the optimizer.  The implementation is side-by-side
with the existing heuristic planner version. The main differences are that it
supports correlated subqueries, integrates with the CBO, and compiles
default and computed properties differently. Rather than keeping these
properties separate, the new Insert operator wraps the input expression
with Project operators that synthesize the default and computed
expressions.


Co-authored-by: Andrew Kimball <[email protected]>

pkg/server/updates_test.go

@@ -656,10 +656,6 @@ func TestReportUsage(t *testing.T) {
 		`[false,false,false] ALTER TABLE _ CONFIGURE ZONE = _`,
 		`[false,false,false] CREATE DATABASE _`,
 		`[false,false,false] CREATE TABLE _ (_ INT, CONSTRAINT _ CHECK (_ > _))`,
-		`[false,false,false] INSERT INTO _ SELECT unnest(ARRAY[_, _, __more2__])`,
-		`[false,false,false] INSERT INTO _ VALUES (_), (__more2__)`,
-		`[false,false,false] INSERT INTO _ VALUES (length($1::STRING)), (__more1__)`,
-		`[false,false,false] INSERT INTO _(_, _) VALUES (_, _)`,
 		`[false,false,false] SET CLUSTER SETTING "cluster.organization" = _`,
 		`[false,false,false] SET CLUSTER SETTING "diagnostics.reporting.send_crash_reports" = _`,
 		`[false,false,false] SET CLUSTER SETTING "server.time_until_store_dead" = _`,
@@ -668,6 +664,10 @@ func TestReportUsage(t *testing.T) {
 		`[false,false,false] SET application_name = _`,
 		`[false,false,false] UPDATE _ SET _ = _ + _`,
 		`[false,false,true] CREATE TABLE _ (_ INT PRIMARY KEY, _ INT, INDEX (_) INTERLEAVE IN PARENT _ (_))`,
+		`[true,false,false] INSERT INTO _ SELECT unnest(ARRAY[_, _, __more2__])`,
+		`[true,false,false] INSERT INTO _ VALUES (_), (__more2__)`,
+		`[true,false,false] INSERT INTO _ VALUES (length($1::STRING)), (__more1__)`,
+		`[true,false,false] INSERT INTO _(_, _) VALUES (_, _)`,
 		`[true,false,false] SELECT (_, _, __more2__) = (SELECT _, _, _, _ FROM _ LIMIT _)`,
 		`[true,false,true] SELECT _ / $1`,
 		`[true,false,true] SELECT _ / _`,

pkg/sql/data_source.go

@@ -440,7 +440,8 @@ func (p *planner) getAliasedTableName(n tree.TableExpr) (*tree.TableName, *tree.
 	if ate, ok := n.(*tree.AliasedTableExpr); ok {
 		n = ate.Expr
 		// It's okay to ignore the As columns here, as they're not permitted in
-		// DML aliases where this function is used.
+		// DML aliases where this function is used. The grammar does not allow
+		// them, so the parser would have reported an error if they were present.
 		if ate.As.Alias != "" {
 			alias = tree.NewUnqualifiedTableName(ate.As.Alias)
 		}

pkg/sql/insert.go

@@ -224,7 +224,7 @@ func (p *planner) Insert(
 					// TODO(justin): this is too restrictive. It should
 					// be possible to allow INSERT INTO (x) VALUES (DEFAULT)
 					// if x is a computed column. See #22434.
-					return nil, sqlbase.CannotWriteToComputedColError(&insertCols[maxInsertIdx])
+					return nil, sqlbase.CannotWriteToComputedColError(insertCols[maxInsertIdx].Name)
 				}
 				arityChecked = true
 			}
@@ -254,7 +254,7 @@ func (p *planner) Insert(
 			return nil, err
 		}
 		if numExprs > maxInsertIdx {
-			return nil, sqlbase.CannotWriteToComputedColError(&insertCols[maxInsertIdx])
+			return nil, sqlbase.CannotWriteToComputedColError(insertCols[maxInsertIdx].Name)
 		}
 	}
 

pkg/sql/logictest/testdata/logic_test/builtin_function

@@ -1987,8 +1987,12 @@ SELECT pg_catalog.pg_encoding_to_char(6), pg_catalog.pg_encoding_to_char(7)
 ----
 UTF8  NULL
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query TITI
 SELECT pg_catalog.inet_client_addr(), pg_catalog.inet_client_port(), pg_catalog.inet_server_addr(), pg_catalog.inet_server_port()
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 ::/0  0  ::/0  0
 

pkg/sql/logictest/testdata/logic_test/computed

@@ -722,7 +722,7 @@ CREATE TABLE error_check (k INT PRIMARY KEY, s STRING, i INT AS (s::INT) STORED)
 statement ok
 INSERT INTO error_check VALUES(1, '1')
 
-statement error computed column i:
+statement error could not parse "foo" as type int: strconv.ParseInt
 INSERT INTO error_check VALUES(2, 'foo')
 
 statement error computed column i:

pkg/sql/logictest/testdata/logic_test/optimizer

@@ -97,8 +97,8 @@ CREATE SEQUENCE seq
 statement ok
 SET OPTIMIZER = ALWAYS
 
-query error pq: unsupported statement: \*tree\.Insert
-INSERT INTO test (k, v) VALUES (5, 50)
+query error pq: unsupported statement: \*tree\.Delete
+DELETE FROM test WHERE k=5
 
 # Don't fall back to heuristic planner in ALWAYS mode.
 query error pq: aggregates with FILTER are not supported yet

pkg/sql/logictest/testdata/logic_test/pgoidtype

@@ -40,26 +40,42 @@ SELECT pg_typeof('upper'::REGPROC), pg_typeof('upper'::REGPROCEDURE), pg_typeof(
 ----
 regproc  regprocedure  regtype
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'pg_constraint'::REGCLASS, 'pg_catalog.pg_constraint'::REGCLASS
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint  pg_constraint
 
 query error pgcode 42P01 relation "foo.pg_constraint" does not exist
 SELECT 'foo.pg_constraint'::REGCLASS
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT '"pg_constraint"'::REGCLASS, '  "pg_constraint" '::REGCLASS
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint  pg_constraint
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'pg_constraint '::REGCLASS, '  pg_constraint '::REGCLASS
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint  pg_constraint
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'pg_constraint '::REGCLASS, '"pg_constraint"'::REGCLASS::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint  139623798
 
@@ -75,16 +91,24 @@ WHERE relname = 'pg_constraint'
 ----
 139623798  pg_constraint  139623798  pg_constraint  pg_constraint
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OOOO
 SELECT 'upper'::REGPROC, 'upper'::REGPROCEDURE, 'pg_catalog.upper'::REGPROCEDURE, 'upper'::REGPROC::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 upper  upper  upper  2896429946
 
 query error invalid function name
 SELECT 'invalid.more.pg_catalog.upper'::REGPROCEDURE
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OOO
 SELECT 'upper(int)'::REGPROC, 'upper(int)'::REGPROCEDURE, 'upper(int)'::REGPROC::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 upper  upper  2896429946
 
@@ -106,28 +130,48 @@ SELECT 'blah(, )'::REGPROC
 query error more than one function named 'sqrt'
 SELECT 'sqrt'::REGPROC
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OOOO
 SELECT 'array_in'::REGPROC, 'array_in(a,b,c)'::REGPROC, 'pg_catalog.array_in'::REGPROC, 'pg_catalog.array_in( a ,b, c )'::REGPROC
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 array_in  array_in  array_in  array_in
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OOOO
 SELECT 'array_in'::REGPROCEDURE, 'array_in(a,b,c)'::REGPROCEDURE, 'pg_catalog.array_in'::REGPROCEDURE, 'pg_catalog.array_in( a ,b, c )'::REGPROCEDURE
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 array_in  array_in  array_in  array_in
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'public'::REGNAMESPACE, 'public'::REGNAMESPACE::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 public  2397796629
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'bool'::REGTYPE, 'bool'::REGTYPE::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 boolean  16
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OO
 SELECT 'numeric(10,3)'::REGTYPE, 'numeric( 10, 3 )'::REGTYPE
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 numeric  numeric
 
@@ -151,14 +195,22 @@ SELECT 'blah'::REGTYPE
 
 ## Test other cast syntaxes
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query O
 SELECT CAST ('pg_constraint' AS REGCLASS)
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 # This forces the b_expr form of the cast syntax.
 query OO
 SELECT ('pg_constraint')::REGCLASS, ('pg_constraint')::REGCLASS::OID
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 pg_constraint  139623798
 
@@ -289,13 +341,21 @@ SELECT NOT (prorettype::regtype::text = 'foo') AND proretset FROM pg_proc WHERE
 ----
 false
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query TTTTT
 SELECT crdb_internal.create_regtype(10, 'foo'), crdb_internal.create_regclass(10, 'foo'), crdb_internal.create_regproc(10, 'foo'), crdb_internal.create_regprocedure(10, 'foo'), crdb_internal.create_regnamespace(10, 'foo')
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 foo foo foo foo foo
 
+# TODO(jordan): Restore this to original form by removing FROM
+# clause once issue 32422 is fixed.
 query OOOOO
 SELECT crdb_internal.create_regtype(10, 'foo')::oid, crdb_internal.create_regclass(10, 'foo')::oid, crdb_internal.create_regproc(10, 'foo')::oid, crdb_internal.create_regprocedure(10, 'foo')::oid, crdb_internal.create_regnamespace(10, 'foo')::oid
+FROM pg_class
+WHERE relname = 'pg_constraint'
 ----
 10 10 10 10 10
 

pkg/sql/logictest/testdata/logic_test/statement_statistics

@@ -112,7 +112,7 @@ SELECT key,flags
  WHERE application_name = 'valuetest' ORDER BY key, flags
 ----
 key                                                               flags
-INSERT INTO test VALUES (_, _, __more1__), (__more1__)            -
+INSERT INTO test VALUES (_, _, __more1__), (__more1__)            ·
 SELECT (_, _, __more3__) FROM test WHERE _                        ·
 SELECT key FROM test.crdb_internal.node_statement_statistics      ·
 SELECT sin(_)                                                     ·

pkg/sql/logictest/testdata/logic_test/txn

@@ -931,19 +931,19 @@ statement ok
 COMMIT
 
 statement error cannot execute CREATE TABLE in a read-only transaction
-CREATE TABLE a (a int)
+CREATE TABLE tab (a int)
 
 statement error cannot execute INSERT in a read-only transaction
-INSERT INTO a VALUES(1)
+INSERT INTO kv VALUES('foo')
 
 statement error cannot execute UPDATE in a read-only transaction
-UPDATE a SET a = 1
+UPDATE kv SET v = 'foo'
 
 statement error cannot execute INSERT in a read-only transaction
-UPSERT INTO a VALUES(2)
+UPSERT INTO kv VALUES('foo')
 
 statement error cannot execute DELETE in a read-only transaction
-DELETE FROM a
+DELETE FROM kv
 
 statement error cannot execute nextval\(\) in a read-only transaction
 SELECT nextval('a')

pkg/sql/logictest/testdata/logic_test/views

@@ -525,3 +525,21 @@ query T
 SELECT create_statement FROM [SHOW CREATE w3]
 ----
 CREATE VIEW w3 (x) AS (WITH t AS (SELECT x FROM test.public.w) SELECT x FROM t)
+
+# Test CRUD privilege in view.
+
+statement ok
+CREATE TABLE t (a INT PRIMARY KEY, b INT)
+
+statement ok
+CREATE VIEW crud_view AS SELECT a, b FROM [INSERT INTO t (a, b) VALUES (100, 100) RETURNING a, b]
+
+statement ok
+GRANT SELECT ON crud_view TO testuser
+
+user testuser
+
+query error user testuser does not have INSERT privilege on relation t
+SELECT * FROM crud_view
+
+user root

pkg/sql/logictest/testdata/planner_test/distsql_subquery

@@ -1,4 +1,4 @@
-# LogicTest: 5node-dist 5node-dist-opt
+# LogicTest: 5node-dist
 
 query T
 SELECT url FROM [EXPLAIN (DISTSQL) SELECT 1 WHERE EXISTS (SELECT 1)]

pkg/sql/opt/bench/stub_factory.go

@@ -186,3 +186,9 @@ func (f *stubFactory) ConstructExplain(
 func (f *stubFactory) ConstructShowTrace(typ tree.ShowTraceType, compact bool) (exec.Node, error) {
 	return struct{}{}, nil
 }
+
+func (f *stubFactory) ConstructInsert(
+	input exec.Node, table opt.Table, rowsNeeded bool,
+) (exec.Node, error) {
+	return struct{}{}, nil
+}

pkg/sql/opt/catalog.go

@@ -108,17 +108,32 @@ type Table interface {
 	IndexCount() int
 
 	// Index returns the ith index, where i < IndexCount. The table's primary
-	// index is always the 0th index, and is always present (use the
-	// opt.PrimaryIndex to select it). The primary index corresponds to the
-	// table's primary key. If a primary key was not explicitly specified, then
-	// the system implicitly creates one based on a hidden rowid column.
+	// index is always the 0th index, and is always present (use opt.PrimaryIndex
+	// to select it). The primary index corresponds to the table's primary key.
+	// If a primary key was not explicitly specified, then the system implicitly
+	// creates one based on a hidden rowid column.
 	Index(i int) Index
 
 	// StatisticCount returns the number of statistics available for the table.
 	StatisticCount() int
 
 	// Statistic returns the ith statistic, where i < StatisticCount.
 	Statistic(i int) TableStatistic
+
+	// MutationColumnCount returns the number of columns that are in the process
+	// of being added or dropped and that need to be set to their default values
+	// when inserting new rows. These columns are in the DELETE_AND_WRITE_ONLY
+	// state. See this RFC for more details:
+	//
+	//   cockroachdb/cockroach/docs/RFCS/20151014_online_schema_change.md
+	//
+	MutationColumnCount() int
+
+	// MutationColumn returns a Column interface for one of the columns that is
+	// in the process of being added or dropped. The index of the column must be
+	// <= MutationColumnCount. The set of columns returned by MutationColumn are
+	// always disjoint from those returned by the Column method.
+	MutationColumn(i int) Column
 }
 
 // View is an interface to a database view, exposing only the information needed
@@ -149,12 +164,38 @@ type Column interface {
 	// DatumType returns the data type of the column.
 	DatumType() types.T
 
+	// ColTypeStr returns the SQL data type of the column, as a string. Note that
+	// this is sometimes different than DatumType().String(), since datum types
+	// are a subset of column types.
+	ColTypeStr() string
+
 	// IsNullable returns true if the column is nullable.
 	IsNullable() bool
 
 	// IsHidden returns true if the column is hidden (e.g., there is always a
 	// hidden column called rowid if there is no primary key on the table).
 	IsHidden() bool
+
+	// HasDefault returns true if the column has a default value. DefaultExprStr
+	// will be set to the SQL expression string in that case.
+	HasDefault() bool
+
+	// DefaultExprStr is set to the SQL expression string that describes the
+	// column's default value. It is used when the user does not provide a value
+	// for the column when inserting a row. Default values cannot depend on other
+	// columns.
+	DefaultExprStr() string
+
+	// IsComputed returns true if the column is a computed value. ComputedExprStr
+	// will be set to the SQL expression string in that case.
+	IsComputed() bool
+
+	// ComputedExprStr is set to the SQL expression string that describes the
+	// column's computed value. It is always used to provide the column's value
+	// when inserting or updating a row. Computed values cannot depend on other
+	// computed columns, but they can depend on all other columns, including
+	// columns with default values.
+	ComputedExprStr() string
 }
 
 // IndexColumn describes a single column that is part of an index definition.