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

[JDBC]Data inconsistency caused by JDBC calling stored procedures #1899

Closed
winchannelBj opened this issue Aug 22, 2022 · 9 comments
Closed

Comments

@winchannelBj
Copy link

[JDBC]Data inconsistency caused by JDBC calling stored procedures

Driver version

All versions.

SQL Server version

All versions.

Problem description

I have a table and a stored procedure as bellow:

CREATE TABLE [dbo].[TEST_USER](
[ID] [bigint] NOT NULL,
[USER_NAME] nvarchar NULL,
[USER_PWD] nvarchar NULL,
CONSTRAINT [PK_TEST_ACTIVITY] PRIMARY KEY CLUSTERED
(
[ID] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO

-- Create unique key for the user_name field.
CREATE UNIQUE NONCLUSTERED INDEX [IX_TEST_ACTIVITY] ON [dbo].[TEST_USER]
(
[USER_NAME] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
GO

CREATE PROCEDURE [dbo].[PROC_MS_JDBC_TEST]
AS
BEGIN
SET XACT_ABORT ON
BEGIN TRANSACTION
INSERT INTO TEST_USER(ID,USER_NAME,[USER_PWD])
VALUES(1,'unique_error_name','1');
-- The second insert statement will raise an exception.
INSERT INTO TEST_USER(ID,USER_NAME,[USER_PWD])
VALUES(2,'unique_error_name','2');
COMMIT

END

My jdbc code like this:

public class SQLServerJdbcTest {

public static void main(String[] args)  {
    try {
        Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver");

        Connection connection = DriverManager.getConnection("jdbc:sqlserver://xxx.xxx.x.xx:1433;database=xx;encrypt=false;", "xx", "xxxxxxx");

        CallableStatement stat = null;
        try{
            stat = connection.prepareCall("{call PROC_MS_JDBC_TEST}");
            stat.executeUpdate();
            System.out.println("{call PROC_MS_JDBC_TEST} command executed successfully.");
        }catch (Exception ex){
            ex.printStackTrace();
        } finally {
            try {
                if (stat != null) {
                    stat.close();
                }
            } catch (SQLException e) {
                e.printStackTrace();
            } catch (NullPointerException e){
                e.printStackTrace();
            }
        }
    }catch (Exception ex){
        ex.printStackTrace();
    }
}

}

Expected behavior

In my java code,an exception that from the database will be catched.

Actual behavior

Executing the java code above,i can not get any exception. In other words, the Java code is executed successfully, but the database actually has an exception. The behavior of JDBC is somewhat strange and may cause inconsistent data of the system.

Any other details that can be helpful

Through debugging the trace code, I found that the message returned by the stored procedure to the client contains multiple results. Whether the JDBC client throws an error depends on the message parsing. The parsing is handled through a while loop. When the JDBC code does not detect the database exception in the condition of exiting the loop for the first time, the client code will not catch the exception.
In the stored procedure, you can avoid returning multiple results to the JDBC client by adding a “SET NOCOUNT ON” statement.If we do not encounter similar problems, no one knows that we must deal with them in this way.

winchannelBj added a commit to winchannelBj/mssql-jdbc that referenced this issue Aug 22, 2022
fixed microsoft#1899  When executing the stored procedure, force the return of true in the dodone method of the nextresult class to allow the message parsing to continue.
@Jeffery-Wasty
Copy link
Contributor

Hi @winchannelBj,

Thank you for the issue, information and the linked pull request. We'll take a look at all of this and get back to you with a decision. My initial thoughts are that the change proposed may resolve the issue, but might not be the best solution, and so we'll also look into other ways to resolve the problem.

@Jeffery-Wasty Jeffery-Wasty self-assigned this Aug 22, 2022
@Jeffery-Wasty Jeffery-Wasty added the Under Investigation Used for issues under investigation label Aug 22, 2022
@winchannelBj
Copy link
Author

Thanks for your reply. I found that my change can not work for the bellow stored procedure.

CREATE PROC [dbo].[PROC_MS_JDBC_TEST_2]
AS
BEGIN
SELECT * INTO #TEST_USER FROM TEST_USER
SELECT 1/0 ;
END

When the JDBC code executes the stored procedure, the tdsreader. Peektokentype() method returns TDS.TDS_ COLMETADATA,which causes the while loop to be terminated. Therefore, the exception information of the stored procedure could not be captured.

class:
DTSParser
method:
static void parse(TDSReader tdsReader, TDSTokenHandler tdsTokenHandler,
boolean readOnlyWarningsFlag) throws SQLServerException {
final boolean isLogging = logger.isLoggable(Level.FINEST);

    // Process TDS tokens from the token stream until we're told to stop.
    boolean parsing = true;

    // If TDS_LOGIN_ACK is received verify for TDS_FEATURE_EXTENSION_ACK packet
    boolean isLoginAck = false;
    boolean isFeatureExtAck = false;
    while (parsing) {
        int tdsTokenType = tdsReader.peekTokenType();
        if (isLogging) {
            logger.finest(tdsReader.toString() + ": " + tdsTokenHandler.logContext + ": Processing "
                    + ((-1 == tdsTokenType) ? "EOF" : TDS.getTokenName(tdsTokenType)));
        }
        if (readOnlyWarningsFlag && TDS.TDS_MSG != tdsTokenType) {
            parsing = false;
            return;
        }
        switch (tdsTokenType) {
            case TDS.TDS_SSPI:
                parsing = tdsTokenHandler.onSSPI(tdsReader);
                break;
            case TDS.TDS_LOGIN_ACK:
                isLoginAck = true;
                parsing = tdsTokenHandler.onLoginAck(tdsReader);
                break;
            case TDS.TDS_FEATURE_EXTENSION_ACK:
                isFeatureExtAck = true;
                tdsReader.getConnection().processFeatureExtAck(tdsReader);
                parsing = true;
                break;
            case TDS.TDS_ENV_CHG:
                parsing = tdsTokenHandler.onEnvChange(tdsReader);
                break;
            case TDS.TDS_SESSION_STATE:
                parsing = tdsTokenHandler.onSessionState(tdsReader);
                break;
            case TDS.TDS_RET_STAT:
                parsing = tdsTokenHandler.onRetStatus(tdsReader);
                break;
            case TDS.TDS_RETURN_VALUE:
                parsing = tdsTokenHandler.onRetValue(tdsReader);
                break;
            case TDS.TDS_DONEINPROC:
            case TDS.TDS_DONEPROC:
            case TDS.TDS_DONE:
                tdsReader.getCommand().checkForInterrupt();
                parsing = tdsTokenHandler.onDone(tdsReader);
                break;

            case TDS.TDS_ERR:
                parsing = tdsTokenHandler.onError(tdsReader);
                break;
            case TDS.TDS_MSG:
                parsing = tdsTokenHandler.onInfo(tdsReader);
                break;
            case TDS.TDS_ORDER:
                parsing = tdsTokenHandler.onOrder(tdsReader);
                break;
            case TDS.TDS_COLMETADATA:
                parsing = tdsTokenHandler.onColMetaData(tdsReader);
                break;
            case TDS.TDS_ROW:
                parsing = tdsTokenHandler.onRow(tdsReader);
                break;
            case TDS.TDS_NBCROW:
                parsing = tdsTokenHandler.onNBCRow(tdsReader);
                break;
            case TDS.TDS_COLINFO:
                parsing = tdsTokenHandler.onColInfo(tdsReader);
                break;
            case TDS.TDS_TABNAME:
                parsing = tdsTokenHandler.onTabName(tdsReader);
                break;

            case TDS.TDS_FEDAUTHINFO:
                parsing = tdsTokenHandler.onFedAuthInfo(tdsReader);
                break;
            case -1:
                tdsReader.getCommand().onTokenEOF();
                tdsTokenHandler.onEOF(tdsReader);
                parsing = false;
                break;

            default:
                throwUnexpectedTokenException(tdsReader, tdsTokenHandler.logContext);
                break;
        }
    }

    // if TDS_FEATURE_EXTENSION_ACK is not received verify if TDS_FEATURE_EXT_AE was sent
    if (isLoginAck && !isFeatureExtAck)
        tdsReader.tryProcessFeatureExtAck(isFeatureExtAck);
}

@Jeffery-Wasty
Copy link
Contributor

Thank you for the information. How much of a problem is this issue to your current workflow? We've currently backlogged this for at least the next few weeks, however I saw that there was an additional IcM incident raised citing this issue. Please let us know the effect of this issue so we can plan accordingly.

@Jeffery-Wasty
Copy link
Contributor

Hi @winchannelBj,

When dealing with multiple resultSets in a store procedure, as you have above, the approach is a bit different. executeUpdate will only execute the first update, to execute all, and see all results, you have to use getMoreResults and getResultSet. An example would be something like this.

stat.execute();
while (stat.getMoreResults()) {
System.out.println(stat.getResultSet());
}

With this the error is shown correctly. For more information, please see: Using Multiple Result Sets. We'll close this issue and associated PR, but feel free to comment if there are additional questions.

@Jeffery-Wasty Jeffery-Wasty removed the Under Investigation Used for issues under investigation label Aug 24, 2022
@winchannelBj
Copy link
Author

winchannelBj commented Aug 25, 2022

Thank you for your attention to this issue.If you need any help, you can leave me a message.

This problem has a great impact on our system. We use a large number of stored procedures, and many systems have been deployed.

The example you provided still doesn't work. Is there anything wrong ?

        //CallableStatement stat = null;
        Statement stat = null;
        try{
            //stat = connection.prepareCall("{call PROC_MS_JDBC_TEST}");
            stat = connection.createStatement();
           // stat.executeUpdate("{call PROC_MS_JDBC_TEST}");
            stat.execute("{call PROC_MS_JDBC_TEST}");
            while(stat.getMoreResults()){
                System.out.println(stat.getResultSet());
            }
            System.out.println("{call PROC_MS_JDBC_TEST} command executed successfully.");
        }catch (Exception ex){
            ex.printStackTrace();
        } finally {
            try {
                if (stat != null) {
                    stat.close();
                }
            } catch (SQLException e) {
                e.printStackTrace();
            } catch (NullPointerException e){
                e.printStackTrace();
            }
        }

@Jeffery-Wasty Jeffery-Wasty reopened this Aug 25, 2022
@Jeffery-Wasty
Copy link
Contributor

Jeffery-Wasty commented Aug 25, 2022

What exactly is the error you're trying to get? Is it not "cannot insert duplicate key"? Without any changes, I was able to get the same results you were, the test passing when it shouldn't. With the changes I proposed, I get the following:

image

Is this not the desired result?

EDIT: I've confirmed this has the same result with the latest code snippet you provided.

@David-Engel
Copy link
Collaborator

This question comes up frequently and there are doc and wiki pages for it:
https://docs.microsoft.com/en-us/sql/connect/jdbc/parsing-the-results?view=sql-server-ver16
https://github.com/microsoft/mssql-jdbc/wiki/Handling-SQLExceptions

Some past issue references: #367, #399, #826, #937, #995, #1171

@winchannelBj
Copy link
Author

Thanks!It works.

Change the code like this:

try (Connection connection = DriverManager.getConnection(URL); Statement statement = connection.createStatement()) {
boolean resultsAvailable = statement.execute(USER_SQL);
int updateCount = -2;
while (true) {
updateCount = statement.getUpdateCount();
if (!resultsAvailable && updateCount == -1)
break;
if (resultsAvailable) {
// handle ResultSet
} else {
// handle Update Count
}
resultsAvailable = statement.getMoreResults();
}
}

@Jeffery-Wasty
Copy link
Contributor

Right, that looks like it would process all the results as well. Is any additional assistance needed on this issue? If not, we'll move forward with closing it.

@Jeffery-Wasty Jeffery-Wasty removed their assignment Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants