Hi Yong, I'd keep the upgrade as a single statement:
``` DROP TABLE IF EXISTS polaris_schema.idempotency_records; ``` On PostgreSQL, CockroachDB, and H2 that drops idx_idemp_realm_expires and idempotency_records_pkey with the table. Dropping them first does the same thing. It also doesn't help a revert. Those statements remove the same objects; they don't record how to put them back. The create statements are already in schema-v4.sql, and the v3-to-v4 migration in #5349 repeats them. That is the form a rollback needs. We don't document downgrades today. If we want one later, it should be its own script of create statements. The explicit form already failed on #5437: - The primary key is a constraint, so DROP INDEX cannot remove it. - DROP INDEX given a table name is rejected by PostgreSQL regardless of IF EXISTS; that guard covers absence, not the wrong object type. With ON_ERROR_STOP the rest of the migration never runs. - ALTER TABLE ... DROP CONSTRAINT idempotency_records_pkey fails when the table is missing. The 1.7.0 instructions cover v3 and v4, and v3 never had this table. DROP TABLE IF EXISTS is a no-op on v3 and correct on v4. idempotency_records_pkey is the PostgreSQL default name. A deployment that created the constraint under another name fails the ALTER and still succeeds on DROP TABLE. #5349 already uses only DROP TABLE IF EXISTS. I'd match that. I would still list a dependency when another object we are keeping depends on the one we drop. A foreign key from another table makes DROP TABLE fail, and CASCADE then drops that constraint from a table we are keeping — an effect that lands outside the object being removed, so it belongs in the instructions rather than being implied. Nothing in the v4-to-v5 change is in that category: idempotency_records has no inbound references, and as noted in the PR it was never used in released code, so it should be empty as well. -Ayush On 2026/09/22 02:16:00 Yong Zheng wrote: > Hello all, > > I would like to start this discussion regarding if we should list out > cascading dependency deletion during schema version change (see sample > reference in https://github.com/apache/polaris/pull/5437). > > With the sample reference, we will be dropping table > polaris_schema.idempotency_records which has index > "polaris_schema.idx_idemp_realm_expires" and PK on "idempotency_records_pkey". > > When we drop the table, it would drop the PK along with index during > cascading dependency deletion. One reason to keep those is to easy the revert > process if we need to do so. However, keeping all of the dependencies listed > out is a bit tedious and add over-head for regular schema upgrade (in this > case, instead of running one drop sql, we would need to do 2 drop plus 1 > alter table sql statements). Thoughts? > > Thanks, > Yong >
