diff --git a/docs/queries.md b/docs/queries.md index 4478141..58f601d 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -171,8 +171,8 @@ after `code` and disambiguated as `code1` and `code2`. The bounds are bound in textual order, the start bound before the end bound, whatever their placeholder indexes are. A placeholder bound beside a literal bound, as in `code BETWEEN $1 AND 10`, is typed the same way and binds one parameter. A named -bound such as `code BETWEEN :lo AND :hi` is rejected like any other named -placeholder, while a placeholder as the tested value, as in +bound such as `code BETWEEN :lo AND :hi` is typed the same way and named after +its placeholder, while a placeholder as the tested value, as in `$1 BETWEEN code AND code`, and a placeholder inside a computed bound, as in `code BETWEEN $1 + 1 AND $2`, are rejected as unanalyzed placeholder locations. @@ -213,8 +213,10 @@ LIMIT $1 OFFSET $2; parameter. - Both values must be non-negative. sqlcj generates no validation, so PostgreSQL rejects a negative value when the query executes. -- A named value such as `LIMIT :n` or `OFFSET :n` is rejected like any other - named placeholder. A placeholder in a computed value, as in `LIMIT $1 + 1` or +- 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. @@ -225,8 +227,8 @@ A write targets exactly one table of its entry's schema. | Statement | Accepted shape | | --- | --- | -| `INSERT` | an explicit column list and a single `VALUES` row whose values are all `$N` placeholders | -| `UPDATE` | `SET` assignments that each assign one direct column a single `$N` placeholder, with an optional `WHERE` using the read predicate forms | +| `INSERT` | an explicit column list and a single `VALUES` row whose values are all placeholders | +| `UPDATE` | `SET` assignments that each assign one direct column a single placeholder, with an optional `WHERE` using the read predicate forms | | `DELETE` | one target table with an optional `WHERE` using the read predicate forms | ```sql @@ -268,24 +270,40 @@ table expressions are rejected in a returning write. ## Parameters -sqlcj uses PostgreSQL `$N` placeholders. The compiler replaces each real -placeholder token with a JDBC `?` and leaves every other character of the -statement byte-for-byte unchanged, so `$1` inside a string literal, a quoted -identifier, or a comment is not a parameter. +A query writes its parameters either as PostgreSQL `$N` placeholders or as +`:name` placeholders, and one query uses one of the two forms. The compiler +replaces each real placeholder token with a JDBC `?` and leaves every other +character of the statement byte-for-byte unchanged, so `$1` or `:id` inside a +string literal, a quoted identifier, or a comment is not a parameter. - Placeholder indexes must be positive and contiguous from `$1`. +- A named placeholder is a colon followed directly by an unquoted name of ASCII + letters, digits, and underscores that does not start with a digit, such as + `:userId`. Names are compared exactly as written, so `:term` and `:Term` are + two parameters, and a name spelled like a SQL keyword, such as `:limit`, + `:user`, or `:year`, is an ordinary name. +- Each distinct name is one parameter, numbered by its first textual + occurrence, and every occurrence of that name is bound at its own `?` + position. +- A named placeholder is accepted wherever a `$N` placeholder is, and names its + generated method parameter after itself, so `LIMIT :pageSize` generates + `pageSize` rather than `limit`. - The Java type of a placeholder is the type of the column it is compared with, assigned to, or inserted into. -- Anonymous `?` placeholders and named `:name` placeholders are rejected. +- 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` - — is rejected rather than left unbound. + or `lower(name) = :name` — is rejected rather than left unbound. ### Logical order versus textual order The two orders are distinct and both are observable: - **Logical order** is placeholder index order. It is the order of the generated - method parameters: `$1` is the first method parameter, `$2` the second. + method parameters: `$1` is the first method parameter, `$2` the second. In a + named query it is first-occurrence order: the name written first is the first + method parameter. - **Textual order** is the order in which placeholder tokens appear in the SQL. It is the JDBC binding order of the generated `?` positions. @@ -308,11 +326,24 @@ public int updateAuthorBio(Long id, String bio) { } ``` -An index may repeat. A repeated index produces one method parameter, named and -typed from its first occurrence, and its value is bound at every textual -position where the index occurs. Occurrences of one index whose inferred Java +An index and a name may repeat. A repeated index produces one method parameter, +named and typed from its first occurrence, and its value is bound at every +textual position where the index occurs; a repeated name behaves the same way +and keeps its own name. Occurrences of one index or one name whose inferred Java types differ are rejected. +`UpdateAuthorBio` written with named placeholders states the same two orders: + +```sql +-- name: UpdateAuthorBio :exec +UPDATE authors +SET bio = :bio +WHERE id = :id; +``` + +The generated method takes `(bio, id)`, because `:bio` occurs first, and binds +`(bio, id)`. + ## Generated Java Each configuration entry generates one final repository class in the configured @@ -531,7 +562,9 @@ name, and its header line. 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 - computed `LIKE` pattern such as `'%' || $1 || '%'`, a placeholder as the + named placeholder under a function such as `lower(name) = :name` or under a + cast such as `:name::text`, a computed `LIKE` pattern such as + `'%' || $1 || '%'`, a placeholder as the tested value of a range such as `$1 BETWEEN id AND id`, a computed range bound such as `id BETWEEN $1 + 1 AND $2`, a computed pagination value such as `LIMIT $1 + 1` or `OFFSET $1 + 1`, a `LIMIT a, b` row count such as @@ -540,11 +573,10 @@ name, and its header line. - A `LIKE`-family pattern placeholder that is negated, uses another keyword such as `SIMILAR TO`, carries an `ESCAPE` clause or a `BINARY` modifier, tests a non-text column, or stands as the tested value. -- A named range bound such as `id BETWEEN :lo AND :hi` and a named pagination - value such as `LIMIT :n` or `OFFSET :n`, which fail with the named-placeholder - diagnostic. -- Anonymous `?` and named `:name` placeholders, and non-contiguous or - non-positive placeholder indexes. +- Anonymous `?` placeholders, `$N` and `:name` placeholders mixed in one query, + a qualified name such as `:a.b`, a quoted name such as `:"x"`, an `&name` + placeholder, one name whose occurrences have conflicting types, and + non-contiguous or non-positive placeholder indexes. - An `INSERT` without an explicit column list, with more than one `VALUES` row, with a value that is not a placeholder, or built from a `SELECT`; and an `UPDATE` assignment that is not a single placeholder. @@ -569,9 +601,9 @@ them: - `ON CONFLICT`, `UPDATE ... FROM`, and `DELETE ... USING` on a non-returning `:exec` write. -sqlcj itself provides no named parameters, macros, array operators such as -`= ANY`, dynamic `IN` expansion, or query-building API. An array parameter is -one whole list bound at one placeholder, not a placeholder list. +sqlcj itself provides no macros, array operators such as `= ANY`, dynamic `IN` +expansion, or query-building API. An array parameter is one whole list bound at +one placeholder, not a placeholder list. Unsupported schema input and unsupported column types are listed in [PostgreSQL Support](postgresql.md#unsupported-types-and-ddl). @@ -659,8 +691,8 @@ Reads: `QueryAnalyzerTest.shouldResolveRangeBoundParameterBesideLiteralBound`, `QueryAnalyzerTest.shouldNotCreateParameterForLiteralRange`, `QueryAnalyzerTest.shouldRejectUnknownColumnInRangePredicate`, and - `QueryAnalyzerTest.shouldRejectNamedRangeBound` cover the range predicates, - and + `QueryAnalyzerTest.shouldResolveNamedRangeBoundParameters` cover the range + predicates, and `PostgresIntegrationTest.shouldExecuteGeneratedRangePredicatesAgainstPostgres` executes a `BETWEEN` read and a `NOT BETWEEN` read whose bounds use out-of-order placeholder indexes against PostgreSQL 16. @@ -668,7 +700,7 @@ Reads: `QueryAnalyzerTest.shouldResolvePaginationParametersInTextualBindingOrder`, `QueryAnalyzerTest.shouldResolveOffsetParameterBesideUnanalyzedRowCount`, `QueryAnalyzerTest.shouldNotCreateParametersForLiteralPagination`, - `QueryAnalyzerTest.shouldRejectNamedPaginationValue`, and + `QueryAnalyzerTest.shouldNameNamedPaginationParameterAfterItsPlaceholder`, and `QueryAnalyzerTest.shouldRejectPlaceholderInUnsupportedPaginationValue` cover pagination, and `PostgresIntegrationTest.shouldExecuteGeneratedPaginationAgainstPostgres` @@ -731,10 +763,36 @@ Parameters: `QueryAnalyzerTest.shouldRejectGappedParameterIndexes`, `QueryAnalyzerTest.shouldRejectZeroParameterIndex`, `QueryAnalyzerTest.shouldRejectPlaceholderInUnsupportedLocation`, - `QueryAnalyzerTest.shouldRejectAnonymousParameter`, - `QueryAnalyzerTest.shouldRejectNamedParameter`, and + `QueryAnalyzerTest.shouldRejectAnonymousParameter`, and `QueryAnalyzerTest.shouldKeepPlaceholderTextThatIsNotAParameter` cover ordering, repetition, and rejection. +- `SqlParserTest.shouldCompileNamedParametersByFirstOccurrence`, + `SqlParserTest.shouldCompileNamesThatDifferInCaseAsDistinctParameters`, + `SqlParserTest.shouldReportBothPlaceholderFormsOfOneSource`, + `SqlParserTest.shouldNotCompileUnsupportedNamedPlaceholderForms`, + `SqlParserTest.shouldPreserveNamedPlaceholderTextThatIsNotAParameter`, + `SqlParameterCompilerTest.shouldReplaceReportedNamedParameterSpans`, + `SqlParameterCompilerTest.shouldIgnoreNameSeparatedFromItsColon`, and + `SqlParameterCompilerTest.shouldRejectNamedSpanThatDoesNotHoldTheParameterImage` + cover the compiled named form, its numbering, and its span guards. +- `QueryAnalyzerTest.shouldResolveNamedParameter`, + `QueryAnalyzerTest.shouldNumberNamedParametersByFirstOccurrence`, + `QueryAnalyzerTest.shouldResolveNamedParametersOfUpdate`, + `QueryAnalyzerTest.shouldResolveNamedParametersOfInsert`, + `QueryAnalyzerTest.shouldResolveNamedParametersInInList`, + `QueryAnalyzerTest.shouldResolveNamedLikePatternParameter`, + `QueryAnalyzerTest.shouldResolveNamedParametersSpelledLikeKeywords`, + `QueryAnalyzerTest.shouldRejectMixedPlaceholderForms`, + `QueryAnalyzerTest.shouldRejectUnsupportedNamedPlaceholderForm`, + `QueryAnalyzerTest.shouldRejectNamedPlaceholderInUnanalyzedLocation`, and + `QueryAnalyzerTest.shouldRejectNamedPlaceholderWithConflictingTypes` cover the + analyzed named locations, the generated parameter names, and the named + rejections. +- `SqlcjCompilerIntegrationTest.shouldExecuteGeneratedQueryWithNamedPlaceholders` + and + `PostgresIntegrationTest.shouldExecuteGeneratedNamedPlaceholdersAgainstPostgres` + compile and execute a named query that repeats a name and orders its + parameters by first occurrence. - `SqlcjCompilerIntegrationTest.shouldExecuteGeneratedQueryWithOutOfOrderPlaceholders`, `SqlcjCompilerIntegrationTest.shouldExecuteGeneratedQueryWithRepeatedPlaceholder`, `SqlcjCompilerIntegrationTest.shouldExecuteGeneratedUpdateWithOutOfOrderPlaceholders`, 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 11898d7..5e33ade 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java @@ -53,6 +53,21 @@ public final class QueryAnalyzer { private static final String ANONYMOUS_PARAMETER_REJECTION = """ Anonymous '?' parameters are not supported; use an indexed placeholder such as $1"""; + /** + * The rejection of a placeholder written with a colon or ampersand that is + * not a supported named placeholder, which is written with the placeholder + * as the query spells it. + */ + private static final String NAMED_PLACEHOLDER_REJECTION = """ + Named placeholder '%s' is not supported; write an unquoted name of letters, digits, and \ + underscores directly after ':', such as :userId"""; + + private static final String MIXED_PLACEHOLDER_REJECTION = """ + Indexed '$N' and named ':name' placeholders must not be mixed in one query"""; + + /** The character that opens a supported named placeholder. */ + private static final String NAME_PREFIX = ":"; + /** The parameter name of a {@code LIMIT} row count placeholder. */ private static final String LIMIT_PARAMETER_NAME = "limit"; @@ -78,8 +93,90 @@ private record Source(String name, dev.sqlcj.schema.Table table, boolean leftJoi 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. + */ + private record Placeholder(int index, String name) { + + /** + * The name of the parameter this occurrence binds, which a named + * placeholder states itself and every other placeholder takes from the + * column or clause it belongs to. + */ + private String nameOr(String clauseName) { + return name == null + ? clauseName + : name; + } + } + + /** + * The compiled placeholders of one query and the parameter occurrences + * analyzed against them, collected in the textual order of the executable + * {@code ?} positions. + */ + private static final class Placeholders { + + /** The compiled placeholder names in logical parameter order. */ + private final List names; + + private final List occurrences = new ArrayList<>(); + + private Placeholders(List names) { + this.names = names; + } + + /** + * Reports the placeholder an expression is, and {@code null} when the + * expression is absent or binds none. A named placeholder the compiler + * did not replace is rejected as the query spells it, which keeps every + * accepted occurrence bound even though such a placeholder is normally + * already rejected from the compiler's report. + */ + private Placeholder of(Expression expression) { + if (expression instanceof JdbcParameter parameter) { + if (!parameter.isUseFixedIndex()) { + throw new UnsupportedOperationException(ANONYMOUS_PARAMETER_REJECTION); + } + + return new Placeholder(parameter.getIndex(), null); + } + + if (expression instanceof JdbcNamedParameter named) { + return new Placeholder(requireCompiledName(named), named.getName()); + } + + return null; + } + + private int requireCompiledName(JdbcNamedParameter named) { + int index = NAME_PREFIX.equals(named.getParameterCharacter()) + ? names.indexOf(named.getName()) + : -1; + + if (index < 0) { + throw new UnsupportedOperationException( + NAMED_PLACEHOLDER_REJECTION.formatted( + named.getParameterCharacter() + named.getName() + ) + ); + } + + return index + 1; + } + + private void add(QueryParameter occurrence) { + occurrences.add(occurrence); + } + + private List occurrences() { + return occurrences; + } + } + public QueryModel analyze(Query query, ParsedSql parsedSql, Schema schema) { - requireIndexedPlaceholders(parsedSql); + requireSupportedPlaceholders(parsedSql); Statement statement = parsedSql.statement(); @@ -114,14 +211,16 @@ private QueryModel analyzeSelect(Query query, ParsedSql parsedSql, Select select List columns = resolveColumns(plainSelect, sources); - List bindingParameters = resolveBindingParameters(plainSelect, sources); + Placeholders placeholders = toPlaceholders(parsedSql); + + resolveBindingParameters(plainSelect, sources, placeholders); return toQueryModel( query, parsedSql, table.getUnquotedName(), columns, - bindingParameters, + placeholders, resolveSelectRowTable(plainSelect, sources) ); } @@ -144,12 +243,16 @@ private QueryModel analyzeInsert(Query query, ParsedSql parsedSql, Insert insert rowTable = resolveReturningRowTable(returningClause, source); } + Placeholders placeholders = toPlaceholders(parsedSql); + + resolveInsertParameters(insert, source.table(), placeholders); + return toQueryModel( query, parsedSql, table.getUnquotedName(), columns, - resolveInsertParameters(insert, source.table()), + placeholders, rowTable ); } @@ -172,10 +275,12 @@ private QueryModel analyzeUpdate(Query query, ParsedSql parsedSql, Update update rowTable = resolveReturningRowTable(returningClause, source); } - List bindingParameters = resolveUpdateSetParameters(update, source.table()); + Placeholders placeholders = toPlaceholders(parsedSql); + + resolveUpdateSetParameters(update, source.table(), placeholders); if (update.getWhere() != null) { - resolveParameters(update.getWhere(), List.of(source), bindingParameters); + resolveParameters(update.getWhere(), List.of(source), placeholders); } return toQueryModel( @@ -183,7 +288,7 @@ private QueryModel analyzeUpdate(Query query, ParsedSql parsedSql, Update update parsedSql, table.getUnquotedName(), columns, - bindingParameters, + placeholders, rowTable ); } @@ -206,10 +311,10 @@ private QueryModel analyzeDelete(Query query, ParsedSql parsedSql, Delete delete rowTable = resolveReturningRowTable(returningClause, source); } - List bindingParameters = new ArrayList<>(); + Placeholders placeholders = toPlaceholders(parsedSql); if (delete.getWhere() != null) { - resolveParameters(delete.getWhere(), List.of(source), bindingParameters); + resolveParameters(delete.getWhere(), List.of(source), placeholders); } return toQueryModel( @@ -217,7 +322,7 @@ private QueryModel analyzeDelete(Query query, ParsedSql parsedSql, Delete delete parsedSql, table.getUnquotedName(), columns, - bindingParameters, + placeholders, rowTable ); } @@ -408,7 +513,11 @@ private void requireNoCommonTableExpressions(List withItems) { * Resolves the {@code INSERT} parameters by pairing the explicit column * list with the single values row, which is also their textual order. */ - private List resolveInsertParameters(Insert insert, dev.sqlcj.schema.Table table) { + private void resolveInsertParameters( + Insert insert, + dev.sqlcj.schema.Table table, + Placeholders placeholders + ) { ExpressionList columns = insert.getColumns(); if (columns == null || columns.isEmpty()) { @@ -423,24 +532,22 @@ private List resolveInsertParameters(Insert insert, dev.sqlcj.sc ); } - List parameters = new ArrayList<>(); - for (int index = 0; index < columns.size(); index++) { - if (!(values.get(index) instanceof JdbcParameter parameter)) { + Placeholder placeholder = placeholders.of(values.get(index)); + + if (placeholder == null) { throw new UnsupportedOperationException( "INSERT values must be indexed placeholders." ); } addParameter( - parameter, + placeholder, columns.get(index).getUnquotedColumnName(), table, - parameters + placeholders ); } - - return parameters; } private ParenthesedExpressionList resolveInsertValues(Insert insert) { @@ -459,29 +566,30 @@ private ParenthesedExpressionList resolveInsertValues(Insert insert) { * Resolves the {@code UPDATE} assignment parameters in source order, which * precedes any parameter in the {@code WHERE} expression. */ - private List resolveUpdateSetParameters(Update update, dev.sqlcj.schema.Table table) { - List parameters = new ArrayList<>(); - + private void resolveUpdateSetParameters( + Update update, + dev.sqlcj.schema.Table table, + Placeholders placeholders + ) { for (UpdateSet updateSet : update.getUpdateSets()) { - if ( - updateSet.getColumns().size() != 1 - || updateSet.getValues().size() != 1 - || !(updateSet.getValue(0) instanceof JdbcParameter parameter) - ) { + Placeholder placeholder = updateSet.getColumns().size() == 1 + && updateSet.getValues().size() == 1 + ? placeholders.of(updateSet.getValue(0)) + : null; + + if (placeholder == null) { throw new UnsupportedOperationException( "UPDATE assignments must set one column to an indexed placeholder." ); } addParameter( - parameter, + placeholder, updateSet.getColumn(0).getUnquotedColumnName(), table, - parameters + placeholders ); } - - return parameters; } /** @@ -494,9 +602,11 @@ private QueryModel toQueryModel( ParsedSql parsedSql, String tableName, List columns, - List occurrences, + Placeholders placeholders, String rowTable ) { + List occurrences = placeholders.occurrences(); + List bindingParameterIndexes = requireAccountedOccurrences(parsedSql, occurrences); return new QueryModel( @@ -506,7 +616,7 @@ private QueryModel toQueryModel( parsedSql.parameters().executableSql(), bindingParameterIndexes, columns, - toParameters(occurrences), + toParameters(occurrences, parsedSql.parameters().names()), rowTable ); } @@ -521,31 +631,57 @@ private List requireAccountedOccurrences(ParsedSql parsedSql, List placeholders = parsedSql.parameters().indexes(); + List compiled = parsedSql.parameters().indexes(); + List names = parsedSql.parameters().names(); - if (!analyzed.equals(placeholders)) { + if (!analyzed.equals(compiled)) { throw new UnsupportedOperationException( "SQL placeholders %s are not the analyzed parameters %s; a placeholder is in an unsupported location" - .formatted(placeholders, analyzed) + .formatted( + describePlaceholders(compiled, names), + describePlaceholders(analyzed, names) + ) ); } return analyzed; } + /** + * Writes a list of logical parameter numbers as the query spells its + * placeholders, which is {@code :name} for a named query and the number + * itself for an indexed one. + */ + private String describePlaceholders(List indexes, List names) { + if (names.isEmpty()) { + return indexes.toString(); + } + + return indexes.stream() + .map(index -> describePlaceholder(index, names)) + .collect(Collectors.joining(", ", "[", "]")); + } + + /** Writes one logical parameter as the query spells its placeholder. */ + private String describePlaceholder(int index, List names) { + return names.isEmpty() + ? "$" + index + : NAME_PREFIX + names.get(index - 1); + } + /** * Retains one parameter per placeholder index in logical index order. A * repeated index keeps the name and type of its first occurrence and is * accepted only when every occurrence resolves to the same Java type. */ - private List toParameters(List occurrences) { + private List toParameters(List occurrences, List names) { Map parametersByIndex = new LinkedHashMap<>(); for (QueryParameter occurrence : occurrences) { QueryParameter parameter = parametersByIndex.putIfAbsent(occurrence.index(), occurrence); if (parameter != null) { - requireSameParameterType(parameter, occurrence); + requireSameParameterType(parameter, occurrence, names); } } @@ -566,11 +702,26 @@ private List toParameters(List occurrences) { * occurrence is a list of its element's Java type, so it is never the same * type as an occurrence of that element. */ - private void requireSameParameterType(QueryParameter parameter, QueryParameter occurrence) { + private void requireSameParameterType( + QueryParameter parameter, + QueryParameter occurrence, + List names + ) { if (isSameParameterType(parameter, occurrence)) { return; } + if (!names.isEmpty()) { + throw new UnsupportedOperationException( + "Placeholder %s has conflicting types: %s and %s" + .formatted( + describePlaceholder(parameter.index(), names), + describeParameterType(parameter), + describeParameterType(occurrence) + ) + ); + } + throw new UnsupportedOperationException( "Placeholder $%d has conflicting types: %s from '%s' and %s from '%s'" .formatted( @@ -800,54 +951,61 @@ private Source resolveJoinConditionSource(Expression expression, List so * encountered, which is the JDBC binding order of the generated {@code ?} * positions. */ - private List resolveBindingParameters(PlainSelect plainSelect, List sources) { - List parameters = new ArrayList<>(); - + private void resolveBindingParameters( + PlainSelect plainSelect, + List sources, + Placeholders placeholders + ) { if (plainSelect.getWhere() != null) { resolveParameters( plainSelect.getWhere(), sources, - parameters + placeholders ); } - resolvePaginationParameters(plainSelect, parameters); - - return parameters; + resolvePaginationParameters(plainSelect, placeholders); } /** * 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. The parser stores + * {@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. */ - private void resolvePaginationParameters(PlainSelect plainSelect, List parameters) { - JdbcParameter rowCount = resolvePaginationPlaceholder( - resolveLimitRowCount(plainSelect.getLimit()) - ); + private void resolvePaginationParameters(PlainSelect plainSelect, Placeholders placeholders) { + Expression rowCountValue = resolveLimitRowCount(plainSelect.getLimit()); + + Placeholder rowCount = placeholders.of(rowCountValue); Offset offset = plainSelect.getOffset(); - JdbcParameter offsetValue = resolvePaginationPlaceholder( - offset == null ? null : offset.getOffset() - ); + Expression offsetExpression = offset == null + ? null + : offset.getOffset(); + + Placeholder offsetValue = placeholders.of(offsetExpression); - if (rowCount != null && offsetValue != null && sourcePosition(offsetValue) < sourcePosition(rowCount)) { - addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, parameters); - addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, parameters); + 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); return; } if (rowCount != null) { - addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, parameters); + addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, placeholders); } if (offsetValue != null) { - addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, parameters); + addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, placeholders); } } @@ -862,43 +1020,25 @@ private Expression resolveLimitRowCount(Limit limit) { : limit.getRowCount(); } - /** - * Rejects a named pagination value and reports the placeholder to bind, - * which is absent when the value binds none. A value such as a literal or - * {@code ALL} reaches the database as written, while a computed value keeps - * its placeholder unaccounted for the placeholder accounting check. - */ - private JdbcParameter resolvePaginationPlaceholder(Expression value) { - if (value == null) { - return null; - } - - requireIndexedParameter(value); - - return value instanceof JdbcParameter parameter - ? parameter - : null; - } - /** The source position of a placeholder token, counted from one. */ - private int sourcePosition(JdbcParameter parameter) { - return parameter.getASTNode().jjtGetFirstToken().absoluteBegin; + private int sourcePosition(Expression placeholder) { + return placeholder.getASTNode().jjtGetFirstToken().absoluteBegin; } private void resolveParameters( Expression expression, List sources, - List parameters + Placeholders placeholders ) { if (expression instanceof AndExpression and) { - resolveParameters(and.getLeftExpression(), sources, parameters); - resolveParameters(and.getRightExpression(), sources, parameters); + resolveParameters(and.getLeftExpression(), sources, placeholders); + resolveParameters(and.getRightExpression(), sources, placeholders); return; } if (expression instanceof OrExpression or) { - resolveParameters(or.getLeftExpression(), sources, parameters); - resolveParameters(or.getRightExpression(), sources, parameters); + resolveParameters(or.getLeftExpression(), sources, placeholders); + resolveParameters(or.getRightExpression(), sources, placeholders); return; } @@ -907,19 +1047,19 @@ private void resolveParameters( resolveParameters( nestedExpression, sources, - parameters + placeholders ); } return; } if (expression instanceof InExpression in) { - resolveInExpression(in, sources, parameters); + resolveInExpression(in, sources, placeholders); return; } if (expression instanceof LikeExpression like) { - resolveLikeExpression(like, sources, parameters); + resolveLikeExpression(like, sources, placeholders); return; } @@ -929,7 +1069,7 @@ private void resolveParameters( } if (expression instanceof Between between) { - resolveBetweenExpression(between, sources, parameters); + resolveBetweenExpression(between, sources, placeholders); return; } @@ -938,12 +1078,12 @@ private void resolveParameters( comparison.getLeftExpression(), comparison.getRightExpression(), sources, - parameters + placeholders ); } } - private void resolveInExpression(InExpression in, List sources, List parameters) { + private void resolveInExpression(InExpression in, List sources, Placeholders placeholders) { if (!(in.getLeftExpression() instanceof net.sf.jsqlparser.schema.Column column)) { return; } @@ -954,10 +1094,10 @@ private void resolveInExpression(InExpression in, List sources, List expressionList) { for (Expression expression : expressionList) { - requireIndexedParameter(expression); + Placeholder placeholder = placeholders.of(expression); - if (expression instanceof JdbcParameter parameter) { - addParameter(parameter, schemaColumn, parameters); + if (placeholder != null) { + addParameter(placeholder, schemaColumn, placeholders); } } @@ -968,7 +1108,7 @@ private void resolveInExpression(InExpression in, List sources, List sources, - List parameters + Placeholders placeholders ) { - requireIndexedParameter(expression); + Placeholder placeholder = placeholders.of(expression); - if (expression instanceof JdbcParameter parameter) { - addParameter(parameter, schemaColumn, parameters); + if (placeholder != null) { + addParameter(placeholder, schemaColumn, placeholders); return; } @@ -990,14 +1130,14 @@ private void resolveInExpression( and.getLeftExpression(), schemaColumn, sources, - parameters + placeholders ); resolveInExpression( and.getRightExpression(), schemaColumn, sources, - parameters + placeholders ); return; @@ -1008,14 +1148,14 @@ private void resolveInExpression( or.getLeftExpression(), schemaColumn, sources, - parameters + placeholders ); resolveInExpression( or.getRightExpression(), schemaColumn, sources, - parameters + placeholders ); return; @@ -1023,15 +1163,15 @@ private void resolveInExpression( if (expression instanceof ParenthesedExpressionList expressionList) { for (Expression nestedExpression : expressionList) { - requireIndexedParameter(nestedExpression); + Placeholder nested = placeholders.of(nestedExpression); - if (nestedExpression instanceof JdbcParameter parameter) { - addParameter(parameter, schemaColumn, parameters); + if (nested != null) { + addParameter(nested, schemaColumn, placeholders); } else { resolveParameters( nestedExpression, sources, - parameters + placeholders ); } } @@ -1047,21 +1187,20 @@ private void resolveInExpression( private void resolveLikeExpression( LikeExpression like, List sources, - List parameters + Placeholders placeholders ) { Expression left = like.getLeftExpression(); Expression right = like.getRightExpression(); - requireIndexedParameter(left); - requireIndexedParameter(right); - - if (left instanceof JdbcParameter) { + if (placeholders.of(left) != null) { throw new UnsupportedOperationException( "A LIKE placeholder must be the pattern, not the tested value." ); } - if (!(right instanceof JdbcParameter parameter)) { + Placeholder pattern = placeholders.of(right); + + if (pattern == null) { return; } @@ -1076,9 +1215,9 @@ private void resolveLikeExpression( requireSupportedType(schemaColumn); addParameter( - parameter, + pattern, requireTextColumn(schemaColumn), - parameters + placeholders ); } @@ -1158,17 +1297,15 @@ private void resolveIsNullExpression(IsNullExpression isNull, List sourc private void resolveBetweenExpression( Between between, List sources, - List parameters + Placeholders placeholders ) { Expression left = between.getLeftExpression(); - Expression start = between.getBetweenExpressionStart(); - Expression end = between.getBetweenExpressionEnd(); - requireIndexedParameter(left); - requireIndexedParameter(start); - requireIndexedParameter(end); + Placeholder tested = placeholders.of(left); + Placeholder start = placeholders.of(between.getBetweenExpressionStart()); + Placeholder end = placeholders.of(between.getBetweenExpressionEnd()); - if (!(start instanceof JdbcParameter) && !(end instanceof JdbcParameter)) { + if (tested != null || (start == null && end == null)) { return; } @@ -1178,12 +1315,12 @@ private void resolveBetweenExpression( dev.sqlcj.schema.Column schemaColumn = resolveColumn(column, sources).column(); - if (start instanceof JdbcParameter startParameter) { - addParameter(startParameter, schemaColumn, parameters); + if (start != null) { + addParameter(start, schemaColumn, placeholders); } - if (end instanceof JdbcParameter endParameter) { - addParameter(endParameter, schemaColumn, parameters); + if (end != null) { + addParameter(end, schemaColumn, placeholders); } } @@ -1191,84 +1328,85 @@ private void resolveParameterComparison( Expression left, Expression right, List sources, - List parameters + Placeholders placeholders ) { - requireIndexedParameter(left); - requireIndexedParameter(right); + Placeholder leftPlaceholder = placeholders.of(left); + Placeholder rightPlaceholder = placeholders.of(right); - if (left instanceof net.sf.jsqlparser.schema.Column column && right instanceof JdbcParameter parameter) { + if (left instanceof net.sf.jsqlparser.schema.Column column && rightPlaceholder != null) { addParameter( - parameter, + rightPlaceholder, resolveColumn(column, sources).column(), - parameters + placeholders ); return; } - if (left instanceof JdbcParameter parameter && right instanceof net.sf.jsqlparser.schema.Column column) { + if (leftPlaceholder != null && right instanceof net.sf.jsqlparser.schema.Column column) { addParameter( - parameter, + leftPlaceholder, resolveColumn(column, sources).column(), - parameters + placeholders ); } } private void addParameter( - JdbcParameter parameter, + Placeholder placeholder, String columnName, dev.sqlcj.schema.Table table, - List parameters + Placeholders placeholders ) { addParameter( - parameter, + placeholder, findColumn(table, columnName), - parameters + placeholders ); } private void addParameter( - JdbcParameter parameter, + Placeholder placeholder, dev.sqlcj.schema.Column column, - List parameters + Placeholders placeholders ) { addParameter( - parameter, + placeholder, column.name(), requireSupportedType(column), column.enumType(), column.array(), column.blankPadded(), - parameters + placeholders ); } private void addParameter( - JdbcParameter parameter, + Placeholder placeholder, String name, ColumnType type, - List parameters + Placeholders placeholders ) { - addParameter(parameter, name, type, null, false, false, parameters); + 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. + */ private void addParameter( - JdbcParameter parameter, + Placeholder placeholder, String name, ColumnType type, String enumType, boolean array, boolean blankPadded, - List parameters + Placeholders placeholders ) { - if (!parameter.isUseFixedIndex()) { - throw new UnsupportedOperationException(ANONYMOUS_PARAMETER_REJECTION); - } - - parameters.add( + placeholders.add( new QueryParameter( - parameter.getIndex(), - name, + placeholder.index(), + placeholder.nameOr(name), type, enumType, array, @@ -1278,28 +1416,37 @@ private void addParameter( } /** - * Rejects an anonymous placeholder reported anywhere in the SQL source, - * including a clause this analyzer does not traverse, so that an accepted - * query never keeps an unbound placeholder in its executable SQL. + * Rejects the placeholder forms an analyzed query must not contain + * anywhere in its SQL source, including a clause this analyzer does not + * traverse: an anonymous placeholder, which has no parameter to bind, a + * named placeholder the compiler did not replace, which the executable SQL + * would otherwise keep, and indexed and named placeholders in one query, + * whose parameter order would follow two rules at once. */ - private void requireIndexedPlaceholders(ParsedSql parsedSql) { + private void requireSupportedPlaceholders(ParsedSql parsedSql) { if (parsedSql.parameters().hasAnonymousParameter()) { throw new UnsupportedOperationException(ANONYMOUS_PARAMETER_REJECTION); } - } - /** - * Rejects a named placeholder where an indexed placeholder is supported, so - * that an accepted query never keeps an unbound placeholder in its - * executable SQL. - */ - private void requireIndexedParameter(Expression expression) { - if (expression instanceof JdbcNamedParameter named) { + List uncompiled = parsedSql.parameters().uncompiledPlaceholders(); + + if (!uncompiled.isEmpty()) { throw new UnsupportedOperationException( - "Named parameter ':%s' is not supported; use an indexed placeholder such as $1" - .formatted(named.getName()) + NAMED_PLACEHOLDER_REJECTION.formatted(uncompiled.getFirst()) ); } + + if (parsedSql.parameters().hasPositionalParameter() && !parsedSql.parameters().names().isEmpty()) { + throw new UnsupportedOperationException(MIXED_PLACEHOLDER_REJECTION); + } + } + + /** + * The compiled placeholders of one query, against which every analyzed + * placeholder occurrence resolves to its logical parameter. + */ + private Placeholders toPlaceholders(ParsedSql parsedSql) { + return new Placeholders(parsedSql.parameters().names()); } /** diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryModel.java b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryModel.java index 4d908f2..ea24ac6 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryModel.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryModel.java @@ -7,12 +7,14 @@ /** * Analyzed query facts required by code generation. * - * @param executableSql JDBC-executable SQL where every parsed {@code $N} - * parameter token is replaced by {@code ?} - * @param bindingParameterIndexes placeholder indexes in the textual order of the - * {@code ?} positions in {@link #executableSql()} - * @param parameters one query parameter per placeholder index, in - * logical placeholder-index order + * @param executableSql JDBC-executable SQL where every parsed + * {@code $N} and {@code :name} parameter token + * is replaced by {@code ?} + * @param bindingParameterIndexes the logical parameter number of each + * {@code ?} position in + * {@link #executableSql()}, in textual order + * @param parameters one query parameter per logical parameter + * number, in that order * @param rowTable the schema's declared name of the table whose * complete row this query returns, or * {@code null} when the result is specific to 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 c50ada9..18e574f 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/ParsedSql.java @@ -6,8 +6,8 @@ * Syntax-level result of parsing one query SQL source. * * @param statement the parsed statement - * @param parameters the positional parameters compiled from the parsed - * {@code $N} tokens of the same source + * @param parameters the parameters compiled from the parsed {@code $N} and + * {@code :name} placeholder tokens of the same source */ public record ParsedSql( Statement statement, 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 ca16455..8e7ab3e 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameterCompiler.java @@ -1,5 +1,6 @@ package dev.sqlcj.sql; +import net.sf.jsqlparser.expression.JdbcNamedParameter; import net.sf.jsqlparser.parser.CCJSqlParserConstants; import net.sf.jsqlparser.parser.Node; import net.sf.jsqlparser.parser.Token; @@ -9,16 +10,20 @@ import java.util.regex.Pattern; /** - * Compiles the positional {@code $N} parameters of a parsed SQL source into - * JDBC {@code ?} markers. + * Compiles the positional {@code $N} and named {@code :name} parameters of a + * parsed SQL source into JDBC {@code ?} markers. * *

Only tokens that the SQL parser itself reported as parameter tokens are * replaced. Every other character is copied from the source, so placeholder * text inside string literals, quoted identifiers, comments, and identifiers * stays byte-for-byte unchanged. * - *

An anonymous {@code ?} placeholder has no index to bind, so it is - * reported instead of replaced and is rejected by semantic analysis. + *

Each distinct placeholder name is one logical parameter, numbered by its + * first textual occurrence, and every occurrence of that name becomes a + * {@code ?} of its own. A placeholder form this compiler does not recognize, + * 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. */ final class SqlParameterCompiler { @@ -30,6 +35,20 @@ final class SqlParameterCompiler { */ private static final Pattern POSITIONAL_PARAMETER = Pattern.compile("\\$\\d{1,9}"); + /** + * A named parameter name, which is an unquoted identifier of ASCII letters, + * digits, and underscores that does not start with a digit. Names are + * compared exactly as written. + */ + private static final Pattern PARAMETER_NAME = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*"); + + /** + * The image of the token that opens a named parameter. The parser reports + * the cast operator {@code ::} as a token of its own, so only a single + * colon opens a name. + */ + private static final String NAME_PREFIX = ":"; + /** * The image of an anonymous placeholder token. The parser reports it as a * token of its own, so a {@code ?} inside a string literal, a quoted @@ -59,7 +78,9 @@ SqlParameters compile(String sql, Node astRoot) { StringBuilder executableSql = new StringBuilder(); List indexes = new ArrayList<>(); + List names = new ArrayList<>(); boolean anonymous = false; + boolean positional = false; int copied = 0; for (Token token = firstToken; token != null; token = token.next) { @@ -68,7 +89,13 @@ SqlParameters compile(String sql, Node astRoot) { } if (isPositionalParameter(token)) { - int begin = requireSpan(sql, token, copied); + int begin = requireSpan( + sql, + token.image, + token.absoluteBegin, + token.absoluteEnd, + copied + ); executableSql .append(sql, copied, begin) @@ -77,6 +104,29 @@ SqlParameters compile(String sql, Node astRoot) { copied = begin + token.image.length(); indexes.add(Integer.parseInt(token.image.substring(1))); + + positional = true; + } else if (token != lastToken && isNamedParameter(token, token.next)) { + Token name = token.next; + String image = token.image + name.image; + + int begin = requireSpan( + sql, + image, + token.absoluteBegin, + name.absoluteEnd, + copied + ); + + executableSql + .append(sql, copied, begin) + .append('?'); + + copied = begin + image.length(); + + indexes.add(numberOf(name.image, names)); + + token = name; } if (token == lastToken) { @@ -86,7 +136,49 @@ SqlParameters compile(String sql, Node astRoot) { executableSql.append(sql, copied, sql.length()); - return new SqlParameters(executableSql.toString(), indexes, anonymous); + List uncompiled = new ArrayList<>(); + + collectUncompiledPlaceholders(node, names, uncompiled); + + return new SqlParameters( + executableSql.toString(), + indexes, + names, + uncompiled, + positional, + anonymous + ); + } + + /** + * 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. A placeholder the + * parser reports without a parse-tree node of its own, such as one under a + * cast, has no node to collect here. + */ + private void collectUncompiledPlaceholders(Node node, List names, List uncompiled) { + if (node.jjtGetValue() instanceof JdbcNamedParameter named && !isCompiled(named, names)) { + String placeholder = named.getParameterCharacter() + named.getName(); + + if (!uncompiled.contains(placeholder)) { + uncompiled.add(placeholder); + } + } + + for (int child = 0; child < node.jjtGetNumChildren(); child++) { + collectUncompiledPlaceholders(node.jjtGetChild(child), names, uncompiled); + } + } + + /** + * Reports whether a named placeholder the parser produced is one this + * compiler replaced, which requires both its colon and its name to be the + * supported form. + */ + private boolean isCompiled(JdbcNamedParameter named, List names) { + return NAME_PREFIX.equals(named.getParameterCharacter()) + && names.contains(named.getName()); } private boolean isAnonymousParameter(Token token) { @@ -100,23 +192,55 @@ private boolean isPositionalParameter(Token token) { } /** - * Returns the source offset of a parameter token after requiring that its + * Reports whether a colon token and the token after it are one named + * parameter, which requires the name to follow the colon directly. A name + * separated from its colon, a qualified or quoted name, and a name that is + * not an unquoted identifier are other text, which stays in the SQL. + */ + private boolean isNamedParameter(Token colon, Token name) { + return colon.kind == CCJSqlParserConstants.DOUBLE_COLON + && NAME_PREFIX.equals(colon.image) + && name != null + && name.image != null + && name.absoluteBegin == colon.absoluteEnd + && PARAMETER_NAME.matcher(name.image).matches(); + } + + /** + * Reports the logical parameter number of a placeholder name: the number + * the name already has, or the next number, which records the name so that + * distinct names are numbered by their first occurrence. + */ + private int numberOf(String name, List names) { + int index = names.indexOf(name); + + if (index >= 0) { + return index + 1; + } + + names.add(name); + + return names.size(); + } + + /** + * Returns the source offset of a parameter after requiring that its * reported span follows the previous replacement, stays inside the source, - * and holds exactly the token image. + * and holds exactly the parameter image. */ - private int requireSpan(String sql, Token token, int copied) { - int begin = token.absoluteBegin - SOURCE_OFFSET; - int end = token.absoluteEnd - SOURCE_OFFSET; + private int requireSpan(String sql, String image, int absoluteBegin, int absoluteEnd, int copied) { + int begin = absoluteBegin - SOURCE_OFFSET; + int end = absoluteEnd - SOURCE_OFFSET; boolean valid = begin >= copied && end <= sql.length() - && end - begin == token.image.length() - && sql.startsWith(token.image, begin); + && end - begin == image.length() + && sql.startsWith(image, begin); if (!valid) { throw new SqlParseException( "Parameter '%s' reported an unusable source position: [%d, %d)" - .formatted(token.image, begin, end) + .formatted(image, begin, end) ); } diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameters.java b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameters.java index 671f5c2..4497d72 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameters.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/sql/SqlParameters.java @@ -3,24 +3,43 @@ import java.util.List; /** - * Compiled positional parameters of one SQL source. + * Compiled parameters of one SQL source. * - * @param executableSql the source SQL in which every parsed {@code $N} - * parameter token is replaced by a JDBC {@code ?} and - * every other character is preserved - * @param indexes the parameter indexes in the textual order of the - * {@code ?} positions in {@link #executableSql()} + * @param executableSql the source SQL in which every parsed {@code $N} and + * {@code :name} parameter token is replaced by a JDBC + * {@code ?} and every other character is preserved + * @param indexes the logical parameter number of each {@code ?} position + * in {@link #executableSql()}, in textual order, which is + * the index of a {@code $N} placeholder and the + * first-occurrence number of a {@code :name} placeholder + * @param names the compiled {@code :name} placeholder names in logical + * order, so the name of logical parameter {@code n} is the + * element at {@code n - 1}, and an empty list when the + * source compiled no named placeholder + * @param uncompiledPlaceholders + * the named placeholders of the parse tree that were not + * replaced, written as the source spells them, such as + * {@code :a.b}, {@code :"x"}, and {@code &x}, each + * reported once in the order they were found + * @param hasPositionalParameter + * whether the source contains a {@code $N} placeholder + * token, which a named placeholder must not be mixed with * @param hasAnonymousParameter * whether the source contains an anonymous {@code ?} - * placeholder token, which has no index to bind + * placeholder token, which has no parameter to bind */ public record SqlParameters( String executableSql, List indexes, + List names, + List uncompiledPlaceholders, + boolean hasPositionalParameter, boolean hasAnonymousParameter ) { public SqlParameters { indexes = List.copyOf(indexes); + names = List.copyOf(names); + uncompiledPlaceholders = List.copyOf(uncompiledPlaceholders); } } 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 c29b185..2e0dd3e 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java @@ -979,8 +979,8 @@ void shouldRejectUnknownColumnInNullPredicate() { + "|A LIKE pattern placeholder requires a VARCHAR or TEXT column, but id is BIGINT.", "SELECT id FROM users WHERE $1 LIKE name" + "|A LIKE placeholder must be the pattern, not the tested value.", - "SELECT id FROM users WHERE name LIKE :pattern" - + "|Named parameter ':pattern' is not supported; use an indexed placeholder such as $1" + "SELECT id FROM users WHERE :pattern LIKE name" + + "|A LIKE placeholder must be the pattern, not the tested value." } ) void shouldRejectUnsupportedLikePlaceholderForm(String sql, String message) { @@ -1132,20 +1132,28 @@ void shouldRejectUnknownColumnInRangePredicate() { } @Test - void shouldRejectNamedRangeBound() { + void shouldResolveNamedRangeBoundParameters() { String sql = "SELECT id FROM users WHERE id BETWEEN :lo AND :hi"; - Query query = new Query("ListUsers", QueryType.MANY, sql); - ParsedSql parsedSql = parser.parse(sql); + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> analyzer.analyze(query, parsedSql, schema) + assertEquals( + List.of( + new QueryParameter(1, "lo", ColumnType.BIGINT), + new QueryParameter(2, "hi", ColumnType.BIGINT) + ), + model.parameters() ); + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + assertEquals( - "Named parameter ':lo' is not supported; use an indexed placeholder such as $1", - exception.getMessage() + "SELECT id FROM users WHERE id BETWEEN ? AND ?", + model.executableSql() ); } @@ -1262,26 +1270,30 @@ void shouldNotCreateParametersForLiteralPagination() { ); } + /** + * A named pagination value is named after its placeholder rather than after + * its clause, so the generated method states the caller's own name. + */ @ParameterizedTest @ValueSource( strings = { - "SELECT id FROM users ORDER BY id LIMIT :n", - "SELECT id FROM users ORDER BY id OFFSET :n" + "SELECT id FROM users ORDER BY id LIMIT :pageSize", + "SELECT id FROM users ORDER BY id OFFSET :pageSize" } ) - void shouldRejectNamedPaginationValue(String sql) { - Query query = new Query("ListUsers", QueryType.MANY, sql); - ParsedSql parsedSql = parser.parse(sql); - - UnsupportedOperationException exception = assertThrows( - UnsupportedOperationException.class, - () -> analyzer.analyze(query, parsedSql, schema) + void shouldNameNamedPaginationParameterAfterItsPlaceholder(String sql) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema ); assertEquals( - "Named parameter ':n' is not supported; use an indexed placeholder such as $1", - exception.getMessage() + List.of(new QueryParameter(1, "pageSize", ColumnType.INTEGER)), + model.parameters() ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); } @ParameterizedTest @@ -2534,9 +2546,192 @@ void shouldRejectAnonymousParameterInUnsupportedLocation() { } @Test - void shouldRejectNamedParameter() { + void shouldResolveNamedParameter() { String sql = "SELECT * FROM users WHERE id = :userId"; + QueryModel model = analyzer.analyze( + new Query("FindUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, "userId", ColumnType.BIGINT)), + model.parameters() + ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); + + assertEquals( + "SELECT * FROM users WHERE id = ?", + model.executableSql() + ); + } + + /** + * Each distinct name is one parameter numbered by its first textual + * occurrence, while every occurrence binds at its own textual position. + */ + @Test + void shouldNumberNamedParametersByFirstOccurrence() { + String sql = """ + SELECT id FROM users + WHERE (name = :term OR bio = :term) + AND id > :minId + ORDER BY id OFFSET :skip LIMIT :pageSize"""; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + predicateSchema + ); + + assertEquals( + List.of( + new QueryParameter(1, "term", ColumnType.VARCHAR), + new QueryParameter(2, "minId", ColumnType.BIGINT), + new QueryParameter(3, "skip", ColumnType.INTEGER), + new QueryParameter(4, "pageSize", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals(List.of(1, 1, 2, 3, 4), model.bindingParameterIndexes()); + } + + /** + * An update names its parameters after its placeholders, and keeps binding + * its assignments before its predicate. + */ + @Test + void shouldResolveNamedParametersOfUpdate() { + String sql = "UPDATE users SET name = :newName WHERE id = :id"; + + QueryModel model = analyzer.analyze( + new Query("UpdateUser", QueryType.EXEC, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "newName", ColumnType.VARCHAR), + new QueryParameter(2, "id", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2), model.bindingParameterIndexes()); + + assertEquals( + "UPDATE users SET name = ? WHERE id = ?", + model.executableSql() + ); + } + + @Test + void shouldResolveNamedParametersOfInsert() { + String sql = "INSERT INTO users (id, name) VALUES (:id, :name)"; + + QueryModel model = analyzer.analyze( + new Query("CreateUser", QueryType.EXEC, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "id", ColumnType.BIGINT), + new QueryParameter(2, "name", ColumnType.VARCHAR) + ), + model.parameters() + ); + + assertEquals( + "INSERT INTO users (id, name) VALUES (?, ?)", + model.executableSql() + ); + } + + @Test + void shouldResolveNamedParametersInInList() { + String sql = "SELECT id FROM users WHERE id IN (:first, :second)"; + + QueryModel model = analyzer.analyze( + new Query("FindUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "first", ColumnType.BIGINT), + new QueryParameter(2, "second", ColumnType.BIGINT) + ), + model.parameters() + ); + + assertEquals( + "SELECT id FROM users WHERE id IN (?, ?)", + model.executableSql() + ); + } + + @Test + void shouldResolveNamedLikePatternParameter() { + String sql = "SELECT id FROM users WHERE name LIKE :pattern"; + + QueryModel model = analyzer.analyze( + new Query("SearchUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, "pattern", ColumnType.VARCHAR)), + model.parameters() + ); + + assertEquals( + "SELECT id FROM users WHERE name LIKE ?", + model.executableSql() + ); + } + + /** + * A placeholder name is the word written after the colon, whatever the SQL + * parser makes of that word elsewhere, so a name spelled like a keyword is + * a parameter of its own. + */ + @Test + void shouldResolveNamedParametersSpelledLikeKeywords() { + String sql = "SELECT id FROM users WHERE id = :limit AND name = :user ORDER BY id LIMIT :year"; + + QueryModel model = analyzer.analyze( + new Query("FindUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "limit", ColumnType.BIGINT), + new QueryParameter(2, "user", ColumnType.VARCHAR), + new QueryParameter(3, "year", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals( + "SELECT id FROM users WHERE id = ? AND name = ? ORDER BY id LIMIT ?", + model.executableSql() + ); + } + + @Test + void shouldRejectMixedPlaceholderForms() { + String sql = "SELECT * FROM users WHERE id IN ($1, :other)"; + Query query = new Query("FindUsers", QueryType.MANY, sql); ParsedSql parsedSql = parser.parse(sql); @@ -2546,22 +2741,88 @@ void shouldRejectNamedParameter() { ); assertEquals( - "Named parameter ':userId' is not supported; use an indexed placeholder such as $1", + "Indexed '$N' and named ':name' placeholders must not be mixed in one query", exception.getMessage() ); } + /** + * A qualified, quoted, or {@code &name} placeholder is rejected wherever it + * appears, including a clause this analyzer does not traverse, because the + * executable SQL would otherwise keep it. + */ + @ParameterizedTest + @CsvSource( + delimiter = '|', + quoteCharacter = '`', + value = { + "SELECT * FROM users WHERE id = :a.b|:a.b", + "SELECT * FROM users WHERE id = :\"x\"|:\"x\"", + "SELECT * FROM users WHERE id = &x|&x", + "SELECT id FROM users WHERE id = :id ORDER BY :\"x\"|:\"x\"", + "SELECT id FROM users WHERE id = :id ORDER BY &x|&x", + "SELECT id FROM users WHERE id = abs(:\"x\")|:\"x\"", + "SELECT id FROM users WHERE id = abs(&x)|&x", + "SELECT id FROM users WHERE id = abs(:a.b)|:a.b" + } + ) + void shouldRejectUnsupportedNamedPlaceholderForm(String sql, String placeholder) { + Query query = new Query("FindUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, schema) + ); + + assertEquals( + "Named placeholder '%s' is not supported; write an unquoted name of letters, digits, " + .formatted(placeholder) + + "and underscores directly after ':', such as :userId", + exception.getMessage() + ); + } + + /** + * A named placeholder in a location this analyzer does not visit is + * reported by the placeholder accounting, which writes the placeholder as + * the query does. + */ @Test - void shouldRejectNamedParameterInInList() { - String sql = "SELECT * FROM users WHERE id IN ($1, :other)"; + void shouldRejectNamedPlaceholderInUnanalyzedLocation() { + String sql = "SELECT id FROM users WHERE lower(name) = :name"; Query query = new Query("FindUsers", QueryType.MANY, sql); ParsedSql parsedSql = parser.parse(sql); - assertThrows( + UnsupportedOperationException exception = assertThrows( UnsupportedOperationException.class, () -> analyzer.analyze(query, parsedSql, schema) ); + + assertEquals( + "SQL placeholders [:name] are not the analyzed parameters []; " + + "a placeholder is in an unsupported location", + exception.getMessage() + ); + } + + @Test + void shouldRejectNamedPlaceholderWithConflictingTypes() { + String sql = "SELECT id FROM users WHERE id = :value AND name = :value"; + + Query query = new Query("FindUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, schema) + ); + + assertEquals( + "Placeholder :value has conflicting types: Long and String", + exception.getMessage() + ); } @Test 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 1bc7cb3..72b8999 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java @@ -832,6 +832,56 @@ INSERT INTO users (id, code, name) } } + /** + * Covers named placeholders end to end: a read whose placeholders are named + * and whose first name repeats is generated, compiled, and executed against + * PostgreSQL, so its method parameters follow first-occurrence order while + * each occurrence binds at its own textual position. + */ + @Test + void shouldExecuteGeneratedNamedPlaceholdersAgainstPostgres() throws Exception { + Path classesDirectory = generateAndCompile(""" + -- name: ListUsersByTerm :many + SELECT id, name + FROM users + WHERE (name = :term OR bio = :term) + AND id > :minId + ORDER BY id + OFFSET :skip LIMIT :pageSize; + """); + + execute(""" + INSERT INTO users (id, code, name, bio) + VALUES + (1, 1, 'Alice', NULL), + (2, 2, 'Alice', NULL), + (3, 3, 'Bob', 'Alice'), + (4, 4, 'Bob', NULL) + """); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Object repository = newRepository(classLoader); + + Method listUsersByTerm = repository.getClass().getMethod( + "listUsersByTerm", + String.class, + Long.class, + Integer.class, + Integer.class + ); + + assertEquals( + List.of("Alice", "Bob"), + names(listUsersByTerm.invoke(repository, "Alice", 0L, 1, 2)) + ); + + assertEquals( + List.of("Bob"), + names(listUsersByTerm.invoke(repository, "Alice", 2L, 0, 1)) + ); + } + } + /** * Covers the scalar count end to end: a {@code :one} count read is * generated, compiled, and executed against PostgreSQL, so its result 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 34a5352..a3c9446 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java @@ -602,6 +602,63 @@ void shouldExecuteGeneratedQueryWithOutOfOrderPlaceholders() throws Exception { } } + /** + * A repository generated from named placeholders compiles and executes: its + * method parameters follow first-occurrence order and carry the placeholder + * names, while a repeated name binds at each of its textual positions. + */ + @Test + void shouldExecuteGeneratedQueryWithNamedPlaceholders() throws Exception { + Path classesDirectory = generateAndCompile( + """ + -- name: FindUser :one + SELECT id, name, active + FROM users + WHERE (name = :term OR name = :term) + AND id = :userId; + """ + ); + + String source = Files.readString(tempDir.resolve("generated/generated/UsersRepository.java")); + + assertTrue( + source.contains( + "public FindUserResult findUser(String term, Long userId)" + ) + ); + + assertTrue(source.contains("java.util.Arrays.asList(term, term, userId)")); + assertTrue(source.contains("WHERE (name = ? OR name = ?)")); + assertTrue(source.contains("AND id = ?")); + + QueryExecutor executor = new JdbcQueryExecutor(usersDataSource()); + + try (URLClassLoader classLoader = classLoader(classesDirectory)) { + Class generatedClass = Class.forName( + "generated.UsersRepository", + true, + classLoader + ); + + Object generatedQuery = generatedClass + .getConstructor(QueryExecutor.class) + .newInstance(executor); + + Method method = generatedClass.getMethod( + "findUser", + String.class, + Long.class + ); + + Object result = method.invoke(generatedQuery, "Alice", 1L); + + assertNotNull(result); + + assertEquals(1L, getRecordComponent(result, "id")); + assertEquals("Alice", getRecordComponent(result, "name")); + } + } + @Test void shouldExecuteGeneratedQueryWithRepeatedPlaceholder() throws Exception { Path classesDirectory = generateAndCompile( 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 ab011e2..32b98c1 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParameterCompilerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParameterCompilerTest.java @@ -89,6 +89,56 @@ void shouldRejectSpanOutsideTheSource() { ); } + @Test + void shouldReplaceReportedNamedParameterSpans() { + String sql = "SELECT * FROM users WHERE id = :id"; + + SqlParameters parameters = compile( + sql, + token(CCJSqlParserConstants.DOUBLE_COLON, ":", 31), + token(CCJSqlParserConstants.S_IDENTIFIER, "id", 32) + ); + + assertEquals("SELECT * FROM users WHERE id = ?", parameters.executableSql()); + assertEquals(List.of(1), parameters.indexes()); + assertEquals(List.of("id"), parameters.names()); + } + + /** A name that does not follow its colon directly is other text. */ + @Test + void shouldIgnoreNameSeparatedFromItsColon() { + String sql = "SELECT * FROM users WHERE id = : id"; + + SqlParameters parameters = compile( + sql, + token(CCJSqlParserConstants.DOUBLE_COLON, ":", 31), + token(CCJSqlParserConstants.S_IDENTIFIER, "id", 33) + ); + + assertEquals(sql, parameters.executableSql()); + assertTrue(parameters.indexes().isEmpty()); + assertTrue(parameters.names().isEmpty()); + } + + @Test + void shouldRejectNamedSpanThatDoesNotHoldTheParameterImage() { + String sql = "SELECT * FROM users WHERE id = :id"; + + Token colon = token(CCJSqlParserConstants.DOUBLE_COLON, ":", 30); + Token name = token(CCJSqlParserConstants.S_IDENTIFIER, "id", 31); + + SqlParseException exception = assertThrows( + SqlParseException.class, + () -> compile(sql, colon, name) + ); + + assertTrue( + exception.getMessage() + .startsWith("Parameter ':id' reported an unusable source position"), + exception.getMessage() + ); + } + @Test void shouldRejectSpansThatAreNotInTextualOrder() { String sql = "SELECT * FROM users WHERE id = $1 AND id = $2"; 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 732b1a8..89ac9a1 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/sql/SqlParserTest.java @@ -99,6 +99,119 @@ void shouldCompileParametersOfSupportedWrites() { ); } + /** + * Each distinct name is one logical parameter numbered by its first + * textual occurrence, and every occurrence becomes a {@code ?} of its own. + */ + @Test + void shouldCompileNamedParametersByFirstOccurrence() { + ParsedSql parsedSql = parser.parse( + "SELECT * FROM users WHERE (name = :term OR bio = :term) AND id > :minId" + ); + + assertEquals( + "SELECT * FROM users WHERE (name = ? OR bio = ?) AND id > ?", + parsedSql.parameters().executableSql() + ); + + assertEquals(List.of(1, 1, 2), parsedSql.parameters().indexes()); + assertEquals(List.of("term", "minId"), parsedSql.parameters().names()); + assertFalse(parsedSql.parameters().hasPositionalParameter()); + } + + /** Names are compared exactly as written, so case distinguishes them. */ + @Test + void shouldCompileNamesThatDifferInCaseAsDistinctParameters() { + ParsedSql parsedSql = parser.parse( + "SELECT * FROM users WHERE name = :term AND bio = :Term" + ); + + assertEquals(List.of(1, 2), parsedSql.parameters().indexes()); + assertEquals(List.of("term", "Term"), parsedSql.parameters().names()); + } + + @Test + void shouldReportBothPlaceholderFormsOfOneSource() { + ParsedSql parsedSql = parser.parse( + "SELECT * FROM users WHERE id = $1 AND name = :name" + ); + + assertTrue(parsedSql.parameters().hasPositionalParameter()); + assertEquals(List.of("name"), parsedSql.parameters().names()); + } + + /** + * Only a name written directly after a single colon is a parameter, so a + * quoted name, an ampersand placeholder, a separated name, and a numeric + * bind stay in the SQL for semantic analysis to reject. + */ + @Test + void shouldNotCompileUnsupportedNamedPlaceholderForms() { + for ( + String sql : List.of( + "SELECT * FROM users WHERE id = :\"x\"", + "SELECT * FROM users WHERE id = &x", + "SELECT * FROM users WHERE id = : x", + "SELECT * FROM users WHERE id = :1" + ) + ) { + ParsedSql parsedSql = parser.parse(sql); + + assertEquals(sql, parsedSql.parameters().executableSql()); + assertTrue(parsedSql.parameters().indexes().isEmpty()); + assertTrue(parsedSql.parameters().names().isEmpty()); + } + } + + /** + * A named placeholder the parser reported and this compiler did not replace + * is reported as the source spells it, so semantic analysis can reject it + * wherever it appears. + */ + @Test + void shouldReportNamedPlaceholdersThatWereNotCompiled() { + ParsedSql parsedSql = parser.parse( + "SELECT id FROM users WHERE id = :id AND id = abs(:a.b) ORDER BY :\"x\", &y" + ); + + assertEquals( + List.of(":a.b", ":\"x\"", "&y"), + parsedSql.parameters().uncompiledPlaceholders() + ); + } + + @Test + void shouldReportNoUncompiledPlaceholderForSupportedForms() { + ParsedSql parsedSql = parser.parse( + "SELECT id FROM users WHERE id = $1 AND name = '&x' -- :\"x\"" + ); + + assertTrue(parsedSql.parameters().uncompiledPlaceholders().isEmpty()); + } + + @Test + void shouldPreserveNamedPlaceholderTextThatIsNotAParameter() { + String sql = """ + SELECT id, ':id literal' AS "c:id" + FROM users -- :id line comment + WHERE id = :id /* :id block comment */ + """; + + ParsedSql parsedSql = parser.parse(sql); + + assertEquals( + """ + SELECT id, ':id literal' AS "c:id" + FROM users -- :id line comment + WHERE id = ? /* :id block comment */ + """, + parsedSql.parameters().executableSql() + ); + + assertEquals(List.of(1), parsedSql.parameters().indexes()); + assertEquals(List.of("id"), parsedSql.parameters().names()); + } + @Test void shouldReportAnonymousParameterOutsideAnalyzedExpressions() { ParsedSql parsedSql = parser.parse("SELECT id FROM users WHERE id = $1 LIMIT ?");