FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix float inference to use DoubleParameter (64-bit) instead of FloatP… by Shubhambhusate · Pull Request #742 · databricks/databricks-sql-python · GitHub

/ databricks-sql-python Public

Fix float inference to use DoubleParameter (64-bit) instead of FloatP… - #742

Merged
jprakash-db merged 2 commits into
databricks:mainfrom
Shubhambhusate:fix/float-to-double-parameter-inference
Mar 10, 2026
ConversationCommits (2)ChecksFiles changed
Merged

Fix float inference to use DoubleParameter (64-bit) instead of FloatP…#742
jprakash-db merged 2 commits into
databricks:mainfrom
Shubhambhusate:fix/float-to-double-parameter-inference

Conversation

Shubhambhusate commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

…arameter (32-bit)

What type of PR is this?

  • Bug Fix

Description

dbsql_parameter_from_primitive() in src/databricks/sql/parameters/native.py currently maps Python's float type to FloatParameter, which binds values as Databricks SQL FLOAT (32-bit single-precision). However, Python's float is a 64-bit IEEE 754 double-precision number, so this mapping silently truncates precision.
This PR changes the inference to use DoubleParameter instead, which binds values as Databricks SQL DOUBLE (64-bit) and correctly preserves the full precision of Python floats.

Before (buggy):

cursor.execute("INSERT INTO t (val) VALUES (?)", [12345.678901234])
# Stored as 12345.0 — precision lost silently

After (fixed):

cursor.execute("INSERT INTO t (val) VALUES (?)", [12345.678901234])
# Stored as 12345.678901234 — full precision preserved

How is this tested?

  • Unit tests
  • Manually

All 55 unit tests in tests/unit/test_parameters.py pass, including test_inference[DoubleParameter-Primitive.FLOAT].

Manually verified that parameterized queries with raw Python floats now preserve full 64-bit precision when inserted into DOUBLE columns.

Related Tickets & Documents

Fix float inference to use DoubleParameter (64-bit) instead of FloatP…
5f08474 
…arameter (32-bit)

Signed-off-by: Shubhambhusate <bhusates6@gmail.com>

Shubhambhusate commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

Issue link: #741

Shubhambhusate commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @msrathore-db Can you please review the PR

msrathore-db commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Hi @msrathore-db Can you please review the PR

Adding @jprakash-db for review as he has more context on the same

Shubhambhusate commented Feb 27, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jprakash-db Can you please review this PR?

gopalldb reviewed Feb 27, 2026
View reviewed changes
Comment thread tests/unit/test_parameters.py
gopalldb reviewed Feb 27, 2026
View reviewed changes

gopalldb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A minor comment on test, will also let Jothi also take a look

jprakash-db commented Feb 27, 2026

Copy link
Copy Markdown
Contributor

Overall LGTM.
I think this is fine, even if customers define FLOAT table and insert DOUBLE from client the server auto truncates so not an issue.

Shubhambhusate commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jprakash-db

Do we have any ETA for the PR to be merged, and the fix will be available?

jprakash-db commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

@Shubhambhusate Looks like the creds are not available in this forked branch to run the e2e. Let me run them manually and I will update the status here

jprakash-db merged commit 12bfd5b into databricks:main Mar 10, 2026
33 of 36 checks passed

jprakash-db commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

@Shubhambhusate We have merged it , thanks for the contribution

Shubhambhusate commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jprakash-db

Thanks for merging it. Could you please let me know when the fix will be available to the user?

jprakash-db commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

@Shubhambhusate We are going to cut a release in this week tentatively, so you can expect by coming Monday

Shubhambhusate commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the update.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

gopalldb gopalldb left review comments
jprakash-db jprakash-db approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL