Hi Yufei, > This fails on older schemas that don't have the events table.
If a user follows the migrations sequentially, the events table is guaranteed to exist by the time the v4 to v5 script runs. The UPDATE statement only fails if someone tries to apply the v4 to v5 migration on a schema that never went through previous steps. That's an operator error, not a documentation issue. > I'd suggest removing the entire Schema upgrades section That's a net loss for end users. Why not keep it? I agree it's subject to drift, but as I mentioned earlier, 1) it's still better than nothing, and 2) we can prevent drift by creating migration tests that exercise the migration steps. Thanks, Alex On Wed, Sep 23, 2026 at 5:18 AM Yong Zheng <[email protected]> wrote: > > Thanks for the feedback all. I will proceed with the change. > > Thanks, > Yong > > On 2026/09/22 23:45:05 Yufei Gu wrote: > > I think users shouldn't rely on SQL snippets in the upgrade guide as the > > source of truth for schema changes. > > > > For example: UPDATE polaris_schema.events SET catalog_id = NULL WHERE > > catalog_id = '__realm__'; > > This fails on older schemas that don't have the events table. Like the > > ALTER TABLE example discussed above, it shows why migration steps need to > > account for the starting schema version. > > > > I'd suggest removing the entire Schema upgrades section > > <https://polaris.apache.org/releases/1.7.0/metastores/relational-jdbc/#schema-upgrades>. > > The historical schema SQL files should remain the source of truth for each > > schema version. Maintaining migration SQL separately in the documentation > > is prone to errors and drift. > > > > > > On Tue, Sep 22, 2026 at 10:40 AM Alex Dutra <[email protected]> wrote: > > > > > Hi all, > > > > > > I agree with Ayush. For the record, I initially included a DROP INDEX > > > statement in my own PR [5349], but in hindsight, I think that was a > > > bad idea that sparked this whole discussion. I apologize for that. > > > > > > > #5349 removes the runtime fallback and makes the migration mandatory > > > with a fail fast on version mismatch. That turns these snippets from > > > advisory into load bearing, and today they are > > > untested prose [...] the durable fix is a test per backend > > > > > > +1000. That is my intention as soon as [5349] gets merged. I think we > > > can also improve on the visual presentation and propose migration > > > steps for each database type, using Docsy tabbed panes [1]. But that > > > PR is already big, so I think we need to move forward incrementally > > > here. > > > > > > Thanks, > > > Alex > > > > > > [1]: https://www.docsy.dev/docs/content/shortcodes/#tabbed-panes > > > [5349]: https://github.com/apache/polaris/pull/5349 > > > > > > On Tue, Sep 22, 2026 at 6:30 AM Jean-Baptiste Onofré <[email protected]> > > > wrote: > > > > > > > > Hi guys, > > > > > > > > +1 to what Ayush said: I would keep the upgrade as a single DROP TABLE > > > > IF > > > > EXISTS polaris_schema.idempotency_records. > > > > > > > > The decisive argument for me is that the block is documented as covering > > > > "an existing v3/v4 database". One snippet, two source versions, so every > > > > statement in it has to be a no-op on the lower one. v3 never had > > > > idempotency_records (it is created in the v3 -> v4 section), so ALTER > > > TABLE > > > > ... DROP CONSTRAINT idempotency_records_pkey fails there, and with > > > > ON_ERROR_STOP the rest of the "migration" never runs. DROP TABLE IF > > > EXISTS > > > > is a no-op on v3 and correct on v4. That property is worth more to > > > > operators than explicitness. > > > > > > > > On the revert motivation: drop statements do not record how to recreate > > > > anything. A "downgrade" needs the CREATE statements, and those already > > > > exist in schema-v4.sql and are repeated in the v3 -> v4 section of > > > > #5349. > > > > Listing more drops adds nothing there imho. If we want documented > > > downgrade > > > > later, it should be its own script of CREATE statements. > > > > > > > > Also, idempotency_records_pkey is just the PostgreSQL default constraint > > > > name. A deployment where the constraint was created under a different > > > name > > > > fails the ALTER and succeeds on the DROP TABLE. We claim PostgreSQL, > > > > CockroachDB and H2 for this block, so I would avoid generated names in > > > > operator facing SQL. > > > > > > > > As a convention, Ayush carve-out is the right line: list a dependency > > > only > > > > when the drop reaches outside the object being removed, typically a > > > foreign > > > > key from a table we are keeping, where CASCADE silently alters that > > > > retained table. That is an operator visible side effect and belongs in > > > the > > > > instructions. Cascades confined to the dropped object are noise. > > > > > > > > One broader point while we are here: #5349 removes the runtime fallback > > > and > > > > makes the migration mandatory with a fail fast on version mismatch. That > > > > turns these snippets from advisory into load bearing, and today they are > > > > untested prose (which is how a statement that errors on PostgreSQL got > > > > as > > > > far as it did). I think the durable fix is a test per backend: bootstrap > > > > schema-vN, apply the documented N -> N+1 SQL, assert the result matches > > > > schema-v(N+1). That would catch this class of issue in CI rather than on > > > > the list. Happy to open an issue for it. > > > > > > > > Regards > > > > JB > > > > > > > > On Tue, Sep 22, 2026 at 4:16 AM Yong Zheng <[email protected]> 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 > > > > > > > > > >
