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

SNOW-1842116: Typescript: getResultsFromQueryId expects option of type StatementOption. sqlText should therefore be optional. #978

Open
sjclemmy opened this issue Dec 2, 2024 · 1 comment
Assignees
Labels
bug Something isn't working status-triage_done Initial triage done, will be further handled by the driver team

Comments

@sjclemmy
Copy link

sjclemmy commented Dec 2, 2024

The typescript type for the connect.getResultsFromQueryId options argument is StatementOption, This is shared with connection.execute so I would expect the sqlText to be optional.

It would probably be a better idea to give getResultsFromQueryId its own argument type.
This code yields a typescript error

    connection.getResultsFromQueryId({
      queryId: queryId,
      complete: function (err, stmt, rows) {
        if (err) {
          console.error(
            "Failed to execute statement due to the following error: " +
              err.message
          );
        } else {
          console.log(rows);
        }
      },
    });
Argument of type '{ queryId: string; complete: (err: SnowflakeError | undefined, stmt: RowStatement | FileAndStageBindStatement, rows: any[] | undefined) => void; }' is not assignable to parameter of type 'StatementOption'.
  Property 'sqlText' is missing in type '{ queryId: string; complete: (err: SnowflakeError | undefined, stmt: RowStatement | FileAndStageBindStatement, rows: any[] | undefined) => void; }' but required in type 'StatementOption'.ts(2345)
index.d.ts(604, 9): 'sqlText' is declared here.

This code passes the typescript validation, but I don't know if it actually works.

    connection.getResultsFromQueryId({
      // not optional (bug) so need to pass empty string
      sqlText: "",
      queryId: queryId,
      complete: function (err, stmt, rows) {
        if (err) {
          console.error(
            "Failed to execute statement due to the following error: " +
              err.message
          );
          reject(err);
        } else {
          resolve(rows);
        }
      },
    });
@sjclemmy sjclemmy added the bug Something isn't working label Dec 2, 2024
@github-actions github-actions bot changed the title Typescript: getResultsFromQueryId expects option of type StatementOption. sqlText should therefore be optional. SNOW-1842116: Typescript: getResultsFromQueryId expects option of type StatementOption. sqlText should therefore be optional. Dec 2, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_done Initial triage done, will be further handled by the driver team label Dec 4, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

hi - thanks for raising this and providing the details too! we'll take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants