Skip to content

[DE-4902] enforcing string casting when column type is INTEGER with (38,0) precision and scale#12

Merged
gregosaurus merged 3 commits into
upstream-masterfrom
de-4902-fix-int-overflow-in-cursor
Jun 27, 2025
Merged

[DE-4902] enforcing string casting when column type is INTEGER with (38,0) precision and scale#12
gregosaurus merged 3 commits into
upstream-masterfrom
de-4902-fix-int-overflow-in-cursor

Conversation

@gregosaurus

@gregosaurus gregosaurus commented Jun 26, 2025

Copy link
Copy Markdown

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

Fixing JS Int overflow by casting all INTEGER types to String

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Comment thread redash/query_runner/snowflake.py Outdated
for row in cursor:
row_dict = {}
for column, value in zip(columns, row):
if column["type"] == TYPE_INTEGER:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[self-review] this isn't going to work since line 93 the type is changed.

@gregosaurus gregosaurus changed the title DE-4902: enforcing string casting when column type is INTEGER with (38,0) precision and scale [WIP][DE-4902] enforcing string casting when column type is INTEGER with (38,0) precision and scale Jun 26, 2025
@gregosaurus gregosaurus changed the title [WIP][DE-4902] enforcing string casting when column type is INTEGER with (38,0) precision and scale [DE-4902] enforcing string casting when column type is INTEGER with (38,0) precision and scale Jun 27, 2025
@inytar

inytar commented Jun 27, 2025

Copy link
Copy Markdown

Hey Greg,
Let's take an other approach here.
The issue is clearly in the json -> javascript object. As we can see in this screenshot.
Screenshot 2025-06-27 at 07 43 00

Let's try and tackle that. A bit of investigations shows that Redash uses axios as helper to do http calls. And axios is automatically transforming the json response to a javascript object.

The good part is that we can overwrite this transformResponse function in the axios config.
Now we just use a json library that works with big numbers, and we should be good.

@gregosaurus gregosaurus requested a review from a team June 27, 2025 05:50
Comment thread redash/query_runner/snowflake.py Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you doing here? Is this a if scale is 0? Could you add a comment to make that clear.

@gregosaurus gregosaurus Jun 27, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll enhance, i refactored to make this part "shorter" but i do agree it's blurry now.

Basically cursor.description has different metadata for each index (see: https://docs.snowflake.com/en/developer-guide/python-connector/python-connector-api#description), index 1 being the data_type code.

The snowflake doc is blurry in terms of what is provided as metadata in description in case of INTEGER:

Synonymous with NUMBER, except that precision and scale can’t be specified (that is, it always defaults to NUMBER(38, 0))

Does that mean that precision and scale are set to None in the description object when it's a pure integer ? but when it is a float it is ? the existing code from redash team in snowflake query_runner might lead me to think so:

def determine_type(cls, data_type, scale):
        t = TYPES_MAP.get(data_type, None)
        if t == TYPE_INTEGER and scale > 0:
            return TYPE_FLOAT
        return t

TL;DR, to fix the issue i had to check on the type as it was done by the snippet above, minus the scale.

(further read https://docs.snowflake.com/en/developer-guide/python-connector/python-connector-api#label-python-connector-type-codes and https://docs.snowflake.com/en/sql-reference/data-types-numeric)

Comment thread redash/query_runner/snowflake.py Outdated
Comment on lines 150 to 152

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this change all values types that aren't number to strings?

Suggested change
if column[1] == 0 and column[5] > 0:
row_dict[column_name] = value
else:
# Cast INT to STRING to avoid potential overflow in JS
row_dict[column_name] = str(value)
if column[1] == 0 and column[5] == 0:
# Cast INT to STRING to avoid potential overflow in JS
row_dict[column_name] = str(value)
else:
row_dict[column_name] = value

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed the wrong commit...

@gregosaurus gregosaurus force-pushed the de-4902-fix-int-overflow-in-cursor branch from cfd6e54 to 1359a65 Compare June 27, 2025 08:51
@gregosaurus gregosaurus merged commit fd3f8be into upstream-master Jun 27, 2025
2 checks passed
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 this pull request may close these issues.

2 participants