Hello, +1. No concern with current PR.
Thanks, Yong > On Sep 29, 2026, at 11:40 AM, Yufei Gu <[email protected]> wrote: > > The current approach looks good to me. Feel free to move forward. > > Yufei > > >> On Tue, Sep 29, 2026 at 7:29 AM Dmitri Bourlatchkov <[email protected]> >> wrote: >> >> Hi All, >> >> +1 to merge 5349 (I also approved in GH). >> >> Cheers, >> Dmitri. >> >>> On Tue, Sep 29, 2026 at 9:58 AM Alex Dutra <[email protected]> wrote: >>> >>> Hi all, >>> >>> The PR [5349] has received two approvals already. Are we OK to merge it >> as >>> is? >>> >>> Thanks, >>> Alex >>> >>> [5349]: https://github.com/apache/polaris/pull/5349 >>> >>> On Sat, Sep 26, 2026 at 1:44 AM Yufei Gu <[email protected]> wrote: >>>> >>>> Hi Alex, >>>> >>>> Thanks for the update. The PR looks good to me now. >>>> >>>> Creating a document to list all source-of-truth schema files in the Git >>> rep >>>> works for now. We can address any further improvements as follow-ups. >>>> >>>> Yufei >>>> >>>> >>>> On Fri, Sep 25, 2026 at 2:28 PM Prithvi S <[email protected] >>> >>>> wrote: >>>> >>>>> 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 >>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>>> >>> >>
