Hi all, +1 on landing #5349 this way. The version check belongs with the single schema.sql, and the upgrade notes can wait.
The page still says to apply the migration SQL below, and that SQL is gone. What's left is a diff of schema.sql, from v6 on. I think we should still give people some SQL. A schema diff shows the end state, not what to run when the tables already have data. idempotency_records only exists in v4, and the v5 file says it was never wired up. v3 to v5 has nothing to do for that table. Running every hop creates it and then drops it. On a real v4 database, DROP TABLE IF EXISTS is enough, index and primary key included. I'd only list a dependency when the drop changes some other table we're keeping, like Ayush said. Same kind of gap elsewhere. The 1.7 events change needs an UPDATE to clear '__realm__', and a schema diff only shows the nullable column. The v6 notes drop and recreate idx_locations, and Cockroach needs indexes even when version already says 6. #5349 checks that number, so the database still starts. #5437 already added the idempotency drop on the published 1.7.0 page. The changelog and the doc on main never picked it up. So a short note per release, only for those steps. New tables and columns come from the diff. Anything added and removed in between can be skipped. I'd keep the SQL as a file in the repo and test it the way JB described. Load the old schema, run the file, check columns and indexes against the next schema. The doc links the file. Thanks, Prithvi S On Fri, Sep 25, 2026 at 6:53 PM Dmitri Bourlatchkov <[email protected]> wrote: > Hi All, > > Alex's proposal for making progress here sounds good to me. +1 to PR [5349] > in its current state. > > I think it is also a good idea to reopen the discussion on schema upgrade > instructions after merging this PR. > > [5349] https://github.com/apache/polaris/pull/5349 > > Cheers, > Dmitri. > > On Fri, Sep 25, 2026 at 9:03 AM Alex Dutra <[email protected]> wrote: > > > Hi Yufei, hi all, > > > > In [5349], I do believe that the instructions were complete and correct. > > > > However, in the spirit of making progress in that PR, which is stuck > > until we settle on this specific topic, I went ahead and removed ALL > > migration SQL snippets from that PR, **including** what was already > > present on main, as you requested. Only generic instructions about how > > to get the schema diffs were retained. > > > > I hope this move will help get that PR unstuck. > > > > But now I think this discussion thread (or a new one maybe) should > > answer a broader question: should we provide any form of specific SQL > > migration guidance to users, and if so, in what form? > > > > Thanks, > > Alex > > > > [5349]: https://github.com/apache/polaris/pull/5349 > > > > > > On Thu, Sep 24, 2026 at 10:53 PM Yufei Gu <[email protected]> wrote: > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
