Hi Alex, Thanks for your input!
1. I'll keep my changes on the PR that add the default quarkus.datasource.jdbc.additional-jdbc-properties.currentSc hema=POLARIS_SCHEMA 2. I'll brainstorm further and see if I can find some way! 3. Thanks for the great suggestion. I totally agree and will reflect this in the PR. With regards, Eundo 2026년 7월 21일 (화) 오전 5:23, Alexandre Dutra <[email protected]>님이 작성: > Hi Eundo, > > Thanks for the thorough analysis! I'm glad to see that we're > converging to a really nice solution. > > To your points: > > > I'd therefore propose shipping a default in runtime/defaults: > quarkus.datasource.jdbc.additional-jdbc-properties.currentSchema=POLARIS_SCHEMA > > +1 on this idea, I think it makes migration a lot easier. > > > I think the two-step procedure is the right call for production [...] > but it does add a manual step to every quickstart, docker-compose, and Helm > example > > Indeed. I'm personally OK with that and am sure we can find a smart > way to update the quickstart examples holistically. > > > the Helm chart's persistence.relationalJdbc.schemaName value could stay > > I would rather suggest a new field: > persistence.relationalJdbc.additionalProperties, with default value: { > currentSchema: POLARIS_SCHEMA }. This way we also leave the door open > for users that want to customize other JDBC properties. > > Thank you for working on this! > > Alex > > > On Sun, Jul 19, 2026 at 1:24 PM Eundo Lee <[email protected]> wrote: > > > > Hi Alex, > > > > Thanks for taking this further — I agree with the direction. Driver-level > > schema selection is the logical endpoint of the review so far > > (statement-level -> connection-level -> driver-level), and it removes the > > hardest parts of the PR from Polaris's responsibility entirely: quoting, > > identifier case folding, validation, and the per-database SET statements > > all become the driver's documented behavior. It also helps with MySQL > > support later (#3960), and — procedurally nice — it eliminates the > > RelationalJdbcConfiguration change altogether, so the PR no longer > touches > > an extension point at all. Most of the already-reviewed work (unqualified > > generated SQL, schema-agnostic scripts for the current schema version) > > carries over unchanged. > > > > Two points I'd like to settle before reworking the PR in this direction: > > > > 1. Upgrade compatibility. Today every relational-jdbc deployment has its > > tables in POLARIS_SCHEMA because the code and scripts force it. If the > > persistence layer becomes fully schema-agnostic, an existing > deployment > > that upgrades without adding currentSchema to its JDBC configuration > > would resolve unqualified names against the default search path — > > "public" on PostgreSQL — and break (or quietly bootstrap a second set > > of tables in public, which is what #1116 set out to avoid). I'd > > therefore propose shipping a default in runtime/defaults: > > > > > > > quarkus.datasource.jdbc.additional-jdbc-properties.currentSchema=POLARIS_SCHEMA > > > > This covers PostgreSQL and CockroachDB (the shipped drivers; H2 is > > test-only), keeps upgrades and the out-of-the-box experience > unchanged, > > and is trivially overridable: pgjdbc documents that when a property is > > specified both in the URL and in the Properties object, the Properties > > value is ignored [1] — and additional-jdbc-properties reaches the > driver > > through the Properties channel. I verified this empirically with > pgjdbc > > 42.7.13 against both PostgreSQL 17 and CockroachDB: a currentSchema > URL > > parameter overrides the connection property in both, the property > alone > > selects the schema correctly in both, and with neither set the session > > lands in "public" — which is exactly the silent failure mode the > shipped > > default would prevent. (The currentSchema value is also case-folded > like > > any unquoted identifier on both databases, consistent with the casing > > behavior we already agreed on.) > > > > [1] https://jdbc.postgresql.org/documentation/use/ > > > > 2. Bootstrap friction for fresh deployments. With no CREATE SCHEMA issued > > by Polaris, even a default fresh installation needs a DBA (or init > > script) to run CREATE SCHEMA polaris_schema before the admin tool can > > bootstrap. I think the two-step procedure is the right call for > > production — schema creation is privileged DBA work — but it does add > a > > manual step to every quickstart, docker-compose, and Helm example, so > > I'd like the community to explicitly bless that trade-off. The > > getting-started materials would gain an init step as part of the PR. > > > > One suggestion to keep the operator UX: the Helm chart's > > persistence.relationalJdbc.schemaName value could stay, but instead of > > mapping to a Polaris property it would simply render the > > additional-jdbc-properties.currentSchema datasource property. Operators > > keep one discoverable knob; the persistence code stays 100% > > schema-agnostic. > > > > On tests: agreed this is solvable — H2 supports running initialization > SQL > > via the INIT= URL parameter (e.g. CREATE SCHEMA IF NOT EXISTS ... \; > > SET SCHEMA ...), which covers schema creation and selection for the > > H2-based tests cleanly. > > > > I'll rework the PR accordingly — it's mostly deletions at this point, but > > we can > > make additional changes if any disagreements arise. > > > > Also, as Yufei mentioned in the PR comment [2], I believe the community > has > > reached a level of consensus high enough for this PR to be moved out of > > draft > > state. Moving to your new direction would actually eliminate the new > > extension point (polaris.persistence.relational.jdbc.schema-name) as > well. > > > > [2] https://github.com/apache/polaris/pull/4945#discussion_r3583285567 > > > > Thanks, > > Eundo > > > > 2026년 7월 16일 (목) 오전 1:47, Alexandre Dutra <[email protected]>님이 작성: > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
