Add Configurable Schema support for PostreSQL DB.#4566
Merged
sadilchamishka merged 1 commit intowso2:4.12.xfrom Apr 3, 2026
Merged
Add Configurable Schema support for PostreSQL DB.#4566sadilchamishka merged 1 commit intowso2:4.12.xfrom
sadilchamishka merged 1 commit intowso2:4.12.xfrom
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 |
📝 WalkthroughSummary by CodeRabbitBug Fixes
WalkthroughThe change adds PostgreSQL-specific schema handling to the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java (3)
1535-1548: Avoid redundantgetDatabaseProductName()calls.The
getDatabaseProductName()is called twice: once at line 1535 (for DB2 check) and again at line 1543 (for PostgreSQL check). Consider caching the result in a local variable to avoid redundant metadata queries.♻️ Proposed refactor to cache database product name
if (metaData.storesLowerCaseIdentifiers()) { tableName = tableName.toLowerCase(); } - if (connection.getMetaData().getDatabaseProductName().toLowerCase().contains(DB2)) { + String databaseProductName = metaData.getDatabaseProductName(); + if (databaseProductName != null && databaseProductName.toLowerCase().contains(DB2)) { return connection.getMetaData().getTables(null, null, tableName, new String[]{"TABLE"}).next(); } String schemaName = connection.getSchema(); String catalogName = connection.getCatalog(); // For PostgreSQL, use the configured schema if available. This handles cases where tables reside in a // non-default schema that differs from the first entry in the database user's search path. - if (POSTGRE_SQL.equalsIgnoreCase(connection.getMetaData().getDatabaseProductName())) { + if (POSTGRE_SQL.equalsIgnoreCase(databaseProductName)) { String postgresSchema = getPostgresSchema(); if (org.apache.commons.lang.StringUtils.isNotBlank(postgresSchema)) { schemaName = postgresSchema.trim(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java` around lines 1535 - 1548, The code in HybridRoleManager repeatedly calls connection.getMetaData().getDatabaseProductName(); cache this value in a local variable (e.g., dbProductName) immediately after obtaining the metadata and use that variable for the DB2 check (contains(DB2)) and the POSTGRE_SQL.equalsIgnoreCase check; ensure you apply the same case handling used currently (toLowerCase() for DB2 check or call toLowerCase() on the cached value) and retain the existing logic that reads and possibly overrides schemaName using getPostgresSchema().
25-25: Tight coupling to Tomcat JDBC pool.The import of
DataSourceProxycouples this feature specifically to Tomcat's JDBC connection pool. If a different pool implementation is used (e.g., HikariCP), the PostgreSQL schema configuration will silently be ignored. The graceful fallback (return null) prevents failures, but the feature simply won't work with non-Tomcat pools.Consider documenting this limitation or adding a warning log when the data source is not a
DataSourceProxy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java` at line 25, The code in HybridRoleManager currently assumes a Tomcat DataSourceProxy, which silently makes Postgres schema config a no-op for other pools; update the logic that checks the DataSource instance (in HybridRoleManager) to detect when the datasource is not an instance of org.apache.tomcat.jdbc.pool.DataSourceProxy and emit a warning via the class logger describing that only Tomcat's DataSourceProxy is supported and the Postgres schema configuration will be ignored; also add a short Javadoc or comment on HybridRoleManager noting this limitation so callers are aware.
1562-1573: Consider adding debug logging for troubleshooting.When the data source is not a
DataSourceProxyor when thepostgresSchemaproperty is not configured, the method silently returnsnull. Adding debug-level logging would help users troubleshoot when the PostgreSQL schema configuration isn't being applied as expected.🔍 Proposed enhancement with debug logging
private String getPostgresSchema() { + if (log.isDebugEnabled()) { + log.debug("Retrieving PostgreSQL schema configuration."); + } String customSchema = null; if (!(dataSource instanceof DataSourceProxy)) { + if (log.isDebugEnabled()) { + log.debug("DataSource is not an instance of DataSourceProxy. PostgreSQL schema configuration " + + "is only supported with Tomcat JDBC pool."); + } return null; } Properties properties = ((DataSourceProxy) dataSource).getDbProperties(); if (properties != null) { customSchema = properties.getProperty(POSTGRES_SCHEMA); + if (log.isDebugEnabled() && customSchema != null) { + log.debug("Found PostgreSQL schema configuration: " + customSchema); + } } return customSchema; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java` around lines 1562 - 1573, getPostgresSchema currently returns null silently when dataSource is not a DataSourceProxy or when POSTGRES_SCHEMA is missing; add debug logging to both failure paths to aid troubleshooting. In getPostgresSchema, use the class logger (or create one if missing) to emit debug messages: one when !(dataSource instanceof DataSourceProxy) indicating the actual dataSource class and that POSTGRES_SCHEMA lookup was skipped, and another when properties != null but properties.getProperty(POSTGRES_SCHEMA) is null indicating the property is not set; keep return behavior unchanged but ensure messages reference getPostgresSchema, DataSourceProxy, POSTGRES_SCHEMA and dataSource for context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java`:
- Around line 1535-1548: The code in HybridRoleManager repeatedly calls
connection.getMetaData().getDatabaseProductName(); cache this value in a local
variable (e.g., dbProductName) immediately after obtaining the metadata and use
that variable for the DB2 check (contains(DB2)) and the
POSTGRE_SQL.equalsIgnoreCase check; ensure you apply the same case handling used
currently (toLowerCase() for DB2 check or call toLowerCase() on the cached
value) and retain the existing logic that reads and possibly overrides
schemaName using getPostgresSchema().
- Line 25: The code in HybridRoleManager currently assumes a Tomcat
DataSourceProxy, which silently makes Postgres schema config a no-op for other
pools; update the logic that checks the DataSource instance (in
HybridRoleManager) to detect when the datasource is not an instance of
org.apache.tomcat.jdbc.pool.DataSourceProxy and emit a warning via the class
logger describing that only Tomcat's DataSourceProxy is supported and the
Postgres schema configuration will be ignored; also add a short Javadoc or
comment on HybridRoleManager noting this limitation so callers are aware.
- Around line 1562-1573: getPostgresSchema currently returns null silently when
dataSource is not a DataSourceProxy or when POSTGRES_SCHEMA is missing; add
debug logging to both failure paths to aid troubleshooting. In
getPostgresSchema, use the class logger (or create one if missing) to emit debug
messages: one when !(dataSource instanceof DataSourceProxy) indicating the
actual dataSource class and that POSTGRES_SCHEMA lookup was skipped, and another
when properties != null but properties.getProperty(POSTGRES_SCHEMA) is null
indicating the property is not set; keep return behavior unchanged but ensure
messages reference getPostgresSchema, DataSourceProxy, POSTGRES_SCHEMA and
dataSource for context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: aedc30c0-5f4a-4410-9064-51d681149111
📒 Files selected for processing (1)
core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/hybrid/HybridRoleManager.java
There was a problem hiding this comment.
Pull request overview
Adds support for checking table existence in a configured (non-default) PostgreSQL schema so HybridRoleManager works correctly when PostgreSQL tables live outside the default search path.
Changes:
- Updates
isTableExiststo prefer a configured PostgreSQL schema when the DB product is PostgreSQL. - Introduces
getPostgresSchema()to readpostgresSchemafrom datasource DB properties. - Adds constants for PostgreSQL product name and the
postgresSchemaproperty key.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sadilchamishka
approved these changes
Apr 3, 2026
|
PR builder started |
|
PR builder completed |
jenkins-is-staging
approved these changes
Apr 3, 2026
jenkins-is-staging
left a comment
jenkins-is-staging
left a comment
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/23940509067
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
In order to configure a non default schema for postgreSQL following config should be added to db configs.
See the following example
This pull request enhances the
HybridRoleManagerto better support PostgreSQL databases, particularly when tables are located in a non-default schema. The main changes ensure that the correct schema is used when checking for table existence, improving compatibility in environments with custom PostgreSQL schemas.PostgreSQL schema support improvements:
isTableExiststo detect when the database is PostgreSQL and override the schema name with a configured value if present, ensuring accurate table existence checks in custom schemas.getPostgresSchemamethod to retrieve thepostgresSchemaproperty from the data source configuration, used to determine the correct schema for PostgreSQL.POSTGRE_SQLandPOSTGRES_SCHEMAfor improved code clarity and maintainability.Dependency and import updates:
DataSourceProxyandPropertiesto support the new schema detection logic. [1] [2]