diff --git a/docs/queries.md b/docs/queries.md index cf0b858..a3055dd 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -155,6 +155,38 @@ 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. +### Pagination + +A read may page its rows with `LIMIT` and `OFFSET`. Each clause takes either a +literal value or a `$N` placeholder: + +```sql +-- name: ListAuthorPage :many +SELECT id, name +FROM authors +ORDER BY id +LIMIT $1 OFFSET $2; +``` + +- A `LIMIT` row count placeholder is an `INTEGER` parameter named `limit`, and an + `OFFSET` value placeholder is an `INTEGER` parameter named `offset`, so + `ListAuthorPage` generates `listAuthorPage(Integer limit, Integer offset)`. +- The pagination parameters are bound after every `WHERE` parameter, in the + textual order of the two clauses, so both `LIMIT $1 OFFSET $2` and + `OFFSET $2 LIMIT $1` generate the same `(limit, offset)` method parameters + while the second binds the offset first. +- A value that binds no placeholder contributes no parameter and reaches the + database as written, so `LIMIT 10 OFFSET 5` binds none, while + `LIMIT 10 OFFSET $1` and `LIMIT ALL OFFSET $1` each bind one `offset` + 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 + `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. + ## Writes A write targets exactly one table of its entry's schema. @@ -213,8 +245,8 @@ 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. - Anonymous `?` placeholders and named `:name` placeholders are rejected. -- A placeholder in a location sqlcj does not analyze — for example `LIMIT $1` — - is rejected rather than left unbound. +- A placeholder in a location sqlcj does not analyze — for example `ORDER BY $1` + — is rejected rather than left unbound. ### Logical order versus textual order @@ -413,16 +445,19 @@ 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 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 `LIMIT $1`, a +- A placeholder in a location sqlcj does not analyze, including `ORDER BY $1`, a computed `LIKE` pattern such as `'%' || $1 || '%'`, a placeholder as the - tested value of a range such as `$1 BETWEEN id AND id`, and a computed range - bound such as `id BETWEEN $1 + 1 AND $2`, so parameterized pagination and - dynamic `IN` expansion are unavailable. + 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 + `LIMIT 5, $1`, and a `FETCH FIRST $1 ROWS ONLY` clause, so dynamic `IN` + expansion is unavailable. - 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`, which fails with the - named-placeholder diagnostic. +- 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. - An `INSERT` without an explicit column list, with more than one `VALUES` row, @@ -446,7 +481,6 @@ them: literal pattern, a range whose bounds are both literal such as `id BETWEEN 1 AND 10`, or `IN` with a subquery, - common table expressions, and subqueries outside the `FROM` item, -- `LIMIT` and `OFFSET` with literal values, - `ON CONFLICT`, `UPDATE ... FROM`, and `DELETE ... USING` on a non-returning `:exec` write. @@ -544,6 +578,16 @@ Reads: `PostgresIntegrationTest.shouldExecuteGeneratedRangePredicatesAgainstPostgres` executes a `BETWEEN` read and a `NOT BETWEEN` read whose bounds use out-of-order placeholder indexes against PostgreSQL 16. +- `QueryAnalyzerTest.shouldResolvePaginationParametersAfterPredicateParameters`, + `QueryAnalyzerTest.shouldResolvePaginationParametersInTextualBindingOrder`, + `QueryAnalyzerTest.shouldResolveOffsetParameterBesideUnanalyzedRowCount`, + `QueryAnalyzerTest.shouldNotCreateParametersForLiteralPagination`, + `QueryAnalyzerTest.shouldRejectNamedPaginationValue`, and + `QueryAnalyzerTest.shouldRejectPlaceholderInUnsupportedPaginationValue` cover + pagination, and + `PostgresIntegrationTest.shouldExecuteGeneratedPaginationAgainstPostgres` + executes a `LIMIT ... OFFSET ...` page and the same page written as + `OFFSET ... LIMIT ...` against PostgreSQL 16. - `QueryAnalyzerTest.shouldResolveRowTableForFullRowSelect`, `QueryAnalyzerTest.shouldNotResolveRowTableForQuerySpecificResult`, `QueryAnalyzerTest.shouldNotResolveRowTableForJoinedWildcard`, and 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 dab22e6..0796087 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java @@ -29,6 +29,8 @@ import net.sf.jsqlparser.statement.select.AllColumns; import net.sf.jsqlparser.statement.select.AllTableColumns; import net.sf.jsqlparser.statement.select.Join; +import net.sf.jsqlparser.statement.select.Limit; +import net.sf.jsqlparser.statement.select.Offset; import net.sf.jsqlparser.statement.select.PlainSelect; import net.sf.jsqlparser.statement.select.Select; import net.sf.jsqlparser.statement.select.SelectItem; @@ -50,6 +52,12 @@ public final class QueryAnalyzer { private static final String ANONYMOUS_PARAMETER_REJECTION = """ Anonymous '?' parameters are not supported; use an indexed placeholder such as $1"""; + /** The parameter name of a {@code LIMIT} row count placeholder. */ + private static final String LIMIT_PARAMETER_NAME = "limit"; + + /** The parameter name of an {@code OFFSET} value placeholder. */ + private static final String OFFSET_PARAMETER_NAME = "offset"; + /** * Resolves the Java type of parameter occurrence, which decides whether a * repeated placeholder index can share one generated parameter. @@ -714,19 +722,88 @@ private Source resolveJoinConditionSource(Expression expression, List so * positions. */ private List resolveBindingParameters(PlainSelect plainSelect, List sources) { - if (plainSelect.getWhere() == null) { - return List.of(); + List parameters = new ArrayList<>(); + + if (plainSelect.getWhere() != null) { + resolveParameters( + plainSelect.getWhere(), + sources, + parameters + ); } - List parameters = new ArrayList<>(); + resolvePaginationParameters(plainSelect, parameters); - resolveParameters( - plainSelect.getWhere(), - sources, - parameters + return parameters; + } + + /** + * 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 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()) ); - return parameters; + Offset offset = plainSelect.getOffset(); + + JdbcParameter offsetValue = resolvePaginationPlaceholder( + offset == null ? null : offset.getOffset() + ); + + 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); + + return; + } + + if (rowCount != null) { + addParameter(rowCount, LIMIT_PARAMETER_NAME, ColumnType.INTEGER, parameters); + } + + if (offsetValue != null) { + addParameter(offsetValue, OFFSET_PARAMETER_NAME, ColumnType.INTEGER, parameters); + } + } + + /** + * Reports the analyzed row count of a {@code LIMIT} clause, which is absent + * when the clause carries an offset of its own, as in the {@code LIMIT a, b} + * form. + */ + private Expression resolveLimitRowCount(Limit limit) { + return limit == null || limit.getOffset() != null + ? null + : 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 void resolveParameters( @@ -1064,6 +1141,20 @@ private void addParameter( JdbcParameter parameter, dev.sqlcj.schema.Column column, List parameters + ) { + addParameter( + parameter, + column.name(), + column.type(), + parameters + ); + } + + private void addParameter( + JdbcParameter parameter, + String name, + ColumnType type, + List parameters ) { if (!parameter.isUseFixedIndex()) { throw new UnsupportedOperationException(ANONYMOUS_PARAMETER_REJECTION); @@ -1072,8 +1163,8 @@ private void addParameter( parameters.add( new QueryParameter( parameter.getIndex(), - column.name(), - column.type() + name, + type ) ); } 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 1143eac..3a2cdcd 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java @@ -1096,6 +1096,142 @@ void shouldRejectPlaceholderThatIsNotAnAnalyzedPredicateOperand(String sql) { ); } + @Test + void shouldResolvePaginationParametersAfterPredicateParameters() { + String sql = "SELECT id FROM users WHERE name = $1 ORDER BY id LIMIT $2 OFFSET $3"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of( + new QueryParameter(1, "name", ColumnType.VARCHAR), + new QueryParameter(2, "limit", ColumnType.INTEGER), + new QueryParameter(3, "offset", ColumnType.INTEGER) + ), + model.parameters() + ); + + assertEquals(List.of(1, 2, 3), model.bindingParameterIndexes()); + + assertEquals( + "SELECT id FROM users WHERE name = ? ORDER BY id LIMIT ? OFFSET ?", + model.executableSql() + ); + } + + @Test + void shouldResolvePaginationParametersInTextualBindingOrder() { + String sql = "SELECT id FROM users ORDER BY id OFFSET $2 LIMIT $1"; + + 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()); + + assertEquals( + "SELECT id FROM users ORDER BY id OFFSET ? LIMIT ?", + model.executableSql() + ); + } + + @ParameterizedTest + @ValueSource( + strings = { + "SELECT id FROM users ORDER BY id LIMIT 10 OFFSET $1", + "SELECT id FROM users ORDER BY id LIMIT ALL OFFSET $1" + } + ) + void shouldResolveOffsetParameterBesideUnanalyzedRowCount(String sql) { + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertEquals( + List.of(new QueryParameter(1, "offset", ColumnType.INTEGER)), + model.parameters() + ); + + assertEquals(List.of(1), model.bindingParameterIndexes()); + } + + @Test + void shouldNotCreateParametersForLiteralPagination() { + String sql = "SELECT id FROM users ORDER BY id LIMIT 10 OFFSET 5"; + + QueryModel model = analyzer.analyze( + new Query("ListUsers", QueryType.MANY, sql), + parser.parse(sql), + schema + ); + + assertTrue(model.parameters().isEmpty()); + assertTrue(model.bindingParameterIndexes().isEmpty()); + + assertEquals( + "SELECT id FROM users ORDER BY id LIMIT 10 OFFSET 5", + model.executableSql() + ); + } + + @ParameterizedTest + @ValueSource( + strings = { + "SELECT id FROM users ORDER BY id LIMIT :n", + "SELECT id FROM users ORDER BY id OFFSET :n" + } + ) + 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) + ); + + assertEquals( + "Named parameter ':n' is not supported; use an indexed placeholder such as $1", + exception.getMessage() + ); + } + + @ParameterizedTest + @ValueSource( + strings = { + "SELECT id FROM users ORDER BY id LIMIT $1 + 1", + "SELECT id FROM users ORDER BY id OFFSET $1 + 1", + "SELECT id FROM users ORDER BY id LIMIT 5, $1", + "SELECT id FROM users ORDER BY id LIMIT $1, $2", + "SELECT id FROM users ORDER BY id FETCH FIRST $1 ROWS ONLY" + } + ) + void shouldRejectPlaceholderInUnsupportedPaginationValue(String sql) { + Query query = new Query("ListUsers", QueryType.MANY, sql); + ParsedSql parsedSql = parser.parse(sql); + + assertThrows( + UnsupportedOperationException.class, + () -> analyzer.analyze(query, parsedSql, schema) + ); + } + @Test void shouldResolveQueryParametersInsideInExpression() { Query query = new Query( @@ -1965,7 +2101,7 @@ void shouldRejectZeroParameterIndex() { @Test void shouldRejectPlaceholderInUnsupportedLocation() { - String sql = "SELECT * FROM users WHERE id = $1 LIMIT $2"; + String sql = "SELECT * FROM users WHERE id = $1 LIMIT $2 + 1"; Query query = new Query("FindUsers", QueryType.MANY, sql); ParsedSql parsedSql = parser.parse(sql); 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 77565f3..358ce2a 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java @@ -503,6 +503,64 @@ INSERT INTO users (id, code, name) } } + /** + * Covers pagination end to end: a {@code LIMIT ... OFFSET ...} page 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 shouldExecuteGeneratedPaginationAgainstPostgres() throws Exception { + Path classesDirectory = generateAndCompile(""" + -- name: ListUserPage :many + SELECT id, name + FROM users + ORDER BY id + LIMIT $1 OFFSET $2; + + -- name: ListUserPageWithLeadingOffset :many + SELECT id, name + FROM users + ORDER BY id + OFFSET $2 LIMIT $1; + """); + + 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( + "listUserPage", + Integer.class, + Integer.class + ); + + Method pageWithLeadingOffset = repository.getClass().getMethod( + "listUserPageWithLeadingOffset", + 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)) + ); + } + } + @Test void shouldExecuteGeneratedWriteAgainstPostgres() throws Exception { Path classesDirectory = generateAndCompile("""