sql: warn when creating non-partitioned index

[solongordon]

In most cases, secondary indexes created on a partitioned table should
also be partitioned. To protect users from this gotcha, we now return an
error in this case if sql_safe_updates is on.

This isn't an ideal solution to the problem. It only really protects
users who are using the interactive CLI, and if a user really does want
an unpartitioned index it will force them to turn off sql_safe_updates,
at least temporarily. However, I didn't have much luck finding another
way to deliver a warning which users were likely to see.

Fixes #38475

Release note: None

[cockroach-teamcity]

This change is 

[solongordon]

Open to other ideas about how to deliver this warning if people have them!

[rohany]

Is there a way to make a unit/logic test for this?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis, @rohany, and @solongordon)

---

pkg/sql/create_index.go, line 95 at r1 (raw file):

	// which is undesirable in most cases.
	if params.SessionData().SafeUpdates && n.n.PartitionBy == nil &&
		n.tableDesc.PrimaryIndex.Partitioning.NumColumns > 0 {

do you need to check all indexes, not just the primary index?

---

pkg/sql/create_table.go, line 142 at r1 (raw file):

	if params.SessionData().SafeUpdates && n.n.PartitionBy != nil {
		for _, def := range n.n.Defs {
			if d, ok := def.(*tree.IndexTableDef); ok {

I think you also want to check here if the primary key isn't partitioned, but a secondary index is partitioned.

[solongordon]

Yup, added a few logic tests. (I was trying to slip them in before anyone reviewed but you were too fast.)

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @rohany)

---

pkg/sql/create_index.go, line 95 at r1 (raw file):

Previously, rohany (Rohan Yadav) wrote…

do you need to check all indexes, not just the primary index?

My understanding is that checking the primary index tells you whether the "table" is partitioned, so I think it's enough? I don't think we need to error if the user is adding an unpartitioned index to a table where the primary index isn't partitioned but some other index is.

---

pkg/sql/create_table.go, line 142 at r1 (raw file):

Previously, rohany (Rohan Yadav) wrote…

I think you also want to check here if the primary key isn't partitioned, but a secondary index is partitioned.

Interesting, yeah that seems unlikely but I suppose it would be weird enough to warn about.

[rohany]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @solongordon)

---

pkg/sql/create_index.go, line 95 at r1 (raw file):

Previously, solongordon (Solon) wrote…

My understanding is that checking the primary index tells you whether the "table" is partitioned, so I think it's enough? I don't think we need to error if the user is adding an unpartitioned index to a table where the primary index isn't partitioned but some other index is.

yeah that makes sense

---

pkg/sql/create_table.go, line 142 at r1 (raw file):

Previously, solongordon (Solon) wrote…

Interesting, yeah that seems unlikely but I suppose it would be weird enough to warn about.

yup, since we are warning anyway, might as well.

[solongordon]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)