diff --git a/docs/queries.md b/docs/queries.md index 815c064..46853b5 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -179,8 +179,12 @@ WHERE author_id = $1; and so whether it can read as `NULL` is unknown at compile time. - The operand itself is not analyzed and reaches the database as written, so PostgreSQL rather than sqlcj checks its column references and its functions. - A placeholder in a projection is still rejected, including a cast placeholder - such as `$1::int AS x`. +- A placeholder the projection's cast casts directly, as in + `SELECT $1::text AS label`, is one parameter of the query, typed by that cast + and named after its own placeholder, so an indexed one is `param` and a + named one keeps its name. It binds before a `WHERE` placeholder written after + it, and a named one is also the first logical parameter. An uncast + placeholder in a projection is still rejected. ### Predicates @@ -284,9 +288,30 @@ placeholder takes. `ORDER BY` over direct columns is supported for a stable list order, as in `ORDER BY id`. sqlcj rewrites only `$N` parameter tokens; the rest of the -statement, including the ordering clause, reaches JDBC exactly as written. A -placeholder in `ORDER BY` is rejected, because it is not an analyzed parameter -location. +statement, including the ordering clause, reaches JDBC exactly as written. + +An `ORDER BY` placeholder that states its type with a cast is one parameter of +the query, which is how a sort key chosen by the caller compiles: + +```sql +-- name: ListAuthorsSorted :many +SELECT id, name +FROM authors +ORDER BY + CASE WHEN :sort::text = 'name' THEN name END, + CASE WHEN :sort::text = 'id' THEN id END; +``` + +- `ListAuthorsSorted` generates `listAuthorsSorted(String sort)`, whose one + argument is bound at both of its textual positions, so each accepted value + returns the rows in its own order. +- The clause itself is still not analyzed: the ordering expression, including + its column references, reaches the database as written, and sqlcj checks only + the cast that types the placeholder. +- An uncast `ORDER BY` placeholder, such as `ORDER BY $1`, is still rejected, + because nothing states its type. sqlcj never pastes a column name or a + direction into the SQL, so an ordering column chosen at run time stays + outside the subset. ### Pagination @@ -317,10 +342,18 @@ LIMIT $1 OFFSET $2; - A named value is named after its placeholder rather than after its clause, so `LIMIT :pageSize OFFSET :skip` generates `listAuthorPage(Integer pageSize, Integer skip)`. -- A placeholder in a computed value, as in `LIMIT $1 + 1` or - `OFFSET $1 + 1`, in the `LIMIT a, b` form, as in `LIMIT 5, $1`, and in a - `FETCH FIRST $1 ROWS ONLY` clause are rejected as unanalyzed placeholder - locations. +- A value that is a cast placeholder keeps its clause's name and takes the type + the cast states, so `LIMIT $1::int OFFSET $2::int` generates the same + `(Integer limit, Integer offset)` method parameters as the uncast page, while + `LIMIT $1::bigint` generates `Long limit` and `LIMIT :pageSize::int` + generates `pageSize`. +- A placeholder in a computed value, as in `LIMIT $1 + 1` or `OFFSET $1 + 1`, + in the `LIMIT a, b` form, as in `LIMIT 5, $1`, and in a + `FETCH FIRST $1 ROWS ONLY` clause is rejected as an unanalyzed placeholder + location unless a cast states its type. A cast placeholder there is one + parameter named after its own placeholder rather than after a pagination + clause, so `LIMIT LEAST($1::int, 100)` and `FETCH FIRST $1::int ROWS ONLY` + each generate `param1`. ## Writes @@ -480,8 +513,11 @@ string literal, a quoted identifier, or a comment is not a parameter. - The Java type of a placeholder is the type of the column it is compared with, assigned to, or inserted into. - A placeholder written as the direct operand of `::type` or - `CAST(... AS type)` is typed by that cast instead, wherever it appears inside - a `WHERE` clause, an `INSERT` value, or an `UPDATE` assignment. The cast type + `CAST(... AS type)` is typed by that cast instead, wherever it appears in an + accepted statement: a projection, `WHERE`, `GROUP BY`, `HAVING`, `ORDER BY`, + `LIMIT`, `OFFSET`, `FETCH`, an `INSERT` value, an `UPDATE` or + `DO UPDATE` assignment, a `WITH` body, and a subquery at any depth, such as + `EXISTS`, `IN (SELECT ...)`, or a scalar subquery. The cast type may be any type a column may declare and sqlcj maps, including a declared enum name, written unquoted and matched case-insensitively, and a one-dimensional array, which binds a `List`. A cast type sqlcj does not map, @@ -493,9 +529,15 @@ string literal, a quoted identifier, or a comment is not a parameter. `ILIKE` pattern, which is the shape that accepts an uncast pattern, the compared column of an analyzed `= ANY` list, or an inserted or assigned column — so `name = $1::text` names `name` as - `name = $1` does. Every other indexed cast placeholder is named `param` + `name = $1` does. An indexed cast placeholder that is the whole `LIMIT` row + count or `OFFSET` value is named `limit` or `offset`, as an uncast one is. + Every other indexed cast placeholder is named `param` after its own index, as in `param1` for `$1`, and colliding names take the usual numeric suffix. +- Every placeholder occurrence binds in textual order across the clauses and + nesting depths above, whatever order sqlcj analyzes those clauses in, so a + projected placeholder binds before a `WHERE` one and + `OFFSET $2::int LIMIT $1::int` binds the offset first. - Occurrences of one placeholder may mix a cast and an uncast location, and the parameter keeps the name and type of its first occurrence, so `(:name::text IS NULL OR name = :name)` is one `String` parameter named @@ -503,8 +545,9 @@ string literal, a quoted identifier, or a comment is not a parameter. - Mixing `$N` and `:name` placeholders in one query is rejected, as are anonymous `?` placeholders, a qualified name such as `:a.b`, a quoted name such as `:"x"`, and an `&name` placeholder. -- A placeholder in a location sqlcj does not analyze — for example `ORDER BY $1` - or `lower(name) = :name` — is rejected rather than left unbound. +- A placeholder that neither a clause nor a cast types — for example + `ORDER BY $1`, `lower(name) = :name`, or `(:x)::int`, whose placeholder is not + the direct operand of its cast — is rejected rather than left unbound. ### Logical order versus textual order @@ -773,7 +816,9 @@ name, and its header line. - A `FROM` item that is not a table, a comma-separated source list, a join that is not a plain inner or left join, a join predicate that is not one qualified equality, and a set operation such as `UNION`. -- A placeholder in a location sqlcj does not analyze, including `ORDER BY $1`, a +- An uncast placeholder in a location sqlcj does not analyze, including + `ORDER BY $1`, a projection such as `SELECT $1 AS x`, a grouping list, a + `HAVING` predicate, a subquery, a named placeholder under a function such as `lower(name) = :name`, a computed `LIKE` pattern such as `'%' || $1 || '%'`, a placeholder as the @@ -782,10 +827,10 @@ name, and its header line. `LIMIT $1 + 1` or `OFFSET $1 + 1`, a `LIMIT a, b` row count such as `LIMIT 5, $1`, a placeholder inside a write value such as `COALESCE($2, bio)`, and a `FETCH FIRST $1 ROWS ONLY` clause, so dynamic `IN` - expansion is unavailable. A cast does not widen these locations: a cast - placeholder in a projection, `ORDER BY`, a join condition, or a pagination - value, such as `ORDER BY $1::int`, stays rejected, as does a placeholder that - is not the direct operand of its cast, such as `(:x)::int`. + expansion is unavailable. A cast states the type of such a placeholder in + every one of those locations except a join condition, where any placeholder + stays rejected, and it does not accept a placeholder that is not the direct + operand of its cast, such as `(:x)::int`. - A cast type sqlcj does not map, such as `$1::interval` or a multi-dimensional `$1::int[][]`, which is rejected naming the placeholder and the declared type, or the result column and the declared type when the cast @@ -843,6 +888,12 @@ them: - the unique index an accepted `ON CONFLICT` target matches, and the column references of an `EXCLUDED` value that is not exactly `EXCLUDED.column`. +A clause that is not analyzed still binds the placeholders a cast types inside +it, in textual order, and nothing else of it is checked. A cast placeholder in a +`GROUP BY`, `HAVING`, `ORDER BY`, `WITH` body, or subquery therefore generates +one typed method parameter while PostgreSQL rather than sqlcj checks the +surrounding expression, including its tables and columns. + sqlcj itself provides no macros, dynamic `IN` expansion, or query-building API. The `= ANY` list predicate is the only analyzed array operator. An array parameter is one whole list bound at one placeholder, not a placeholder list. @@ -965,12 +1016,39 @@ Reads: `QueryAnalyzerTest.shouldResolvePaginationParametersInTextualBindingOrder`, `QueryAnalyzerTest.shouldResolveOffsetParameterBesideUnanalyzedRowCount`, `QueryAnalyzerTest.shouldNotCreateParametersForLiteralPagination`, - `QueryAnalyzerTest.shouldNameNamedPaginationParameterAfterItsPlaceholder`, and + `QueryAnalyzerTest.shouldNameNamedPaginationParameterAfterItsPlaceholder`, + `QueryAnalyzerTest.shouldResolveCastPaginationParameters`, + `QueryAnalyzerTest.shouldResolveCastPaginationParametersInTextualBindingOrder`, + `QueryAnalyzerTest.shouldTypeCastPaginationParameterByItsCast`, and `QueryAnalyzerTest.shouldRejectPlaceholderInUnsupportedPaginationValue` cover - pagination, and - `PostgresIntegrationTest.shouldExecuteGeneratedPaginationAgainstPostgres` - executes a `LIMIT ... OFFSET ...` page and the same page written as - `OFFSET ... LIMIT ...` against PostgreSQL 16. + pagination, including the cast values and their clause names, and + `PostgresIntegrationTest.shouldExecuteGeneratedPaginationAgainstPostgres` and + `PostgresIntegrationTest.shouldExecuteGeneratedCastPaginationAgainstPostgres` + execute a `LIMIT ... OFFSET ...` page and the same page written as + `OFFSET ... LIMIT ...`, uncast and cast, against PostgreSQL 16. +- `QueryAnalyzerTest.shouldResolveCastPlaceholdersOfOrderBy`, + `QueryAnalyzerTest.shouldNameIndexedCastPlaceholderOfOrderByAfterItsIndex`, + `QueryAnalyzerTest.shouldResolveCastPlaceholderOfEveryReadClause`, + `QueryAnalyzerTest.shouldNameIndexedCastPlaceholderOfGroupPredicateAfterItsIndex`, + `QueryAnalyzerTest.shouldResolveCastPlaceholderOfSubquery`, + `QueryAnalyzerTest.shouldResolveCastPlaceholderOfWithBody`, + `QueryAnalyzerTest.shouldShareOneParameterBetweenAgreeingCastsOfDifferentClauses`, + `QueryAnalyzerTest.shouldRejectConflictingCastsOfDifferentClauses`, + `QueryAnalyzerTest.shouldRejectUncastPlaceholderOutsideTheAnalyzedClauses`, and + `QueryAnalyzerTest.shouldRejectUnsupportedCastTypeOutsideTheAnalyzedClauses` + cover the cast placeholders of the ordering, grouping, group-predicate, + pagination, `FETCH`, `WITH`, and subquery clauses, their names, their shared + and conflicting repetitions, and the uncast, indirect, and unmapped forms that + stay rejected, while + `SqlParserTest.shouldReportPlaceholderOccurrencesInTextualOrder` and + `SqlParserTest.shouldReportTheCastThatCastsAPlaceholderDirectly` cover the + syntax-level occurrence order and the cast each occurrence carries, + `SqlcjCompilerIntegrationTest.shouldGenerateCompilableRepositoryForCastPlaceholdersOfEveryClause` + compiles a repository generated from those clauses, and + `PostgresIntegrationTest.shouldExecuteGeneratedDynamicSortAgainstPostgres` and + `PostgresIntegrationTest.shouldExecuteGeneratedExistsSubqueryAgainstPostgres` + execute a dynamic sort over two sort keys and an `EXISTS` membership check + against PostgreSQL 16. - `QueryAnalyzerTest.shouldResolveScalarCountColumnFromItsAlias`, `QueryAnalyzerTest.shouldResolveScalarCountBesidePredicateParameter`, `QueryAnalyzerTest.shouldResolveScalarCountBesideDirectColumn`, @@ -989,11 +1067,14 @@ Reads: - `QueryAnalyzerTest.shouldResolveCastProjectionColumnFromItsAlias`, `QueryAnalyzerTest.shouldResolveEnumCastProjectionColumn`, `QueryAnalyzerTest.shouldResolveArrayCastProjectionColumn`, - `QueryAnalyzerTest.shouldRejectUnsupportedCastProjectionForm`, and - `QueryAnalyzerTest.shouldRejectCastPlaceholderProjection` cover both cast + `QueryAnalyzerTest.shouldRejectUnsupportedCastProjectionForm`, + `QueryAnalyzerTest.shouldResolveCastPlaceholderProjection`, and + `QueryAnalyzerTest.shouldNumberNamedProjectionPlaceholderBeforeThePredicateOne` + cover both cast spellings and both alias spellings, the nullable column of the cast type including a declared enum and an array, the missing alias and the unmapped - cast type, and the projected placeholder that stays rejected, and + cast type, and the projected cast placeholder, its name, and its logical and + binding positions, and `PostgresIntegrationTest.shouldExecuteGeneratedCastProjectionAgainstPostgres` executes a `:one` cast `SUM` against PostgreSQL 16 whose `BigDecimal` component is the sum and `null` when no row matches. @@ -1130,8 +1211,7 @@ Parameters: `QueryAnalyzerTest.shouldResolveCastParametersOfInsertValues` cover the cast types, the analyzed cast locations, and the name a cast placeholder takes, while `QueryAnalyzerTest.shouldRejectUnsupportedCastType`, - `QueryAnalyzerTest.shouldRejectCastOccurrenceWithConflictingType`, - `QueryAnalyzerTest.shouldRejectCastPlaceholderInUnanalyzedLocation`, and the + `QueryAnalyzerTest.shouldRejectCastOccurrenceWithConflictingType`, and the cast rows of `QueryAnalyzerTest.shouldRejectUnsupportedNamedPlaceholderForm` cover the cast rejections. 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 f07081d..75558ad 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java @@ -7,12 +7,12 @@ import dev.sqlcj.schema.parser.ColumnTypeMapping; import dev.sqlcj.schema.parser.SchemaNamespace; import dev.sqlcj.sql.ParsedSql; +import dev.sqlcj.sql.PlaceholderOccurrence; import dev.sqlcj.type.DefaultTypeResolver; import dev.sqlcj.type.TypeResolver; import net.sf.jsqlparser.expression.Alias; import net.sf.jsqlparser.expression.CastExpression; import net.sf.jsqlparser.expression.Expression; -import net.sf.jsqlparser.expression.ExpressionVisitorAdapter; import net.sf.jsqlparser.expression.Function; import net.sf.jsqlparser.expression.JdbcNamedParameter; import net.sf.jsqlparser.expression.JdbcParameter; @@ -56,6 +56,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Comparator; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -125,9 +126,11 @@ private record ResolvedColumn(Source source, dev.sqlcj.schema.Column column) { /** * One placeholder occurrence, resolved to the logical parameter it binds - * and, for a named placeholder, to the name that parameter carries. + * and, for a named placeholder, to the name that parameter carries. The + * placeholder expression identifies the occurrence, which is how a clause's + * analysis meets the placeholder occurrences the parser reported. */ - private record Placeholder(int index, String name) { + private record Placeholder(Expression expression, int index, String name) { /** * The name of the parameter this occurrence binds, which a named @@ -155,9 +158,10 @@ private record CastPlaceholder(Placeholder placeholder, ColumnTypeMapping.Mapped } /** - * The compiled placeholders of one query and the parameter occurrences - * analyzed against them, collected in the textual order of the executable - * {@code ?} positions. + * The compiled placeholders of one query and the parameter occurrences its + * clauses typed, each recorded against the placeholder expression it + * resolves. The textual order of the occurrences is the order the parser + * reported them in, not the order the clauses were analyzed in. */ private static final class Placeholders { @@ -170,7 +174,8 @@ private static final class Placeholders { /** Resolves the type a cast states. */ private final ColumnTypeMapping columnTypeMapping; - private final List occurrences = new ArrayList<>(); + /** The occurrences a clause typed, by their placeholder expression. */ + private final Map typedOccurrences = new IdentityHashMap<>(); private Placeholders( List names, @@ -195,11 +200,15 @@ private Placeholder of(Expression expression) { throw new UnsupportedOperationException(ANONYMOUS_PARAMETER_REJECTION); } - return new Placeholder(parameter.getIndex(), null); + return new Placeholder(expression, parameter.getIndex(), null); } if (expression instanceof JdbcNamedParameter named) { - return new Placeholder(requireCompiledName(named), named.getName()); + return new Placeholder( + expression, + requireCompiledName(named), + named.getName() + ); } return null; @@ -258,12 +267,38 @@ private int requireCompiledName(JdbcNamedParameter named) { return index + 1; } - private void add(QueryParameter occurrence) { - occurrences.add(occurrence); + /** + * Records the parameter occurrence a clause of the query typed for one + * placeholder. Each placeholder occurrence stands in one clause, so + * typing one twice is a defect of this analyzer. + */ + private void record(Placeholder placeholder, QueryParameter occurrence) { + if (typedOccurrences.put(placeholder.expression(), occurrence) != null) { + throw new IllegalStateException( + "Placeholder %s was typed twice.".formatted(placeholder.spelled()) + ); + } + } + + /** + * Takes the occurrence a clause typed for one placeholder, and + * {@code null} when no clause typed it. + */ + private QueryParameter typed(Expression placeholder) { + return typedOccurrences.remove(placeholder); } - private List occurrences() { - return occurrences; + /** + * Requires that every occurrence a clause typed was one of the + * placeholder occurrences the parser reported, so that no analyzed + * parameter is left out of the binding order. + */ + private void requireReportedOccurrences() { + if (!typedOccurrences.isEmpty()) { + throw new IllegalStateException( + "A typed placeholder is not a reported placeholder occurrence." + ); + } } } @@ -765,7 +800,7 @@ private void resolveExcludedColumns( } /** - * Builds the analyzed model from the parameter occurrences collected in + * Builds the analyzed model from the parameter occurrences resolved in * textual order, keeping that order for JDBC binding and exposing one * logical parameter per placeholder index. */ @@ -777,7 +812,7 @@ private QueryModel toQueryModel( Placeholders placeholders, String rowTable ) { - List occurrences = placeholders.occurrences(); + List occurrences = resolveOccurrences(parsedSql, placeholders); List bindingParameterIndexes = requireAccountedOccurrences(parsedSql, occurrences); @@ -793,6 +828,43 @@ private QueryModel toQueryModel( ); } + /** + * Resolves the parameter occurrences of one query in the textual order of + * the placeholder occurrences the SQL parser reported, which is the JDBC + * binding order of the generated {@code ?} positions. + * + *

