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

It doesn't work when set properties for ShardingPreparedStatement. #2981

Closed
tristaZero opened this issue Sep 5, 2019 · 4 comments · Fixed by #2999
Closed

It doesn't work when set properties for ShardingPreparedStatement. #2981

tristaZero opened this issue Sep 5, 2019 · 4 comments · Fixed by #2999

Comments

@tristaZero
Copy link
Contributor

tristaZero commented Sep 5, 2019

Bug Report

For English only, other languages will not accept.

Before report a bug, make sure you have:

Please pay attention on issues you submitted, because we maybe need more details.
If no response more than 7 days and we cannot reproduce it on current information, we will close it.

Please answer these questions before submitting your issue. Thanks!

Which version of ShardingSphere did you use?

4.0.0.RC3

Which project did you use? Sharding-JDBC or Sharding-Proxy?

Sharding-JDBC

Expected behavior

Actual behavior

When set queryTimeout property for ShardingPreparedStatement, it doesn't work.

Reason analyze (If you can)

Because when ShardingPreparedStatement is created, there is no actual PreparedStatement in ShardingPreparedStatement. Only when you call execute() or executeQuery(), PreparedStatement is created. So before execute() is called, any assignment for PreparedStatement will not work.

Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

One solution is to create PreparedStatement once ShardingPreparedStatement is created. You need to split function of initPreparedStatementExecutor() in ShardingPreparedStatement.
Here are related codes in ShardingPreparedStatement:

public ShardingPreparedStatement(final ShardingConnection connection, final String sql) {
        this(connection, sql, ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT, false);
    }
---
@Override
    public ResultSet executeQuery() throws SQLException {
        ResultSet result;
        try {
            clearPrevious();
            shard();
            initPreparedStatementExecutor();
            MergeEngine mergeEngine = MergeEngineFactory.newInstance(connection.getRuntimeContext().getDatabaseType(), 
                    connection.getRuntimeContext().getRule(), sqlRouteResult, connection.getRuntimeContext().getMetaData().getTables(), preparedStatementExecutor.executeQuery());
            result = getResultSet(mergeEngine);
        } finally {
            clearBatch();
        }
        currentResultSet = result;
        return result;
    }
---
private void initPreparedStatementExecutor() throws SQLException {
        preparedStatementExecutor.init(sqlRouteResult);
        setParametersForStatements();
    }
---
In PreparedStatementExecutor.java
public void init(final SQLRouteResult routeResult) throws SQLException {
        setOptimizedStatement(routeResult.getShardingStatement());
        getExecuteGroups().addAll(obtainExecuteGroups(routeResult.getRouteUnits()));
        cacheStatements();
    }

Example codes for reproduce this issue (such as a github link).

@sunbufu
Copy link
Contributor

sunbufu commented Sep 5, 2019

After reading #2923, I agree with trista. Because init PreparedStatemen in advance is more direct, simpler, easier than cache some parameters.

@tristaZero
Copy link
Contributor Author

@sunbufu Are u interested in this, so easy. As i said here

@sunbufu
Copy link
Contributor

sunbufu commented Sep 6, 2019

Yes, I'd like to do.

@tristaZero
Copy link
Contributor Author

Waiting for your pr. 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants