From 31d968c3550228a95e23e1c734103bbbc37be21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20=C3=87engel?= Date: Wed, 14 Oct 2026 21:44:25 +0300 Subject: [PATCH] feat(analysis): analyze UPDATE ... FROM and DELETE ... USING sources Added sources (tables, CTEs, and aliased subqueries) join the target in the WHERE scope, so their columns type and name placeholders, and a join inside FROM types its condition over its own comma-separated item. RETURNING may return a column of any source, and RETURNING .* returns the target's shared row on every write. A bare * on a joined write and a wildcard of another source are rejected, and the remaining multi-table UPDATE and joined DELETE forms get messages naming the supported clause. --- docs/queries.md | 191 ++++-- .../dev/sqlcj/analysis/QueryAnalyzer.java | 380 +++++++++--- .../dev/sqlcj/analysis/QueryAnalyzerTest.java | 554 +++++++++++++++++- .../compiler/PostgresIntegrationTest.java | 119 ++++ .../ReferenceServiceIntegrationTest.java | 10 - .../SqlcjCompilerIntegrationTest.java | 110 +++- .../unsupported/queries/delete_using.sql | 7 - .../unsupported/queries/update_from.sql | 7 - 8 files changed, 1236 insertions(+), 142 deletions(-) delete mode 100644 sqlcj-cli/src/test/resources/reference-service/unsupported/queries/delete_using.sql delete mode 100644 sqlcj-cli/src/test/resources/reference-service/unsupported/queries/update_from.sql diff --git a/docs/queries.md b/docs/queries.md index 597e333..0b530cc 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -483,13 +483,17 @@ LIMIT $1 OFFSET $2; ## Writes -A write targets exactly one table of its entry's schema. +A write targets exactly one table of its entry's schema, whose rows it changes. +An `UPDATE` and a `DELETE` may read further sources of that schema to select +those rows, as +[Added sources in `UPDATE` and `DELETE`](#added-sources-in-update-and-delete) +states. | Statement | Accepted shape | | --- | --- | | `INSERT` | an explicit column list and a row source — `VALUES` rows or a `SELECT` — writing one value per written column by position, each a placeholder or an expression that binds none, with an optional `ON CONFLICT` clause | -| `UPDATE` | `SET` assignments that each assign one direct column a placeholder or an expression that binds none, with an optional `WHERE` using the read predicate forms | -| `DELETE` | one target table with an optional `WHERE` using the read predicate forms | +| `UPDATE` | `SET` assignments that each assign one direct column a placeholder or an expression that binds none, with an optional `FROM` clause of added sources and an optional `WHERE` using the read predicate forms | +| `DELETE` | one target table, with an optional `USING` clause of added sources and an optional `WHERE` using the read predicate forms | ```sql -- name: CreateAuthor :one @@ -558,6 +562,66 @@ reaches the database as written. An uncast placeholder written there is rejected as an unanalyzed placeholder location, in an `UPDATE` or `DO UPDATE` assignment, in a `VALUES` value, and in an item of an insert's `SELECT` alike. +### Added sources in `UPDATE` and `DELETE` + +An `UPDATE` may select the rows it writes through an `UPDATE ... FROM` clause, +and a `DELETE` through a `DELETE ... USING` clause. The write still targets one +table, whose rows it changes, while the added sources only decide which of its +rows it writes and what the write may read: + +```sql +-- name: SetRoyaltyAmountsByCountry :exec +UPDATE royalties r +SET amount = :amount +FROM authors a +WHERE a.id = r.author_id + AND a.country = :country; + +-- name: DeleteAuthorRoyalties :many +DELETE +FROM royalties r + USING authors a +WHERE a.id = r.author_id + AND a.name = :name +RETURNING r.id, a.name; +``` + +`SetRoyaltyAmountsByCountry` generates +`setRoyaltyAmountsByCountry(BigDecimal amount, String country)`, whose second +parameter is typed and named from `authors.country`, and +`DeleteAuthorRoyalties` returns one record of the deleted royalty's `id` and +its author's `name`. + +- An `UPDATE ... FROM` clause takes one source followed by inner or left joins, + by the join rules [Sources](#sources) states, or by further comma-separated + sources. A comma-separated list is accepted here, unlike in a `SELECT`, + because it is PostgreSQL's own form of the clause. +- A `DELETE ... USING` clause takes a comma-separated list of sources, and no + join: JSqlParser does not read a join there, so `USING a JOIN b` fails to + parse. +- Each added source is a table of the entry's schema, a `WITH` clause of the + write in scope, or an aliased subquery, resolved exactly as a read's source + is, so a `LATERAL` source, a doubly parenthesized source, and a subquery + without an alias keep their diagnostics. +- A join condition inside `FROM` sees only the sources of the comma-separated + item it joins onto, up to and including its own joined source, because + PostgreSQL binds a `JOIN` more tightly than a comma. The target is never one + of them, so a condition that names it is rejected as + `Unknown source qualifier in query: `, as PostgreSQL also rejects it. + Its placeholders are typed by the `WHERE` predicate rules. +- The `WHERE` predicate reads the target and every added source as one level, + with the write's `WITH` clauses in scope of its predicate subqueries, so a + placeholder compared with a column of an added source is typed and named from + that column. `SET` assignments keep reading the target's columns alone. +- The exposed names of the target and of every added source must differ, as the + sources of a read must, so a repeated name is rejected as + `Duplicate source name in query: `. +- Every column read from a left-joined source is nullable, in `RETURNING` as in + a read's projection. +- Placeholders bind in the textual order of the query, across the `SET` + assignments, the added sources and their join conditions, and the `WHERE` + predicate. + ### Values that bind no placeholder An `INSERT` value or an `UPDATE` assignment that contains no placeholder — such @@ -696,34 +760,57 @@ collation or an operator class. A supported `INSERT`, `UPDATE`, or `DELETE` declared `:one`, `:optional`, or `:many` may end with a `RETURNING` clause that lists either: -- direct columns of the target table, in the order they are declared, or -- a bare `*`, which expands in the target table's schema column order. +- direct columns of the write's sources — the target and, on a joined write, + every source its `FROM` or `USING` clause adds — in the order they are + declared, or +- a wildcard of the target, which expands in the target table's schema column + order: a bare `*` on a single-table write, and `.*` on any write, + written with the name the target exposes, which is its alias when the write + declares one. A returned column may carry an alias, written as `id AS user_id` or `id user_id`, which names its result component exactly as a `SELECT` item's alias does, while -its type and nullability still come from the schema column. +its type comes from the schema column and its nullability from that column and +its source, so a column of a left-joined source is nullable. A returning write produces the same generated record, positional row mapper, and cardinality behavior as a read, so a database-generated `SERIAL` or `BIGSERIAL` -value is read back with its declared type. A clause that is exactly `*` returns -the target table's shared `Row` record, while a `RETURNING` column -list keeps the query's own `Result` record. - -An aliased `*`, a qualified `table.*`, an unknown column, and `RETURNING` on -`:exec` are rejected, and a computed `RETURNING` item, with or without an alias, -is rejected as +value is read back with its declared type. A clause that is exactly a wildcard +of the target returns the target table's shared `Row` record, on a +joined and on a single-table write alike, while a `RETURNING` column list, a +wildcard written beside another item included, keeps the query's own +`Result` record. + +An unknown column and `RETURNING` on `:exec` are rejected, and a computed +`RETURNING` item, with or without an alias, is rejected as `Unsupported RETURNING item ; return columns of the target table or *`, naming the expression as the query writes it, on one line. A cast does not make such an item supported, because a `RETURNING` clause returns columns of the -target table. A reference written with an array subscript or slice, such as +write's sources. A reference written with an array subscript or slice, such as `RETURNING labels[1]` or `RETURNING labels[1:2] AS pair`, is not a direct -column of the target table either, so it is rejected as a computed item, with or -without an alias. +column either, so it is rejected as a computed item, with or without an alias. + +A wildcard returns the complete row of the target alone, so: + +- an aliased wildcard, bare or qualified, is rejected as + `RETURNING must not be aliased.`, and a modified one, such as + `RETURNING * EXCEPT (id)`, as `RETURNING must not be modified.`; +- a wildcard whose qualifier names no source of the write, the `old` and `new` + row aliases of a write included, is rejected as + `Unknown source qualifier in query: `; +- a wildcard of an added source is rejected as + `RETURNING .* is not supported; return the columns of by name.`; +- a bare `*` on a joined write, which PostgreSQL returns the columns of every + source for, is rejected as + `RETURNING * is not supported with UPDATE ... FROM or DELETE ... USING; write + RETURNING .* for the target's row.` A returning insert accepts every accepted row source — several `VALUES` rows, a -parenthesized row source, and a `SELECT` — while `UPDATE ... FROM` and -`DELETE ... USING` are rejected in every write. An accepted `ON CONFLICT` -clause is analyzed the same way with and without `RETURNING`. +parenthesized row source, and a `SELECT` — and a returning `UPDATE` and +`DELETE` accept the added sources +[Added sources in `UPDATE` and `DELETE`](#added-sources-in-update-and-delete) +states. An accepted `ON CONFLICT` clause is analyzed the same way with and +without `RETURNING`. A write may declare `WITH` clauses of its own, with or without `RETURNING`. Each body is analyzed by the rules [Sources](#sources) states, so its @@ -892,9 +979,10 @@ a query of an entry reads or binds. of the query name, as in `get_author` and `GetAuthor` to `getAuthor`. - A `:one`, `:optional`, or `:many` query that returns one complete table row — a single-source `SELECT *` or `SELECT .*`, or a write whose `RETURNING` - clause is exactly `*` — returns the `Row` record. That record is a - top-level `public record` in `java.package`, generated once per table into its - own file and shared by every repository of the package that returns that row. + clause is exactly a wildcard of its target — returns the `Row` + record. That record is a top-level `public record` in `java.package`, + generated once per table into its own file and shared by every repository of + the package that returns that row. Each repository keeps its own private `RowMapper` field for the row, generated after the constructor in the order its queries first use it. - Every other `:one`, `:optional`, or `:many` query generates a nested `public record` named @@ -1190,15 +1278,18 @@ query name, and its header line. unqualified conflict-predicate column that both the inserted table and `EXCLUDED` carry, or a predicate column qualified with a name its own predicate does not read, such as `EXCLUDED` in a conflict-target predicate. -- An aliased `*`, a computed, qualified-wildcard, or unknown `RETURNING` item, - and `RETURNING` on `:exec`; a computed item is rejected as +- An aliased or modified `RETURNING` wildcard, a wildcard of a source other + than the write target, a bare `*` on a joined write, and a computed or + unknown `RETURNING` item, and `RETURNING` on `:exec`; each is rejected as [`RETURNING`](#returning) states. -- The shapes that read or write more than the one analyzed table, on an - `:exec` and on a returning write alike: an `UPDATE ... FROM` or joined - `UPDATE`, rejected as - `UPDATE ... FROM and joined updates are not supported.`; and a - `DELETE ... USING` or joined `DELETE`, rejected as - `DELETE ... USING and joined deletes are not supported.` +- The write shapes that change rows of more than the one analyzed table, on an + `:exec` and on a returning write alike: an `UPDATE` that names several + tables before `SET`, written with a comma or with a join, rejected as + `UPDATE of several tables is not supported; write the other sources in + UPDATE ... FROM.`; and a joined `DELETE`, such as + `DELETE a FROM a JOIN b`, rejected as + `A joined DELETE is not supported; write the other sources in + DELETE ... USING.` - Any annotation other than `:one`, `:optional`, `:many`, and `:exec`. An unexpected failure of schema parsing, query analysis, or generation is not a @@ -1539,6 +1630,39 @@ Writes and `RETURNING`: `QueryAnalyzerTest.shouldAnalyzeCommonTableExpressionOfAWriteWithoutReturning` cover the `WITH` clause of a returning and of an `:exec` write, whose body types its own placeholders while the returned columns stay the target's. +- `QueryAnalyzerTest.shouldTypeUpdateWhereParameterFromAnAddedSourceColumn`, + `QueryAnalyzerTest.shouldTypeDeleteWhereParameterFromAUsingSourceColumn`, + `QueryAnalyzerTest.shouldTypeJoinConditionParameterOfAnUpdateFromClause`, + `QueryAnalyzerTest.shouldResolveCommaSeparatedUpdateFromSources`, + `QueryAnalyzerTest.shouldResolveSubquerySourceOfAnUpdateFromClause`, + `QueryAnalyzerTest.shouldResolveCommonTableExpressionSourceOfADeleteUsingClause`, + `QueryAnalyzerTest.shouldRejectAnUpdateFromJoinConditionOutsideItsJoinGroup`, + `QueryAnalyzerTest.shouldResolveNullabilityOfJoinedWriteReturningColumns`, + `QueryAnalyzerTest.shouldResolveRowTableForATargetWildcardOfAWrite`, + `QueryAnalyzerTest.shouldNotResolveRowTableForATargetWildcardBesideAnotherColumn`, + `QueryAnalyzerTest.shouldRejectAnUnsupportedJoinedWriteReturningWildcard`, + `QueryAnalyzerTest.shouldRejectAnUnknownReturningWildcardQualifier`, + `QueryAnalyzerTest.shouldRejectARepeatedJoinedWriteSourceName`, + `QueryAnalyzerTest.shouldBindJoinedUpdateParametersInTextualOrder`, and + `QueryAnalyzerTest.shouldShareOneNamedParameterAcrossAJoinedUpdate` cover + `UPDATE ... FROM` and `DELETE ... USING`: a `WHERE` placeholder typed from a + column of an added source, a join condition's own placeholders, a + comma-separated list, a subquery and a common table expression as sources + with their bodies' placeholders typed, a join condition that names the + target or an earlier comma-separated source rejected as an unknown + qualifier, the nullability of a returned column of a left-joined source, the + target's shared row returned by `RETURNING .*` on a joined and on a + single-table write, the query-specific result of a wildcard beside another + column, the bare `*`, other-source wildcard, aliased wildcard, unknown + qualifier, and repeated source-name rejections, and the textual binding + order across `SET`, the added sources, and `WHERE`, with indexed and named + placeholders, while the executable SQL stays the query's own. + `SqlcjCompilerIntegrationTest.shouldGenerateCompilableJavaForJoinedWrites` + compiles the generated sources of a joined update, a left-joined update, and + a `USING` delete, and + `PostgresIntegrationTest.shouldExecuteGeneratedJoinedWritesAgainstPostgres` + executes a joined update returning the target's shared row and a `USING` + delete returning a column of each source against PostgreSQL 16. - `QueryAnalyzerTest.shouldAnalyzeInsertWithSeveralValuesRows`, `QueryAnalyzerTest.shouldAnalyzeIndexedInsertWithSeveralValuesRows`, `QueryAnalyzerTest.shouldAnalyzeInsertSelectWithoutFrom`, @@ -1663,8 +1787,8 @@ Unsupported queries and internal errors: subquery alias, a subquery source without an alias, a `WITH RECURSIVE` clause, a data-modifying `WITH` body, a set operation as an insert's row source, an insert whose `SELECT` writes fewer values than its column list, - and an `:exec` `UPDATE ... FROM`, comma-joined `UPDATE`, - `DELETE ... USING`, and joined `DELETE`, while + a comma-joined and a join-joined multi-table `UPDATE`, a joined `DELETE`, + and a bare `RETURNING *` on a joined update, while `QueryAnalyzerTest.shouldRejectUnsupportedCountProjectionForm`, `QueryAnalyzerTest.shouldRejectUnsupportedJoinModifier`, `QueryAnalyzerTest.shouldRejectUnsupportedReturningItem`, @@ -1836,8 +1960,7 @@ Reference service: covers the whole located diagnostic line, and the absence of generated output, of every entry of the reference service's unsupported query catalogue: the two set operations, the `RIGHT`, `FULL`, `USING`, `NATURAL`, - `CROSS`, comma, and `LATERAL` joins, an `UPDATE ... FROM`, a - `DELETE ... USING`, an uncast + `CROSS`, comma, and `LATERAL` joins, an uncast `lower(:email)`, and a placeholder beside a subscripted column, and `ReferenceServiceIntegrationTest.shouldCatalogueExactlyTheUnsupportedQueryFiles` covers that the catalogue directory holds exactly those queries. diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java index a158212..805c6e4 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java @@ -119,6 +119,57 @@ public final class QueryAnalyzer { private static final String COMPUTED_RETURNING_REJECTION = """ Unsupported RETURNING item %s; return columns of the target table or *"""; + /** + * The rejection of an aliased returned wildcard, which is written as the + * query spells the wildcard, because an alias names one result component + * and a wildcard returns a row of them. + */ + private static final String RETURNING_WILDCARD_ALIAS_REJECTION = """ + RETURNING %s must not be aliased."""; + + /** + * The rejection of a modified returned wildcard, which is written as the + * query spells the wildcard, because a modifier changes which columns the + * wildcard returns. + */ + private static final String RETURNING_WILDCARD_MODIFIER_REJECTION = """ + RETURNING %s must not be modified."""; + + /** + * The rejection of a bare {@code *} on a joined write, which PostgreSQL + * returns the columns of every source for, while the only complete row such + * a write returns here is its target's. + */ + private static final String JOINED_RETURNING_ALL_REJECTION = """ + RETURNING * is not supported with UPDATE ... FROM or DELETE ... USING; \ + write RETURNING %s.* for the target's row."""; + + /** + * The rejection of a returned wildcard of a source other than the write + * target, whose complete row the write does not return, named by the source + * the qualifier exposes. + */ + private static final String OTHER_SOURCE_RETURNING_WILDCARD_REJECTION = """ + RETURNING %s.* is not supported; return the columns of %s by name."""; + + /** + * The rejection of an {@code UPDATE} that names several targets, whose + * updated rows this subset does not model, naming the clause that adds a + * source instead. + */ + private static final String MULTI_TABLE_UPDATE_REJECTION = """ + UPDATE of several tables is not supported; write the other sources in UPDATE ... FROM."""; + + /** + * The rejection of a joined {@code DELETE}, whose deleted rows this subset + * does not model, naming the clause that adds a source instead. + */ + private static final String JOINED_DELETE_REJECTION = """ + A joined DELETE is not supported; write the other sources in DELETE ... USING."""; + + /** The rejection of a source of a write's {@code FROM} or {@code USING}. */ + private static final String WRITE_SOURCE_REJECTION = "Only table sources are supported."; + /** The rejection of a {@code LATERAL} source. */ private static final String LATERAL_REJECTION = "LATERAL is not supported."; @@ -832,8 +883,8 @@ private QueryModel analyzeInsert(Query query, ParsedSql parsedSql, Insert insert String rowTable = null; if (returningClause != null) { - columns = resolveReturningColumns(returningClause, source); - rowTable = resolveReturningRowTable(returningClause, source); + columns = resolveReturningColumns(returningClause, List.of(source)); + rowTable = resolveReturningRowTable(returningClause, List.of(source)); } resolveConflictParameters(insert, source, commonTableExpressions, analysis); @@ -855,7 +906,7 @@ private QueryModel analyzeUpdate(Query query, ParsedSql parsedSql, Update update requireSupportedUpdateSources(update); Table table = update.getTable(); - Source source = toSource(table, schema); + Source target = toSource(table, schema); Placeholders placeholders = toPlaceholders(parsedSql, schema); Analysis analysis = new Analysis(parsedSql, schema, placeholders); @@ -865,20 +916,30 @@ private QueryModel analyzeUpdate(Query query, ParsedSql parsedSql, Update update analysis ); + List sources = resolveWriteSources( + target, + update.getFromItem() == null + ? List.of() + : List.of(update.getFromItem()), + update.getJoins(), + analysis, + commonTableExpressions + ); + List columns = List.of(); String rowTable = null; if (returningClause != null) { - columns = resolveReturningColumns(returningClause, source); - rowTable = resolveReturningRowTable(returningClause, source); + columns = resolveReturningColumns(returningClause, sources); + rowTable = resolveReturningRowTable(returningClause, sources); } - resolveUpdateSetParameters(update.getUpdateSets(), source.table(), placeholders); + resolveUpdateSetParameters(update.getUpdateSets(), target.table(), placeholders); if (update.getWhere() != null) { resolveParameters( update.getWhere(), - Scope.of(List.of(source), commonTableExpressions), + Scope.of(sources, commonTableExpressions), analysis ); } @@ -900,7 +961,7 @@ private QueryModel analyzeDelete(Query query, ParsedSql parsedSql, Delete delete requireSupportedDeleteSources(delete); Table table = delete.getTable(); - Source source = toSource(table, schema); + Source target = toSource(table, schema); Placeholders placeholders = toPlaceholders(parsedSql, schema); Analysis analysis = new Analysis(parsedSql, schema, placeholders); @@ -910,18 +971,26 @@ private QueryModel analyzeDelete(Query query, ParsedSql parsedSql, Delete delete analysis ); + List sources = resolveWriteSources( + target, + delete.getUsingFromItemList(), + null, + analysis, + commonTableExpressions + ); + List columns = List.of(); String rowTable = null; if (returningClause != null) { - columns = resolveReturningColumns(returningClause, source); - rowTable = resolveReturningRowTable(returningClause, source); + columns = resolveReturningColumns(returningClause, sources); + rowTable = resolveReturningRowTable(returningClause, sources); } if (delete.getWhere() != null) { resolveParameters( delete.getWhere(), - Scope.of(List.of(source), commonTableExpressions), + Scope.of(sources, commonTableExpressions), analysis ); } @@ -975,10 +1044,14 @@ private boolean producesResult(Query query) { } /** - * Resolves the returned columns in declared order against the single write - * target, expanding a bare {@code *} in schema column order. + * Resolves the returned columns in declared order against the write's + * sources, the target first, expanding a wildcard of the target in schema + * column order. */ - private List resolveReturningColumns(ReturningClause returningClause, Source source) { + private List resolveReturningColumns( + ReturningClause returningClause, + List sources + ) { if ( returningClause.getKeyword() != ReturningClause.Keyword.RETURNING || returningClause.getDataItems() != null @@ -995,7 +1068,7 @@ private List resolveReturningColumns(ReturningClause returningClaus resolveReturningItem( returningItem.getExpression(), returningItem.getAlias(), - source + sources ) ); } @@ -1005,63 +1078,66 @@ private List resolveReturningColumns(ReturningClause returningClaus /** * Reports the table whose complete row a returning write returns, which is - * the write target when the clause is exactly a bare {@code *}, and + * the write target when the clause is exactly a wildcard of the target, and * {@code null} for a returned column list. The name is the schema's own * spelling of the table, so every query returning that row shares one row * identity. + * + *

