fix(sql): fused SUM/AVG folds over double/text columns, SQLite bridge column typing

[bplatz]

Three correctness fixes for SQL graph sources, found by replaying the SQL pushdown lane's cases against a real SQLite database. They are independent of the lane, so they land ahead of it.

Fused SUM/AVG folds silently summed 0. The fused R2RML aggregate operator skipped any physical double or text column mapped as xsd:decimal or xsd:integer, so SUM, AVG and COUNT over such a column returned 0 with no error. Both accumulators now convert any numeric or text column, widening the running scale as needed. text_typed_numeric_columns_fold_like_the_generic_path pins it.

The bridge failed to decode a SQLite NUMERIC cell whose storage class differed from its declared type. SQLite stores 5.00 in a NUMERIC column as an INTEGER and 99.50 as a REAL. Cells are now read by conversion to the reported type.

The bridge typed a SQLite column by its first row. sqlx cannot parse NUMERIC or DECIMAL(10,2) as a declared type and falls back to the first row's storage class, so a column whose first value was 5.00 reported as bigint and every later 99.50 came out as 99; a NULL first row left it varchar. The bridge now reads each column's declared type through sqlite3_column_decltype and maps it by SQLite's affinity rules, so numeric-affinity columns are double whatever the first row holds. Expression columns keep the driver's inference. numeric_columns_are_typed_by_declaration_not_first_row creates the mixed-storage, NULL-first and expression cases and fails with either the declared-type lookup or the conversion removed.

Based on feature/iceberg-static-policy (#1759); the lane branch will be rebased onto this.

[aaj3f]

@bplatz the issue and the design bridge for the fixes makes sense. I'd have no notes myself, but Claude went deep (comments below) and found a few things that are significant enough to deserve being addressed prior to merge. Claude review & comments to follow:

---

Three genuine bugs, @bplatz, and the bridge half is exactly right: typing a SQLite column by its declared type under SQLite's own affinity rules, read through the libsqlite3-sys sqlx already links, with double for NUMERIC affinity so 5.00-as-INTEGER and 99.50-as-REAL both survive, and unchecked cell conversion so a mixed-storage page doesn't error halfway through. I verified rather than read: the bridge suite is 5/5 including the new test, and bypassing the declared-type lookup makes it fail with bigint for both numeric columns; fmt/clippy are clean on both the bridge and fluree-db-query; the fused unit test runs by name.

The fold-in is on the fused half. The commit's rationale is "the generic path converts each value, so the fold must too" — but the generic lane converts via to_string() (shortest round-trip, 19.99 → "19.99") while the fold uses BigDecimal::try_from(f64), the exact binary expansion (scale 48 for 19.99). Probing at HEAD: for Float64 that overflows the i128 fold on the first non-dyadic row, so overflowed flips and the query re-runs on the exact pipeline — right answer, but the fast path never fires for exactly the SQLite NUMERIC-as-double case this PR targets (only dyadic values like the test's 99.5/5.0/2.125 fold). For Float32 the expansion fits, so the fold accepts it and SUM([19.99, 5.0]) renders 24.9899997711181640625 where the generic lane renders 24.99. Parsing v.to_string() instead fixes both in two lines. Alongside it, the Err(_) => true arms silently omit an unparseable cell where the generic lane poisons the group (agg_sum → Unbound, agg-err-01) — returning false escalates to the exact pipeline, which is what this file's own :250 rule asks for.

