sql/schemachanger: fix ALTER PRIMARY KEY idempotency for hash-sharded indexes

[rafiss]

Previously, running ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (j) USING HASH twice on the same table would fail instead of being recognized as a no-op. The idempotency check in
isNewPrimaryKeySameAsOldPrimaryKey compared the old index's key columns (which include the shard column, e.g.
[crdb_internal_j_shard_16, j]) against the user-specified columns ([j]). The length mismatch caused the function to return false, proceeding with a redundant ALTER that then errored.

Fix this by moving the sharded-vs-not-sharded check earlier and, when both old and new PKs are hash-sharded, filtering out the shard column from the old index's key columns before comparison.

Resolves: #130191
Resolves: #113587

Release note (bug fix): Fixed a bug where running ALTER TABLE ... ALTER PRIMARY KEY USING COLUMNS (...) USING HASH a second time on a table that already had the same hash-sharded primary key would fail instead of being treated as a no-op.

[trunk-io]

😎 Merged successfully - details.

[cockroach-teamcity]

This change is 

[fqazi]

One minor issue with this

@fqazi reviewed all commit messages and made 3 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on rafiss and shghasemi).

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go line 408 at r1 (raw file):

	// shard column, but the old index's key columns do.
	if oldPrimaryIndexElem.Sharding != nil {
		shardColID := getColumnIDFromColumnName(

I think this still needs to confirm the bucket count? Since that could have changed?

---

pkg/sql/logictest/testdata/logic_test/alter_primary_key line 1440 at r1 (raw file):

# Repeating the same ALTER PRIMARY KEY with USING HASH should be a no-op.
statement ok
ALTER TABLE t1 ALTER PRIMARY KEY USING COLUMNS (j) USING HASH;

Should we have a builder test to confirm no elements get generated or execute explain?

[rafiss]

Addressed both review comments:

Bucket count check (line 408): The bucket count comparison was already present further down in the same function (lines 440-449 at r1). It evaluates the new shard bucket count and compares it against the old one, returning false if they differ. I've added a builder test case that explicitly demonstrates changing the bucket count is not a no-op.

Builder test (line 1440): Added both:

1. A builder test in scbuild/testdata/alter_table_alter_primary_key confirming zero elements are generated for the no-op case (and that changing bucket count does generate elements).
2. An EXPLAIN (DDL) query in the logic test confirming only UndoAllInTxnImmediateMutationOpSideEffects is produced (no real schema change operations).

[fqazi]

@fqazi reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on rafiss and shghasemi).

[rafiss]

/trunk merge

TFTR!