sql: add syntax to create unique constraints without an index

[rytaft]

This commit adds support for the syntax ... UNIQUE WITHOUT INDEX ...,
both when adding UNIQUE constraints and when adding UNIQUE columns. Using
this syntax will currently return the error "unique constraints without
an index are not yet supported", but support for the syntax serves as a
starting point for adding support for these unique constraints.

Informs #41535

Release note (sql change): Added support for using the syntax
... UNIQUE WITHOUT INDEX ... in CREATE TABLE and ALTER TABLE statements,
both when defining columns and unique constraints. Although this syntax
can now be parsed successfully, using this syntax currently returns an
error "unique constraints without an index are not yet supported".

[cockroach-teamcity]

This change is 

[knz]

parser code changes LGTM

(did not review whether it matches the desired spec)

i'd say @otan is maybe the better person to review the change holistically.

[rytaft]

TFTR! Added @otan as a reviewer.

[RaduBerinde]

One alternative that I want to run by you is specifying something like UNIQUE INDEX (a) PARTITIONED BY (region), which would create unique index (region, a) to be used for the uniqueness checks. That would make it harder to misuse (creating uniqueness constraints without any useful index).

Maybe PARTITIONED BY is not the right word, as there's nothing specific about regions, other ideas are SHARDED BY, PREFIXED BY

[mgartner]

I'm a +1 for eliminating the possibility for users to introduce full table scans for simple INSERTs: #41535 (comment)

[rytaft]

My understanding is that we want customers to be able to move from a single-region deployment to a multi-region deployment without changing their schema, and everything should "just work". So UNIQUE INDEX (a) would effectively become syntactic sugar for something like UNIQUE INDEX (a) PARTITIONED BY (region) (and we don't necessarily even need to support the PARTITIONED BY syntax). I'm not opposed to adding that syntax in case power users want to specify a column other than region to use for partitioning -- but that would be a separate PR.

The syntax in this PR is unrelated to the new Multi-Region abstractions, and will allow us to separate the concept of unique constraints from unique indexes. I don't think it will be easy to misuse, because users will need to knowingly write WITHOUT INDEX in their schema. I also don't expect it to be a commonly used feature other than for advanced users and for testing purposes.

By separating the concept of unique constraints from unique indexes, we'll be able to add a unique check by default to any mutation that touches a unique column or set of columns, and as an optimization, remove it if there exists a unique index with those columns. Otherwise, the check itself can be optimized by taking advantage of any indexes available (such as a partitioned unique index). This has the advantage of keeping the uniqueness checks general and not tied to the concept of regions.

[otan]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball, @otan, and @rytaft)

---

docs/generated/sql/bnf/col_qualification.bnf, line 13 at r1 (raw file):

	| 'NOT' 'NULL'
	| 'NULL'
	| 'UNIQUE' opt_without_index

i'm reading https://medium.com/flatiron-engineering/uniqueness-in-postgresql-constraints-versus-indexes-4cf957a472fd and it seems as though indexes and constraints for UNIQUE have different properties.

it sounds like for this case, this should be a constraint, not an index, and so only a constraint CONSTRAINT ... WITHOUT INDEX syntax is the thing we want.

is this something we care about?

[rytaft]

---

docs/generated/sql/bnf/col_qualification.bnf, line 13 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

i'm reading https://medium.com/flatiron-engineering/uniqueness-in-postgresql-constraints-versus-indexes-4cf957a472fd and it seems as though indexes and constraints for UNIQUE have different properties.

it sounds like for this case, this should be a constraint, not an index, and so only a constraint CONSTRAINT ... WITHOUT INDEX syntax is the thing we want.

is this something we care about?

My understanding from reading that article is that UNIQUE constraints and UNIQUE indexes in Postgres are in fact identical due to the implementation of UNIQUE constraints using an index. I know it says "the preferred method is to use constraints", but that seems to be purely for academic reasons.

But either way, it's not clear to me that there is a difference between CONSTRAINT foo UNIQUE (a) and UNIQUE (a) other than the fact that the first one has a name. I don't think the second one necessarily implies the presence of an index any more than the first one does. Am I missing something?

[otan]

Reviewed 1 of 15 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball and @rytaft)

---

docs/generated/sql/bnf/col_qualification.bnf, line 13 at r1 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

My understanding from reading that article is that UNIQUE constraints and UNIQUE indexes in Postgres are in fact identical due to the implementation of UNIQUE constraints using an index. I know it says "the preferred method is to use constraints", but that seems to be purely for academic reasons.

But either way, it's not clear to me that there is a difference between CONSTRAINT foo UNIQUE (a) and UNIQUE (a) other than the fact that the first one has a name. I don't think the second one necessarily implies the presence of an index any more than the first one does. Am I missing something?

Taking a read of https://stackoverflow.com/questions/23542794/postgres-unique-constraint-vs-index

It seems like UNIQUE CONSTRAINT doesn't actually need to be backed by an index -- just that all the databases decided to implemented that way.

From what I can tell online, there are things CONSTRAINTS cannot do that indexes can or vice versa, e.g. DEFERRED (constraints only), CONCURRENTLY (index only), ON CONFLICT (constraints only).

The difference seems subtle but I would not be surprised if people relied on it (and not surprised if we treat them the same when we shouldn't).

Taking a re-read of this though, seems like UNIQUE in this clause makes it a constraint so it doesn't really matter, my bad.

[otan]

---

docs/generated/sql/bnf/col_qualification.bnf, line 13 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

Taking a read of https://stackoverflow.com/questions/23542794/postgres-unique-constraint-vs-index

It seems like UNIQUE CONSTRAINT doesn't actually need to be backed by an index -- just that all the databases decided to implemented that way.

From what I can tell online, there are things CONSTRAINTS cannot do that indexes can or vice versa, e.g. DEFERRED (constraints only), CONCURRENTLY (index only), ON CONFLICT (constraints only).

The difference seems subtle but I would not be surprised if people relied on it (and not surprised if we treat them the same when we shouldn't).

Taking a re-read of this though, seems like UNIQUE in this clause makes it a constraint so it doesn't really matter, my bad.

(there's also some fun differences involving NULLS on constraints vs indexes too, but I'm going to stop reading there because my head hurts)

[rytaft]

---

docs/generated/sql/bnf/col_qualification.bnf, line 13 at r1 (raw file):

Previously, otan (Oliver Tan) wrote…

(there's also some fun differences involving NULLS on constraints vs indexes too, but I'm going to stop reading there because my head hurts)

Haha sounds good -- thanks for checking this out. I don't think we currently treat the two concepts differently in CRDB, but that's good to know that there are differences in other DBs.

[mgartner]

Thanks for the additional context @rytaft. LGTM!

[RaduBerinde]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball, @otan, and @rytaft)

---

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

		ShardBuckets Expr
	}
	Unique               bool

[nit] maybe group these into a Unique struct like the others. Also maybe WithoutIndex instead of NoIndex would make it easier for people to discover the corresponding syntax.

[rytaft]

TFTR!

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @andy-kimball and @otan)

---

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

Previously, RaduBerinde wrote…

[nit] maybe group these into a Unique struct like the others. Also maybe WithoutIndex instead of NoIndex would make it easier for people to discover the corresponding syntax.

Done.

[awoods187]

I like the proposed syntax and think we may not need Radus suggestion but its good to have in reserve if we need it.

[rytaft]

TFTRs!

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @andy-kimball and @otan)

[craig]

Build succeeded:

• GitHub CI (Cockroach)