sql: check CHECK constraints during UPDATE too

[dt]

This change is

[danhhz]

Previously, dt (David Taylor) wrote…

sql: check CHECK constraints during UPDATE too

---

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

---

sql/update.go, line 323 [r1] (raw file):

  u.checkHelper.loadRow(u.tw.ru.fetchColIDtoRowIndex, oldValues, false)
  u.checkHelper.updateRow(u.updateCols, updateValues)

make checkHelper.updateRow take the map[ColumnID]int and cache it somewhere instead of computing it once per row.

maybe have makeRowUpdater compute it and put it on rowUpdater?

---

Comments from Reviewable

[vivekmenezes]

I haven't been following these CHECK constraint PRs. Just checking if we
are rolling out these constraints properly like a schema change? The F1
paper talks about rolling them out using an intermediate WRITE_ONLY state.
Feel free to create an issue we aren't

On Tue, May 17, 2016 at 2:20 PM Daniel Harrison notifications@github.com
wrote:

[image: ]
https://camo.githubusercontent.com/41b3ec3419116e3b3f84507ad965646ce4ce1f0c/68747470733a2f2f72657669657761626c652e696f2f6c67746d2e706e67

Previously, dt (David Taylor) wrote…

sql: check CHECK constraints during UPDATE too

---

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved

discussion, some commit checks pending.

sql/update.go, line 323 [r1]
https://reviewable.io:443/reviews/cockroachdb/cockroach/6753#-KHzvVnJZEW-_h5VwSju:-KHzvVnJZEW-_h5VwSjv:2139562696
(raw file

cockroach/sql/update.go

Line 323 in 7be6bf6

u.checkHelper.updateRow(u.updateCols, updateValues)

):

u.checkHelper.loadRow(u.tw.ru.fetchColIDtoRowIndex, oldValues, false)
u.checkHelper.updateRow(u.updateCols, updateValues)

make checkHelper.updateRow take the map[ColumnID]int and cache it
somewhere instead of computing it once per row.

maybe have makeRowUpdater compute it and put it on rowUpdater?

Comments from Reviewable
https://reviewable.io:443/reviews/cockroachdb/cockroach/6753#-:-KHzwU37KWSrT4qj_-ek:1417088721

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#6753 (comment)

[dt]

They're not checking schema changes at all right now. The last couple PRs were refactors/cleanups of the initial impl, but I'll look at handling schema changes in a follow-up.

[dt]

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

---

sql/update.go, line 323 [r1] (raw file):

Previously, paperstreet (Daniel Harrison) wrote…

make checkHelper.updateRow take the map[ColumnID]int and cache it somewhere instead of computing it once per row.

maybe have makeRowUpdater compute it and put it on rowUpdater?

Done.

Left it on updateNode for now though I imagine it's long-term home will become clearer once we see how this looks in UPSERT.

---

Comments from Reviewable