sql: parser support for GENERATED {ALWAYS | BY DEFAULT} AS IDENTITY syntax

[ZhouXing19]

This commit is to allow the parser to support
the GENERATED {ALWAYS | BY DEFAULT} AS IDENTITY syntax
under CREATE TABLE statement.

Release note (sql change): Added support for GENERATED {ALWAYS | BY DEFAULT} AS IDENTITY
syntax in a column definition. This will automatically create
a sequence for the given column.
This matches PostgreSQL syntax and functionality.
See https://www.postgresql.org/docs/current/sql-createtable.html

[cockroach-teamcity]

This change is 

[mgartner]

Great work! I think it's almost there - I left a few minor comments and questions.

Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ZhouXing19)

---

pkg/sql/parser/testdata/create_table, line 2268 at r1 (raw file):

  a INT UNIQUE,
  b INT GENERATED ALWAYS AS IDENTITY,
  c INT GENERATED BY DEFAULT AS IDENTITY

nit: add a column with NOT NULL and GENERATED ... AS IDENTITY to ensure that it is allowed.

---

pkg/sql/sem/tree/create.go, line 407 at r1 (raw file):

// GeneratedIdentityType represents either GENERATED ALWAYS AS IDENTITY
// or GENERATED BY DEFAULT AS IDENTITY

nit: end comment sentences with a period .

---

pkg/sql/sem/tree/create.go, line 410 at r1 (raw file):

type GeneratedIdentityType int

// The values of GeneratedIdentity.GeneratedAsIdentityType

nit: add . to the end of the sentence

---

pkg/sql/sem/tree/create.go, line 553 at r1 (raw file):

			}
			d.GeneratedIdentity.IsGeneratedAsIdentity = true
			d.GeneratedIdentity.GeneratedAsIdentityType = GeneratedAlways

It looks like Postgres automatically marks GENERATED [ALWAYS | BY DEFAULT] AS IDENTITY columns as NOT NULL. Should we be doing the same?

In Postgres:

marcus=# CREATE TABLE t (a INT GENERATED ALWAYS AS IDENTITY);
CREATE TABLE

marcus=# \d t
                            Table "public.t"
 Column |  Type   | Collation | Nullable |           Default
--------+---------+-----------+----------+------------------------------
 a      | integer |           | not null | generated always as identity

---

pkg/sql/sem/tree/create.go, line 554 at r1 (raw file):

			d.GeneratedIdentity.IsGeneratedAsIdentity = true
			d.GeneratedIdentity.GeneratedAsIdentityType = GeneratedAlways
		case *GeneratedByDefAsIdentity:

nit: I think there's only 1 line that is different between these two cases, so you could combine them a bit. Up to you.

case *GeneratedAlwaysAsIdentity, *GeneratedByDefAsIdentity:
    // Do the error checks...

    d.GeneratedIdentity.IsGeneratedAsIdentity = true
    switch t := c.Qualification.(type) {
    case *GeneratedAlwaysAsIdentity:
        d.GeneratedIdentity.GeneratedAsIdentityType = GeneratedAlways
    case *GeneratedByDefAsIdentity:
        d.GeneratedIdentity.GeneratedAsIdentityType = GeneratedByDefault
    }

---

pkg/sql/sem/tree/create.go, line 824 at r1 (raw file):

}

// GeneratedAlwaysAsIdentity represents a column generated always as identity

nit: .

---

pkg/sql/sem/tree/create.go, line 827 at r1 (raw file):

type GeneratedAlwaysAsIdentity struct{}

// GeneratedByDefAsIdentity represents a column generated by default as identity

nit: .

[ZhouXing19]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mgartner)

---

pkg/sql/parser/testdata/create_table, line 2268 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: add a column with NOT NULL and GENERATED ... AS IDENTITY to ensure that it is allowed.

Done.

---

pkg/sql/sem/tree/create.go, line 407 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: end comment sentences with a period .

Done.

---

pkg/sql/sem/tree/create.go, line 410 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: add . to the end of the sentence

Done.

---

pkg/sql/sem/tree/create.go, line 553 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

It looks like Postgres automatically marks GENERATED [ALWAYS | BY DEFAULT] AS IDENTITY columns as NOT NULL. Should we be doing the same?

In Postgres:

marcus=# CREATE TABLE t (a INT GENERATED ALWAYS AS IDENTITY);
CREATE TABLE

marcus=# \d t
                            Table "public.t"
 Column |  Type   | Collation | Nullable |           Default
--------+---------+-----------+----------+------------------------------
 a      | integer |           | not null | generated always as identity

Done.

[mgartner]

Nice work!

Reviewed 9 of 9 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ZhouXing19)

---

pkg/sql/sem/tree/create.go, line 579 at r1 (raw file):

			d.Nullable.ConstraintName = c.Name
		case NullConstraint:
			if d.Nullable.Nullability == NotNull || d.GeneratedIdentity.IsGeneratedAsIdentity {

I think the || d.GeneratedIdentity.IsGeneratedAsIdentity is still needed here because a INT NULL GENERATED ALWAYS AS IDENTITY is not allowed, correct? Maybe a test with the order of qualifications switched would catch it, like: a INT GENERATED ALWAYS AS IDENTITY NULL.

---

pkg/sql/sem/tree/create.go, line 544 at r3 (raw file):

					"multiple identity specifications for column %q", name)
			}
			d.GeneratedIdentity.IsGeneratedAsIdentity = true

super nit: move this after the checks for computed columns and null columns

[ZhouXing19]

---

pkg/sql/sem/tree/create.go, line 579 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I think the || d.GeneratedIdentity.IsGeneratedAsIdentity is still needed here because a INT NULL GENERATED ALWAYS AS IDENTITY is not allowed, correct? Maybe a test with the order of qualifications switched would catch it, like: a INT GENERATED ALWAYS AS IDENTITY NULL.

We've restrict that the a NULL token will raise error if this column has been marked as NOT NULL by other qualification (check here), and we mark a GeneratedAsIdentity column as NOT NULL.

So even without || d.GeneratedIdentity.IsGeneratedAsIdentity, I think neither a INT NULL GENERATED ALWAYS AS IDENTITY nor a INT GENERATED ALWAYS AS IDENTITY NULL will work, right?

[mgartner]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ZhouXing19)

---

pkg/sql/sem/tree/create.go, line 579 at r1 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

We've restrict that the a NULL token will raise error if this column has been marked as NOT NULL by other qualification (check here), and we mark a GeneratedAsIdentity column as NOT NULL.

So even without || d.GeneratedIdentity.IsGeneratedAsIdentity, I think neither a INT NULL GENERATED ALWAYS AS IDENTITY nor a INT GENERATED ALWAYS AS IDENTITY NULL will work, right?

Oh right! Thanks for adding the tests.

[ZhouXing19]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)