Skip to content

fix(parameters): honor explicit decimal precision and scale - #988

Open
leninworld wants to merge 1 commit into
databricks:mainfrom
leninworld:fix-decimal-precision-scale-901
Open

leninworld wants to merge 1 commit into
databricks:mainfrom
leninworld:fix-decimal-precision-scale-901

Conversation

@leninworld

@leninworld leninworld commented Oct 18, 2026 •

Copy link
Copy Markdown

This PR addresses part of issue #901.

What type of PR is this?

  • Bug Fix

Description

Correct the explicit DecimalParameter type to use SQL's DECIMAL(precision,scale) order, and honor an explicitly declared zero scale. Correct the parameter descriptions and document the compatibility change for callers who followed the old reversed descriptions. The public signature, validation rules, automatic sizing and value payload are unchanged. Explicit zero precision is now sent as declared instead of silently using automatic sizing.

This is a partial fix: it does not change the issue's broader under-declared-scale/truncation semantics or claim that its entire conformance test now passes. The adjacent automatic-sizing work in PR #952 is untouched; its full patch applies cleanly alongside this change.

AI-assisted implementation, independently reviewed. @leninworld personally certified DCO 1.1 and confirmed the commit sign-off.

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually

Linux aarch64 Docker, Python 3.13.16, Poetry 2.5.1, official optional kernel wheel 1.1.0:

  • Before the fix: two new regression cases fail (DECIMAL(2,10) and DECIMAL(3,0)); the automatic-sizing control passes.
  • After: all 59 parameter tests pass. The ordinary unit suite with the kernel wheel installed reports 1,250 passed, 4 skipped, 351 subtests passed. Its one deselected real-kernel test passes in a separate fresh invocation as required by the project.
  • Source and touched-test Black checks pass; mypy reports no issues in 67 source files; source distribution and wheel builds pass.
  • The real local binding API produces DECIMAL(10,2) and DECIMAL(10,0) for explicit sizes, and preserves automatic DECIMAL(5,2) sizing. Named and positional bindings preserve their string payloads.
poetry run python -m pytest tests/unit/test_parameters.py -q
poetry run python -m pytest tests/unit -m 'not realkernel' -q -rs
poetry run python -m pytest tests/unit/test_session.py -m realkernel -v -rs
poetry run black --check src tests/unit/test_parameters.py
poetry run mypy --install-types --non-interactive src
poetry build

The four skips are existing upstream markers: two legacy interface cases, an outdated fetch benchmark, and a Thrift native-binding case. Eleven fixture/FutureWarning reports remain. No test, skip marker or warning filter was weakened.

Not run: live Databricks/e2e tests, kernel/SEA SQL-server behavior, other operating systems or the hosted CI matrix. No server-acceptance or performance claim.

Docker validation summary

Related Tickets & Documents

Refs #901 (explicit precision/scale and zero-scale handling only).

Render declared decimal types in SQL precision,scale order and preserve an
explicit zero scale. Correct the parameter descriptions and document the
compatibility change for callers that followed the reversed descriptions.

Add regression coverage through actual parameter binding, preserving automatic
sizing and the string payload. This addresses part of issue databricks#901 only.

Validated in Linux aarch64 Docker: 1250 ordinary unit tests and 351 subtests,
a separate real-kernel test, formatting, type checks and package builds pass.
Four existing tests remain skipped; no live Databricks/e2e tests were run.

Signed-off-by: Lenin Mookiah <lenin.world@gmail.com>

This branch has not been deployed

No deployments
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.

1 participant