Hi Yufei, Thanks for the quick action. I've updated the PR to leave historical DDL scripts unchanged.
With regards, Eundo 2026년 7월 14일 (화) 오전 1:45, Yufei Gu <[email protected]>님이 작성: > Hi Eundo, > > Thanks for the clarification. Just a small update, the --schema-version > option has already been removed from the bootstrap command( > https://github.com/apache/polaris/pull/5044), so I don't think we need to > update the historical DDL scripts anymore. > > With that out of the way, I think this PR can focus on making the current > schema configurable and updating the current bootstrap path accordingly. > > Thanks again for working on this! > > Yufei > > > On Sat, Jul 11, 2026 at 2:01 AM Eundo Lee <[email protected]> wrote: > > > Hi Yufei, > > > > I was not aware of the discussion regarding keeping the latest schema > > version only. > > Thanks for the heads up. > > > > It seems that you've opened a separate discussion thread on the removal > of > > --schema-version > > option from the bootstrap command. So I'm guessing that the removal > should > > be out of scope > > from this PR. > > > > In that case, in order for the polaris deployment to work as-is with a > > configurable schema, > > I think we should keep the changes to the historical DDL scripts, and > > remove the scripts once > > the --schema-version option is removed from the bootstrap command. > > > > Please feel free to correct me if I'm wrong. > > > > Thanks, > > > > Eundo > > > > > > 2026년 7월 11일 (토) 오전 9:00, Yufei Gu <[email protected]>님이 작성: > > > > > Hi Eundo, > > > > > > Thanks for working on this. Making the schema name configurable is a > good > > > idea. I also agree with Alex that we should avoid setting the schema > > every > > > time we obtain a connection from the pool. > > > > > > I do have one minor concern. I don't think we need to touch the > > historical > > > JDBC schemas. We've previously discussed on the dev mailing list that > > each > > > Polaris release should only keep the latest schema version. One small > > > behavior change would naturally follow from this. We could remove the > > > --schema-version option from the bootstrap command[1]. That option > allows > > > users to bootstrap a realm with a historical schema version. While it > may > > > be useful in some rare cases, I don't think it provides enough value to > > > justify the added complexity. > > > > > > 1. > > > > > > > > > https://polaris.apache.org/releases/1.6.0/admin-tool/#bootstrapping-realms-and-principal-credentials > > > > > > Thanks, > > > > > > Yufei > > > > > > > > > On Thu, Jul 2, 2026 at 2:08 AM Eundo Lee <[email protected]> wrote: > > > > > > > Thank you for taking your time to review. > > > > > > > > I've applied your suggestion on using an explicit default value for > the > > > > helm chart. > > > > For other points I agree very much with your direction. > > > > However, I just wanted to confirm with you the design / > implementation > > > > details > > > > before diving in to create more commits. > > > > > > > > I would greatly appreciate additional inputs. > > > > > > > > Thanks, > > > > Eundo Lee > > > > > > > > > > > > 2026년 7월 1일 (수) 오후 11:57, Alexandre Dutra <[email protected]>님이 작성: > > > > > > > > > Hi Eundo, > > > > > > > > > > Thank you for raising this topic and providing an initial draft > PR! I > > > > > left a few comments. > > > > > > > > > > I generally agree with the idea of making the schema name > > > > > configurable. But I'm not sure I agree with some design choices > made > > > > > in the PR: for example, I think it would be easier to let the > Agroal > > > > > pool set the schema on each JDBC connection, rather than having it > > set > > > > > on each SQL statement generated by the QueryGenerator. > > > > > > > > > > Let's keep discussing implementation details in the PR though. > > > > > > > > > > Thanks, > > > > > Alex > > > > > > > > > > On Wed, Jul 1, 2026 at 11:30 AM 이은도 <[email protected]> wrote: > > > > > > > > > > > > Hi all, > > > > > > > > > > > > I'd like to get feedback on a small enhancement to the Relational > > > JDBC > > > > > > persistence backend before moving the PR out of draft. > > > > > > > > > > > > Problem > > > > > > > > > > > > The backend currently hard-codes its database schema as > > > POLARIS_SCHEMA > > > > > > -- both in QueryGenerator (there's an existing "// TODO: make > > schema > > > > name > > > > > > configurable.") and in the bootstrap SQL scripts. This makes it > > > > > impossible > > > > > > to comply with a schema-naming policy that organizations might > > have. > > > > > > It came up while looking at issue #1116 (the older EclipseLink > > "avoid > > > > > > public schema" > > > > > > request); the JDBC backend fixed the "not public" part by > > > hard-coding a > > > > > > name, > > > > > > but the value still isn't configurable. > > > > > > > > > > > > Issue: https://github.com/apache/polaris/issues/4944 > > > > > > Draft PR: https://github.com/apache/polaris/pull/4945 > > > > > > > > > > > > Proposed change > > > > > > > > > > > > Add polaris.persistence.relational.jdbc.schema-name, defaulting > to > > > > > > POLARIS_SCHEMA so existing deployments are unaffected. The value > is > > > > > > validated as a plain SQL identifier (it's interpolated into SQL, > > not > > > > > > bound). Bootstrap scripts use a ${schema} placeholder substituted > > at > > > > > > script-execution time so the configured schema is created and > used > > > > > > consistently. > > > > > > > > > > > > Points I'd like input on > > > > > > > > > > > > 1. This adds a method to RelationalJdbcConfiguration, which is an > > > > > > extension point -- hence this discussion per CONTRIBUTING.md. > > > > > > > > > > > > 2. To wire the schema through, I converted QueryGenerator from a > > > static > > > > > > utility to an instance bound to the schema. That's the bulk of > > the > > > > > diff > > > > > > (mechanical call-site updates). An alternative is threading > the > > > > schema > > > > > > through method parameters; the instance approach seemed > cleaner, > > > but > > > > > I'm > > > > > > happy to change direction. > > > > > > > > > > > > 3. For the admin tool, I exposed the schema as the same config > > > property > > > > > > rather than a separate CLI flag, so the admin tool and server > > > can't > > > > > > bootstrap into different schemas. Let me know if a CLI flag is > > > > > preferred > > > > > > despite that drift risk. > > > > > > > > > > > > Feedback welcome -- I'll hold the PR in draft until there's > > > consensus. > > > > > > > > > > > > Thanks, > > > > > > Eundo Lee > > > > > > > > > > > > > > >