An occurrence a clause of the query typed is that clause's parameter. + * Every other occurrence is typed by the cast it is the direct operand of, + * wherever in the statement it stands, and is named after its own + * placeholder. A placeholder neither a clause nor a cast types contributes + * no parameter, so the accounting reports it as unaccounted. + */ + private List resolveOccurrences(ParsedSql parsedSql, Placeholders placeholders) { + List occurrences = new ArrayList<>(); + + for (PlaceholderOccurrence occurrence : parsedSql.placeholders()) { + QueryParameter typed = placeholders.typed(occurrence.placeholder()); + + if (typed != null) { + occurrences.add(typed); + + continue; + } + + CastPlaceholder cast = placeholders.ofCast(occurrence.cast()); + + if (cast != null) { + occurrences.add( + toCastParameter(cast, indexedParameterName(cast.placeholder())) + ); + } + } + + placeholders.requireReportedOccurrences(); + + return occurrences; + } + /** * Requires that the occurrences resolved against the schema are exactly the * placeholder tokens the SQL parser reported, in the same textual order, so @@ -1135,9 +1207,8 @@ private Source resolveJoinConditionSource(Expression expression, List so } /** - * Resolves the supported parameters in the textual order in which they are - * encountered, which is the JDBC binding order of the generated {@code ?} - * positions. + * Resolves the parameters the clauses of a read type, which are the + * {@code WHERE} parameters and the pagination parameters. */ private void resolveBindingParameters( PlainSelect plainSelect, @@ -1156,44 +1227,54 @@ private void resolveBindingParameters( } /** - * Resolves the pagination parameters, which follow every {@code WHERE} - * parameter in textual order. A {@code LIMIT} row count and an - * {@code OFFSET} value that is a placeholder each becomes an - * {@code INTEGER} parameter named after its own clause, or after the - * placeholder when the placeholder is named. The parser stores - * {@code OFFSET a LIMIT b} exactly like {@code LIMIT b OFFSET a}, so two - * placeholders are ordered by their source positions. + * Resolves the pagination parameters, which are the {@code LIMIT} row count + * and the {@code OFFSET} value. Either value binds one parameter named after + * its own clause, or after the placeholder when the placeholder is named. + * The parser stores {@code OFFSET a LIMIT b} exactly like + * {@code LIMIT b OFFSET a}, so the two bind in the order the parser reported + * their placeholders, not in the order they are resolved here. */ private void resolvePaginationParameters(PlainSelect plainSelect, Placeholders placeholders) { - Expression rowCountValue = resolveLimitRowCount(plainSelect.getLimit()); - - Placeholder rowCount = placeholders.of(rowCountValue); - Offset offset = plainSelect.getOffset(); - Expression offsetExpression = offset == null - ? null - : offset.getOffset(); + resolvePaginationValue( + resolveLimitRowCount(plainSelect.getLimit()), + LIMIT_PARAMETER_NAME, + placeholders + ); - Placeholder offsetValue = placeholders.of(offsetExpression); + resolvePaginationValue( + offset == null + ? null + : offset.getOffset(), + OFFSET_PARAMETER_NAME, + placeholders + ); + } - if ( - rowCount != null - && offsetValue != null - && sourcePosition(offsetExpression) < sourcePosition(rowCountValue) - ) { - addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, placeholders); - addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, placeholders); + /** + * Resolves one pagination value: a placeholder, which counts rows and is + * therefore an {@code INTEGER}, or a cast placeholder, which states its own + * type and keeps its clause's name. Every other value reaches the database + * as written. + */ + private void resolvePaginationValue( + Expression value, + String name, + Placeholders placeholders + ) { + Placeholder placeholder = placeholders.of(value); + + if (placeholder != null) { + addParameter(placeholder, name, ColumnType.INTEGER, placeholders); return; } - if (rowCount != null) { - addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, placeholders); - } + CastPlaceholder cast = placeholders.ofCast(value); - if (offsetValue != null) { - addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, placeholders); + if (cast != null) { + addCastParameter(cast, name, placeholders); } } @@ -1208,11 +1289,6 @@ private Expression resolveLimitRowCount(Limit limit) { : limit.getRowCount(); } - /** The source position of a placeholder token, counted from one. */ - private int sourcePosition(Expression placeholder) { - return placeholder.getASTNode().jjtGetFirstToken().absoluteBegin; - } - private void resolveParameters( Expression expression, List sources, @@ -1252,7 +1328,7 @@ private void resolveParameters( } if (expression instanceof IsNullExpression isNull) { - resolveIsNullExpression(isNull, sources, placeholders); + resolveIsNullExpression(isNull, sources); return; } @@ -1263,17 +1339,11 @@ private void resolveParameters( if (expression instanceof ComparisonOperator comparison) { resolveParameterComparison(comparison, sources, placeholders); - return; } - - resolveCastPlaceholders(expression, placeholders); } private void resolveInExpression(InExpression in, List sources, Placeholders placeholders) { if (!(in.getLeftExpression() instanceof net.sf.jsqlparser.schema.Column column)) { - resolveCastPlaceholders(in.getLeftExpression(), placeholders); - resolveCastPlaceholders(in.getRightExpression(), placeholders); - return; } @@ -1368,10 +1438,7 @@ private void resolveInExpression( if (cast != null) { addCastParameter(cast, schemaColumn.name(), placeholders); - return; } - - resolveCastPlaceholders(expression, placeholders); } /** @@ -1435,12 +1502,7 @@ && unsupportedLikePatternReason(like) == null resolveColumn(column, sources).column().name(), placeholders ); - - return; } - - resolveCastPlaceholders(left, placeholders); - resolveCastPlaceholders(right, placeholders); } /** @@ -1509,22 +1571,15 @@ private dev.sqlcj.schema.Column requireTextColumn(dev.sqlcj.schema.Column column /** * Resolves the tested column of {@code IS NULL} and {@code IS NOT NULL} * against the query sources. The predicate names no parameter after its - * tested value, so a tested value that is not a direct column contributes - * only the parameters its own cast placeholders state. + * tested value, so a tested value that is not a direct column is left to + * the cast that types its own placeholder. */ - private void resolveIsNullExpression( - IsNullExpression isNull, - List sources, - Placeholders placeholders - ) { + private void resolveIsNullExpression(IsNullExpression isNull, List sources) { Expression tested = isNull.getLeftExpression(); if (tested instanceof net.sf.jsqlparser.schema.Column column) { resolveColumn(column, sources); - return; } - - resolveCastPlaceholders(tested, placeholders); } /** @@ -1555,13 +1610,7 @@ private void resolveBetweenExpression( resolveColumnValue(start, schemaColumn, placeholders); resolveColumnValue(end, schemaColumn, placeholders); - - return; } - - resolveCastPlaceholders(left, placeholders); - resolveCastPlaceholders(start, placeholders); - resolveCastPlaceholders(end, placeholders); } /** @@ -1631,13 +1680,9 @@ private void resolveParameterComparison( resolveColumn(column, sources).column().name(), placeholders ); - return; } } } - - resolveCastPlaceholders(left, placeholders); - resolveCastPlaceholders(right, placeholders); } /** @@ -1767,8 +1812,8 @@ private dev.sqlcj.schema.Column requireArrayElementColumn(dev.sqlcj.schema.Colum /** * Resolves one value of a clause that names its parameter after a column: a * placeholder typed from the column, a cast placeholder that states its own - * type and keeps the column's name, or an expression that is neither, which - * contributes only the parameters its own cast placeholders state. + * type and keeps the column's name, or an expression that is neither, whose + * own cast placeholders are typed where they stand. */ private void resolveColumnValue( Expression value, @@ -1786,10 +1831,7 @@ private void resolveColumnValue( if (cast != null) { addCastParameter(cast, column.name(), placeholders); - return; } - - resolveCastPlaceholders(value, placeholders); } /** @@ -1800,42 +1842,6 @@ private boolean bindsValue(Expression value, Placeholders placeholders) { return placeholders.of(value) != null || placeholders.ofCast(value) != null; } - /** - * Binds every cast placeholder an expression contains that no clause of the - * query names, each after its own placeholder, in textual order. The - * expression visitor reports an expression's parts in the order the query - * writes them and does not descend into a subquery, whose placeholders are - * not analyzed. - */ - private void resolveCastPlaceholders(Expression expression, Placeholders placeholders) { - if (expression == null) { - return; - } - - expression.accept( - new ExpressionVisitorAdapter() { - - @Override - public Void visit(CastExpression cast, S context) { - CastPlaceholder castPlaceholder = placeholders.ofCast(cast); - - if (castPlaceholder == null) { - return super.visit(cast, context); - } - - addCastParameter( - castPlaceholder, - indexedParameterName(castPlaceholder.placeholder()), - placeholders - ); - - return null; - } - }, - null - ); - } - /** * The name an indexed placeholder takes where no column of the query names * it, which is the placeholder's own index. A named placeholder states its @@ -1846,25 +1852,28 @@ private String indexedParameterName(Placeholder placeholder) { } /** - * Records one parameter occurrence a cast types, named after the column - * whose value it is where a clause names one, and otherwise after the - * placeholder itself. + * Records one parameter occurrence a cast types in the clause that names + * it, which names it after the column whose value it is. */ private void addCastParameter( CastPlaceholder cast, String name, Placeholders placeholders ) { + placeholders.record(cast.placeholder(), toCastParameter(cast, name)); + } + + /** One parameter occurrence a cast types, under the given name. */ + private QueryParameter toCastParameter(CastPlaceholder cast, String name) { ColumnTypeMapping.MappedType type = cast.type(); - addParameter( + return toParameter( cast.placeholder(), name, type.type(), type.enumType(), type.array(), - type.blankPadded(), - placeholders + type.blankPadded() ); } @@ -1893,11 +1902,7 @@ private void addParameter( addParameter(placeholder, name, type, null, false, false, placeholders); } - /** - * Records one parameter occurrence, named after its placeholder when the - * placeholder is named, and otherwise after the column or clause it belongs - * to. - */ + /** Records one parameter occurrence a clause of the query typed. */ private void addParameter( Placeholder placeholder, String name, @@ -1907,15 +1912,31 @@ private void addParameter( boolean blankPadded, Placeholders placeholders ) { - placeholders.add( - new QueryParameter( - placeholder.index(), - placeholder.nameOr(name), - type, - enumType, - array, - blankPadded - ) + placeholders.record( + placeholder, + toParameter(placeholder, name, type, enumType, array, blankPadded) + ); + } + + /** + * One parameter occurrence, named after its placeholder when the placeholder + * is named, and otherwise after the column or clause it belongs to. + */ + private QueryParameter toParameter( + Placeholder placeholder, + String name, + ColumnType type, + String enumType, + boolean array, + boolean blankPadded + ) { + return new QueryParameter( + placeholder.index(), + placeholder.nameOr(name), + type, + enumType, + array, + blankPadded ); } diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java b/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java index 18e574f..86ac5e7 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java @@ -2,15 +2,25 @@ import net.sf.jsqlparser.statement.Statement; +import java.util.List; + /** * Syntax-level result of parsing one query SQL source. * - * @param statement the parsed statement - * @param parameters the parameters compiled from the parsed {@code $N} and - * {@code :name} placeholder tokens of the same source + * @param statement the parsed statement + * @param parameters the parameters compiled from the parsed {@code $N} and + * {@code :name} placeholder tokens of the same source + * @param placeholders the placeholder occurrences of the same source in textual + * order, one per placeholder the parse tree spells, at every + * clause and nesting depth */ public record ParsedSql( Statement statement, - SqlParameters parameters + SqlParameters parameters, + List placeholders ) { + + public ParsedSql { + placeholders = List.copyOf(placeholders); + } } diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/sql/PlaceholderOccurrence.java b/sqlcj-cli/src/main/java/dev/sqlcj/sql/PlaceholderOccurrence.java new file mode 100644 index 0000000..bcfc47c --- /dev/null +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/PlaceholderOccurrence.java @@ -0,0 +1,24 @@ +package dev.sqlcj.sql; + +import net.sf.jsqlparser.expression.CastExpression; +import net.sf.jsqlparser.expression.Expression; + +/** + * One placeholder token of a parsed SQL source, as the parse tree spells it. + * + *

An occurrence is syntax, not a type: it reports where a placeholder stands + * and whether the query casts it, and leaves to semantic analysis which + * parameter it binds and of which type. + * + * @param placeholder the placeholder expression the parser produced, which + * identifies this occurrence + * @param cast the cast whose direct operand the placeholder is, which is + * the innermost cast of a chain such as + * {@code :x::text::int}, and {@code null} when the + * placeholder is not cast directly + */ +public record PlaceholderOccurrence( + Expression placeholder, + CastExpression cast +) { +} diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java index 409bd07..a9ae3f7 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java @@ -3,6 +3,7 @@ import net.sf.jsqlparser.expression.CastExpression; import net.sf.jsqlparser.expression.Expression; import net.sf.jsqlparser.expression.JdbcNamedParameter; +import net.sf.jsqlparser.expression.JdbcParameter; import net.sf.jsqlparser.parser.CCJSqlParserConstants; import net.sf.jsqlparser.parser.Node; import net.sf.jsqlparser.parser.Token; @@ -26,6 +27,10 @@ * such as a qualified, quoted, or {@code &name} placeholder, stays in the SQL * and is reported as written for semantic analysis to reject, as is an * anonymous {@code ?} placeholder, which has no parameter to bind. + * + *

The same walk reports the placeholder occurrences of the source in + * textual order, each with the cast that casts it, which is the syntax-level + * order in which semantic analysis binds them. */ final class SqlParameterCompiler { @@ -66,7 +71,12 @@ final class SqlParameterCompiler { */ private static final int SOURCE_OFFSET = 1; - SqlParameters compile(String sql, Node astRoot) { + /** + * Compiles the parameters of one parsed source and collects its placeholder + * occurrences into {@code placeholders}, in the textual order of the + * {@code ?} positions of the compiled SQL. + */ + SqlParameters compile(String sql, Node astRoot, List placeholders) { if (astRoot == null) { throw new SqlParseException("SQL parse tree is unavailable."); } @@ -140,7 +150,7 @@ SqlParameters compile(String sql, Node astRoot) { List uncompiled = new ArrayList<>(); - collectUncompiledPlaceholders(astRoot, names, uncompiled); + collectPlaceholders(astRoot, names, uncompiled, placeholders); return new SqlParameters( executableSql.toString(), @@ -153,33 +163,84 @@ SqlParameters compile(String sql, Node astRoot) { } /** - * Collects the named placeholders of the parse tree that this compiler did - * not replace, written as the source spells them, so that an accepted query - * never keeps such a placeholder in its executable SQL. + * Walks the parse tree in pre-order, collecting the placeholder occurrences + * of the source in textual order and the named placeholders this compiler + * did not replace, written as the source spells them, so that an accepted + * query never keeps such a placeholder in its executable SQL. * *

The parser gives the operand of a {@code ::} cast no parse-tree node * of its own, so a cast reports the placeholder it casts, through any * further cast of the same operand. */ - private void collectUncompiledPlaceholders(Node node, List names, List uncompiled) { - collectUncompiledPlaceholder(node.jjtGetValue(), names, uncompiled); + private void collectPlaceholders( + Node node, + List names, + List uncompiled, + List placeholders + ) { + Object value = node.jjtGetValue(); + + collectUncompiledPlaceholder(value, names, uncompiled); + + if (value instanceof CastExpression cast) { + CastExpression innermost = innermostCast(cast); + Expression operand = innermost.getLeftExpression(); + + collectUncompiledPlaceholder(operand, names, uncompiled); + collectPlaceholder(operand, innermost, placeholders); + } - if (node.jjtGetValue() instanceof CastExpression cast) { - collectUncompiledPlaceholder(castOperand(cast), names, uncompiled); + if (value instanceof Expression expression) { + collectPlaceholder(expression, null, placeholders); } for (int child = 0; child < node.jjtGetNumChildren(); child++) { - collectUncompiledPlaceholders(node.jjtGetChild(child), names, uncompiled); + collectPlaceholders(node.jjtGetChild(child), names, uncompiled, placeholders); } } - /** The operand a cast casts, which is the operand of a chained cast. */ - private Expression castOperand(CastExpression cast) { - Expression operand = cast.getLeftExpression(); + /** + * The cast a chain of casts applies first, which is the cast whose operand + * a chain such as {@code :x::text::int} casts directly. + */ + private CastExpression innermostCast(CastExpression cast) { + return cast.getLeftExpression() instanceof CastExpression chained + ? innermostCast(chained) + : cast; + } + + /** + * Collects one parse-tree value when it is a placeholder, in the order the + * walk reaches it. + * + *

One expression can hang under two parse-tree nodes, so a placeholder + * is recorded once, identified by the expression itself, and the cast that + * casts it is kept whichever node reports it first. + */ + private void collectPlaceholder( + Object value, + CastExpression cast, + List placeholders + ) { + if (!(value instanceof JdbcParameter) && !(value instanceof JdbcNamedParameter)) { + return; + } + + Expression placeholder = (Expression) value; + + for (int index = 0; index < placeholders.size(); index++) { + PlaceholderOccurrence collected = placeholders.get(index); + + if (collected.placeholder() == placeholder) { + if (cast != null && collected.cast() == null) { + placeholders.set(index, new PlaceholderOccurrence(placeholder, cast)); + } + + return; + } + } - return operand instanceof CastExpression chained - ? castOperand(chained) - : operand; + placeholders.add(new PlaceholderOccurrence(placeholder, cast)); } /** diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParser.java b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParser.java index c650ae4..4e71445 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParser.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParser.java @@ -5,6 +5,8 @@ import net.sf.jsqlparser.parser.CCJSqlParserUtil; import net.sf.jsqlparser.statement.Statement; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; public final class SqlParser { @@ -12,8 +14,8 @@ public final class SqlParser { private final SqlParameterCompiler parameterCompiler = new SqlParameterCompiler(); /** - * Parses one SQL source into its statement and its compiled positional - * parameters. + * Parses one SQL source into its statement, its compiled positional + * parameters, and its placeholder occurrences in textual order. * *

The parser used for the returned statement is captured while parsing. * {@link CCJSqlParserUtil} hands a newly created parser to the consumer @@ -35,9 +37,14 @@ public ParsedSql parse(String sql) { throw new SqlParseException("SQL source contains no statement."); } - return new ParsedSql( - statement, - parameterCompiler.compile(sql, parser.get().getASTRoot()) + List placeholders = new ArrayList<>(); + + SqlParameters parameters = parameterCompiler.compile( + sql, + parser.get().getASTRoot(), + placeholders ); + + return new ParsedSql(statement, parameters, placeholders); } } 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 c3a7a66..505a5a7 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java @@ -1344,6 +1344,90 @@ void shouldNameNamedPaginationParameterAfterItsPlaceholder(String sql) { assertEquals(List.of(1), model.bindingParameterIndexes()); } + /** + * A cast pagination value states its own type and keeps its clause's name. + */ + @Test + void shouldResolveCastPaginationParameters() { + String sql = "SELECT id FROM users ORDER BY id LIMIT $1::int OFFSET $2::int"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "limit", ColumnType.INTEGER), + new QueryParameter(2, "offset", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + + assertEquals( + "SELECT id FROM users ORDER BY id LIMIT ?::int OFFSET ?::int", + model.executableSql() + ); + } + + /** + * The parser stores an {@code OFFSET} written before its {@code LIMIT} like + * one written after it, so the two cast values bind in the order the query + * writes them. + */ + @Test + void shouldResolveCastPaginationParametersInTextualBindingOrder() { + String sql = "SELECT id FROM users ORDER BY id OFFSET $2::int LIMIT $1::int"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "limit", ColumnType.INTEGER), + new QueryParameter(2, "offset", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals(List.of(2, 1), model.bindingParameterIndexes()); + } + + /** + * A cast pagination value is typed by its cast rather than by its clause, + * and a named one is still named after its placeholder. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + value = { + "SELECT id FROM users ORDER BY id LIMIT $1::bigint|limit|BIGINT", + "SELECT id FROM users ORDER BY id OFFSET $1::bigint|offset|BIGINT", + "SELECT id FROM users ORDER BY id LIMIT :pageSize::int|pageSize|INTEGER", + "SELECT id FROM users ORDER BY id OFFSET CAST(:skip AS int)|skip|INTEGER" + } + ) + void shouldTypeCastPaginationParameterByItsCast(String sql, String name, ColumnType type) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, name, type)), + model.parameters() + ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); + } + @ParameterizedTest @ValueSource( strings = { @@ -1573,33 +1657,68 @@ void shouldRejectUnsupportedCastProjectionForm(String sql, String message) { } /** - * A cast does not make a projected placeholder analyzable, so it stays - * rejected by the placeholder accounting in either spelling. + * A cast projection whose operand is a placeholder types that placeholder. + * No column names it, so it is named after its own index, and the cast + * still names the result column after the projection's alias. */ - @ParameterizedTest - @CsvSource( - delimiter = '|', - quoteCharacter = '"', - value = { - "SELECT $1::int AS x FROM users|[1]", - "SELECT :n::int AS x FROM users|[:n]" - } - ) - void shouldRejectCastPlaceholderProjection(String sql, String compiled) { - Query query = new Query("ListUsers", QueryType.MANY, sql); - ParsedSql parsedSql = parser.parse(sql); + @Test + void shouldResolveCastPlaceholderProjection() { + String sql = "SELECT $1::text AS label, id FROM users WHERE id = $2"; - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> analyzer.analyze(query, parsedSql, schema) + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema ); assertEquals( - "SQL placeholders %s are not the analyzed parameters []; " - .formatted(compiled) - + "a placeholder is in an unsupported location", - exception.getMessage() + List.of( + new QueryColumn("label", ColumnType.TEXT, true), + new QueryColumn("id", ColumnType.BIGINT, false) + ), + model.columns() + ); + + assertEquals( + List.of( + new QueryParameter(1, "param1", ColumnType.TEXT), + new QueryParameter(2, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + + assertEquals( + "SELECT ?::text AS label, id FROM users WHERE id = ?", + model.executableSql() + ); + } + + /** + * A named placeholder is numbered by its first textual occurrence, so a + * projected one is the query's first logical parameter even though the + * {@code WHERE} clause is analyzed before the projection's placeholders. + */ + @Test + void shouldNumberNamedProjectionPlaceholderBeforeThePredicateOne() { + String sql = "SELECT :label::text AS label, id FROM users WHERE id = :id"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "label", ColumnType.TEXT), + new QueryParameter(2, "id", ColumnType.BIGINT) + ), + model.parameters() ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); } @ParameterizedTest @@ -3437,13 +3556,247 @@ void shouldRejectCastOccurrenceWithConflictingType() { } /** - * A cast does not make a placeholder analyzable in a clause this analyzer - * does not traverse, so such a placeholder stays rejected by the - * placeholder accounting. + * A cast types a placeholder in a clause this analyzer does not otherwise + * traverse, which is how a dynamic sort states its type. The sort + * placeholder is repeated, so it is one logical parameter bound at both of + * its positions, after the predicate parameter. + */ + @Test + void shouldResolveCastPlaceholdersOfOrderBy() { + String sql = """ + SELECT id, name + FROM users + WHERE id = :projectId + ORDER BY + CASE WHEN :sort::text = 'name' THEN name END, + CASE WHEN :sort::text = 'id' THEN id END"""; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "projectId", ColumnType.BIGINT), + new QueryParameter(2, "sort", ColumnType.TEXT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2, 2), model.bindingParameterIndexes()); + + assertEquals( + """ + SELECT id, name + FROM users + WHERE id = ? + ORDER BY + CASE WHEN ?::text = 'name' THEN name END, + CASE WHEN ?::text = 'id' THEN id END""", + model.executableSql() + ); + } + + /** + * An indexed cast placeholder outside the clauses that name a parameter is + * named after its own index, in the {@code ::} and the {@code CAST} form + * alike. + */ + @ParameterizedTest + @ValueSource( + strings = { + "SELECT id FROM users WHERE id = $1 ORDER BY $2::int", + "SELECT id FROM users WHERE id = $1 ORDER BY CAST($2 AS int)" + } + ) + void shouldNameIndexedCastPlaceholderOfOrderByAfterItsIndex(String sql) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "id", ColumnType.BIGINT), + new QueryParameter(2, "param2", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + } + + /** + * A cast types a placeholder in every other clause of a read: the grouping + * list, the group predicate, and a computed pagination value, which is not + * the whole row count and is therefore named after its own placeholder. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + quoteCharacter = '"', + value = { + "SELECT name FROM users GROUP BY name, :sort::text|sort|TEXT", + "SELECT name FROM users GROUP BY name HAVING COUNT(*) > :minimum::bigint" + + "|minimum|BIGINT", + "SELECT id FROM users ORDER BY id LIMIT LEAST(:pageSize::int, 100)" + + "|pageSize|INTEGER", + "SELECT id FROM users ORDER BY id FETCH FIRST :pageSize::int ROWS ONLY" + + "|pageSize|INTEGER", + "SELECT id FROM users ORDER BY id LIMIT LEAST($1::int, 100)|param1|INTEGER", + "SELECT id FROM users ORDER BY id FETCH FIRST $1::int ROWS ONLY|param1|INTEGER" + } + ) + void shouldResolveCastPlaceholderOfEveryReadClause(String sql, String name, ColumnType type) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, name, type)), + model.parameters() + ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); + } + + /** + * An indexed cast placeholder of a clause that names no parameter is named + * after its own index wherever it stands. + */ + @Test + void shouldNameIndexedCastPlaceholderOfGroupPredicateAfterItsIndex() { + String sql = "SELECT name, COUNT(*) AS total FROM users GROUP BY name HAVING COUNT(*) > $1::bigint"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, "param1", ColumnType.BIGINT)), + model.parameters() + ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); + } + + /** + * A cast types a placeholder inside a subquery at any depth, in a read and + * in a write, even though the subquery itself is not analyzed. Such a + * placeholder is named after its own placeholder, because no analyzed + * column names it. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + quoteCharacter = '"', + value = { + "SELECT id FROM users" + + " WHERE EXISTS (SELECT 1 FROM orders WHERE orders.total = :amount::numeric)" + + "|amount", + "SELECT id FROM users" + + " WHERE id IN (SELECT user_id FROM orders WHERE orders.total = :amount::numeric)" + + "|amount", + "SELECT id FROM users WHERE id =" + + " (SELECT MAX(user_id) FROM orders WHERE orders.total = :amount::numeric)" + + "|amount", + "SELECT id FROM users WHERE EXISTS (" + + "SELECT 1 FROM orders" + + " WHERE EXISTS (SELECT 1 FROM profiles WHERE profiles.nickname = :amount::text))" + + "|amount", + "DELETE FROM users" + + " WHERE id IN (SELECT user_id FROM orders WHERE orders.total = :amount::numeric)" + + "|amount" + } + ) + void shouldResolveCastPlaceholderOfSubquery(String sql, String name) { + QueryType type = sql.startsWith("DELETE") + ? QueryType.EXEC + : QueryType.MANY; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", type, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals(1, model.parameters().size()); + assertEquals(name, model.parameters().get(0).name()); + assertEquals(List.of(1), model.bindingParameterIndexes()); + } + + /** + * A {@code WITH} body is carried unanalyzed, so its cast placeholder is + * typed by that cast alone and binds before the analyzed clauses that + * follow it. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + quoteCharacter = '"', + value = { + "WITH recent AS (SELECT id FROM orders WHERE orders.total = $1::numeric)" + + " SELECT id FROM users WHERE id = $2|MANY", + "WITH recent AS (SELECT id FROM orders WHERE orders.total = $1::numeric)" + + " DELETE FROM users WHERE id = $2|EXEC" + } + ) + void shouldResolveCastPlaceholderOfWithBody(String sql, QueryType type) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", type, sql), + parser.parse(sql), + joinSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "param1", ColumnType.DECIMAL), + new QueryParameter(2, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + } + + /** + * Two occurrences of one placeholder whose casts agree are one logical + * parameter, even in different clauses. + */ + @Test + void shouldShareOneParameterBetweenAgreeingCastsOfDifferentClauses() { + String sql = "SELECT id FROM users WHERE name = :term::text" + + " ORDER BY CASE WHEN :term::text = 'name' THEN name END"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, "term", ColumnType.TEXT)), + model.parameters() + ); + + assertEquals(List.of(1, 1), model.bindingParameterIndexes()); + } + + /** + * Two occurrences of one placeholder whose casts state different Java types + * are rejected like any other conflicting placeholder. */ @Test - void shouldRejectCastPlaceholderInUnanalyzedLocation() { - String sql = "SELECT id FROM users WHERE id = $1 ORDER BY $2::int"; + void shouldRejectConflictingCastsOfDifferentClauses() { + String sql = "SELECT id FROM users WHERE name = :term::text" + + " ORDER BY CASE WHEN :term::int = 1 THEN name END"; Query query = new Query("ListUsers", QueryType.MANY, sql); ParsedSql parsedSql = parser.parse(sql); @@ -3454,12 +3807,66 @@ void shouldRejectCastPlaceholderInUnanalyzedLocation() { ); assertEquals( - "SQL placeholders [1, 2] are not the analyzed parameters [1]; " + "Placeholder :term has conflicting types: String and Integer", + exception.getMessage() + ); + } + + /** + * Only a placeholder a cast states the type of is typed outside the clauses + * that name a parameter, so an uncast placeholder, and a placeholder a cast + * does not cast directly, stay rejected by the placeholder accounting. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + quoteCharacter = '"', + value = { + "SELECT id FROM users ORDER BY $1|[1]", + "SELECT id FROM users ORDER BY CASE WHEN :sort = 'name' THEN name END|[:sort]", + "SELECT id FROM users ORDER BY (:sort)::int|[:sort]", + "SELECT id FROM users WHERE EXISTS (SELECT 1 FROM orders WHERE orders.id = :id)|[:id]" + } + ) + void shouldRejectUncastPlaceholderOutsideTheAnalyzedClauses(String sql, String compiled) { + Query query = new Query("ListUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, joinSchema) + ); + + assertEquals( + "SQL placeholders %s are not the analyzed parameters []; " + .formatted(compiled) + "a placeholder is in an unsupported location", exception.getMessage() ); } + /** + * A cast type sqlcj does not map is rejected naming the placeholder also + * where the placeholder stands outside the clauses that name a parameter. + */ + @Test + void shouldRejectUnsupportedCastTypeOutsideTheAnalyzedClauses() { + String sql = "SELECT id FROM users ORDER BY $1::interval"; + + Query query = new Query("ListUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, schema) + ); + + assertEquals( + "Placeholder $1 has unsupported cast type INTERVAL", + exception.getMessage() + ); + } + @Test void shouldAnalyzeSelectDeclaredAsOptional() { String sql = "SELECT id, name FROM users WHERE id = $1"; 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 44e03da..b680520 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java @@ -149,6 +149,24 @@ created_at TIMESTAMP(3) WITHOUT TIME ZONE, ); """; + /** + * A snapshot of a membership relation, whose rows a read reaches only + * through an {@code EXISTS} subquery. + */ + private static final String MEMBERSHIP_SCHEMA = """ + CREATE TABLE teams + ( + id BIGINT PRIMARY KEY, + name VARCHAR(255) NOT NULL + ); + + CREATE TABLE memberships + ( + team_id BIGINT NOT NULL, + role VARCHAR(32) NOT NULL + ); + """; + /** * A snapshot that declares every accepted floating-point, binary, and time * spelling, each of them nullable so that one row can carry values and @@ -550,6 +568,8 @@ void resetDatabase() throws Exception { execute("DROP FUNCTION IF EXISTS greet_listener(text)"); execute("DROP TABLE IF EXISTS customer_orders"); execute("DROP TABLE IF EXISTS customers"); + execute("DROP TABLE IF EXISTS memberships"); + execute("DROP TABLE IF EXISTS teams"); execute("DROP TABLE IF EXISTS measurements"); execute("DROP TABLE IF EXISTS documents"); execute("DROP TABLE IF EXISTS user_aliases"); @@ -989,6 +1009,162 @@ INSERT INTO users (id, code, name) } } + /** + * Covers a dynamic sort end to end: one cast placeholder repeated in two + * {@code ORDER BY} keys is generated, compiled, and executed against + * PostgreSQL, so each of its two values returns the rows in its own order. + */ + @Test + void shouldExecuteGeneratedDynamicSortAgainstPostgres() throws Exception { + Path classesDirectory = generateAndCompile(""" + -- name: ListUsersSorted :many + SELECT id, name + FROM users + ORDER BY + CASE WHEN :sort::text = 'name' THEN name END, + CASE WHEN :sort::text = 'id' THEN id END; + """); + + execute(""" + INSERT INTO users (id, code, name) + VALUES + (1, 1, 'Cara'), + (2, 2, 'Alice'), + (3, 3, 'Bob') + """); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Object repository = newRepository(classLoader); + + Method listUsersSorted = repository.getClass().getMethod( + "listUsersSorted", + String.class + ); + + assertEquals( + List.of("Alice", "Bob", "Cara"), + names(listUsersSorted.invoke(repository, "name")) + ); + + assertEquals( + List.of("Cara", "Alice", "Bob"), + names(listUsersSorted.invoke(repository, "id")) + ); + } + } + + /** + * Covers a cast page end to end: a {@code LIMIT ... OFFSET ...} page whose + * values are cast placeholders, and the same page written as + * {@code OFFSET ... LIMIT ...}, are generated, compiled, and executed + * against PostgreSQL, so a swapped binding order would change the returned + * page. + */ + @Test + void shouldExecuteGeneratedCastPaginationAgainstPostgres() throws Exception { + Path classesDirectory = generateAndCompile(""" + -- name: ListUserCastPage :many + SELECT id, name + FROM users + ORDER BY id + LIMIT $1::int OFFSET $2::int; + + -- name: ListUserCastPageWithLeadingOffset :many + SELECT id, name + FROM users + ORDER BY id + OFFSET $2::int LIMIT $1::int; + """); + + execute(""" + INSERT INTO users (id, code, name) + VALUES + (1, 1, 'Alice'), + (2, 2, 'Bob'), + (3, 3, 'Cara'), + (4, 4, 'Dora') + """); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Object repository = newRepository(classLoader); + + Method page = repository.getClass().getMethod( + "listUserCastPage", + Integer.class, + Integer.class + ); + + Method pageWithLeadingOffset = repository.getClass().getMethod( + "listUserCastPageWithLeadingOffset", + Integer.class, + Integer.class + ); + + assertEquals( + List.of("Bob", "Cara"), + names(page.invoke(repository, 2, 1)) + ); + + assertEquals( + List.of("Bob", "Cara"), + names(pageWithLeadingOffset.invoke(repository, 2, 1)) + ); + } + } + + /** + * Covers a membership check end to end: a read whose {@code EXISTS} + * subquery compares a cast placeholder is generated, compiled, and executed + * against PostgreSQL, so the placeholder reaches the subquery and selects + * the teams of the given role alone. + */ + @Test + void shouldExecuteGeneratedExistsSubqueryAgainstPostgres() throws Exception { + execute(MEMBERSHIP_SCHEMA); + + Path classesDirectory = generateAndCompile(MEMBERSHIP_SCHEMA, """ + -- name: ListTeamsByRole :many + SELECT id, name + FROM teams + WHERE EXISTS ( + SELECT 1 + FROM memberships + WHERE memberships.team_id = teams.id + AND memberships.role = :role::varchar + ) + ORDER BY id; + """); + + execute(""" + INSERT INTO teams (id, name) + VALUES (1, 'Platform'), (2, 'Billing'), (3, 'Support') + """); + + execute(""" + INSERT INTO memberships (team_id, role) + VALUES (1, 'owner'), (2, 'member'), (3, 'owner') + """); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Object repository = newRepository(classLoader); + + Method listTeamsByRole = repository.getClass().getMethod( + "listTeamsByRole", + String.class + ); + + assertEquals( + List.of("Platform", "Support"), + names(listTeamsByRole.invoke(repository, "owner")) + ); + + assertEquals( + List.of("Billing"), + names(listTeamsByRole.invoke(repository, "member")) + ); + } + } + /** * Covers named placeholders end to end: a read whose placeholders are named * and whose first name repeats is generated, compiled, and executed against 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 8c43f63..dc45072 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java @@ -3146,6 +3146,91 @@ name VARCHAR(255), ); } + /** + * A repository generated from cast placeholders outside the clauses that + * name a parameter compiles: a projected cast, a dynamic sort, a cast page, + * a group predicate, and a subquery each contribute one typed method + * parameter, in the logical order of their placeholders. + */ + @Test + void shouldGenerateCompilableRepositoryForCastPlaceholdersOfEveryClause() throws IOException { + generateAndCompile( + """ + CREATE TABLE users + ( + id BIGINT NOT NULL, + name VARCHAR(255) + ); + + CREATE TABLE orders + ( + id BIGINT NOT NULL, + user_id BIGINT NOT NULL, + total DECIMAL(10, 2) + ); + """, + """ + -- name: ListUsers :many + SELECT :label::text AS label, id, name + FROM users + WHERE id = :id + ORDER BY CASE WHEN :sort::text = 'name' THEN name END + LIMIT :pageSize::int OFFSET :skip::int; + + -- name: CountUsersByName :many + SELECT name, COUNT(*) AS total + FROM users + GROUP BY name + HAVING COUNT(*) > :minimum::bigint; + + -- name: ListBuyers :many + SELECT id, name + FROM users + WHERE EXISTS ( + SELECT 1 + FROM orders + WHERE orders.user_id = users.id + AND orders.total = :total::numeric + ) + ORDER BY id; + """ + ); + + String repository = Files.readString( + tempDir + .resolve("generated") + .resolve("generated") + .resolve("UsersRepository.java") + ); + + assertTrue( + repository.contains( + "public List listUsers(" + + "String label, Long id, String sort, Integer pageSize, Integer skip)" + ), + repository + ); + + assertEquals( + List.of("String label", "Long id", "String name"), + recordComponents(repository, "ListUsersResult") + ); + + assertTrue( + repository.contains( + "public List countUsersByName(Long minimum)" + ), + repository + ); + + assertTrue( + repository.contains( + "public List listBuyers(BigDecimal total)" + ), + repository + ); + } + /** * 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/java/dev/sqlcj/sql/SqlParameterCompilerTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParameterCompilerTest.java index 0038fde..2e31d6d 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParameterCompilerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParameterCompilerTest.java @@ -9,6 +9,7 @@ import net.sf.jsqlparser.statement.create.table.ColDataType; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -202,7 +203,7 @@ void shouldNotReportCompiledNamedParameterThatIsACastOperand() { } private SqlParameters compile(String sql, Token... tokens) { - return compiler.compile(sql, node(tokens)); + return compiler.compile(sql, node(tokens), new ArrayList<>()); } /** @@ -221,7 +222,7 @@ private SqlParameters compile(String sql, Token[] tokens, Expression... expressi root.jjtAddChild(child, index); } - return compiler.compile(sql, root); + return compiler.compile(sql, root, new ArrayList<>()); } private Node node(Token... tokens) { diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java index 89ac9a1..2ffebce 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java @@ -1,5 +1,6 @@ package dev.sqlcj.sql; +import net.sf.jsqlparser.expression.JdbcNamedParameter; import net.sf.jsqlparser.statement.select.Select; import org.junit.jupiter.api.Test; @@ -323,6 +324,68 @@ void shouldReportLexicalFailureWithoutExceptionClassNames() { assertFalse(exception.getMessage().contains("net.sf.jsqlparser")); } + /** + * One occurrence is reported per placeholder the source spells, in textual + * order at every clause and nesting depth, each with the cast whose direct + * operand it is. + */ + @Test + void shouldReportPlaceholderOccurrencesInTextualOrder() { + ParsedSql parsedSql = parser.parse(""" + WITH recent AS (SELECT id FROM orders WHERE total = :amount::numeric) + SELECT :label::text AS label, id + FROM users + WHERE id = :id + AND EXISTS (SELECT 1 FROM tasks WHERE tasks.name = CAST(:task AS text)) + ORDER BY id + OFFSET :skip::int + LIMIT :pageSize::int"""); + + assertEquals( + List.of("amount", "label", "id", "task", "skip", "pageSize"), + placeholderNames(parsedSql) + ); + + assertEquals( + List.of("numeric", "text", "none", "text", "int", "int"), + castTypes(parsedSql) + ); + } + + /** + * Every occurrence of one placeholder is reported, and only a placeholder a + * cast casts directly carries a cast, which for a chain of casts is the + * cast that applies first. + */ + @Test + void shouldReportTheCastThatCastsAPlaceholderDirectly() { + ParsedSql parsedSql = parser.parse( + "SELECT id FROM users WHERE id = :a::text::int AND id = (:b)::int AND id = :a" + ); + + assertEquals(List.of("a", "b", "a"), placeholderNames(parsedSql)); + assertEquals(List.of("text", "none", "none"), castTypes(parsedSql)); + assertEquals(List.of(1, 2, 1), parsedSql.parameters().indexes()); + } + + private List placeholderNames(ParsedSql parsedSql) { + return parsedSql.placeholders().stream() + .map(occurrence -> ((JdbcNamedParameter) occurrence.placeholder()).getName()) + .toList(); + } + + /** The declared cast type of each occurrence, and {@code none} for none. */ + private List castTypes(ParsedSql parsedSql) { + return parsedSql.placeholders().stream() + .map(PlaceholderOccurrence::cast) + .map( + cast -> cast == null + ? "none" + : cast.getColDataType().getDataType() + ) + .toList(); + } + /** A block comment nests as in PostgreSQL and ends at its last delimiter. */ @Test void shouldPreserveParameterTextInsideNestedBlockComment() {