GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [sqllib] Fix overflow bugs by mihaibudiu · Pull Request #7691 · feldera/feldera

Conversation

@mihaibudiu

@mihaibudiu mihaibudiu commented Oct 28, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Checklist

  • Unit tests added/updated

A code audit found misbehaviors in arithmetic around extreme values (e.g., abs(MIN_VALUE) was negative).
There are 4 commits, each with an explanatory commit message.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mihaibudiu
mihaibudiu requested a review from blp October 28, 2026 23:32
blp
blp reviewed Oct 28, 2026
View reviewed changes
Comment thread crates/fxp/src/dynamic.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Oct 28, 2026
View reviewed changes
Comment thread crates/fxp/src/dynamic.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mythical-fred-oss

mythical-fred-oss Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test --locked -p feldera-fxp ✅ 43 passed
cargo test --locked -p feldera-sqllib (abs/neg/round/interval filters) ✅
mvn test FunctionsTest, PostgresInt8Tests, BasicQueriesTests#negate*, CastTests#castInterval, TimeTests#makeTime*, VariantTests#numberWith* ✅ 67 passed
Extra QA (scratch tests, not pushed): -TINYINT min and ABS(SMALLINT) min report overflow; -/ABS on nullable columns; negating the minimum INTERVAL MONTH ✅
Extra QA: DynamicDecimal(-10^37) + 0.25 ❌ multiply overflow (see inline)

Findings

  • bug (medium): DynamicDecimal add with a large negative operand overflows in the fast path. It panics in debug builds and wraps silently in release builds, and it never reaches the fixed 256-bit branch. The bug predates this PR, but it is in the code being fixed, and add_wide only tests positive operands.
  • missing (medium), commit quality: [SQL] Do not warn when a Calcite optimization step fails has no body. It turns a user-visible compiler warning into a debug log, so the history should say why (is it the new non-foldable ABS making a reduce step throw?).
  • missing (low-medium), tests: no SQL-level test for ABS of the minimum TINYINT/SMALLINT/INT, or for -x with TINYINT/INT columns. I checked these by hand and they pass, but they aren't in the suite.

Otherwise the gates are met: tests cover each fix, docs are updated (integer.md), there are no new deps or unsafe blocks, and there is no flakiness risk.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

blp
blp approved these changes Oct 28, 2026
View reviewed changes
mihaibudiu added 5 commits October 28, 2026 21:08
@mihaibudiu
[sqllib] Fix DynamicDecimal arithmetic, years of an interval, and DEC…
9e7b2f3
…IMAL ROUND

- DynamicDecimal multiplication and the 256-bit branches of addition added
  the digits dropped from the result to the exponent instead of subtracting
  them.  CAST(DECIMAL AS INTERVAL DAYS) and MAKE_TIME with many fractional
  digits gave small wrong values.
- The conversion of a DynamicDecimal with more than 38 fractional digits to
  DOUBLE panicked, e.g. CAST(PARSE_JSON('1e-100') AS DOUBLE).
- LongInterval::years negated the months, which overflows for the minimum
  interval.
- ROUND and BROUND of a DECIMAL panicked when rounding more than 38 digits
  away from the last digit of the scale.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[SQL] Report overflow for ABS and unary minus of the minimum value
5e9ced5
- ABS of the minimum integer or interval, and unary minus of the minimum
  integer now report an overflow.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[sqllib] Fix ROUND and TRUNCATE of floating point values with many di…
2f9da40
…gits

10^digits is infinity or 0 for a large digits argument: ROUND(1.5e0, 400)
was NaN, and ROUND(1e300, 20) was infinity.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[SQL] Do not warn when a Calcite optimization step fails
4f7d6ab
Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
[fxp] Add DynamicDecimal values whose exponents differ by more than 38
bf0201e
Adding two DynamicDecimal values whose exponents differed by more than
38 panicked, because the value with fewer fractional digits was scaled
by a power of 10 that does not fit in an i128.  The datagen connector
adds such values for a DECIMAL range bound with more than 38 fractional
digits.  The sum now keeps the digits of the other value down to 37
places past the last digit of the first, and is truncated toward zero.

Signed-off-by: Mihai Budiu <[email protected]>
@mihaibudiu
mihaibudiu force-pushed the sqllib-overflow-bugs branch from 23d4993 to bf0201e Compare October 29, 2026 04:13
@mihaibudiu
mihaibudiu added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit b5a07bc Oct 29, 2026
1 check passed

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu deleted the sqllib-overflow-bugs branch October 29, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@blp blp blp approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@mihaibudiu @blp
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.