sql: implement SERIAL column type

[danhhz]

Closes #4491.

There are two good options for this, each with tradeoffs. One is for SERIAL to
be an alias for DEFAULT unique_rowid and the other is to emulate Postgres's
SEQUENCEs.

Emulating SEQUENCE funnels every insert through a single point of contention,
but would be a drop-in replacement.

unique_rowid has no contention, is faster, and works for the common use of a
unique and increasing identifier (modulo overlapping transactions).
Unfortunately, it may also cause user confusion and break ORMs that (correctly
in Postgres) assume that SERIAL starts at 0 and (incorrectly) that SERIAL has no
holes. An interesting side benefit of unique_rowid is that it prevents the
problem of inadvertantly exposing user growth numbers (sign up for a service
once per day and keep track of the userids), which is a common issue with
SERIAL.

If it was known that user confusion and ORM compatibility would not be an issue,
unique_rowid would be the clear decision. Since it's still early enough to run
small experiments, this is the approach used here. If this results in sufficient
evidence that it was the wrong decision, a later version of Cockroach will
switch the behavior to emulate SEQUENCEs. Existing SERIAL columns will, however,
retain the old behavior.

---

This change is

[petermattis]

I think we'll need some good documentation explaining this choice.

Previously, paperstreet (Daniel Harrison) wrote…

sql: implement SERIAL column type

Closes #4491.

There are two good options for this, each with tradeoffs. One is for SERIAL to

be an alias for DEFAULT unique_rowid and the other is to emulate Postgres's

SEQUENCEs.

Emulating SEQUENCE funnels every insert through a single point of contention,

but would be a drop-in replacement.

unique_rowid has no contention, is faster, and works for the common use of a

unique and increasing identifier (modulo overlapping transactions).

Unfortunately, it may also cause user confusion and break ORMs that (correctly

in Postgres) assume that SERIAL starts at 0 and (incorrectly) that SERIAL has no

holes. An interesting side benefit of unique_rowid is that it prevents the

problem of inadvertantly exposing user growth numbers (sign up for a service

once per day and keep track of the userids), which is a common issue with

SERIAL.

If it was known that user confusion and ORM compatibility would not be an issue,

unique_rowid would be the clear decision. Since it's still early enough to run

small experiments, this is the approach used here. If this results in sufficient

evidence that it was the wrong decision, a later version of Cockroach will

switch the behavior to emulate SEQUENCEs. Existing SERIAL columns will, however,

retain the old behavior.

---

Review status: 0 of 6 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

sql/sqlbase/table.go, line 199 [r1] (raw file):

      if t.IsSerial() {
          if d.DefaultExpr != nil {
              return nil, nil, fmt.Errorf("multiple default values specified for column %q", col.Name)

Is that the postgres error? Seems like it would be clearer to mention a default value was specified for the SERIAL column type.

---

Comments from Reviewable

[danhhz]

Agreed

Previously, petermattis (Peter Mattis) wrote…

:lgtm:

I think we'll need some good documentation explaining this choice.

---

Review status: 0 of 6 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

sql/sqlbase/table.go, line 199 [r1] (raw file):

Previously, petermattis (Peter Mattis) wrote…

Is that the postgres error? Seems like it would be clearer to mention a default value was specified for the SERIAL column type.

Yeah, that was Postgres's. I thought it was wonky too, but tend to go with Postgres when I can for Google-ability of error messages. You noticing the same thing tips the scales for me in favor of improving the message

---

Comments from Reviewable

[petermattis]

Mind working with @jseldess on those docs? In particular, we should highlight how SERIAL/AUTO_INCREMENT in other databases can have both holes and rows inserted out of order.

Previously, paperstreet (Daniel Harrison) wrote…

Agreed

---

Review status: 0 of 6 files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable

[jseldess]

Thanks for the mention. @paperstreet, I'll set up time to discuss.

[danhhz]

Happy to! In the meantime, I wrote up some quick notes cockroachdb/docs#356.

Previously, jseldess wrote…

Thanks for the mention. @paperstreet, I'll set up time to discuss.

---

Review status: 0 of 6 files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable

[jseldess]

Excellent. Thanks, @paperstreet

[jseldess]

Docs covered by cockroachdb/docs#362