-
Notifications
You must be signed in to change notification settings - Fork 494
relational-jdbc: add DB-agnostic idempotency store + model #3584
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
Changes from 5 commits
8a2ba83
236cb0f
d2ef040
fc2ac61
a3427fc
323484b
feceb77
6d20fa3
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -50,6 +50,15 @@ public record PreparedBatchQuery(String sql, List<List<Object>> parametersList) | |||||||||
| /** A container for the query fragment SQL string and the ordered parameter values. */ | ||||||||||
| record QueryFragment(String sql, List<Object> parameters) {} | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Returns the fully-qualified table name used by relational-jdbc queries. | ||||||||||
| * | ||||||||||
| * @param tableName Target table name. | ||||||||||
| */ | ||||||||||
| public static String fullyQualifiedTableName(@Nonnull String tableName) { | ||||||||||
| return getFullyQualifiedTableName(tableName); | ||||||||||
| } | ||||||||||
|
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 am confused, why create a new public function ? can we not have private function itself made public ?
Contributor
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. Right, we don't need this. removed. |
||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Generates a SELECT query with projection and filtering. | ||||||||||
| * | ||||||||||
|
|
@@ -192,6 +201,65 @@ public static PreparedQuery generateUpdateQuery( | |||||||||
| return new PreparedQuery(sql, bindingParams); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Builds an UPDATE query that updates only the specified columns and supports richer WHERE | ||||||||||
| * predicates (equality, greater-than, less-than, IS NULL, IS NOT NULL). | ||||||||||
| * | ||||||||||
| * <p>Callers should prefer passing an ordered map (e.g. {@link java.util.LinkedHashMap}) for the | ||||||||||
| * set clause so generated SQL and parameter order are stable. | ||||||||||
|
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. nit: feel a bit easier to understand to use
Suggested change
Contributor
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. Fixed. Thanks for the suggestion! |
||||||||||
| * | ||||||||||
| * @param tableColumns List of valid table columns. | ||||||||||
| * @param tableName Target table. | ||||||||||
| * @param setClause Column-value pairs to update. | ||||||||||
| * @param whereEquals Column-value pairs used in WHERE equality filtering. | ||||||||||
| * @param whereGreater Column-value pairs used in WHERE greater-than filtering. | ||||||||||
| * @param whereLess Column-value pairs used in WHERE less-than filtering. | ||||||||||
| * @param whereIsNull Columns that must be NULL. | ||||||||||
| * @param whereIsNotNull Columns that must be NOT NULL. | ||||||||||
| * @return UPDATE query with parameter bindings. | ||||||||||
| */ | ||||||||||
| public static PreparedQuery generateUpdateQuery( | ||||||||||
| @Nonnull List<String> tableColumns, | ||||||||||
| @Nonnull String tableName, | ||||||||||
| @Nonnull Map<String, Object> setClause, | ||||||||||
| @Nonnull Map<String, Object> whereEquals, | ||||||||||
| @Nonnull Map<String, Object> whereGreater, | ||||||||||
| @Nonnull Map<String, Object> whereLess, | ||||||||||
| @Nonnull Set<String> whereIsNull, | ||||||||||
| @Nonnull Set<String> whereIsNotNull) { | ||||||||||
| if (setClause.isEmpty()) { | ||||||||||
| throw new IllegalArgumentException("Empty setClause"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| Set<String> columns = new HashSet<>(tableColumns); | ||||||||||
| validateColumns(columns, setClause.keySet()); | ||||||||||
| validateColumns(columns, whereEquals.keySet()); | ||||||||||
| validateColumns(columns, whereGreater.keySet()); | ||||||||||
| validateColumns(columns, whereLess.keySet()); | ||||||||||
| validateColumns(columns, whereIsNull); | ||||||||||
| validateColumns(columns, whereIsNotNull); | ||||||||||
|
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. Same here
Contributor
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. Removed |
||||||||||
|
|
||||||||||
| QueryFragment where = | ||||||||||
| generateWhereClauseExtended( | ||||||||||
| columns, whereEquals, whereGreater, whereLess, whereIsNull, whereIsNotNull); | ||||||||||
|
|
||||||||||
| List<String> setParts = new ArrayList<>(); | ||||||||||
| List<Object> params = new ArrayList<>(); | ||||||||||
| for (Map.Entry<String, Object> entry : setClause.entrySet()) { | ||||||||||
| setParts.add(entry.getKey() + " = ?"); | ||||||||||
| params.add(entry.getValue()); | ||||||||||
| } | ||||||||||
| params.addAll(where.parameters()); | ||||||||||
|
|
||||||||||
| String sql = | ||||||||||
| "UPDATE " | ||||||||||
| + getFullyQualifiedTableName(tableName) | ||||||||||
| + " SET " | ||||||||||
| + String.join(", ", setParts) | ||||||||||
| + where.sql(); | ||||||||||
| return new PreparedQuery(sql, params); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Builds a DELETE query with the given conditions. | ||||||||||
| * | ||||||||||
|
|
@@ -209,6 +277,32 @@ public static PreparedQuery generateDeleteQuery( | |||||||||
| "DELETE FROM " + getFullyQualifiedTableName(tableName) + where.sql(), where.parameters()); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Builds a DELETE query that supports richer WHERE predicates (equality, greater-than, less-than, | ||||||||||
| * IS NULL, IS NOT NULL). | ||||||||||
| */ | ||||||||||
| public static PreparedQuery generateDeleteQuery( | ||||||||||
|
huaxingao marked this conversation as resolved.
|
||||||||||
| @Nonnull List<String> tableColumns, | ||||||||||
| @Nonnull String tableName, | ||||||||||
| @Nonnull Map<String, Object> whereEquals, | ||||||||||
| @Nonnull Map<String, Object> whereGreater, | ||||||||||
| @Nonnull Map<String, Object> whereLess, | ||||||||||
| @Nonnull Set<String> whereIsNull, | ||||||||||
| @Nonnull Set<String> whereIsNotNull) { | ||||||||||
| Set<String> columns = new HashSet<>(tableColumns); | ||||||||||
| validateColumns(columns, whereEquals.keySet()); | ||||||||||
| validateColumns(columns, whereGreater.keySet()); | ||||||||||
| validateColumns(columns, whereLess.keySet()); | ||||||||||
| validateColumns(columns, whereIsNull); | ||||||||||
| validateColumns(columns, whereIsNotNull); | ||||||||||
|
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. Do we need to validate them here as the method
Contributor
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. You are right. Removed. |
||||||||||
|
|
||||||||||
| QueryFragment where = | ||||||||||
| generateWhereClauseExtended( | ||||||||||
| columns, whereEquals, whereGreater, whereLess, whereIsNull, whereIsNotNull); | ||||||||||
| return new PreparedQuery( | ||||||||||
| "DELETE FROM " + getFullyQualifiedTableName(tableName) + where.sql(), where.parameters()); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static PreparedQuery generateSelectQuery( | ||||||||||
| @Nonnull List<String> columnNames, | ||||||||||
| @Nonnull String tableName, | ||||||||||
|
|
@@ -231,22 +325,55 @@ static QueryFragment generateWhereClause( | |||||||||
| @Nonnull Set<String> tableColumns, | ||||||||||
| @Nonnull Map<String, Object> whereEquals, | ||||||||||
| @Nonnull Map<String, Object> whereGreater) { | ||||||||||
| return generateWhereClauseExtended( | ||||||||||
| tableColumns, whereEquals, whereGreater, Map.of(), Set.of(), Set.of()); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static void validateColumns( | ||||||||||
| @Nonnull Set<String> tableColumns, @Nonnull Set<String> columns) { | ||||||||||
| for (String column : columns) { | ||||||||||
| if (!tableColumns.contains(column) && !column.equals("realm_id")) { | ||||||||||
| throw new IllegalArgumentException("Invalid query column: " + column); | ||||||||||
|
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.
Contributor
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. Makes sense. Fixed. |
||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @VisibleForTesting | ||||||||||
| static QueryFragment generateWhereClauseExtended( | ||||||||||
| @Nonnull Set<String> tableColumns, | ||||||||||
| @Nonnull Map<String, Object> whereEquals, | ||||||||||
| @Nonnull Map<String, Object> whereGreater, | ||||||||||
| @Nonnull Map<String, Object> whereLess, | ||||||||||
| @Nonnull Set<String> whereIsNull, | ||||||||||
| @Nonnull Set<String> whereIsNotNull) { | ||||||||||
| // Preserve the original behavior of rejecting unknown columns. This is used by SELECT query | ||||||||||
| // generation too, not only by callers of the extended UPDATE/DELETE helpers. | ||||||||||
|
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. Nit: I guess this comment is more like a dev log. Do we need them as a comment?
Contributor
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. Removed. |
||||||||||
| validateColumns(tableColumns, whereEquals.keySet()); | ||||||||||
| validateColumns(tableColumns, whereGreater.keySet()); | ||||||||||
| validateColumns(tableColumns, whereLess.keySet()); | ||||||||||
| validateColumns(tableColumns, whereIsNull); | ||||||||||
| validateColumns(tableColumns, whereIsNotNull); | ||||||||||
|
|
||||||||||
| List<String> conditions = new ArrayList<>(); | ||||||||||
| List<Object> parameters = new ArrayList<>(); | ||||||||||
| for (Map.Entry<String, Object> entry : whereEquals.entrySet()) { | ||||||||||
| if (!tableColumns.contains(entry.getKey()) && !entry.getKey().equals("realm_id")) { | ||||||||||
| throw new IllegalArgumentException("Invalid query column: " + entry.getKey()); | ||||||||||
| } | ||||||||||
| conditions.add(entry.getKey() + " = ?"); | ||||||||||
| parameters.add(entry.getValue()); | ||||||||||
| } | ||||||||||
| for (Map.Entry<String, Object> entry : whereGreater.entrySet()) { | ||||||||||
| if (!tableColumns.contains(entry.getKey()) && !entry.getKey().equals("realm_id")) { | ||||||||||
| throw new IllegalArgumentException("Invalid query column: " + entry.getKey()); | ||||||||||
| } | ||||||||||
| conditions.add(entry.getKey() + " > ?"); | ||||||||||
| parameters.add(entry.getValue()); | ||||||||||
| } | ||||||||||
| for (Map.Entry<String, Object> entry : whereLess.entrySet()) { | ||||||||||
| conditions.add(entry.getKey() + " < ?"); | ||||||||||
| parameters.add(entry.getValue()); | ||||||||||
| } | ||||||||||
| for (String column : whereIsNull) { | ||||||||||
| conditions.add(column + " IS NULL"); | ||||||||||
| } | ||||||||||
| for (String column : whereIsNotNull) { | ||||||||||
| conditions.add(column + " IS NOT NULL"); | ||||||||||
| } | ||||||||||
| String clause = conditions.isEmpty() ? "" : " WHERE " + String.join(" AND ", conditions); | ||||||||||
| return new QueryFragment(clause, parameters); | ||||||||||
| } | ||||||||||
|
|
||||||||||
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.
Do we need this new method given that we've got a
getDatabaseType()already(line 82)? We could change the scope topublicif 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.
Removed. Thanks!