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

opt: Add Update support to CBO #32774

Merged
merged 10 commits into from Dec 10, 2018

Conversation

Projects
None yet
6 participants
@andy-kimball
Copy link
Contributor

andy-kimball commented Dec 3, 2018

Add a new Update operator to the CBO which is used to optimize the
UPDATE SQL statement. As part of compiling the UPDATE statement, the
optbuilder constructs an expression that outputs the existing values
for all rows from the target table that match the WHERE clause.
Additional column(s) that provide updated values are projected for
each of the SET expressions, as well as for any computed columns. For
example:

  CREATE TABLE abc (a INT PRIMARY KEY, b INT, c INT)
  UPDATE abc SET b=1 WHERE a=2

This would create an input expression similar to this SQL:

  SELECT a AS oa, b AS ob, c AS oc, 1 AS nb FROM abc WHERE a=2

The execution engine evaluates this relational expression and uses
the resulting values to form the KV keys and values.

@andy-kimball andy-kimball requested review from justinj , rytaft and RaduBerinde Dec 3, 2018

@andy-kimball andy-kimball requested review from cockroachdb/sql-execution-prs as code owners Dec 3, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Dec 3, 2018

This change is Reviewable

@andy-kimball

This comment has been minimized.

Copy link
Contributor

andy-kimball commented Dec 3, 2018

Note that I commented out the Max1Row operator when compiling SET subqueries. That won't work properly until this PR merges:

#32753

I'll rebase once that happens. In the meantime, I want to get this PR posted, since there's a lot to review.

@justinj
Copy link
Member

justinj left a comment

Only nits from me for now - might take a more holistic look in a while though

Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 6 of 6 files at r3, 10 of 10 files at r4, 33 of 33 files at r5, 20 of 20 files at r6, 3 of 3 files at r7, 21 of 21 files at r8, 21 of 21 files at r9.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/opt_exec_factory.go, line 1009 at r9 (raw file):

	}

	// serialize the data-modifying plan to ensure that no data is

nit: Serialize


pkg/sql/logictest/testdata/logic_test/default, line 111 at r9 (raw file):


statement ok
UPDATE v SET b = DEFAULT

