Skip to content

Fix truediv binary#672

Merged
sfc-gh-jcieslak merged 7 commits into
mainfrom
jcieslak/fix-truediv-workflow
Apr 20, 2026
Merged

Fix truediv binary#672
sfc-gh-jcieslak merged 7 commits into
mainfrom
jcieslak/fix-truediv-workflow

Conversation

@sfc-gh-jcieslak

@sfc-gh-jcieslak sfc-gh-jcieslak commented Apr 15, 2026

Copy link
Copy Markdown
Member

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-2503709: visit_truediv_binary does not have backward compatibility #618

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding new credentials
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    This PR restores backward-compatible SQL compilation for true division (/) when force_div_is_floordiv=True by delegating visit_truediv_binary() to SQLAlchemy’s base implementation, which preserves the expected CAST(... AS NUMERIC) behavior. Additional tests were needed because this was a compiler-level regression in generated SQL, so runtime result checks alone would not catch it.

The coverage was added to the existing compiler tests in tests/test_compiler.py (test_division_operator_with_force_div_is_floordiv_default_true and test_division_operator_with_denominator_expr_force_div_is_floordiv_default_true), which already verify nearby / and // behavior for both force_div_is_floordiv=True and False. This is not a new breaking change; it restores the intended behavior of the existing compatibility path while leaving force_div_is_floordiv=False unchanged.

@sfc-gh-jcieslak sfc-gh-jcieslak changed the title wip Fix truediv binary Apr 15, 2026
@sfc-gh-jcieslak sfc-gh-jcieslak marked this pull request as ready for review April 16, 2026 11:31
@sfc-gh-jcieslak sfc-gh-jcieslak requested a review from a team as a code owner April 16, 2026 11:31
@sfc-gh-mraba sfc-gh-mraba self-requested a review April 17, 2026 08:08

@sfc-gh-mraba sfc-gh-mraba left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe tests for true div require adjustment.

@sfc-gh-jcieslak sfc-gh-jcieslak merged commit 433c07b into main Apr 20, 2026
63 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the jcieslak/fix-truediv-workflow branch April 20, 2026 12:28
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SNOW-2503709: visit_truediv_binary does not have backward compatibility

2 participants