Hi Eundo, I did another review and I think things are shaping up nicely.
I think though that we need to go one step further and make the whole JDBC persistence layer completely agnostic of the schema name. The schema name can be provided through configuration, using standard Quarkus configuration properties; the Agroal datasource would then take care of setting the schema on each connection. There is nothing to do code-wise. This imho eliminates all the issues stemming from quoting / casing the schema names, and makes the code a lot more portable. This approach, of course, makes it impossible for Polaris to issue a CREATE SCHEMA statement on the user's behalf. I understand that this is a breaking change. But I also think it is a good thing to stop creating schemas on the fly: such a statement should be executed by DB admins, as they require elevated privileges. If we agree with this direction, then the bootstrap story becomes a two-step procedure: a DB admin first creates the schema, then a Polaris admin invokes the admin tool to bootstrap the realm, and passes the schema to use via Quarkus config. I'm aware that this may require some changes to tests using H2, but I'm fairly confident there is a way to initialize the schema in these cases. Let me know what you think! Thanks, Alex On Tue, Jul 14, 2026 at 4:24 AM Eundo Lee <[email protected]> wrote: > > 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 > > > > > > > > > > > > > > > > > > > >
