databricks / databricks-sql-nodejs

Databricks SQL Connector for Node.js
Apache License 2.0
24 stars 32 forks source link

Fetching structs containing datetimes fails silently #103

Open jnj16180340 opened 1 year ago

jnj16180340 commented 1 year ago

In this step,
https://github.com/databricks/databricks-sql-nodejs/blob/c99dfd1762420bd50d7f244d3dcef0accf2e376b/lib/result/JsonResult.ts#L121

https://github.com/databricks/databricks-sql-nodejs/blob/c99dfd1762420bd50d7f244d3dcef0accf2e376b/lib/result/JsonResult.ts#L89

If the data being parsed contains datetimes, it fails and returns defaultValue. Example error and data are below:

SyntaxError: Unexpected number in JSON at position 39
    at JSON.parse (<anonymous>)
    at JsonResult.toJSON (./databricks-sql-nodejs/dist/result/JsonResult.js:93:25)
    at JsonResult.convertData (./databricks-sql-nodejs/dist/result/JsonResult.js:64:29)
    at ./databricks-sql-nodejs/dist/result/JsonResult.js:47:25
    at Array.map (<anonymous>)
    at JsonResult.getSchemaValues (./databricks-sql-nodejs/dist/result/JsonResult.js:43:35)
    at ./databricks-sql-nodejs/dist/result/JsonResult.js:27:62
    at Array.reduce (<anonymous>)
    at JsonResult.getRows (./databricks-sql-nodejs/dist/result/JsonResult.js:27:28)
    at ./databricks-sql-nodejs/dist/result/JsonResult.js:16:31

The string that's failing to parse is below (partially scrubbed) '{"id":414247,"created_at":2021-12-21 21:33:59.339,"updated_at":2021-12-21 21:33:59.339,"deleted_at":null,"s3_bucket":"thebucket","s3_key":"c411f24d-1b4a-4eb0-b25b-d2287c7ba3c0"}'

Also, would it make sense to at least log a warning if parsing fails and returns the default value?

kravets-levko commented 1 year ago

Hi @jnj16180340! First of all, thank you for reporting this issue, and also I really appreciate all the additional info you provided. I can see that json is definitely malformed, and it probably is some bug. I need to investigate it, but I'm pretty sure that bug is on backend. And I totally agree that library should emit some warning in such cases, that's what we'll definitely implement. I'll keep you posted if any news on this issue. Sorry for the inconvenience

jnj16180340 commented 1 year ago

Thanks @kravets-levko - Still working on understanding everything going on under the hood, however if "backend" means the remote SQL Warehouse it seems unlikely since the JDBC interface and python databricks-sql both handle structs+arrays without issue

kravets-levko commented 1 year ago

Thank you @jnj16180340, that's very important update!

kravets-levko commented 1 year ago

@jnj16180340 sorry for being silent for so long. A very quick update re. your issue. It looks that there is some bug with serializing complex structures, we're trying to figure it out. JDBC and python drivers use Apache Arrow to represent query results by default and very likely that's why they work (complex structures are handled by Arrow and not represented as JSON). Node driver uses JSON-based results format and that's why it is affected by this bug. Arrow support for Node driver is currently in progress (/databricks/databricks-sql-nodejs#94) - I checked your case on that branch and can confirm that with Arrow support enabled it works fine. I'll keep you posted once I have more news. Sorry for the inconvenience