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 > > >
