> That's a net loss for end users. This assumes the documented instructions are correct and sufficient to complete the upgrade. I don’t think that’s the case. For example, what steps should users follow to upgrade from a schema version earlier than v4 to v5? Why do users need to create a table and then delete it during a upgrade? It is getting even more complex when data backfill is needed. In a real env, users still need to consult the versioned schema files,* the source of truth*, to understand what has changed.
Schema upgrades are critical operations that require great care; even a small error can break the system. Given these gaps, I don’t think users can confidently rely solely on the Schema upgrades section <https://polaris.apache.org/releases/1.7.0/metastores/relational-jdbc/#schema-upgrades> . Maintaining separate migration instructions also adds a maintenance burden for the community when we already have the source of truth in the rep. Besides, incomplete instructions can give users a false sense of confidence that following the page is sufficient for a safe upgrade, which is disastrous. Yufei On Wed, Sep 23, 2026 at 6:06 AM Alex Dutra <[email protected]> wrote: > 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 > > > > > > > > > > > > > >
