pkg: Allow user defined primary key in CREATE TABLE ... AS #38589

adityamaru27 · 2019-07-01T18:46:23Z

This change allows users to specify which column in the newly created table should serve as the primary key (#20940). If no such constraint is specified, we generate a unique row_id to serve as the primary key.

cockroach-teamcity · 2019-07-01T18:46:32Z

This change is

adityamaru27 · 2019-07-09T14:26:33Z

RFAL now that the core BulkRowWriter is merged in.

dt · 2019-07-10T20:28:48Z

pkg/sql/create_table.go

 	}

 	ct := &createTableNode{n: n, dbDesc: dbDesc, sourcePlan: sourcePlan}
 	ct.run.synthRowID = synthRowID
+	ct.run.fromHeuristicPlanner = true


can you comment this / where used below (explaining the why in addition to the what)?

dt · 2019-07-10T20:31:55Z

pkg/sql/parser/sql.y

@@ -802,7 +802,7 @@ func newNameFromStr(s string) *tree.Name {
 %type <tree.IsolationLevel> iso_level
 %type <tree.UserPriority> user_priority

-%type <tree.TableDefs> opt_table_elem_list table_elem_list
+%type <tree.TableDefs> opt_table_elem_list table_elem_list ctas_opt_col_list ctas_table_elem_list


here/thoughout: for someone else coming into this code or trying to read the diagrams, I think "ctas" could be more descriptive. maybe "create_as" ?

dt · 2019-07-10T20:39:47Z

pkg/sql/parser/sql.y

  {
    name := $7.unresolvedObjectName().ToTableName()
    $$.val = &tree.CreateTable{
      Table: name,
      IfNotExists: true,
      Interleave: nil,
-      Defs: nil,
+      Defs: $8.tblDefs(),


can you expand the commit message (or better yet, a comment on the AST node fields) about what's going in Defs in an AS stmt? I'm having a bit of trouble grokking this part.

dt · 2019-07-10T20:41:35Z

pkg/sql/create_table.go

+		parentID, id,
+		creationTime,
+		privileges,
+		affected,


I don't think there is any point in plumbing this as an arg just to pass nil. might as well pass nil here, right?

affected is only for resolving FK defs which we don't expect (or support) here.

dt · 2019-07-10T20:50:41Z

pkg/sql/parser/sql.y

+  }
+
+ctas_table_elem:
+  ctas_column_def


another catch from @rolandcrosby: in additional to column def (indeed, more important than being able to put a qualifier on one of the names in fact) we also want the ability to say PRIMARY KEY(a, b) as to define a new multi-col PK (that's where this really gets interesting in repartitioning). I think we could allow it as an alternative _elem case if we wanted, and then handle it or a qualifier. Alternatively, that could be the only syntax for specifying the PK (instead of the qualifier) which eliminated conflicts. I don't have a preference so I'll let you and @rolandcrosby figure that out

for me, supporting multi-col PK at all is a higher priority than supporting multiple syntaxes to define PKs, but otherwise I don't have a strong opinion on whether it's worth supporting this via both inline qualifiers and the PRIMARY KEY (a, b) syntax.

adityamaru27

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @lucy-zhang)

pkg/sql/create_table.go, line 98 at r2 (raw file):

Previously, dt (David Taylor) wrote…

can you comment this / where used below (explaining the why in addition to the what)?

Done.

pkg/sql/create_table.go, line 1030 at r2 (raw file):

Previously, dt (David Taylor) wrote…

I don't think there is any point in plumbing this as an arg just to pass nil. might as well pass nil here, right?

affected is only for resolving FK defs which we don't expect (or support) here.

Done.

pkg/sql/parser/sql.y, line 805 at r2 (raw file):

Previously, dt (David Taylor) wrote…

here/thoughout: for someone else coming into this code or trying to read the diagrams, I think "ctas" could be more descriptive. maybe "create_as" ?

Done.

pkg/sql/parser/sql.y, line 4040 at r2 (raw file):

Previously, dt (David Taylor) wrote…

can you expand the commit message (or better yet, a comment on the AST node fields) about what's going in Defs in an AS stmt? I'm having a bit of trouble grokking this part.

Done.

adityamaru27

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @lucy-zhang)

pkg/sql/parser/sql.y, line 4446 at r2 (raw file):

Previously, rolandcrosby (Roland Crosby) wrote…

for me, supporting multi-col PK at all is a higher priority than supporting multiple syntaxes to define PKs, but otherwise I don't have a strong opinion on whether it's worth supporting this via both inline qualifiers and the PRIMARY KEY (a, b) syntax.

Done.

This change adds grammar rules to support users specifying the primary key columns in a CREATE TABLE...AS query. It allows for both column level qualification, as well as constraint style declaration of PKs. eg: CREATE TABLE a (id PRIMARY KEY) AS SELECT * FROM b CREATE TABLE a (id, PRIMARY KEY(id)) AS SELECT * FROM b CREATE TABLE a (id, idtwo, PRIMARY KEY(id, idtwo)) AS SELECT * FROM b Due to CREATE TABLE and CREATE TABLE ... AS having an identical syntax for declaring PKs, the logic resolving qualifiers and/or constraints had to be pushed to after a column_name without a type is encountered. This avoids reduce/reduce conflicts, but introduces a restriction that a PRIMARY KEY constraint must be preceded by at least one column name. Release note (sql change): Allows user defined PK in CTAS statements.

This change removes the need for a separate `AsColumnNames` field in the CreateTable node, and uses `ColumnTableDefs` instead. This is similar to a normal CREATE TABLE query, thereby allowing us to piggyback on most of the existing logic. It ensures `row_id` key generation only occurs if no PK is explicitly specified. Some special handling is required in pretty printing, as column types are not populated at the time of parsing. Release note: None

adityamaru27 · 2019-07-16T13:16:13Z

@dt ready for a final look. I addressed your comments, added the modified grammar, and a few more logic tests.

adityamaru27 · 2019-07-16T18:47:46Z

TFTR!
bors r+

38589: pkg: Allow user defined primary key in CREATE TABLE ... AS r=adityamaru27 a=adityamaru27 This change allows users to specify which column in the newly created table should serve as the primary key (#20940). If no such constraint is specified, we generate a unique row_id to serve as the primary key. Co-authored-by: Aditya Maru <[email protected]>

craig · 2019-07-16T19:12:22Z

Build succeeded

GitHub CI (Cockroach)

Parameter updates Diagram updates Examples now use MovR Code PRs: cockroachdb/cockroach#38589 cockroachdb/cockroach#38904 Fixes #5190

Parameter updates Diagram updates Examples now use MovR New primary key example for partitioning Code PRs: cockroachdb/cockroach#38589 cockroachdb/cockroach#38904 Fixes #5190

adityamaru27 requested review from dt, thoszhang and a team July 1, 2019 18:46

adityamaru27 requested a review from a team as a code owner July 1, 2019 18:46

adityamaru27 requested review from a team July 1, 2019 18:46

adityamaru27 force-pushed the ctas-user-setting-pk branch 4 times, most recently from c0d2b21 to f390742 Compare July 3, 2019 03:05

adityamaru27 force-pushed the ctas-user-setting-pk branch from f390742 to 83e361d Compare July 9, 2019 13:43

dt approved these changes Jul 10, 2019

View reviewed changes

dt reviewed Jul 10, 2019

View reviewed changes

adityamaru27 force-pushed the ctas-user-setting-pk branch from 83e361d to 9c3757c Compare July 15, 2019 15:59

adityamaru27 commented Jul 15, 2019

View reviewed changes

adityamaru27 force-pushed the ctas-user-setting-pk branch 2 times, most recently from 8e26c79 to 48442c7 Compare July 16, 2019 02:49

adityamaru27 commented Jul 16, 2019

View reviewed changes

adityamaru27 added 2 commits July 15, 2019 22:56

adityamaru27 force-pushed the ctas-user-setting-pk branch from 48442c7 to f8333e7 Compare July 16, 2019 02:57

dt approved these changes Jul 16, 2019

View reviewed changes

adityamaru27 mentioned this pull request Jul 16, 2019

parser: Add user defined column families to CTAS. #38904

Merged

thoszhang approved these changes Jul 16, 2019

View reviewed changes

adityamaru27 mentioned this pull request Jul 16, 2019

parser: Adds new grammar to support user specified PK in CTAS query. #38884

Closed

craig bot merged commit f8333e7 into cockroachdb:master Jul 16, 2019

jseldess mentioned this pull request Aug 8, 2019

pkg: Allow user defined primary key in CREATE TABLE ... AS cockroachdb/docs#5190

Closed

ericharmeling added a commit to cockroachdb/docs that referenced this pull request Aug 21, 2019

CREATE TABLE AS updates:

d1f6003

Parameter updates Diagram updates Examples now use MovR Code PRs: cockroachdb/cockroach#38589 cockroachdb/cockroach#38904 Fixes #5190

ericharmeling added a commit to cockroachdb/docs that referenced this pull request Aug 21, 2019

CREATE TABLE AS updates:

355041b

Parameter updates Diagram updates Examples now use MovR Code PRs: cockroachdb/cockroach#38589 cockroachdb/cockroach#38904 Fixes #5190

ericharmeling mentioned this pull request Aug 21, 2019

CREATE TABLE AS updates cockroachdb/docs#5247

Merged

ericharmeling added a commit to cockroachdb/docs that referenced this pull request Aug 26, 2019

CREATE TABLE AS updates:

fde3d70

Parameter updates Diagram updates Examples now use MovR Code PRs: cockroachdb/cockroach#38589 cockroachdb/cockroach#38904 Fixes #5190

lnhsingh mentioned this pull request Sep 16, 2019

Improved CREATE TABLE AS cockroachdb/docs#4760

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg: Allow user defined primary key in CREATE TABLE ... AS #38589

pkg: Allow user defined primary key in CREATE TABLE ... AS #38589

adityamaru27 commented Jul 1, 2019 •

edited

Loading

cockroach-teamcity commented Jul 1, 2019

adityamaru27 commented Jul 9, 2019

dt Jul 10, 2019

dt Jul 10, 2019

dt Jul 10, 2019

dt Jul 10, 2019

dt Jul 10, 2019

rolandcrosby Jul 10, 2019

adityamaru27 left a comment

adityamaru27 left a comment

adityamaru27 commented Jul 16, 2019

adityamaru27 commented Jul 16, 2019

craig bot commented Jul 16, 2019

pkg: Allow user defined primary key in CREATE TABLE ... AS #38589

pkg: Allow user defined primary key in CREATE TABLE ... AS #38589

Conversation

adityamaru27 commented Jul 1, 2019 • edited Loading

cockroach-teamcity commented Jul 1, 2019

adityamaru27 commented Jul 9, 2019

dt Jul 10, 2019

Choose a reason for hiding this comment

dt Jul 10, 2019

Choose a reason for hiding this comment

dt Jul 10, 2019

Choose a reason for hiding this comment

dt Jul 10, 2019

Choose a reason for hiding this comment

dt Jul 10, 2019

Choose a reason for hiding this comment

rolandcrosby Jul 10, 2019

Choose a reason for hiding this comment

adityamaru27 left a comment

Choose a reason for hiding this comment

adityamaru27 left a comment

Choose a reason for hiding this comment

adityamaru27 commented Jul 16, 2019

adityamaru27 commented Jul 16, 2019

craig bot commented Jul 16, 2019

Build succeeded

adityamaru27 commented Jul 1, 2019 •

edited

Loading