fix: route every pd.to_numeric call through a shared exponent-overflow guard - #409
Merged
Merged
Conversation
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
pandas < 3 reads the exponent digits of a numeric string into a C int with no overflow check, before it rejects trailing text (pandas-dev/pandas#62617, #63089, #63167; fixed in pandas 3.0 by pandas-dev/pandas#62741). #407 masked such cells in dtype inference only; about 30 other pd.to_numeric calls (domain validators, fieldcheck, CSV leading-zero detection, time-series scoring, MissForest, semantic checks, learning) still handed hash-like text such as "81e3104049863b72" straight to the parser and could crash the process. Add freshdata._numeric.safe_to_numeric, which keeps those cells away from pandas and otherwise forwards to pd.to_numeric unchanged (errors=, downcast=, dtype_backend=, index, name and dtype). A cell is guarded only when its leading exponent has ten or more significant digits, the smallest size that can overflow the C int accumulator; every shorter exponent, including subnormal and out-of-range values, is parsed exactly as pandas parses it. Numeric, boolean and datetime inputs skip the check; text is screened as one joined string, so clean columns stay close to free. The guard pieces move there and dtypes.py imports them, so dtype inference uses the same bound and keeps valid subnormal and underflow values. Calls on provably numeric dtypes stay direct, and a static test fails when a new unguarded call appears.
kevincostner17
force-pushed
the
fix/to-numeric-shared-guard
branch
from
September 15, 2026 13:22
a02b481 to
7a651fd
Compare
This was referenced Sep 15, 2026
kevincostner17
added a commit
that referenced
this pull request
Sep 15, 2026
…#417) On pandas < 3, pd.to_numeric reads the exponent digits of a numeric string into a C int with no overflow check, before it rejects trailing text (pandas-dev/pandas#62617). A cell such as "81e3104049863b72" can segfault the process whatever errors= says. enterprise/contracts.py still had four direct calls. The one in _contract_values runs on any non-datetime column with min_value or max_value, so a text column holding such a token reached the parser from enforce_contract and compare_to_baseline(contract=...). The other three (_profile_column, _ks_statistic, _psi_numeric) only see int/float-family columns, but go through the guard too so the file needs no exemption. safe_to_numeric hands input with no unsafe cell to pandas untouched, so results are unchanged. This completes the migration from #409: _DEFERRED in tests/test_numeric.py is now empty and the static guard covers contracts.py. New tests run enforce_contract, build_baseline and compare_to_baseline on crash tokens in a child interpreter behind a to_numeric tripwire, and check parity with raw pd.to_numeric on ordinary frames.
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
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.
Summary
Builds on the merged #407. pandas < 3 can segfault in
pd.to_numericon text that starts with a scientific-notation token whose exponent overflows a C int, even when trailing text follows (for example"81e3104049863b72"). #407 guarded dtype inference only. About 30 other directpd.to_numericcalls insrc/could still pass such text to the parser and crash the process: domain validators, fieldcheck, CSV leading-zero detection, time-series scoring, MissForest, semantic checks and learning.freshdata._numeric.safe_to_numeric(values, **kwargs)keeps unsafe cells away from pandas and otherwise forwards topd.to_numericunchanged (errors=,downcast=,dtype_backend=; index, name and dtype preserved). It accepts a Series, Index, array-like or scalar.errors="coerce", raise pandas' ownUnable to parse stringerror witherrors="raise", and come back unchanged witherrors="ignore"._numeric.py, andsteps/dtypes.pyimports them, so dtype inference uses the same bound.streaming/_state.py,streaming/_drift.py,imputation/missforest.py_features, andsteps/memory.py.Behaviour change: dtype inference now keeps valid subnormal and underflow values (e.g.
"4.9e-324","1e-310", and"5e-400"→ 0.0). The previous guard masked any exponent beyond ±308 and dropped them to missing.Root cause
pandas < 3
precise_xstrtodaccumulates the exponent digits in a C int (n = n * 10 + digit, up to 17 digits). It adds that to a mantissa adjustment with no overflow check, and does so before it rejects trailing text. See pandas-dev/pandas#62617, #63089 and #63167; the fix is in pandas 3.0 via pandas-dev/pandas#62741. This repo pinspandas<3. #407 fixed the prefix match in dtype inference; this PR applies the guard to every other call site and narrows it to exponents that can actually overflow.Tests
tests/test_numeric.py:pd.to_numericin everyerrors=mode and withdowncast, across Series, Index, list, ndarray and scalars. Inputs are 4,000 seeded hex tokens plus subnormal, underflow, out-of-range and 9- vs 10-digit exponent tokens. The raw pandas baseline runs in a child interpreter, so a missed crash token fails the test instead of killing the run.1e999999999and1e0000000001are not guarded.1e1000000000,1e2147483648and a 5000-digit exponent are.1e-1000000000. It has a ten-digit exponent, so it is guarded, while pandas underflows it to 0.0. A test records this.string, bytes, categorical, NaN/None, empty input, numeric passthrough,downcast,dtype_backend(pandas ≥ 2), invalid arguments and 2-D input.pd.to_numericchecks that coerce, raise and ignore never hand an overflowing exponent to pandas.-X faulthandler, with a timeout) runsfd.validate_fields,run_domain(..., "finance")andfd.cleanon crash tokens, with the same tripwire.src/freshdataand fails on anyto_numericreference outside_numeric.py, the call sites left for a follow-up, and the justified numeric-only sites.tests/test_dtypes.py:Verification
src/freshdata: no issues (204 files).-m "not online and not large":safe_to_numeric, identical on pandas 2.3.3 and 1.5.3:epd.to_numericesafe_to_numericeOn a text column, the helper adds about 17 ms per 1M rows. That is the fixed cost of the joined-string screen. Float columns cost nothing.
validate_fieldsand the finance domain validator.Remaining call sites
Left for a follow-up PR:
src/freshdata/enterprise/contracts.py:779src/freshdata/enterprise/contracts.py:877src/freshdata/enterprise/contracts.py:948src/freshdata/enterprise/contracts.py:1793When migrating these, remove their entries from
_DEFERREDintests/test_numeric.py.