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 >
