-
Notifications
You must be signed in to change notification settings - Fork 506
Make the Relational JDBC schema name configurable #4945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
71a1007
f284f34
add93e0
1d1d534
ca6b1ca
79faeed
5b73c66
998e702
d75c444
5433bbe
2eb56ff
b955915
06dd6f3
13a6ed8
4876c41
6900912
eb39400
700e54b
142c64c
9b9a6c9
cd827ce
8dcf2df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,10 @@ | |
| /** | ||
| * Utility class to generate parameterized SQL queries (SELECT, INSERT, UPDATE, DELETE). Ensures | ||
| * consistent SQL generation and protects against injection by managing parameters separately. | ||
| * | ||
| * <p>Generated queries reference tables by their unqualified names; the schema holding the Polaris | ||
| * tables is selected through the datasource configuration (for example the PostgreSQL driver's | ||
| * {@code currentSchema} connection property), so the persistence code is agnostic of it. | ||
| */ | ||
| public class QueryGenerator { | ||
|
|
||
|
|
@@ -127,8 +131,7 @@ public static PreparedQuery generateDeleteQueryForEntityGrantRecords( | |
| List<Object> params = | ||
| Arrays.asList( | ||
| entity.getId(), entity.getCatalogId(), entity.getId(), entity.getCatalogId(), realmId); | ||
| return new PreparedQuery( | ||
| "DELETE FROM " + getFullyQualifiedTableName(ModelGrantRecord.TABLE_NAME) + where, params); | ||
| return new PreparedQuery("DELETE FROM " + ModelGrantRecord.TABLE_NAME + where, params); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -195,14 +198,7 @@ public static PreparedQuery generateInsertQuery( | |
| finalValues.add(realmId); | ||
| String columns = String.join(", ", finalColumns); | ||
| String placeholders = finalColumns.stream().map(c -> "?").collect(Collectors.joining(", ")); | ||
| String sql = | ||
| "INSERT INTO " | ||
| + getFullyQualifiedTableName(tableName) | ||
| + " (" | ||
| + columns | ||
| + ") VALUES (" | ||
| + placeholders | ||
| + ")"; | ||
| String sql = "INSERT INTO " + tableName + " (" + columns + ") VALUES (" + placeholders + ")"; | ||
| return new PreparedQuery(sql, finalValues); | ||
| } | ||
|
|
||
|
|
@@ -223,8 +219,7 @@ public static PreparedQuery generateUpdateQuery( | |
| List<Object> bindingParams = new ArrayList<>(values); | ||
| QueryFragment where = generateWhereClause(new HashSet<>(allColumns), whereClause, Map.of()); | ||
| String setClause = allColumns.stream().map(c -> c + " = ?").collect(Collectors.joining(", ")); | ||
| String sql = | ||
| "UPDATE " + getFullyQualifiedTableName(tableName) + " SET " + setClause + where.sql(); | ||
| String sql = "UPDATE " + tableName + " SET " + setClause + where.sql(); | ||
| bindingParams.addAll(where.parameters()); | ||
| return new PreparedQuery(sql, bindingParams); | ||
| } | ||
|
|
@@ -242,8 +237,7 @@ public static PreparedQuery generateDeleteQuery( | |
| @NonNull String tableName, | ||
| @NonNull Map<String, Object> whereClause) { | ||
| QueryFragment where = generateWhereClause(new HashSet<>(tableColumns), whereClause, Map.of()); | ||
| return new PreparedQuery( | ||
| "DELETE FROM " + getFullyQualifiedTableName(tableName) + where.sql(), where.parameters()); | ||
| return new PreparedQuery("DELETE FROM " + tableName + where.sql(), where.parameters()); | ||
| } | ||
|
|
||
| private static PreparedQuery generateSelectQuery( | ||
|
|
@@ -263,12 +257,7 @@ private static PreparedQuery generateSelectQuery( | |
| if (limit != null && limit <= 0) { | ||
| throw new IllegalArgumentException("Limit must be positive"); | ||
| } | ||
| String sql = | ||
| "SELECT " | ||
| + String.join(", ", columnNames) | ||
| + " FROM " | ||
| + getFullyQualifiedTableName(tableName) | ||
| + filter; | ||
| String sql = "SELECT " + String.join(", ", columnNames) + " FROM " + tableName + filter; | ||
| if (orderByColumn != null) { | ||
| sql += " ORDER BY " + orderByColumn + " ASC"; | ||
| } | ||
|
|
@@ -336,7 +325,7 @@ static QueryFragment generateWhereClauseExtended( | |
|
|
||
| @VisibleForTesting | ||
| static PreparedQuery generateVersionQuery() { | ||
| return new PreparedQuery("SELECT version_value FROM POLARIS_SCHEMA.VERSION", List.of()); | ||
| return new PreparedQuery("SELECT version_value FROM VERSION", List.of()); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -348,17 +337,14 @@ public static PreparedQuery generateExistsQuery( | |
| @NonNull String tableName, | ||
| @NonNull Map<String, Object> whereClause) { | ||
| QueryFragment where = generateWhereClause(new HashSet<>(tableColumns), whereClause, Map.of()); | ||
| String sql = | ||
| "SELECT 1 FROM " + getFullyQualifiedTableName(tableName) + where.sql() + " LIMIT 1"; | ||
| String sql = "SELECT 1 FROM " + tableName + where.sql() + " LIMIT 1"; | ||
| return new PreparedQuery(sql, where.parameters()); | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| static PreparedQuery generateEntityTableExistQuery() { | ||
| return new PreparedQuery( | ||
| String.format( | ||
| "SELECT * FROM %s LIMIT 1", getFullyQualifiedTableName(ModelEntity.TABLE_NAME)), | ||
| List.of()); | ||
| String.format("SELECT * FROM %s LIMIT 1", ModelEntity.TABLE_NAME), List.of()); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -413,9 +399,4 @@ public static PreparedQuery generateOverlapQuery( | |
| null); | ||
| return new PreparedQuery(query.sql(), where.parameters()); | ||
| } | ||
|
|
||
| static String getFullyQualifiedTableName(String tableName) { | ||
| // TODO: make schema name configurable. | ||
| return "POLARIS_SCHEMA." + tableName; | ||
| } | ||
|
Comment on lines
-417
to
-420
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible to have 2 part identifier still ? can we get the schema and add this here ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or to put it in a different way are we logging what is the current_schema
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO, keeping the 2 part identifier coded into SQL statements seems a little redundant. However, I'm open to suggestions!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also think it's best to not deal with schemas in the code. However it should certainly be possible to log the current schema somewhere; it should also be possible to put the schema name in the MDC context if needed. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the
docker-entrypoint-initdb.dapproach 😄