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

Reply via email to