8000 Parameters for OFFSET and LIMIT in a prepared statement get swapped · Issue #15466 · duckdb/duckdb · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Parameters for OFFSET and LIMIT in a prepared statement get swapped #15466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
javafanboy opened this issue Dec 25, 2024 · 8 comments · Fixed by #15484
Closed

Parameters for OFFSET and LIMIT in a prepared statement get swapped #15466

javafanboy opened this issue Dec 25, 2024 · 8 comments · Fixed by #15484
Assignees
@ashwaniYDV
Labels
reproduced

Comments

@javafanboy
Copy link
javafanboy commented Dec 25, 2024 •
edited
Loading

I am experiencing a strange effect with DuckDB and Java when passing in OFFSET and LIMIT as parameters in a prepared statement.

To make it work I have to swap the assignment of the parameters compared to the expected i.e. set the desired limit as first parameter and the desired offset as second even though the order is the oposite in the prepared statement....
If I do not switch the order I get zero records in the result (because the limit seems to become zero).

What am I doing wrong here or are there some known quirks/problems with DuckDB (1.1.3) parameter mapping?

My program (somewhat simplified) looks like this:
import java.sql.*;

public class DuckDBParameterBindingTest {

    private static final String JDBC_URL = "jdbc:duckdb:demo.db";
    private static final String QUERY = "SELECT a, b, c, d, e FROM table ORDER BY a, b, c, d, e OFFSET ? LIMIT ?";