Is this no longer legal? Postgres allows it despite it being kind of silly and pointless (I don't think it's a big deal if we don't support it).


pkg/sql/opt/catalog.go, line 114 at r6 (raw file):

	//
	// If the column is not a mutation column, then ok=false.
	MutationColumn(i int) (col MutationColumn, ok bool)

OOC, how come this and not making MutationColumn and interface and using a cast to determine if a column is a mutation column or not?


pkg/sql/opt/norm/testdata/rules/decorrelate, line 86 at r8 (raw file):


opt expect=DecorrelateJoin
UPDATE xy SET (x, y)=(SELECT * FROM uv WHERE u=x)

This is pretty cool 👍


pkg/sql/opt/ops/statement.opt, line 32 at r4 (raw file):

[Private]
define MutationPrivate {
    # Table identifies the table into which to insert. It is an id that can be

nit: the table which is being mutated


pkg/sql/opt/ops/statement.opt, line 60 at r8 (raw file):

    # Fetch columns are referenced by update, computed, and constraint
    # expressions. They're also needed to formulate the final key/value pairs;
    # updating even one column in a family requires the entire the entire value

nit: the entire the entire


pkg/sql/opt/optbuilder/scope_column.go, line 41 at r6 (raw file):

	// hidden is true if the column is not selected by a '*' wildcard operator.
	// The column must be explicitly referenced by name, or otherwise is hidden.

nit: maybe "or otherwise is not included"? since hidden is what's already being discussed.


pkg/sql/opt/optbuilder/scope_column.go, line 44 at r6 (raw file):

	hidden bool

	// mutation is true if the column is in process of being dropped or added to

nit: is in the process

@rytaft
Copy link
Contributor

rytaft left a comment

Still have a bit more to review, but looks good so far!

Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 6 of 6 files at r3, 10 of 10 files at r4, 33 of 33 files at r5, 20 of 20 files at r6, 3 of 3 files at r7, 14 of 21 files at r8.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/opt_exec_factory.go, line 981 at r7 (raw file):

	colCfg := scanColumnsConfig{
		wantedColumns: make([]tree.ColumnID, 0, cols.Len()),
		visibility:    publicAndNonPublicColumns,

Should this always be the visibility? What if this is a read only query?


pkg/sql/opt/metadata.go, line 329 at r5 (raw file):

					}
				}
				break

Seems like you could be excluding some ambiguous cases here. What if there is one column that has the same name and is from the same table, but another column that has the same name and is from a different table? Seems like you might miss that other column.


pkg/sql/opt/exec/factory.go, line 233 at r6 (raw file):

	// input columns are inserted into a subset of columns in the table, in the
	// same order they're defined. The insertCols set contains the ordinal
	// positions of columns in the table into which values are inserted. The

What happens to the other columns? Do they get a default value?


pkg/sql/opt/memo/check_expr.go, line 220 at r8 (raw file):

		_, ok := tab.MutationColumn(i)
		if ok {
			mutCols.Add(int(private.InsertCols[i]))

Isn't this going to panic for updates?


pkg/sql/opt/memo/statistics_builder.go, line 1752 at r8 (raw file):

func (sb *statisticsBuilder) colStatMutation(
	colSet opt.ColSet, insUpd RelExpr,

[nit] insUpd -> mutation (to be consistent with buildMutation)


pkg/sql/opt/memo/testdata/logprops/insert, line 39 at r3 (raw file):

insert abcde
 ├── columns: <none>
 ├── insert:

[nit] it's a little weird to have this child node also called insert. Not sure what a better name would be, but maybe something that evokes column mappings...


pkg/sql/opt/optbuilder/mutation_builder.go, line 64 at r8 (raw file):

	// in the target table, including mutation columns. Table columns which will
	// not have values inserted are set to zero (e.g. delete-only mutation
	// columns). insertColList empty if this is not an Insert operator.

[nit] insertColList is empty


pkg/sql/opt/optbuilder/select.go, line 280 at r6 (raw file):

	ordinals []int,
	indexFlags *tree.IndexFlags,
	includeMutations bool,

This is shadowing the constants you defined above - was that intentional?

@RaduBerinde

This comment has been minimized.

Copy link
Member

RaduBerinde commented Dec 4, 2018


pkg/sql/opt/catalog.go, line 114 at r6 (raw file):

Previously, justinj (Justin Jaffray) wrote…

OOC, how come this and not making MutationColumn and interface and using a cast to determine if a column is a mutation column or not?

I had a similar question (can't we add methods to Column to retrieve this info?), but I think it's harder to implement because we use *ColumnDescriptor as opt.Column; we would need to add a wrapper (which means more allocations).

@rytaft
Copy link
Contributor

rytaft left a comment

Nice work! Done with the first pass.

Reviewed 7 of 21 files at r8, 21 of 21 files at r9.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/opt_exec_factory.go, line 974 at r9 (raw file):

	}

	// Determine the relational type of the generated insert node.

update node?


pkg/sql/opt/optbuilder/mutation_builder.go, line 609 at r8 (raw file):

		// references (like computed expressions) should point to the new column
		// rather than the old.
		projectionsScope.cols[ord].name = ""

This is a bit confusing. It might help to say that sourceCol was added to the projectionsScope as well (or will be added later). Would also help to explain somewhere why the table columns are in the same scope at all (e.g., why not just add a new scope with the old columns replaced).


pkg/sql/opt/optbuilder/mutation_builder.go, line 655 at r8 (raw file):

				mb.outScope.expr = mb.b.factory.ConstructLeftJoinApply(
					mb.outScope.expr,
					//mb.b.factory.ConstructMax1Row(subqueryScope.expr),

Don't forget to uncomment (I presume this depends on the Max1Row issue...)


pkg/sql/opt/optbuilder/mutation_builder.go, line 708 at r8 (raw file):

	// Allow mutation columns to be referenced by other computed mutation columns
	// (otherwise the scope will raise an error if a mutation column is
	// referenced).

Do we need to reset the mutation field later?


pkg/sql/opt/optbuilder/testdata/update, line 1108 at r8 (raw file):

UPDATE mutation SET m=1 RETURNING "o:write-only"
----
error (42703): column "o:write-only" does not exist

should this return the same error as below? (o:write-only is being backfilled)?

@RaduBerinde
Copy link
Member

RaduBerinde left a comment

Great stuff! Only have a few comments so far, will give it another pass

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/logictest/testdata/logic_test/insert, line 652 at r6 (raw file):

BEGIN; ALTER TABLE mutation DROP COLUMN y

# TODO(andyk): re-enable once issue #32759 is fixed.

I think this is fixed now.


pkg/sql/logictest/testdata/logic_test/subquery, line 242 at r9 (raw file):

# Error is different between HP and CBO, but is reasonable in both cases. Just
# verify there's an error that contains the word "columns".
statement error columns

Nit: I think this is a regexp so it could contain both errors with |


pkg/sql/opt/optbuilder/scope.go, line 583 at r6 (raw file):

			}

			if err := checkNoMutationColumn(col); err != nil {

In this loop, we are looking for candidates.. Shouldn't we ignore this column if it's a mutation instead of erroring out right away? The col name could refer to a column in a parent scope. (I'd add a test like this)

@andy-kimball andy-kimball force-pushed the andy-kimball:update branch from ebc3720 to 3d64f27 Dec 6, 2018

@andy-kimball andy-kimball requested review from cockroachdb/core-prs as code owners Dec 6, 2018

@andy-kimball
Copy link
Contributor

andy-kimball left a comment

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/opt_exec_factory.go, line 981 at r7 (raw file):

Previously, rytaft wrote…

Should this always be the visibility? What if this is a read only query?

I added this comment:

	// Set visibility=publicAndNonPublicColumns, since all columns in the "cols"
	// set should be projected, regardless of whether they're public or non-
	// public. The caller decides which columns to include (or not include). Note
	// that when wantedColumns is non-empty, the visibility flag will never
	// trigger the addition of more columns.

pkg/sql/opt_exec_factory.go, line 974 at r9 (raw file):

Previously, rytaft wrote…

update node?

Done.


pkg/sql/logictest/testdata/logic_test/subquery, line 242 at r9 (raw file):

Previously, RaduBerinde wrote…

Nit: I think this is a regexp so it could contain both errors with |

Oh, good to know.


pkg/sql/opt/catalog.go, line 114 at r6 (raw file):

Previously, RaduBerinde wrote…

I had a similar question (can't we add methods to Column to retrieve this info?), but I think it's harder to implement because we use *ColumnDescriptor as opt.Column; we would need to add a wrapper (which means more allocations).

Yes, extra allocations were one of my worries as well. But I think it's fine in this case, since mutation columns are rare. I went ahead and switched the implementation to use a wrapper.


pkg/sql/opt/metadata.go, line 329 at r5 (raw file):

Previously, rytaft wrote…

Seems like you could be excluding some ambiguous cases here. What if there is one column that has the same name and is from the same table, but another column that has the same name and is from a different table? Seems like you might miss that other column.

I removed the break to avoid that case.


pkg/sql/opt/exec/factory.go, line 233 at r6 (raw file):

Previously, rytaft wrote…

What happens to the other columns? Do they get a default value?

I added this to the comment:

All columns are expected to be present except delete-only mutation columns, since those do not need to participate in an insert operation.

pkg/sql/opt/memo/check_expr.go, line 220 at r8 (raw file):

Previously, rytaft wrote…

Isn't this going to panic for updates?

Yes, it will. Fixed.


pkg/sql/opt/memo/statistics_builder.go, line 1752 at r8 (raw file):

Previously, rytaft wrote…

[nit] insUpd -> mutation (to be consistent with buildMutation)

Done.


pkg/sql/opt/memo/testdata/logprops/insert, line 39 at r3 (raw file):

Previously, rytaft wrote…

[nit] it's a little weird to have this child node also called insert. Not sure what a better name would be, but maybe something that evokes column mappings...

I changed it to be insert-mapping and update-mapping.


pkg/sql/opt/ops/statement.opt, line 60 at r8 (raw file):

Previously, justinj (Justin Jaffray) wrote…

nit: the entire the entire

The repetition is a clever literary device that emphasizes the fact that not just one, but multiple columns may need to be updated at once.


pkg/sql/opt/optbuilder/mutation_builder.go, line 64 at r8 (raw file):

Previously, rytaft wrote…

[nit] insertColList is empty

Done.


pkg/sql/opt/optbuilder/mutation_builder.go, line 609 at r8 (raw file):

Previously, rytaft wrote…

This is a bit confusing. It might help to say that sourceCol was added to the projectionsScope as well (or will be added later). Would also help to explain somewhere why the table columns are in the same scope at all (e.g., why not just add a new scope with the old columns replaced).

I updated the comment to be this:

		// "Shadow" the existing column of the same name, since any future
		// references (like computed expressions) should point to the new column
		// containing the updated value rather than the old column containing the
		// original value. The old columns need to be retained in the projection
		// because some original values are needed to formulate the update keys.

Is that clear enough?


pkg/sql/opt/optbuilder/mutation_builder.go, line 655 at r8 (raw file):

Previously, rytaft wrote…

Don't forget to uncomment (I presume this depends on the Max1Row issue...)

Done.


pkg/sql/opt/optbuilder/mutation_builder.go, line 708 at r8 (raw file):

Previously, rytaft wrote…

Do we need to reset the mutation field later?

No, b/c mutation columns are not projected by the Update operator. I added that to the comment.


pkg/sql/opt/optbuilder/scope.go, line 583 at r6 (raw file):

Previously, RaduBerinde wrote…

In this loop, we are looking for candidates.. Shouldn't we ignore this column if it's a mutation instead of erroring out right away? The col name could refer to a column in a parent scope. (I'd add a test like this)

Good catch. Fixed.


pkg/sql/opt/optbuilder/select.go, line 280 at r6 (raw file):

Previously, rytaft wrote…

This is shadowing the constants you defined above - was that intentional?

Not intentional, I fixed it.


pkg/sql/opt/optbuilder/testdata/update, line 1108 at r8 (raw file):

Previously, rytaft wrote…

should this return the same error as below? (o:write-only is being backfilled)?

This is the error given by HP. It'd be messy to make it be the backfilled error, b/c the RETURNING clause does not project mutation columns, and it'd be messy to make it do so.


pkg/sql/logictest/testdata/logic_test/default, line 111 at r9 (raw file):

Previously, justinj (Justin Jaffray) wrote…

Is this no longer legal? Postgres allows it despite it being kind of silly and pointless (I don't think it's a big deal if we don't support it).

I changed code to allow it, so now a NOT-NULL error would only be raised at runtime if there is at least one row to update.

@rytaft

rytaft approved these changes Dec 6, 2018

Copy link
Contributor

rytaft left a comment

:lgtm_strong:

Reviewed 3 of 86 files at r10, 3 of 10 files at r13, 27 of 30 files at r14, 5 of 20 files at r15, 2 of 3 files at r16, 25 of 25 files at r17, 23 of 23 files at r18.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/sql/opt/optbuilder/mutation_builder.go, line 609 at r8 (raw file):

Previously, andy-kimball (Andy Kimball) wrote…

I updated the comment to be this:

		// "Shadow" the existing column of the same name, since any future
		// references (like computed expressions) should point to the new column
		// containing the updated value rather than the old column containing the
		// original value. The old columns need to be retained in the projection
		// because some original values are needed to formulate the update keys.

Is that clear enough?

👍 Thanks!


pkg/sql/opt/optbuilder/mutation_builder.go, line 703 at r18 (raw file):

	// Allow mutation columns to be referenced by other computed mutation columns
	// (otherwise the scope will raise an error if a mutation column is
	// referenced). These does not need to be set back to true again because

[nit] These does -> These do


pkg/sql/opt/optbuilder/testdata/insert, line 730 at r8 (raw file):

error (42703): column "unk" does not exist

# Cannot insert null into non-null column.

Why did you take out this test?


pkg/sql/opt/optbuilder/testdata/insert, line 689 at r18 (raw file):


# Allow NULL DEFAULT value to be inserted into not-null column (would fail at
# runtime if there are any rows to update).

I don't understand this comment... isn't this going to insert one row with a null? Or is the point that this is an execution error, not a planning error?

@RaduBerinde
Copy link
Member

RaduBerinde left a comment

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/sql/opt/catalog.go, line 198 at r18 (raw file):

// dropped from a table. Mutation columns require special handling by mutation
// operators like Insert, Update, and Delete.
type MutationColumn struct {

Wouldn't it be simpler to add IsMutationColumn (ok, deleteOnly bool) to Column and make this wrapper be private in the implementation? (we'd need to add a shell implementation of this to ColumnDescriptor but that seems fine)


pkg/sql/opt/catalog.go, line 209 at r18 (raw file):

// ColName is part of the opt.Column interface.
func (c *MutationColumn) ColName() tree.Name {

[nit] Column could be embedded so we don't have to define these


pkg/sql/opt/ops/statement.opt, line 60 at r8 (raw file):

Previously, andy-kimball (Andy Kimball) wrote…

The repetition is a clever literary device that emphasizes the fact that not just one, but multiple columns may need to be updated at once.

nit: I think you meant "The repetition is a clever is a clever literary device"

@andy-kimball andy-kimball force-pushed the andy-kimball:update branch from 3d64f27 to 212a917 Dec 8, 2018

@andy-kimball
Copy link
Contributor

andy-kimball left a comment

I also added a commit to this PR that only enables CBO UPDATE when the experimental_optimizer_updates session flag is set to True, in order to address concerns raised by +@jordanlewis that column family perf would otherwise regress due to missing "needed column" optimizations.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/sql/opt/catalog.go, line 198 at r18 (raw file):

Previously, RaduBerinde wrote…

Wouldn't it be simpler to add IsMutationColumn (ok, deleteOnly bool) to Column and make this wrapper be private in the implementation? (we'd need to add a shell implementation of this to ColumnDescriptor but that seems fine)

Hmm, maybe, but then if we add additional mutation fields (like add vs. drop), we'd need to add more return values to IsMutationColumn, which seems messy.


pkg/sql/opt/catalog.go, line 209 at r18 (raw file):

Previously, RaduBerinde wrote…

[nit] Column could be embedded so we don't have to define these

Nice, I didn't know Go supported embedding an interface in a struct like that.


pkg/sql/opt/ops/statement.opt, line 60 at r8 (raw file):

Previously, RaduBerinde wrote…

nit: I think you meant "The repetition is a clever is a clever literary device"

I'd hardly classify hardly classify your excellent suggestion as a nit.


pkg/sql/opt/optbuilder/mutation_builder.go, line 703 at r18 (raw file):

Previously, rytaft wrote…

[nit] These does -> These do

Done.


pkg/sql/opt/optbuilder/testdata/insert, line 730 at r8 (raw file):

Previously, rytaft wrote…

Why did you take out this test?

It's no longer a compile-time error. It's now a runtime error. I added a new test above to ensure that it gets compiled correctly:

# Verify that there is no compile-time error when trying to insert a NULL
# DEFAULT value into a not-null column (it will fail at runtime).
build
INSERT INTO abcde (a) VALUES (DEFAULT)
----

I did this in response to Justin's comment on an earlier iteration, in which he pointed out that UPDATE only fails at runtime if trying to set a not-NULL column to NULL.


pkg/sql/opt/optbuilder/testdata/insert, line 689 at r18 (raw file):

Previously, rytaft wrote…

I don't understand this comment... isn't this going to insert one row with a null? Or is the point that this is an execution error, not a planning error?

It's an execution error, not a planning error. I'll fix the comment.

@rytaft

rytaft approved these changes Dec 10, 2018

Copy link
Contributor

rytaft left a comment

Reviewed 1 of 86 files at r10, 3 of 10 files at r22, 27 of 30 files at r23, 5 of 20 files at r24, 2 of 3 files at r25, 25 of 26 files at r26, 22 of 23 files at r27, 11 of 11 files at r28.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

andy-kimball added some commits Nov 26, 2018

opt: Move insertBuilder to new file
Prepare to rename to mutationBuilder and generalize it for other
mutation operators.

Release note: None
opt: Rename insertBuilder to mutationBuilder
This is in preparation for adding Update/Upsert support.

Release note: None
opt: Re-format mutation operator output
Create a separate treeprinter child for each column changed by a mutation,
like this:

  ├── insert:
  │    ├──  column1:7(int) => a:1(int)
  │    └──  column2:8(int) => b:2(int)

This is easier to read, and it will work better for Update mutations.

Release note: None
opt: Generalize mutationBuilder
Make mutationBuilder methods more flexible, in preparation for adding
Update/Upsert support.

Release note: None
opt: Don't qualify names that are identical
If two columns with the same name are added to query metadata, then
the formatter will qualify the names with the table name. This commit
changes the algorithm to not qualify if the table names are the same.
There is no extra clarity if that's the case, and it looks cluttered.

This is important for supporting the Update operator, where the same
table is always used twice in the same query.

Release note: None
opt: Change mutation column metadata interface
After working on Update, I decided that mutation columns should be part
of the main Column list exposed by the opt.Table interface. There are
problems with each choice, but after trying both ways, I found that most
code that calls the Column method wants to include mutation columns if
present. In the cases where that's not true, there's a new
opt.IsMutationColumn helper method to make to make it easy to exclude
them.

In addition, the new design exposes a MutationColumn struct that indicates
whether the mutation column is delete-only or write-only. This will be
needed for Update. That in turn means that Insert may not insert values
into all table columns (i.e. it won't insert into delete-only columns),
and so the ConstructInsert exec factory method takes a new insertCols
parameter that specifies the subset of table columns into which values
will be inserted.

Release note: None

andy-kimball added some commits Dec 2, 2018

sql: Do not automatically add mutation columns
If "wantedColumns" are specified as an argument to the scanNode, don't
add mutation columns automatically. Require caller to add them if they
want them. Note that if wantedColumns are not specified, then mutation
columns are always added (as are all other columns).

This change is important for implementing the CBO Update operator, since
it sometimes needs to fetch a subset of the mutation columns.

Release note: None
opt: Add Update support to CBO
Add a new Update operator to the CBO which is used to optimize the
UPDATE SQL statement. As part of compiling the UPDATE statement, the
optbuilder constructs an expression that outputs the existing values
for all rows from the target table that match the WHERE clause.
Additional column(s) that provide updated values are projected for
each of the SET expressions, as well as for any computed columns. For
example:

  CREATE TABLE abc (a INT PRIMARY KEY, b INT, c INT)
  UPDATE abc SET b=1 WHERE a=2

This would create an input expression similar to this SQL:

  SELECT a AS oa, b AS ob, c AS oc, 1 AS nb FROM abc WHERE a=2

The execution engine evaluates this relational expression and uses
the resulting values to form the KV keys and values.

Release note: None
opt: Add execbuilder support for the Update operator
Build an updateNode from the optimizer's new Update operator. For now,
always fetch all columns from the table. In the future, do "needed
column" analysis to select only those columns that are needed for the
update.

Release note: None

@andy-kimball andy-kimball force-pushed the andy-kimball:update branch from 212a917 to 9564220 Dec 10, 2018

opt: Add experimental_optimizer_updates flag
When enabled, this flag uses the cost-based optimizer rather than the
heuristic planner to plan UPDATE statements. The CBO doesn't yet handle
multiple column families, so this flag avoids turning it on by default
so that performance in column family cases will not be affected for the
alpha release.

Release note (sql change): Added experimental_optimize_updates flag,
which uses the cost-based optimizer to plan UPDATE statements when set
to true.

Release note: None

@andy-kimball andy-kimball force-pushed the andy-kimball:update branch from 9564220 to 9be6b72 Dec 10, 2018

@andy-kimball

This comment has been minimized.

Copy link
Contributor

andy-kimball commented Dec 10, 2018

bors r+

craig bot pushed a commit that referenced this pull request Dec 10, 2018

Merge #32774
32774: opt: Add Update support to CBO r=andy-kimball a=andy-kimball

Add a new Update operator to the CBO which is used to optimize the
UPDATE SQL statement. As part of compiling the UPDATE statement, the
optbuilder constructs an expression that outputs the existing values
for all rows from the target table that match the WHERE clause.
Additional column(s) that provide updated values are projected for
each of the SET expressions, as well as for any computed columns. For
example:
```
  CREATE TABLE abc (a INT PRIMARY KEY, b INT, c INT)
  UPDATE abc SET b=1 WHERE a=2
```
This would create an input expression similar to this SQL:
```
  SELECT a AS oa, b AS ob, c AS oc, 1 AS nb FROM abc WHERE a=2
```
The execution engine evaluates this relational expression and uses
the resulting values to form the KV keys and values.

Co-authored-by: Andrew Kimball <andyk@cockroachlabs.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Dec 10, 2018

Build succeeded

@craig craig bot merged commit 9be6b72 into cockroachdb:master Dec 10, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@andy-kimball andy-kimball deleted the andy-kimball:update branch Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment