fix: SPARQL 1.1 numeric arithmetic and expression ORDER BY correctness#1282
Merged
Conversation
Two related SPARQL 1.1 arithmetic bugs in the expression evaluator (ArithmeticOp::apply), found by the BSBM Business Intelligence query mix: - Mixed-type arithmetic such as xsd:float(?x) / ?int errored with a type mismatch. xsd:float casts are carried as a String-backed TypedLiteral, so the operand never matched a numeric arm. Coerce numeric TypedLiteral operands (xsd:float/double/decimal/integer) to their primitive variant before applying, letting XPath numeric type promotion proceed. - xsd:integer / xsd:integer truncated to an integer (10 / 4 -> 2). Per XPath op:numeric-divide, integer division yields xsd:decimal (10 / 4 -> 2.5). Long/Long and BigInt/BigInt division now return a decimal, capped at 34 significant digits (IEEE-754 decimal128), mirroring AVG. Adds unit tests plus SPARQL and JSON-LD parity integration tests.
Addresses review of the prior arithmetic commit. coerce_numeric_operand hand-rolled only a partial XSD numeric coercion (f64 for float/double, i64 for integer/long/int), so numeric typed literals like xsd:short, large xsd:integer values needing BigInt, or the INF/-INF/NaN float lexicals fell through to a TypeMismatch. Delegate string->numeric parsing to fluree_db_core::coerce_value, which already covers the full XSD numeric lattice: the integer family (with BigInt overflow and per-type range validation), xsd:decimal, and xsd:float/double including special lexicals. Only a numeric result is adopted; non-numeric datatypes and parse failures keep the literal as-is so it still falls to a TypeMismatch in apply. Adds a unit test covering xsd:short, an oversized xsd:integer (BigInt), and the xsd:float INF lexical.
Expression ORDER BY (e.g. `ORDER BY DESC(xsd:float(?c) / ?n)`) was rejected at lowering. Desugar each non-trivial order key into a synthetic bind computed once per solution, carried on a new `Query.order_binds` field and applied as a dedicated stage after grouping/aggregation/HAVING/post-binds and before the sort. - Uniform across no-grouping, dedup-only GROUP BY, and aggregating queries. - Under grouping, validate the order expression references only group keys, aggregate outputs, or post-binds (clean error instead of evaluating over a Grouped binding). - Allow ORDER BY on post-aggregation bind outputs (e.g. `ORDER BY ?ceil`). - Fast paths that sort on `query.ordering` decline when order binds are present; the order-bind stage runs only in the generic pipeline. - Subquery expression ORDER BY is rejected at lowering rather than silently mis-sorting on a grabbed variable; bare/bracketed variables still work. Tests: lowering + execution coverage incl. dedup-only GROUP BY, grouped-var rejection, the stats count-by-predicate fast path, and the BSBM BI Q5 ratio shape.
(+ ?sal (/ ?sal 10)) now yields an xsd:decimal because xsd:integer / xsd:integer returns xsd:decimal per XPath op:numeric-divide. The summed salaries are exact decimals (110, 220) rendered as strings; the untouched sales employee stays an integer. Adjusts assertions accordingly.
Allow an aggregate call directly inside an ORDER BY expression (e.g. `ORDER BY DESC(COUNT(?x))` or `DESC(COUNT(?x) * 2)`), which previously failed at lowering because the order key was lowered before any aggregate alias map existed. Lowering-only change; the executor already accepts aggregate outputs as order-bind sort keys. - Defer aggregate-bearing ORDER BY expressions until aggregate hoisting runs, then lower them into `order_binds` with the alias map in scope. - Share one aggregate-alias map across HAVING and ORDER BY hoisting (rename collect_having_aggregates -> collect_inline_aggregates) so synthetic names stay unique and an ORDER BY aggregate reuses a matching SELECT alias instead of recomputing it. - Hoisted aggregates feed the aggregate list before the auto-GROUP-BY check, so implicit single-group aggregation triggers. - CONSTRUCT/DESCRIBE reject inline-aggregate ORDER BY (no aggregation stage). Tests: explicit GROUP BY (deduped with SELECT alias), aggregate not in SELECT, implicit single group, aggregate inside a compound expression, plus lowering unit tests for dedup and hoisting.
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
2 participants
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
Fixes three SPARQL 1.1 correctness gaps in aggregate/analytic query evaluation. Each is an independent spec-compliance issue in core expression handling.
1. Mixed numeric-type arithmetic no longer errors
Arithmetic between two different XSD numeric types — e.g.
xsd:float(?x) / ?int— failed with a type mismatch. XPath operator mapping requires numeric type promotion: the narrower operand is promoted to the wider type and the operation proceeds. Numeric operands carried as typed literals (string-backedxsd:float, the full integer family including values needingBigInt,xsd:decimal, and theINF/-INF/NaNfloat lexicals) are now normalized through the canonical core coercion layer before the operation, so promotion applies uniformly instead of falling through to an error.2. Integer division yields
xsd:decimalxsd:integer / xsd:integertruncated to an integer (10 / 4→2) — a silent wrong-value deviation. Per XPathop:numeric-divide, integer division yieldsxsd:decimal(10 / 4→2.5). Division now returns a decimal, with precision capped at 34 significant digits (IEEE-754 decimal128, matchingAVG) so recurring quotients stay bounded. As elsewhere,xsd:decimalserializes as a string to preserve exactness, whilexsd:doubleremains a JSON number.3. Expression-based
ORDER BYis supportedORDER BYwith an expression rather than a bare variable was rejected at lowering time. SPARQL 1.1 §15.1 permits any expression as an order condition. Expression order keys (e.g.ORDER BY DESC(?a / ?b)) are now desugared to a synthetic per-solution bind evaluated after grouping/aggregation and before the sort, so the key may referenceGROUP BYkeys, aggregate outputs, andSELECT-expression aliases. Inline aggregates in the order condition (ORDER BY DESC(COUNT(?x)), SPARQL 1.1 §18.2.4.1) are hoisted into the grouping phase and deduplicated against any matchingSELECTaggregate so the count is computed once.Scope / limitations
ORDER BYinside a subquery is intentionally rejected: the subquery operator sorts its projected output and has no stage to evaluate a synthetic sort key. Top-level queries are unaffected.xsd:floatarithmetic uses the existing double representation (no distinct 32-bit float type), consistent with howxsd:float/xsd:doublealready collapse elsewhere in the engine.