Hi All, +1 to single DROP TABLE
Cheers, Dmitri. On Mon, Sep 21, 2026 at 11:22 PM Ayush Saxena <[email protected]> wrote: > 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 > > >
