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-375565: How to fetchAsString but keep null values? #176

Closed
datbth opened this issue Jul 1, 2021 · 9 comments
Closed

SNOW-375565: How to fetchAsString but keep null values? #176

datbth opened this issue Jul 1, 2021 · 9 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team

Comments

@datbth
Copy link

datbth commented Jul 1, 2021

Please answer these questions before submitting your issue. Thanks!

  1. What version of NodeJS are you using (node --version and npm --version)?
    Node: v14.17.0
    Npm: 6.14.14

  2. What operating system and processor architecture are you using?

Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.2 LTS
Release:	20.04
Codename:	focal
  1. What are the component versions in the environment (npm list)?
    I'm using snowflake-sdk@1.6.1

  2. What did you do?
    Run a SQL with fetchAsString option:

connection.execute({
  sqlText: 'select null::float',
  fetchAsString: ['NUMBER'],
  complete (err, stmt, rows) {
    console.log(rows[0][0] === null);
    console.log(JSON.stringify(rows[0][0]));
  }
  1. What did you expect to see?
    Console logging true and null

  2. What did you see instead?
    Console logging false and "null"

  3. Add this to get standard output.

{"message":"[10:51:52.5152 AM]: Contacting SF: /session/v1/login-request?requestId=REQUEST_ID&warehouse=TEST&databaseName=TEST, (1/5)","level":"DEBUG"}
{"message":"[10:51:52.5152 AM]: SNOWFLAKE_URL/session/v1/login-request?requestId=REQUEST_ID&warehouse=TEST&databaseName=TEST","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: Reading OCSP cache file. /home/datbth/.cache/snowflake/ocsp_response_cache.json","level":"DEBUG"}
{"message":"[10:51:52.5152 AM]: Returning OCSP status for certificate 075EBC4CCBC3E05D031EF502C6175674 from cache","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: Returning OCSP status for certificate 067F94578587E8AC77DEB253325BBC998B560D from cache","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: Returning OCSP status for certificate 067F944A2A27CDF3FAC2AE2B01F908EEB9C4C6 from cache","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: Returning OCSP status for certificate A70E4A4C3482B77F from cache","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: socket reused = false","level":"TRACE"}
{"message":"[10:51:52.5152 AM]: OCSP validation succeeded for holistics.ap-southeast-2.snowflakecomputing.com","level":"TRACE"}
{"message":"[10:51:53.5153 AM]: SNOWFLAKE_URL/queries/v1/query-request?requestId=ab0b5126-6fe0-4762-94b0-96587c53b8ef","level":"TRACE"}
{"message":"[10:51:53.5153 AM]: socket reused = true","level":"TRACE"}

I would like to fetch all values as raw Strings, so that I don't have to worry about any lossy/distorted values due to Javascript processing. But currently, with fetchAsString option, null values are being turned into 'NULL' (NULL_UPPERCASE string). I could process through the non-text columns and revert 'NULL' values into nulls, but that would be very inefficient as the values have to be processed unnecessarily multiple times.
Thus, may I ask for the reasons behind the decision of this behavior of turning null into 'NULL'? And whether there are any better setup or workaround for my case?
Thank you.

@github-actions github-actions bot changed the title How to fetchAsString but keep null values? SNOW-375565: How to fetchAsString but keep null values? Jul 1, 2021
@barooo
Copy link

barooo commented Aug 2, 2022

Is there a resolution here? This seems to still be an issue.

@sfc-gh-dszmolka
Copy link
Collaborator

hi, thank you for submitting this issue. using latest (v1.6.18) of the driver for testing.

  1. executing select null, SYSTEM$TYPEOF(null) as typeof with or without fetchAsString returns
{ NULL: null, TYPEOF: 'NULL[LOB]' }
  1. executing select null::float, SYSTEM$TYPEOF(null::float) as typeof with fetchAsString: ['NUMBER'] returns:
{ 'NULL::FLOAT': 'NULL', TYPEOF: 'FLOAT[DOUBLE]' }

per the Snowflake documentation for Fetching Data Types as Strings

When connection.execute() is called, the fetchAsString option can be set to force all numbers or dates to be returned as strings.

as i see it, this is what is happening, forcing the number to be returned as string. ('NULL') so at first glance this seems to be the expected and documented behaviour: fetches number as string. When fetchAsString is omitted, the null seems to be returned as null

what happens in your case when you leave fetchAsString: ['NUMBER'] but do not transform the null into float so it would be fetched as string ?
also does perhaps the Snowflake function TO_VARCHAR provide any workaround? It should cast the column's value to string, and if it's NULL then leaves as NULL. Since the transformation is happening on server-side, you would not need fetchAsString in this case.

@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Feb 24, 2023
@datbth
Copy link
Author

datbth commented Feb 27, 2023

Hi @sfc-gh-dszmolka, thank you for the response.

what happens in your case when you leave fetchAsString: ['NUMBER'] but do not transform the null into float so it would be fetched as string ?

select null::float is only an example to demonstrate the issue.
I'm seeing the same issue when querying NULL values from a float column of a table. E.g.

CREATE TABLE a (x float);
INSERT INTO a VALUES (null);
SELECT x FROM a;

also does perhaps the Snowflake function TO_VARCHAR provide any workaround?

  • In my case, I'm developing a BI (Business Intelligence) tool that does not have control over the user's SQLs. But even in other post-processing use cases, especially in data processing pipelines, I think this would still be quite an inconvenient workaround and it's limiting the connector's usability in data processing pipelines.
  • More importantly, this issue is about the inefficiency of handling NULL values. I believe using something like TO_VARCHAR would still yield some unnecessary processing overhead.

@sfc-gh-dszmolka
Copy link
Collaborator

i see, thank you for the clarification @datbth . looking around a bit I found (maybe a bit late) that this item has already been submitted in 2021 to the team to pick up and prioritize, to implement the necessary capability into the driver. hopefully it can happen sometimes in the coming months. until then, we'll need to resort to some kind of workaround i'm afraid. (server side or client side)

@sfc-gh-dszmolka sfc-gh-dszmolka added enhancement The issue is a request for improvement or a new feature status-in_progress Issue is worked on by the driver team labels Mar 29, 2023
@MasterOdin
Copy link

I hit this today and was surprised by the behavior, where we were doing fetchAsString: [ 'Date' ]. Our expectation from the docs was that only actual date values would be turned into strings, not null as that's not a date. Minimally, it could be nice to have the docs be updated to indicate that null will be returned as the string 'NULL'?

Our workaround was to use patch-package to modify this line:

diff --git a/node_modules/snowflake-sdk/lib/connection/result/column.js b/node_modules/snowflake-sdk/lib/connection/result/column.js
index 58da09f..6d1045f 100644
--- a/node_modules/snowflake-sdk/lib/connection/result/column.js
+++ b/node_modules/snowflake-sdk/lib/connection/result/column.js
@@ -9,7 +9,7 @@ var SfTimestamp = require('./sf_timestamp');
 var SqlTypes = require('./data_types').SqlTypes;
 var bigInt = require('big-integer');
 
-var NULL_UPPERCASE = 'NULL';
+var NULL_UPPERCASE = null;
 
 /**
  * Creates a new Column.

I'd be kind of curious to know what the use case is for getting the string 'NULL' back in a result set. Is it just for making it easier to display the value?

@sfc-gh-dszmolka
Copy link
Collaborator

thank you for sharing your approach of how to work around this issue ! sadly I cannot answer the question why NULL_UPPERCASE is defined this way, it looks like per the blame, it was always like this. Doesn't mean needs to stay like this though. I'll keep this ticket posted once we have progress on it.

@sfc-gh-dszmolka sfc-gh-dszmolka removed the status-in_progress Issue is worked on by the driver team label Dec 28, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_done Initial triage done, will be further handled by the driver team label Feb 11, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-in_progress Issue is worked on by the driver team label Apr 29, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review and removed status-in_progress Issue is worked on by the driver team labels May 7, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

PR in review at #831

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels May 21, 2024
@sfc-gh-dszmolka
Copy link
Collaborator

PR merged and fix will be part of the next release cycle, probably end of May 2024

@sfc-gh-dszmolka
Copy link
Collaborator

released with May 2024 release cycle, version 1.11.0

thank you for your patience here !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

6 participants