Also worth knowing: nothing ran in CI for this head (stacked on #1759), so the gates above are local; it should land after #1759 or be retargeted so the Linux lane sees it.

Adherence to repo commitments:

• Patterns/abstractions: ✔ bridge extends its existing type mapping with the declared-type source; fold extends the existing accumulator; ⚠️ fold's conversion should reuse the generic lane's lexical rather than a second one.
• Performance (speed first, memory second): ⚠️ no regression (BASE was wrong; the exact pipeline is what non-fused queries run) but the fused fast path is not delivered for non-dyadic double columns as written — see the inline note.
• Testing: ✔ bridge test covers mixed-storage / NULL-first / expression and is mutation-red; fused unit test pins the text and dyadic-double shapes; ⚠️ no non-dyadic double or Float32 case, which is why the two divergences are invisible to it.
• Conventions: ✔ three focused, well-explained commits; docs updated; libsqlite3-sys pinned to sqlx's version with a comment; unsafe justified inline.

Verified locally at branch HEAD 0256fd864: fluree-sql-bridge cargo test --test sqlite_protocol → 5 passed; mutation (declared → inferred) → FAILED as expected, restored; bridge fmt/clippy -D warnings clean; cargo test -p fluree-db-query fused_aggregate → 66 passed; cargo clippy -p fluree-db-query --all-targets -- -D warnings + fmt --check clean; throwaway probes of try_from(19.99) (exp 48 vs 2), Float64[19.99] (fold bails, count=0), Float32[19.99,5.0] (24.9899997711181640625), String["abc"] (accepted, count=0), all removed, worktree clean.

Approving so you can land this behind #1759 — but I'd fold the to_string() conversion in first, since it's the fast path the PR is for.

[aaj3f]

Should-fix (performance + lane parity, fold in now). BigDecimal::try_from(f64) is the exact binary expansion, not the lexical the generic lane materializes.

column_to_string (fluree-db-r2rml/src/materialize/term.rs:501-502) renders f32/f64 cells with to_string() — shortest round-trip, so 19.99f64 → "19.99" — and the generic agg_sum parses that. Here 19.99f64 becomes 19.989999999999998436805981327779591083526611328125 (scale 48), so add_exact has to rescale the i128 sum by 10^48, checked_pow overflows, the arm returns false on the first row, overflowed flips (:2118-2122) and the whole query re-runs on the exact pipeline.

I probed it at HEAD: accumulate_exact_row(Float64[19.99]) → false, count=0. Correct answer, but for SQLite NUMERIC (now reported double) holding money-shaped values the fused path never fires — only dyadic values like the test's 99.5/5.0/2.125 survive. For Float32 it's worse in a different way: f64::from(19.99f32) = 19.9899997711181640625 (scale 19) fits, so the fold accepts it and SUM([19.99f32, 5.0f32]) renders 24.9899997711181640625 where the generic lane gives 24.99 — two lanes, two lexicals.

Parsing the same lexical the generic lane uses fixes both:

Column::Float64(values) => match values.get(row) {
    Some(Some(v)) => match v.to_string().parse::<BigDecimal>() {
        Ok(d) => add_exact(d, sum, scale, count),
        Err(_) => false, // NaN/inf: escalate, the exact pipeline poisons correctly
    },
    _ => true,
},
Column::Float32(values) => match values.get(row) {
    Some(Some(v)) => match v.to_string().parse::<BigDecimal>() {
        Ok(d) => add_exact(d, sum, scale, count),
        Err(_) => false,
    },
    _ => true,
},

Not a regression — BASE skipped these rows and reported 0 — but it's the fast path this PR set out to restore, and a two-line change. If you agree it's right I'd rather see it here than in the backlog.

[bplatz]

Agreed and folded in (3ae69b8). Both float arms now go through accumulate_lexical(&v.to_string(), …), joining the text arm that already parsed a lexical.

Confirmed your probe independently before touching it: BigDecimal::try_from(19.99f64) has exponent 48 and 10i128.checked_pow(48) is None, so the f64 fold bailed on the first row; 19.99f32 expands to exponent 19, which fits, so it was accepted. Reverting only the f32 arm now reports (249899997711181640625, 19) against the expected (2499, 2) — the 24.9899997711181640625 you predicted, exactly.

The existing test could not see either shape because 99.5/5.0/2.125 are all dyadic. float_columns_fold_on_the_generic_lane_lexical adds the non-dyadic double and the f32 case, and goes red on either arm reverted.

[aaj3f]

Should-fix (semantics, fold in now). Err(_) => true on an unparseable text cell (and the try_from Err arms above for NaN/∞) accepts the row and adds nothing, where the generic lane materializes "abc"^^xsd:decimal, binding_to_numeric fails, and agg_sum/agg_avg poison the group to Unbound (fluree-db-query/src/aggregate.rs:895-905, SPARQL §18.5.1 / agg-err-01).

This file's own rule at :250 is that such a row "must escalate to the exact BigDecimal pipeline rather than silently omit" — returning false does that, and the exact pipeline then poisons the way the generic lane does. Probed at HEAD: String["abc"] → true, count=0.

Pre-existing in spirit (BASE skipped the whole column), but now that the arm exists it should agree with the lane it mirrors.

[bplatz]

Right, and fixed in the same commit. accumulate_lexical returns false on an unparseable lexical, which escalates to the exact pipeline and lets it poison the group the way agg_sum does.

Folding the three arms into one helper also picked up NaN and infinity for free: they arrive as "NaN"/"inf" from to_string(), fail the parse, and take the same escalation the old try_from Err arms silently swallowed. unconvertible_exact_cells_escalate_rather_than_omit covers "abc" plus f64 NaN, f64 infinity and f32 NaN, and asserts a null still returns true so the legitimate drop is not caught up in it.

[aaj3f]

Praise. add_exact may rescale sum/scale and then bail on the digit multiply, and that's fine: the pair still denotes the same value and the caller discards the accumulator on false. Worth a one-line comment so nobody "fixes" it into a two-phase check.

[bplatz]

Good call — comment added on add_exact saying the mid-rescale bail is deliberate, that the rescaled sum/scale pair still denotes the same value, and that the caller discards the accumulator wholesale on false.

[aaj3f]

Praise, and one question. declared_type reproduces SQLite's documented affinity order exactly (INT → CHAR/CLOB/TEXT → BLOB/empty → REAL/FLOA/DOUB → NUMERIC), and double for NUMERIC affinity is the one Trino type every storage class converts to.

The question, more than a suggestion: an INTEGER-affinity column can still hold a REAL cell (SQLite keeps 2.5 as REAL when it can't convert losslessly), and with try_get_unchecked::<i64> that cell now decodes as 2 where the old checked decode errored mid-page. I don't think it's common, but silent truncation is the worse failure of the two — maybe a per-cell sqlite3_column_type check that falls back to f64 for a REAL cell in a bigint column, if that's cheap in this loop. Happy to be told it isn't worth it.

[bplatz]

Worth it — and it turned out worse than either of us framed it. Measured through the bridge on an INTEGER-declared column:

stored	typeof	emitted before	emitted now
1, 3.0	integer	1, 3	unchanged
2.5	real	2	error
abc	text	0	error
x'0102'	blob	0	error

So not only near-integer truncation: a text or blob cell emitted a fabricated 0, indistinguishable from a real zero, and it feeds straight into the SUM/AVG path this PR is fixing on the other side. The pre-PR checked decode errored on all three, so 81f46795c turned a loud failure quiet — that is what decided it for me rather than the frequency.

Your suggested remedy does not quite work though: falling back to f64 for the cell would contradict the bigint already published in ColumnMeta, so the honest choice is truncate-or-error, not truncate-or-convert. Fixed in f95fe7c by erroring, which restores the pre-PR posture for exactly the bad-data case while leaving the mixed-storage case the unchecked decode was added for untouched — NUMERIC/DECIMAL map to double and never reach that arm.

Two things I checked rather than assumed, since the guard rests on both: sqlx's raw.type_info().name() reports the cell's storage class, matching SQLite's own typeof() on every case; and SQLite converts every losslessly-integral value on insert, so 3.0 in an INT… column is stored as an integer and still reads. A cell that survives as REAL/TEXT/BLOB there is therefore always bad data, never a representation choice.

One gotcha the test caught the hard way: CAST(v AS NUMERIC) is not a workaround, because an expression column has no sqlite3_column_decltype and falls back to the driver's first-row inference. The remedy is declaring the column NUMERIC, which is what the error message and docs/graph-sources/sql.md now say.

[aaj3f]

Praise. The unsafe block is bounded the way I'd want: prepare and finalize under lock_handle, c_sql outlives the call, and SQLite's contract leaves *ppStmt NULL on a failed prepare so the early return leaks nothing. The second prepare (sqlx's describe already did one) is SQLite-local and not worth avoiding.

[aaj3f]

Praise. Mixed-storage, NULL-first, and expression-column cases in one test, and it goes red the moment the declared-type lookup is bypassed (I checked: ["bigint","bigint",…]).

[bplatz]

Thanks for the depth here — all three actionable findings are in, plus the one you raised as a question.

• 3ae69b8 — float arms fold the generic lane's to_string() lexical instead of the exact binary expansion, and an unreadable cell escalates rather than being silently omitted. Two new tests cover the non-dyadic double, the f32 divergence and the escalation.
• f95fe7c — a bigint column now rejects an off-type SQLite cell instead of coercing it. This was the open question on sqlite.rs:48; it was worse than described (a text or blob cell emitted a fabricated 0, not just a truncated 2), and it was a regression this PR introduced, so it belonged here rather than in the backlog. Details in that thread.
• b5866cf — docs, since the previous wording promised that every cell converts, which the guard makes untrue for bigint.

Each new test is mutation-verified: reverting the fix it pins turns it red, and reverting only the f32 arm reproduces your 24.9899997711181640625 exactly.

On CI — #1759 has merged and this is based on main now, so the full matrix is running against this head rather than nothing. Worth a look before merge; the sql-bridge job in particular is the first run where the MySQL and Postgres protocol tests actually execute, since they self-skip locally and report as passing.