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

Fix DBMS_RANDOM.VALUE 38-digit precision by btlqql · Pull Request #2310 · IvorySQL/IvorySQL · GitHub

Repository navigation

Fix DBMS_RANDOM.VALUE 38-digit precision - #2310

Open
btlqql wants to merge 1 commit into
IvorySQL:masterfrom
btlqql:codex/dbms-random-value-precision
Open

btlqql wants to merge 1 commit into
IvorySQL:masterfrom
btlqql:codex/dbms-random-value-precision

Conversation

btlqql commented Oct 3, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes #2308.

DBMS_RANDOM.VALUE converted a float8 draw to NUMBER, limiting its random fraction to roughly 16 decimal digits. This change builds a 38-place NUMBER fraction from two unbiased 19-digit draws. Both the unit interval and range overloads use the same helper. The regression SQL checks the decimal scale, bounds, and the ordering call without parentheses.

Verification

  • Changed dbms_random.c compiled with the existing MSYS2/Cygwin GCC toolchain.
  • pgindent --check on the changed C file and git diff --check passed.
  • A full local make -j4 stopped while linking the untouched liboracle_parser.dll (__imp_OraScanKeywordTokens unresolved). The SQL regression suite could not run locally in that build.

AI assistance

Assisted-by: OpenAI:GPT-6
Percentage of AI-generated code: 100%

Summary by CodeRabbit

  • Bug Fixes
    • DBMS_RANDOM.VALUE() and bounded VALUE(low, high) now retain 38 decimal places, including when using large bounds.
    • DBMS_RANDOM.STRING can now return a backslash as part of its printable character set.

Generate exact 38-place NUMBER fractions from unbiased decimal draws for both VALUE overloads. Cover the result scale and range behavior in the regression suite.

Assisted-by: OpenAI:GPT-6

Percentage of AI-generated code: 100%

coderabbitai Bot commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration
  • Configuration used: Repository: IvorySQL/IvorySQL/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 34ccb63b-55c7-4904-87d0-c9c2da486f24
📥 Commits

Reviewing files that changed from the base of the PR and between 069766e and a67b26d.

📒 Files selected for processing (3)
  • contrib/ivorysql_ora/expected/dbms_random.out
  • contrib/ivorysql_ora/sql/dbms_random.sql
  • contrib/ivorysql_ora/src/builtin_packages/dbms_random/dbms_random.c

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

DBMS_RANDOM now builds random fractions from two 19-digit PRNG draws and uses them in both VALUE overloads. Regression tests check scale 38. The STRING printable character set adds a backslash.

Changes

VALUE precision

Layer / File(s) Summary
Build and apply numeric fractions
contrib/ivorysql_ora/src/builtin_packages/dbms_random/dbms_random.c
A new helper builds a 38-digit fractional Numeric from two PRNG draws. Both VALUE overloads use it for random results; equal bounds still return the bound.
Check VALUE precision
contrib/ivorysql_ora/sql/dbms_random.sql, contrib/ivorysql_ora/expected/dbms_random.out
Seeded regression tests check scale 38 for unbounded VALUE and VALUE with large bounds. The expected row order for ORDER BY dbms_random.value changes.

STRING character set

Layer / File(s) Summary
Update STRING character set and handling
contrib/ivorysql_ora/src/builtin_packages/dbms_random/dbms_random.c
The printable character set adds a backslash. Nearby length checks and conversion are reformatted; the described length behavior remains unchanged.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: unbridled-41

Merge Risk: ⚪ Minimal · up to a67b2

The precision change has no established merge-blocking issue. The exclusive-upper-bound edge case predates this change.

Architecture Summary

Architecture risk: 🟡 Medium · up to a67b2

The change affects 1 system.

Changed systems: contrib

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — contrib (service) was modified; 3 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in contrib/ivorysql_ora/expected/dbms_random.out: Adds a regression check that dbms_random.value() has scale 38 after seeding.
  • observed — Modified behavior in contrib/ivorysql_ora/expected/dbms_random.out: The expected order for ORDER BY dbms_random.value changes from 5, 4, 2, 3, 1 to 3, 5, 4, 2, 1.
  • observed — Modified behavior in contrib/ivorysql_ora/expected/dbms_random.out: Adds a regression check that a VALUE call with large NUMBER bounds returns a value with scale 38.
  • observed — Modified behavior in contrib/ivorysql_ora/sql/dbms_random.sql: Adds a seeded VALUE() test asserting that its result has scale 38.

Reliability and maintainability

  • inferred — Risk-relevant change factors for contrib: blast_radius_2; direct_dependents_2
🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the DBMS_RANDOM.VALUE precision fix, which is the pull request’s main change.
Linked Issues check ✅ Passed [ #2308 ] Both VALUE overloads now use random_fraction(), which combines two uniform 19-digit PRNG draws into a 38-place NUMBER fraction. The unit overload returns this fraction directly. The range ov…
Out of Scope Changes check ✅ Passed The SQL and expected-output changes test the #2308 behavior. The other C edits change formatting only; the printable character-set literals are unchanged. No unrelated behavior change is evident.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (2 skipped: 2 …
✨ Finishing Touches 🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator

Please run the DBMS_RANDOM regression suite in a working build and add a check that digits beyond the previous precision limit vary, since scale() = 38 alone only verifies decimal scale.

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

DBMS_RANDOM.VALUE loses Oracle NUMBER random precision through float8

2 participants


Back | FazBrowse Home | New Git URL