A bare {@code *} is such a wildcard only on a single-table write, + * because a joined write returns the columns of every source for it and is + * rejected where the returned columns are resolved. */ - private String resolveReturningRowTable(ReturningClause returningClause, Source source) { + private String resolveReturningRowTable(ReturningClause returningClause, List sources) { if (returningClause.size() != 1) { return null; } - return returningClause.get(0).getExpression() instanceof AllColumns - ? source.table().name() + Expression expression = returningClause.get(0).getExpression(); + Source target = sources.get(0); + + if (expression instanceof AllTableColumns qualified) { + return lookupSource(sources, returningQualifier(qualified)) + .filter(source -> source == target) + .map(source -> source.table().name()) + .orElse(null); + } + + return expression instanceof AllColumns && sources.size() == 1 + ? target.table().name() : null; } /** - * Resolves one returned item, which is either a bare {@code *} or a direct - * column of the write target, whose alias names its result component. A - * computed item, a subscripted reference included, has no schema type to - * generate, so it is rejected instead of analyzed. + * Resolves one returned item, which is either a wildcard of the write + * target or a direct column of one of the write's sources, whose alias + * names its result component. A computed item, a subscripted reference + * included, has no schema type to generate, so it is rejected instead of + * analyzed. + * + *

A returned column of a left-joined source is nullable even when its + * schema declaration is not, because an unmatched row reads it as + * {@code NULL}. */ - private List resolveReturningItem(Expression expression, Alias alias, Source source) { - if (expression instanceof AllTableColumns) { - throw new UnsupportedOperationException( - "Only an unqualified RETURNING * is supported." - ); - } - + private List resolveReturningItem( + Expression expression, + Alias alias, + List sources + ) { if (expression instanceof AllColumns allColumns) { - if (alias != null) { - throw new UnsupportedOperationException( - "RETURNING * must not be aliased." - ); - } - - if (allColumns.getExceptColumns() != null || allColumns.getReplaceExpressions() != null) { - throw new UnsupportedOperationException( - "RETURNING * must not be modified." - ); - } - - return toQueryColumns(resolveAllColumns(source)); + return resolveReturningWildcard(allColumns, alias, sources); } net.sf.jsqlparser.schema.Column returnedColumn = directColumn(expression); if (returnedColumn != null) { - dev.sqlcj.schema.Column schemaColumn = resolveColumn( - returnedColumn, - Scope.of(List.of(source)) - ).column(); + ResolvedColumn resolved = resolveColumn(returnedColumn, Scope.of(sources)); + dev.sqlcj.schema.Column schemaColumn = resolved.column(); return List.of( new QueryColumn( selectedColumnName(alias, schemaColumn), requireSupportedType(schemaColumn), - schemaColumn.nullable(), + schemaColumn.nullable() || resolved.source().leftJoined(), schemaColumn.enumType(), schemaColumn.array() ) @@ -1073,6 +1149,82 @@ private List resolveReturningItem(Expression expression, Alias alia ); } + /** + * Resolves one returned wildcard, which returns the complete row of the + * write target: a bare {@code *} on a single-table write, and + * {@code target.*} on any write. The columns of another source are + * returnable by name, and a bare {@code *} on a joined write would return + * every source's columns, so each is rejected naming what to write instead. + * + *

An alias names one result component and a modifier changes which + * columns the wildcard returns, so neither is supported on a bare or a + * qualified wildcard. + */ + private List resolveReturningWildcard( + AllColumns allColumns, + Alias alias, + List sources + ) { + String written = writtenReturningWildcard(allColumns); + + if (alias != null) { + throw new UnsupportedOperationException( + RETURNING_WILDCARD_ALIAS_REJECTION.formatted(written) + ); + } + + if (allColumns.getExceptColumns() != null || allColumns.getReplaceExpressions() != null) { + throw new UnsupportedOperationException( + RETURNING_WILDCARD_MODIFIER_REJECTION.formatted(written) + ); + } + + Source target = sources.get(0); + + if (!(allColumns instanceof AllTableColumns qualified)) { + if (sources.size() > 1) { + throw new UnsupportedOperationException( + JOINED_RETURNING_ALL_REJECTION.formatted(target.name()) + ); + } + + return toQueryColumns(resolveAllColumns(target)); + } + + Source source = findSource(sources, returningQualifier(qualified)); + + if (source != target) { + throw new UnsupportedOperationException( + OTHER_SOURCE_RETURNING_WILDCARD_REJECTION.formatted(source.name(), source.name()) + ); + } + + return toQueryColumns(resolveAllColumns(target)); + } + + /** + * The exposed name a qualified returned wildcard names, which the parser + * reports as the wildcard's own table, and as a returning qualifier for the + * {@code old} and {@code new} row aliases of a write. + */ + private String returningQualifier(AllTableColumns allTableColumns) { + Table table = allTableColumns.getTable(); + + return table == null + ? allTableColumns.getReturningQualifier() + : table.getUnquotedName(); + } + + /** + * One returned wildcard as the query writes it, which a diagnostic names, + * without any modifier the wildcard carries. + */ + private String writtenReturningWildcard(AllColumns allColumns) { + return allColumns instanceof AllTableColumns qualified + ? returningQualifier(qualified) + ".*" + : "*"; + } + /** * One expression as the query writes it, on one line: the parser's own * spelling of the expression, with every run of whitespace written as one @@ -1086,34 +1238,27 @@ private String written(Expression expression) { } /** - * Rejects the multi-table {@code UPDATE} forms, whose updated rows this - * subset does not model, on an {@code :exec} and on a returning update - * alike. + * Rejects an {@code UPDATE} that names several tables before {@code SET}, + * whose updated rows this subset does not model, on an {@code :exec} and on + * a returning update alike. The parser reports both the comma-separated and + * the joined spelling of that form as the joins of the target, so a source + * written in {@code FROM}, which this subset models, is not one of them. */ private void requireSupportedUpdateSources(Update update) { - if ( - update.getFromItem() != null - || update.getJoins() != null - || update.getStartJoins() != null - ) { - throw new UnsupportedOperationException( - "UPDATE ... FROM and joined updates are not supported." - ); + if (update.getStartJoins() != null) { + throw new UnsupportedOperationException(MULTI_TABLE_UPDATE_REJECTION); } } /** - * Rejects the multi-table {@code DELETE} forms, whose deleted rows this - * subset does not model, on an {@code :exec} and on a returning delete - * alike. + * Rejects a joined {@code DELETE}, whose deleted rows this subset does not + * model, on an {@code :exec} and on a returning delete alike. The parser + * reports a {@code USING} source, which this subset models, as a source of + * its own clause rather than as a join. */ private void requireSupportedDeleteSources(Delete delete) { - boolean using = delete.getUsingList() != null && !delete.getUsingList().isEmpty(); - - if (using || delete.getJoins() != null) { - throw new UnsupportedOperationException( - "DELETE ... USING and joined deletes are not supported." - ); + if (delete.getJoins() != null) { + throw new UnsupportedOperationException(JOINED_DELETE_REJECTION); } } @@ -1981,6 +2126,105 @@ private List resolveSources( return List.copyOf(sources); } + /** + * Resolves the ordered sources of one write: its target, which its rows + * belong to, followed by every source its {@code FROM} or {@code USING} + * clause adds, in the order the query writes them. An exposed name that + * repeats among them is rejected as it is in a read, the target included. + * + *

Each added item written beside the others — the single {@code FROM} + * item, a comma-separated source after it, and each {@code USING} source — + * starts a join group, and each joined source joins the group it is written + * onto, because PostgreSQL binds a {@code JOIN} more tightly than a comma. + * A join condition therefore resolves its columns and types its + * placeholders over its own group up to and including its joined source, + * which never includes the target, as PostgreSQL scopes a condition inside + * {@code FROM}. + */ + private List resolveWriteSources( + Source target, + List addedItems, + List joins, + Analysis analysis, + List commonTableExpressions + ) { + List sources = new ArrayList<>(); + List joinGroup = new ArrayList<>(); + + addSource(sources, target); + + for (FromItem addedItem : addedItems) { + addWriteSource( + sources, + joinGroup, + resolveSource( + addedItem, + WRITE_SOURCE_REJECTION, + analysis, + commonTableExpressions, + false + ) + ); + } + + if (joins == null) { + return List.copyOf(sources); + } + + for (Join join : joins) { + if (join.isSimple()) { + addWriteSource( + sources, + joinGroup, + resolveSource( + join.getRightItem(), + WRITE_SOURCE_REJECTION, + analysis, + commonTableExpressions, + false + ) + ); + + continue; + } + + addJoinedWriteSource( + sources, + joinGroup, + requireSupportedJoin(join, analysis, commonTableExpressions) + ); + + Expression condition = requireJoinCondition(join); + Scope scope = Scope.of(List.copyOf(joinGroup), commonTableExpressions); + + resolveJoinConditionColumns(condition, scope); + resolveParameters(condition, scope, analysis); + } + + return List.copyOf(sources); + } + + /** + * Adds one source a write's {@code FROM} or {@code USING} clause writes + * beside the others, which starts a join group of its own. + */ + private void addWriteSource(List sources, List joinGroup, Source source) { + addSource(sources, source); + + joinGroup.clear(); + joinGroup.add(source); + } + + /** + * Adds one source a write's {@code FROM} clause joins onto the group it is + * written onto, which is the scope of its own join condition. + */ + private void addJoinedWriteSource(List sources, List joinGroup, Source source) { + addSource(sources, source); + + joinGroup.add(source); + } + private void addSource(List sources, Source source) { boolean duplicate = sources.stream() .anyMatch(existing -> existing.name().equalsIgnoreCase(source.name())); diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java index a9f8791..e5bbe73 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java @@ -4618,10 +4618,8 @@ INSERT INTO users (id) VALUES ($1) RETURNING id AS "User Id" "INSERT INTO users (id) VALUES ($1) RETURNING id + 1 AS x" + "|Unsupported RETURNING item id + 1;" + " return columns of the target table or *", - "INSERT INTO users (id) VALUES ($1) RETURNING users.*" - + "|Only an unqualified RETURNING * is supported.", "INSERT INTO users (id) VALUES ($1) RETURNING users.* AS x" - + "|Only an unqualified RETURNING * is supported.", + + "|RETURNING users.* must not be aliased.", "DELETE FROM users WHERE id = $1 RETURNING count(id)" + "|Unsupported RETURNING item count(id);" + " return columns of the target table or *", @@ -4664,10 +4662,16 @@ void shouldRejectUnknownReturningColumn() { @CsvSource( delimiter = '|', value = { - "UPDATE users SET name = $2 FROM profiles WHERE users.id = $1 RETURNING id" - + "|UPDATE ... FROM and joined updates are not supported.", - "DELETE FROM users USING profiles WHERE users.id = $1 RETURNING id" - + "|DELETE ... USING and joined deletes are not supported." + "UPDATE users, profiles SET name = $2 WHERE users.id = $1 RETURNING id" + + "|UPDATE of several tables is not supported;" + + " write the other sources in UPDATE ... FROM.", + "UPDATE users u JOIN profiles p ON p.user_id = u.id SET name = $2 RETURNING u.id" + + "|UPDATE of several tables is not supported;" + + " write the other sources in UPDATE ... FROM.", + "DELETE users FROM users JOIN profiles ON profiles.user_id = users.id" + + " WHERE users.id = $1 RETURNING id" + + "|A joined DELETE is not supported;" + + " write the other sources in DELETE ... USING." } ) void shouldRejectExcludedReturningWriteForm(String sql, String message) { @@ -8272,4 +8276,540 @@ WHERE id IN (SELECT user_id FROM recent WHERE user_id = $2) assertEquals(List.of(1, 2), model.bindingParameterIndexes()); } + + /** + * A source an {@code UPDATE ... FROM} clause adds is in scope of the + * update's {@code WHERE}, so a placeholder compared with one of its columns + * is typed and named from that column. + */ + @Test + void shouldTypeUpdateWhereParameterFromAnAddedSourceColumn() { + String sql = """ + UPDATE users u + SET name = :name + FROM profiles p + WHERE p.user_id = u.id + AND p.nickname = :nickname + """; + + QueryModel model = analyzer.analyze( + new Query("RenameProfiledUsers", QueryType.EXEC, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals("users", model.table()); + assertTrue(model.columns().isEmpty()); + + assertEquals( + List.of( + new QueryParameter(1, "name", ColumnType.VARCHAR), + new QueryParameter(2, "nickname", ColumnType.VARCHAR) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + } + + /** + * A source a {@code DELETE ... USING} clause adds is in scope of the + * delete's {@code WHERE} and of its {@code RETURNING} clause, and the + * executable SQL keeps the clause as the query writes it. + */ + @Test + void shouldTypeDeleteWhereParameterFromAUsingSourceColumn() { + String sql = """ + DELETE FROM users u + USING profiles p + WHERE p.user_id = u.id + AND p.nickname = :nickname + RETURNING u.id, p.nickname + """; + + QueryModel model = analyzer.analyze( + new Query("DeleteProfiledUsers", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals("users", model.table()); + assertNull(model.rowTable()); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("nickname", ColumnType.VARCHAR, true) + ), + model.columns() + ); + + assertEquals( + List.of(new QueryParameter(1, "nickname", ColumnType.VARCHAR)), + model.parameters() + ); + + assertEquals( + """ + DELETE FROM users u + USING profiles p + WHERE p.user_id = u.id + AND p.nickname = ? + RETURNING u.id, p.nickname + """, + model.executableSql() + ); + } + + /** + * A join inside an {@code UPDATE ... FROM} clause types its own + * {@code ON} placeholders from the columns of the joined sources, and the + * joined source is in scope of the update's {@code WHERE}. + */ + @Test + void shouldTypeJoinConditionParameterOfAnUpdateFromClause() { + String sql = """ + UPDATE users u + SET name = $1 + FROM profiles p + JOIN orders o ON o.user_id = p.user_id + AND o.total = $2 + WHERE u.id = p.user_id + AND u.id = $3 + """; + + QueryModel model = analyzer.analyze( + new Query("RenameOrderingUsers", QueryType.EXEC, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "name", ColumnType.VARCHAR), + new QueryParameter(2, "total", ColumnType.DECIMAL), + new QueryParameter(3, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2, 3), model.bindingParameterIndexes()); + } + + /** + * An {@code UPDATE ... FROM} clause may write its sources as a + * comma-separated list, which is PostgreSQL's own form of the clause, and + * each of them is in scope of the update's {@code WHERE}. + */ + @Test + void shouldResolveCommaSeparatedUpdateFromSources() { + String sql = """ + UPDATE users u + SET name = :name + FROM profiles p, orders o + WHERE p.user_id = u.id + AND o.user_id = u.id + AND o.total = :total + """; + + QueryModel model = analyzer.analyze( + new Query("RenameOrderingUsers", QueryType.EXEC, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "name", ColumnType.VARCHAR), + new QueryParameter(2, "total", ColumnType.DECIMAL) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + } + + /** + * A subquery source of an {@code UPDATE ... FROM} clause is a derived + * source whose columns are its body's result columns, which the update's + * {@code WHERE} and {@code RETURNING} read, while the body types its own + * placeholders from its own columns. + */ + @Test + void shouldResolveSubquerySourceOfAnUpdateFromClause() { + String sql = """ + UPDATE users u + SET name = recent.nickname + FROM (SELECT p.user_id, p.nickname + FROM profiles p + WHERE p.nickname = :nickname) recent + WHERE u.id = recent.user_id + RETURNING u.id, recent.nickname + """; + + QueryModel model = analyzer.analyze( + new Query("RenameUsersFromProfiles", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertNull(model.rowTable()); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("nickname", ColumnType.VARCHAR, true) + ), + model.columns() + ); + + assertEquals( + List.of(new QueryParameter(1, "nickname", ColumnType.VARCHAR)), + model.parameters() + ); + + assertEquals( + """ + UPDATE users u + SET name = recent.nickname + FROM (SELECT p.user_id, p.nickname + FROM profiles p + WHERE p.nickname = ?) recent + WHERE u.id = recent.user_id + RETURNING u.id, recent.nickname + """, + model.executableSql() + ); + } + + /** + * A {@code USING} source that names a common table expression of the delete + * reads that expression, so its columns are the body's own columns and the + * body types its placeholders from them. + */ + @Test + void shouldResolveCommonTableExpressionSourceOfADeleteUsingClause() { + String sql = """ + WITH top_orders AS (SELECT user_id, total + FROM orders + WHERE total = :total) + DELETE FROM users u + USING top_orders + WHERE top_orders.user_id = u.id + AND u.id = :id + RETURNING u.id, top_orders.total + """; + + QueryModel model = analyzer.analyze( + new Query("DeleteTopOrderUsers", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("total", ColumnType.DECIMAL, true) + ), + model.columns() + ); + + assertEquals( + List.of( + new QueryParameter(1, "total", ColumnType.DECIMAL), + new QueryParameter(2, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + } + + /** + * A join condition inside an {@code UPDATE ... FROM} clause sees only the + * sources of the comma-separated item it joins onto, up to its own joined + * source, as PostgreSQL scopes it, so neither the target nor an earlier + * comma-separated source is a known qualifier there. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + value = { + "UPDATE users u SET name = $1 FROM profiles p JOIN orders o ON o.user_id = u.id" + + " WHERE u.id = p.user_id|u", + "UPDATE users u SET name = $1 FROM profiles p, orders o" + + " JOIN users x ON x.id = p.user_id WHERE u.id = o.user_id|p" + } + ) + void shouldRejectAnUpdateFromJoinConditionOutsideItsJoinGroup(String sql, String qualifier) { + Query query = new Query("RenameUsers", QueryType.EXEC, sql); + ParsedSql parsedSql = parser.parse(sql); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> analyzer.analyze(query, parsedSql, joinSchema) + ); + + assertEquals( + "Unknown source qualifier in query: " + qualifier, + exception.getMessage() + ); + } + + /** + * A returned column of a joined write takes the nullability of its schema + * column, and every column of a left-joined source is nullable, because an + * unmatched row reads it as {@code NULL}. + */ + @Test + void shouldResolveNullabilityOfJoinedWriteReturningColumns() { + String sql = """ + UPDATE users u + SET name = :name + FROM profiles p + LEFT JOIN orders o ON o.user_id = p.user_id + WHERE p.user_id = u.id + RETURNING u.id, u.name, o.id AS order_id, o.total + """; + + QueryModel model = analyzer.analyze( + new Query("RenameProfiledUsers", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true), + new QueryColumn("order_id", ColumnType.BIGINT, true), + new QueryColumn("total", ColumnType.DECIMAL, true) + ), + model.columns() + ); + } + + /** + * {@code RETURNING .*} returns the complete row of the write + * target, so it carries the target table's shared row identity on a joined + * and on a single-table insert, update, and delete alike. + */ + @ParameterizedTest + @ValueSource( + strings = { + "INSERT INTO users AS u (id, name) VALUES ($1, $2) RETURNING u.*", + "UPDATE users u SET name = $1 WHERE u.id = $2 RETURNING u.*", + "DELETE FROM users u WHERE u.id = $1 RETURNING u.*", + "UPDATE users u SET name = $1 FROM profiles p WHERE p.user_id = u.id RETURNING u.*", + "DELETE FROM users u USING profiles p WHERE p.user_id = u.id RETURNING u.*" + } + ) + void shouldResolveRowTableForATargetWildcardOfAWrite(String sql) { + QueryModel model = analyzer.analyze( + new Query("WriteUsers", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals("users", model.rowTable()); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true) + ), + model.columns() + ); + } + + /** + * A target wildcard written beside another returned column returns that + * target's columns in their own positions, which is a result specific to + * the query rather than the target's shared row. + */ + @Test + void shouldNotResolveRowTableForATargetWildcardBesideAnotherColumn() { + String sql = """ + UPDATE users u + SET name = :name + FROM profiles p + WHERE p.user_id = u.id + RETURNING u.*, p.nickname + """; + + QueryModel model = analyzer.analyze( + new Query("RenameProfiledUsers", QueryType.MANY, sql), + parser.parse(sql), + joinSchema + ); + + assertNull(model.rowTable()); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true), + new QueryColumn("nickname", ColumnType.VARCHAR, true) + ), + model.columns() + ); + } + + /** + * The only complete row a joined write returns is its target's, so a bare + * {@code *} and a wildcard of another source are rejected naming what to + * write instead, and a wildcard carries no alias. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + value = { + "UPDATE users u SET name = $1 FROM profiles p WHERE p.user_id = u.id RETURNING *" + + "|RETURNING * is not supported with UPDATE ... FROM or DELETE ... USING;" + + " write RETURNING u.* for the target's row.", + "DELETE FROM users USING profiles p WHERE p.user_id = users.id RETURNING *" + + "|RETURNING * is not supported with UPDATE ... FROM or DELETE ... USING;" + + " write RETURNING users.* for the target's row.", + "UPDATE users u SET name = $1 FROM profiles p WHERE p.user_id = u.id RETURNING p.*" + + "|RETURNING p.* is not supported; return the columns of p by name.", + "DELETE FROM users u USING profiles p WHERE p.user_id = u.id RETURNING p.*" + + "|RETURNING p.* is not supported; return the columns of p by name.", + "UPDATE users u SET name = $1 FROM profiles p WHERE p.user_id = u.id RETURNING u.* AS x" + + "|RETURNING u.* must not be aliased." + } + ) + void shouldRejectAnUnsupportedJoinedWriteReturningWildcard(String sql, String message) { + Query query = new Query("WriteUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, joinSchema) + ); + + assertEquals(message, exception.getMessage()); + } + + /** + * A returned wildcard names one of the write's sources, so an unknown + * qualifier, which the {@code old} and {@code new} row aliases of + * PostgreSQL 18 are among them, is rejected as any other unknown source + * qualifier is. + */ + @Test + void shouldRejectAnUnknownReturningWildcardQualifier() { + String sql = "UPDATE users u SET name = $1 FROM profiles p" + + " WHERE p.user_id = u.id RETURNING old.*"; + + Query query = new Query("RenameUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> analyzer.analyze(query, parsedSql, joinSchema) + ); + + assertEquals( + "Unknown source qualifier in query: old", + exception.getMessage() + ); + } + + /** + * The sources of a joined write expose distinct names, the target's name + * included, because a repeated name would make every reference to it + * ambiguous. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + value = { + "UPDATE users u SET name = $1 FROM profiles u WHERE u.user_id = $2|u", + "DELETE FROM users u USING profiles p, orders p WHERE p.user_id = u.id|p" + } + ) + void shouldRejectARepeatedJoinedWriteSourceName(String sql, String name) { + Query query = new Query("WriteUsers", QueryType.EXEC, sql); + ParsedSql parsedSql = parser.parse(sql); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> analyzer.analyze(query, parsedSql, joinSchema) + ); + + assertEquals( + "Duplicate source name in query: " + name, + exception.getMessage() + ); + } + + /** + * The placeholders of a joined write bind in the textual order of the + * query, across its {@code SET} assignments, its {@code FROM} sources, and + * its {@code WHERE} predicate, whatever their indexes are. + */ + @Test + void shouldBindJoinedUpdateParametersInTextualOrder() { + String sql = """ + UPDATE users u + SET name = $2 + FROM (SELECT user_id + FROM profiles + WHERE nickname = $3) recent + WHERE u.id = recent.user_id + AND u.id = $1 + """; + + QueryModel model = analyzer.analyze( + new Query("RenameRecentUsers", QueryType.EXEC, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "id", ColumnType.BIGINT), + new QueryParameter(2, "name", ColumnType.VARCHAR), + new QueryParameter(3, "nickname", ColumnType.VARCHAR) + ), + model.parameters() + ); + + assertEquals(List.of(2, 3, 1), model.bindingParameterIndexes()); + } + + /** + * One named placeholder written in a joined write's {@code SET} clause and + * in its {@code WHERE} predicate is one parameter, bound at each of its + * textual positions, beside the parameter a join condition types. + */ + @Test + void shouldShareOneNamedParameterAcrossAJoinedUpdate() { + String sql = """ + UPDATE users u + SET name = :nickname + FROM profiles p + JOIN orders o ON o.user_id = p.user_id + AND o.total = :total + WHERE p.nickname = :nickname + AND u.id = :id + """; + + QueryModel model = analyzer.analyze( + new Query("RenameOrderingUsers", QueryType.EXEC, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "nickname", ColumnType.VARCHAR), + new QueryParameter(2, "total", ColumnType.DECIMAL), + new QueryParameter(3, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2, 1, 3), model.bindingParameterIndexes()); + } } diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java index 83f7a20..104d5b5 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java @@ -3488,6 +3488,125 @@ INSERT INTO customer_orders (id, customer_id, quantity) } } + /** + * Covers joined writes end to end: an update whose rows a joined source + * selects returns the target's complete row through the shared row record a + * full-row read of the same table returns, and a {@code USING} delete + * returns one column of each of its two sources. PostgreSQL does not + * guarantee the returned row order, so the updated rows are compared as a + * set. + */ + @Test + void shouldExecuteGeneratedJoinedWritesAgainstPostgres() throws Exception { + execute(CONSTRAINT_SCHEMA); + + Path classesDirectory = generateAndCompile( + CONSTRAINT_SCHEMA, + """ + -- name: SetCustomerOrderQuantities :many + UPDATE customer_orders o + SET quantity = :quantity + FROM customers c + WHERE c.id = o.customer_id + AND c.name = :name + RETURNING o.*; + + -- name: DeleteCustomerOrders :many + DELETE FROM customer_orders o + USING customers c + WHERE c.id = o.customer_id + AND c.name = :name + RETURNING o.id, c.name; + + -- name: ListCustomerOrderRows :many + SELECT * + FROM customer_orders + ORDER BY id; + """ + ); + + execute("INSERT INTO customers (id, name) VALUES (1, 'Alice'), (2, 'Bob')"); + + execute( + """ + INSERT INTO customer_orders (id, customer_id, quantity) + VALUES (10, 1, 3), (20, 1, 4), (30, 2, 1) + """ + ); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Object repository = newRepository(classLoader); + + List updated = assertInstanceOf( + List.class, + repository + .getClass() + .getMethod("setCustomerOrderQuantities", Integer.class, String.class) + .invoke(repository, 9, "Alice") + ); + + assertEquals(2, updated.size()); + + assertEquals( + List.of("id", "customerId", "quantity", "status", "createdAt"), + recordComponentNames(updated.get(0)) + ); + + List> updatedRows = new ArrayList<>(); + + for (Object updatedRow : updated) { + updatedRows.add( + List.of( + component(updatedRow, "id"), + component(updatedRow, "customerId"), + component(updatedRow, "quantity") + ) + ); + } + + assertEquals( + Set.of( + List.of(10L, 1L, 9), + List.of(20L, 1L, 9) + ), + Set.copyOf(updatedRows) + ); + + List deletedRows = assertInstanceOf( + List.class, + repository + .getClass() + .getMethod("deleteCustomerOrders", String.class) + .invoke(repository, "Bob") + ); + + assertEquals(1, deletedRows.size()); + + assertEquals( + List.of("id", "name"), + recordComponentNames(deletedRows.get(0)) + ); + + assertEquals(30L, component(deletedRows.get(0), "id")); + assertEquals("Bob", component(deletedRows.get(0), "name")); + + List rows = assertInstanceOf( + List.class, + repository.getClass().getMethod("listCustomerOrderRows").invoke(repository) + ); + + assertEquals(2, rows.size()); + + assertEquals(updated.get(0).getClass(), rows.get(0).getClass()); + + assertEquals(10L, component(rows.get(0), "id")); + assertEquals(9, component(rows.get(0), "quantity")); + + assertEquals(20L, component(rows.get(1), "id")); + assertEquals(9, component(rows.get(1), "quantity")); + } + } + /** * Covers a returning insert: the database-generated serial values and the * target-table order of {@code RETURNING *} are read through the generated diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/ReferenceServiceIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/ReferenceServiceIntegrationTest.java index e8099dc..3a1249f 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/ReferenceServiceIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/ReferenceServiceIntegrationTest.java @@ -143,16 +143,6 @@ class ReferenceServiceIntegrationTest { "ListProjectsWithLatestTask", "LATERAL is not supported." ), - new UnsupportedQuery( - "update_from.sql", - "RaiseProjectTaskPriority", - "UPDATE ... FROM and joined updates are not supported." - ), - new UnsupportedQuery( - "delete_using.sql", - "DeleteProjectComments", - "DELETE ... USING and joined deletes are not supported." - ), new UnsupportedQuery( "uncast_lower.sql", "FindUserByEmail", diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java index a0fc6a3..f166ed0 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java @@ -2468,26 +2468,30 @@ private static Stream rejectedQueries() { "INSERT INTO users (id, name) SELECT id FROM profiles", "INSERT column and value counts must match." ), - Arguments.of( - ":exec", - "UPDATE users SET name = $2 FROM profiles WHERE users.id = $1", - "UPDATE ... FROM and joined updates are not supported." - ), Arguments.of( ":exec", "UPDATE users, profiles SET name = $2 WHERE users.id = $1", - "UPDATE ... FROM and joined updates are not supported." + "UPDATE of several tables is not supported;" + + " write the other sources in UPDATE ... FROM." ), Arguments.of( ":exec", - "DELETE FROM users USING profiles WHERE users.id = $1", - "DELETE ... USING and joined deletes are not supported." + "UPDATE users u JOIN profiles p ON p.user_id = u.id SET name = $1", + "UPDATE of several tables is not supported;" + + " write the other sources in UPDATE ... FROM." ), Arguments.of( ":exec", "DELETE users FROM users JOIN profiles ON profiles.user_id = users.id" + " WHERE users.id = $1", - "DELETE ... USING and joined deletes are not supported." + "A joined DELETE is not supported;" + + " write the other sources in DELETE ... USING." + ), + Arguments.of( + ":many", + "UPDATE users u SET name = $1 FROM profiles p WHERE p.user_id = u.id RETURNING *", + "RETURNING * is not supported with UPDATE ... FROM or DELETE ... USING;" + + " write RETURNING u.* for the target's row." ) ); } @@ -4052,6 +4056,94 @@ INSERT INTO profiles (id, user_id, nickname) assertCompiles(generatedDirectory, tempDir.resolve("classes")); } + /** + * A joined update and a {@code USING} delete generate compilable Java whose + * method parameters are typed from the columns of their added sources, + * whose returned columns of a left-joined source are nullable, and whose + * {@code RETURNING .*} keeps the target's shared row record. + */ + @Test + void shouldGenerateCompilableJavaForJoinedWrites() throws IOException { + Path schemaFile = tempDir.resolve("schema.sql"); + Path queriesFile = tempDir.resolve("queries.sql"); + Path generatedDirectory = tempDir.resolve("generated"); + + Files.writeString(schemaFile, JOIN_SCHEMA); + + Files.writeString( + queriesFile, + """ + -- name: RenameProfiledUsers :many + UPDATE users u + SET name = :name + FROM profiles p + WHERE p.user_id = u.id + AND p.nickname = :nickname + RETURNING u.*; + + -- name: ListRenamedOrderTotals :many + UPDATE users u + SET name = :name + FROM profiles p + LEFT JOIN orders o ON o.user_id = p.user_id + AND o.total = :total + WHERE p.user_id = u.id + RETURNING u.id, o.id AS order_id, o.total; + + -- name: DeleteOrderingUsers :many + WITH top_orders AS (SELECT user_id, total + FROM orders + WHERE total = :total) + DELETE FROM users u + USING top_orders + WHERE top_orders.user_id = u.id + RETURNING u.name, top_orders.total; + """ + ); + + String source = Files.readString( + generateUsersRepository( + List.of(schemaFile.toString()), + queriesFile, + generatedDirectory + ) + ); + + assertTrue( + source.contains( + "public List renameProfiledUsers(String name, String nickname)" + ), + source + ); + + assertTrue( + source.contains( + "public List listRenamedOrderTotals(String name," + + " BigDecimal total)" + ), + source + ); + + assertEquals( + List.of("Long id", "Long orderId", "BigDecimal total"), + recordComponents(source, "ListRenamedOrderTotalsResult") + ); + + assertTrue( + source.contains( + "public List deleteOrderingUsers(BigDecimal total)" + ), + source + ); + + assertEquals( + List.of("String name", "BigDecimal total"), + recordComponents(source, "DeleteOrderingUsersResult") + ); + + assertCompiles(generatedDirectory, tempDir.resolve("classes")); + } + /** * A list predicate generates one {@code List} method parameter of the * compared column's element type, bound as one server array, in compilable diff --git a/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/delete_using.sql b/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/delete_using.sql deleted file mode 100644 index f9d7a6b..0000000 --- a/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/delete_using.sql +++ /dev/null @@ -1,7 +0,0 @@ --- Outside the supported set: sqlcj models no DELETE ... USING. --- name: DeleteProjectComments :exec -DELETE -FROM comments c - USING tasks t -WHERE t.id = c.task_id - AND t.project_id = :projectId; diff --git a/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/update_from.sql b/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/update_from.sql deleted file mode 100644 index aa13db3..0000000 --- a/sqlcj-cli/src/test/resources/reference-service/unsupported/queries/update_from.sql +++ /dev/null @@ -1,7 +0,0 @@ --- Outside the supported set: sqlcj models no UPDATE ... FROM. --- name: RaiseProjectTaskPriority :exec -UPDATE tasks t -SET priority = 1 -FROM projects p -WHERE p.id = t.project_id - AND p.code = :code;