SNOW-1618172: In a multi statement query, snowflake is not sending back responses of queries executed successfully when a statement errors out

rohanricky commented 2 months ago

What is the current behavior?

In a multi statement query, snowflake is not sending back responses of queries executed successfully when one of the queries errors out. In a 4 statement query if there's an error in the 3rd statement, first 2 statements are executed but snowflake doesn't send back the response.

What is the desired behavior?

Snowflake should send back both the success and error responses.

How would this improve `snowflake-connector-nodejs`?

Even in case when one of the queries fail, users might want to work with the success responses of other statements.

References, Other Background

In the below example, we're executing a query containing 3 statements. The tables for first 2 statements are present in snowflake and queries run successfully. If for 3rd statement the table name doesn't exist, complete callback will throw an error. But the success responses for the preceeding queries is not returned.

function executeQuery (connection) {
  return new Promise((resolve, reject) => {
    const MULTI_STATEMENT_COUNT = 0
    // Execute a query using the stream option
    const rows = []
    connection.execute({
      sqlText: "insert into **TABLE_EXISTS** (ID, NAME) values ('test', 'run'); DELETE FROM **TABLE_EXISTS** where PARENTID=12; 
insert into **TABLE_DOESNT_EXIST** (ID, NAME) values ('test', 'run')",
      parameters: { MULTI_STATEMENT_COUNT },
      complete: async function (err, stmt, row, multiResultIds) {
        // we're getting only the error back
        if (err) {
          console.error('Failed to execute statement due to the following error:', err.message)
          return reject(err)
        }

        rows.push(...row)
        if (stmt?.hasNext?.()) {
          stmt.NextResult()
        } else {
          resolve(rows)
        }
      }
    })

    // Handle connection close event
    connection.on('close', () => {
      console.log('Snowflake connection closed')
    })
  })
}

sfc-gh-dszmolka commented 1 month ago

hi - thank you for raising this with us. Taking a look.

sfc-gh-dszmolka commented 1 month ago

so what you're seeing here might be the expected behaviour.

AFAIK per the current implementation ( on the Snowflake backend ) there is an additional 'parent' statement which is executed. So in your example there are 3 statements, but in reality there is 4 - the fourth will be a statement with insert into **TABLE_EXISTS** (ID, NAME) values ('test', 'run'); DELETE FROM **TABLE_EXISTS** where PARENTID=12; insert into **TABLE_DOESNT_EXIST** (ID, NAME) values ('test', 'run'). All of the three child statements are treated as one unit, one object. Which then can fail as one object.

when any of the child statements error out:

the parent statement errors out and is terminated with the same error , with which the erroring statement is terminating (this is thrown as the err in the complete callback too
any following statements are not executed which might follow after the erroring child statement

This (that the whole parent statement errors out, and is considered errored, without returning any possible successful results which might precede the erroring statement) is sorta-kinda documented in other Snowflake clients' documentation. Which behaviour is the same for the node.js driver , since the implementation is done on a central place, the backend.

Some examples:

SQL REST API

If you specified multiple SQL statements in the request and an error occurred when executing any of the statements, Snowflake returns the HTTP response code 422 with a QueryFailureStatus object.
JDBC (this is not super clear, i admit)

..if the statements below are run as a single multi-statement query, the query will fail on the third statement, and an exception will be thrown.

Only error is returned, the result of the successful queries are not.

As a workaround, would it be perhaps applicable calling RESULT_SCAN on the successful queries (preceding the errored-out one) to get their results ?

rohanricky commented 1 month ago

thanks for looking into this @sfc-gh-dszmolka

I tried with RESULT_SCAN and got the following error OperationFailedError: Query {query_id} has no result because it failed'

We have just 1 queryId for all the statements in a query. looks like it is the queryId of parent statement as you mentioned. Do we have a way to access child statement IDs? I see there's a way to get each statement's response, but didn't find any similar documentation for Node.js driver.

I made few changes within Node.js driver to return multiResultIds along with statement, rows in the execute callback, but looks like snowflake doesn't give us multiResultIds if any statement fails.

sfc-gh-dszmolka commented 1 month ago

checked this and looks like calling getStatementId() on the stmt in the complete callback yields the child query's queryId; which in turn might be helpful calling RESULT_SCAN on them later on; if the subsequent query errors our, which errors out the whole multi-statement query.

example:

            const multistatement = connection.execute({
                        sqlText: `
      select 1;
      select 2;
      `,
                        complete: function(err, stmt, rows) {
                                if (err) {
                                    console.log('Error in multistatement: ' + err);
                                } else {
                                    console.log("===> stmt.getStatementId(): " + stmt.getStatementId());
                                    results.push(rows)
                                    ..

would this help?

rohanricky commented 4 weeks ago

@sfc-gh-dszmolka I checked this case. when a multi statement query errors out, the complete callback is called only once. The stmt.getStatementId actually gives the parent query's queryId ( verified this with snowflake UI ). The child queries which are successfully executed have separate query IDs. Not sure how to get these child query IDs. In cases where all child queries are successful we can access child query ID in multiResultIds field ( got this with minor change in snowflake-sdk ).

https://github.com/snowflakedb/snowflake-connector-nodejs/blob/b608860a0680b950220a112bbebd893e0195917b/lib/connection/statement.js#L671 changed above line to: context.complete(null, statement, rows, context.multiResultIds);

but in case a child query fails context.multiResultIds is undefined.

sfc-gh-dszmolka commented 4 weeks ago

okay, got you now. for now, this indeed seems to be a gap - we'll take a look. i'll keep this thread updated on the progress, when there's any.

snowflakedb / snowflake-connector-nodejs