    public static void main(String[] args) {
        try (Connection connection = DriverManager.getConnection(JDBC_URL);
             PreparedStatement preparedStatement = connection.prepareStatement(QUERY)) {

            System.out.println("Test 1: Standard parameter binding (OFFSET=0, LIMIT=5)");
            preparedStatement.setInt(1, 0); // OFFSET
            preparedStatement.setInt(2, 5); // LIMIT
            runQuery(preparedStatement); // Incorrect result: No rows printed

            System.out.println("Test 2: Swapped parameter binding (LIMIT=5, OFFSET=0)");
            preparedStatement.setInt(1, 5); // LIMIT
            preparedStatement.setInt(2, 0); // OFFSET
            runQuery(preparedStatement); // Correct result: 5 rows printed

        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

    private static void runQuery(PreparedStatement preparedStatement) throws SQLException {
        try (ResultSet resultSet = preparedStatement.executeQuery()) {
            while (resultSet.next()) {
                System.out.printf("%s, %s, %d, %d, %d%n",
                        resultSet.getString(1),
                        resultSet.getString(2),
                        resultSet.getLong(3),
                        resultSet.getLong(4),
                        resultSet.getLong(5));
            }
            System.out.println("-------------------------");
        }
    }
}

Originally posted by @javafanboy in #14907
@javafanboy javafanboy changed the title I am experiencing a strange effect with DuckDB and Java when passing in OFFSET and LIMIT as parameters in a prepared statement. My program (somewhat simplified) looks like this: I am experiencing a strange effect with DuckDB and Java when passing in OFFSET and LIMIT as parameters in a prepared statement Dec 25, 2024
@szarnyasg
Copy link
Collaborator

Hi @javafanboy, thanks for reporting this! The issue can be reproduced in the DuckDB CLI client:

PREPARE q AS SELECT x FROM generate_series(1, 10) t(x) OFFSET ? LIMIT ?;
EXECUTE q(3, 5);
┌───────┐
│   x   │
│ int64 │
├───────┤
│     6 │
│     7 │
│     8 │
└───────┘

The reason you're experiencing this issue is that the default order of these clauses is LIMIT followed by OFFSET and while DuckDB allows you to swap them, switching them up is not tracked by prepared statements, causing a mixup. This is similar to issue #13585, where switching up the SELECT and FROM clauses caused prepared statements to evaluate incorrectly.

For now, a workaround is to use LIMIT ... OFFSET. As this is a bug that can lead to uncorrect results, we'll fix it for the next release.

@szarnyasg szarnyasg changed the title I am experiencing a strange effect with DuckDB and Java when passing in OFFSET and LIMIT as parameters in a prepared statement Parameters for OFFSET and LIMIT in a prepared statement get swapped Dec 26, 2024
@javafanboy
Copy link
Author

Thanks for the quick reply - will switch to the "preferred order" to avoid my code stop working once the problem is fixed!

@soerenwolfers
Copy link
Contributor

@javafanboy no need to close issues before they are actually fixed - that only increases the chance they'll slip through unfixed.

@javafanboy javafanboy reopened this Dec 26, 2024
@ashwaniYDV
Copy link
Contributor 8000

hi @szarnyasg, I would like to work on this issue. I have identified the potential fix for this.
Please assign this issue to me.

@szarnyasg
Copy link
Collaborator

@ashwaniYDV thanks for offering your contribution! I assigned you. Please tag me in the PR of the proposed fix once it's ready to review.

ashwaniYDV added a commit to ashwaniYDV/duckdb that referenced this issue Dec 28, 2024
@ashwaniYDV
Fix duckdb#15466 Transform LIMIT or OFFSET first based on order speci…
cf6c823 
…fied in prepared statement
@ashwaniYDV
Copy link
Contributor

hi @szarnyasg I have created a WIP PR. Added some comments in the PR.

@ashwaniYDV ashwaniYDV mentioned this issue Dec 28, 2024
Merged
@Tishj
Copy link
Contributor
Tishj commented Dec 28, 2024

Hmm this is a weird one, I presume the syntactic sugar is expanded before the ? is resolved to a number, I wonder if resolving ? to a number can't be done first, and then the syntactic sugar can be expanded

That should solve others problems as well, like the one Gabor mentioned where things like FROM-first are also not given the correct number

ashwaniYDV added a commit to ashwaniYDV/duckdb that referenced this issue Jan 18, 2025
@ashwaniYDV
Fix duckdb#15466 Transform LIMIT or OFFSET first based on order speci…
4ffe849 
…fied in prepared statement
ashwaniYDV added a commit to ashwaniYDV/duckdb that referenced this issue Jan 18, 2025
@ashwaniYDV
Fix duckdb#15466 Transform LIMIT or OFFSET first based on order speci…
e60e603 
…fied in prepared statement
ashwaniYDV added a commit to ashwaniYDV/duckdb that referenced this issue Jan 18, 2025
@ashwaniYDV
Fix duckdb#15466 Transform LIMIT or OFFSET first based on order speci…
b87f3ea 
…fied in prepared statement
ashwaniYDV added a commit to ashwaniYDV/duckdb that referenced this issue Jan 18, 2025
@ashwaniYDV
Fix duckdb#15466 Transform LIMIT or OFFSET first based on order speci…
23e8455 
…fied in prepared statement
@ashwaniYDV
Copy link
Contributor

hi @szarnyasg , I have created a PR for this fix. Please review it.
#15484

Mytherin added a commit that referenced this issue Feb 6, 2025
@Mytherin
Fix #15466 Transform LIMIT or OFFSET first based on order specified i…
cc99c2c 
…n prepared statement (#15484)

Fixes #15466

I have utilized a boolean parameter `offset_first` to check whether
OFFSET appears first in the query than LIMIT.
Utilized a third parameter `isLimitOffsetFirst ` in Yacc grammar file.
Antonov548 added a commit to Antonov548/duckdb-r that referenced this issue Feb 26, 2025
@Antonov548
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
cd4667c 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
krlmlr pushed a commit to duckdb/duckdb-r that referenced this issue Mar 5, 2025
@Antonov548 @krlmlr
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
d75ebda 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
krlmlr added a commit to duckdb/duckdb-r that referenced this issue May 15, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
aa16139 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
krlmlr added a commit to duckdb/duckdb-r that referenced this issue May 15, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
adfb77b 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
krlmlr added a commit to duckdb/duckdb-r that referenced this issue May 17, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
13fc16b 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
krlmlr added a commit to duckdb/duckdb-r that referenced this issue May 18, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@cc99c2c
c21e6d6 
Fix duckdb/duckdb#15466 Transform LIMIT or OFFSET first based on order specified in prepared statement (duckdb/duckdb#15484)
discussions duckdb/duckdb#15981: remove confusing comment in "duckdb/tools/shell/shell.cpp" (duckdb/duckdb#15984)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ashwaniYDV ashwaniYDV

Labels
reproduced
Projects
None yet
Milestone
No milestone
5 participants
@javafanboy @szarnyasg @soerenwolfers @Tishj @ashwaniYDV
0