Skip to content
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.

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

Ensure thatPreferredCursorExecution is the only configuration flag to control cursor preference #267

Closed
Dabble63 opened this issue Mar 27, 2023 · 5 comments
Labels
type: bug Something isn't working
Milestone

Comments

@Dabble63
Copy link

Dabble63 commented Mar 27, 2023

It should be possible to specify that cursors not be used.
I receive the message "Could not complete cursor operation because the table schema changed after the cursor was declared.". The query completes successfully when run without using cursors. The schema on the database did in fact change.
Whether cursors are used or not is determined by the following code in SimpleMssqlStatement.java

static boolean prefersCursors(String sql) {
        if (sql.isEmpty()) {
            return false;
        }

        String lc = sql.trim().toLowerCase(Locale.ENGLISH);
        if (lc.contains("for xml") || lc.contains("for json")) {
            return false;
        }

        char c = sql.charAt(0);
        return (c == 's' || c == 'S') && lc.startsWith("select");
    }
}

It is not clear whether the author deliberately checks the sql string starting with 's' or 'S' rather than the trimmed string. Anyway both checks do not take into account that the SQL may start with a comment.
As a consequence, it is possible to force cursors to not be used by starting the statement with a space. This feels like exploiting another bug so if this is fixed this workaround will no longer work,
It also means that you cannot use automatically generated statements such as those produced by JPA.

@mp911de
Copy link
Member

mp911de commented Mar 28, 2023

MssqlConnectionConfiguration allows configuring a predicate to force cursor usage. Even if preferCursoredExecution is false, it is a preference and not a directive. We favor cursor usage because cursors work smoother with backpressure.

Currently, you can force direct execution by specifying the fetch size to zero.

@Dabble63
Copy link
Author

Unfortunately the fetchSize is not applied in all situations.
When MssqlConnection.createStatement is run it creates the SimpleMssqlStatement object which initializes the fetchSize to the default value of -1. If the statement starts with the word select (see code in previous post) then preferCursoredExecution will be set to true. The getEffectiveFetchSize routine will then return FETCH_SIZE which has a value of 128.

@OverRide
public MssqlStatement createStatement(String sql) {

    Assert.requireNonNull(sql, "SQL must not be null");
    logger.debug(this.context.getMessage("Creating statement for SQL: [{}]"), sql);

    if (ParametrizedMssqlStatement.supports(sql)) {
        return new ParametrizedMssqlStatement(this.client, this.connectionOptions, sql);
    }

    return new SimpleMssqlStatement(this.client, this.connectionOptions, sql);
}

MssqlStatementSupport:
int getEffectiveFetchSize() {

    if (this.preferCursoredExecution) {
        return this.fetchSize == FETCH_UNCONFIGURED ? FETCH_SIZE : this.fetchSize;
    }

    return this.fetchSize == FETCH_UNCONFIGURED ? 0 : this.fetchSize;
}

@mp911de
Copy link
Member

mp911de commented Mar 29, 2023

Not quite sure we're on the same page. You can set the fetch size yourself via connection.createStatement(…).fetchSize(0). In that case, Statement.execute() is using direct execution instead of cursored execution.

@Dabble63
Copy link
Author

Unfortunately I am running a Spring Boot JPA application with a connection pool.
fetchSize is part of the configuration however it is not always applied.
I don't see how I can interfere without rewriting a good portion of the code.

From the call stack of the creation of SimpleMssqlStatement I see:
new SimpleMssqlStatement(it, this.connectionOptions, this.METADATA_QUERY).execute()

In short, execute is called directly after creation of the statement. Probably the options should include the fetchSize so that it can be applied during the constructor of SimpleMssqlStatement. The fetchSize optimally would be set using @QueryHint in JPA.
My solution has been to ensure that no statement, for which I do not want cursors used, starts with s or S. This does mean that I cannot use the JPA generated selects however in my case that is not terrible.

@mp911de mp911de changed the title PreferredCursorExecution parameter is ignored. Ensure thatPreferredCursorExecution is the only configuration flag to control cursor preference May 25, 2023
@mp911de
Copy link
Member

mp911de commented May 25, 2023

We will move the predicate evaluating SELECT as text into the default cursor preference predicate so you can entirely control cursor preference without having other predicates that would override the preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants