Repository navigation
fix(mongodb): push down only filters MongoDB evaluates as SQL does, and stop claiming a sort order - #77
Conversation
…nd stop claiming a sort order The MongoDB table provider translated SQL predicates operator-for-operator and reported every translation as exact, so DataFusion dropped its own filter and returned whatever MongoDB matched. MongoDB's matching differs from SQL's evaluation of the converted rows: - `$ne`, `$nin`, `$not` and `$nor` match a null or missing field, where SQL's three-valued logic evaluates the predicate to NULL. - An `ObjectId` converts to its hex string, so `_id = '<hex>'` compared a string against an `ObjectId` and matched nothing. - Comparisons are bracketed by BSON type, while a `Utf8` column renders numbers, documents and other types as strings. - Arrays match when any element does; the conversion nulls them in a scalar column. - A cast was unwrapped from the column side, so `CAST(ts AS DATE) = d` compared the raw timestamp with midnight. - LIKE was translated without dotall, with `$` as the end anchor, and without DataFusion's default backslash escape. - NaN compares false against every number, which Arrow orders above them. Each comparison is now guarded by the BSON types its column's Arrow type accepts, excludes arrays where the conversion nulls them, and pushes negation down to the comparisons instead of wrapping `$nor`/`$not`. A translation that can only select a superset (a `Utf8` column, a float range, LIKE) is reported `Inexact`, so DataFusion filters it again. The sort pushdown is removed: MongoDB sorts null and missing fields first, orders by BSON type before value, puts NaN below every number and orders an array by its smallest element, so `find().sort()` cannot reproduce the order DataFusion requires, and returning `Exact` removed the `SortExec` that would.
…tant dates consistent with the converted rows
A `$nin` over strings also excludes a symbol spelled like the excluded
string, which the conversion renders as `Symbol("...")`; the exclusion now
keeps every rendered type.
A collection's default collation can hold unequal strings equal and order
them apart from SQL, which narrows `$nin` and the range operators. A
table now reads its collection's collation: a query that orders strings
names the simple collation on a collated collection, and on a view (whose
collation cannot be replaced) or where the collation cannot be read, only
equality, IN, LIKE and null tests on strings are pushed down.
A cast of a timestamp column to another unit, or of a timestamp without a
time zone into a zone other than UTC, changes values (overflowing, moving
or truncating the instant), so such a cast is no longer looked through.
A date whose milliseconds overflow a microsecond or nanosecond column is
now NULL rather than a wrapped-around instant, and filters on such a
column account for it.
Unnesting whole documents, as a JSON catch-all requires, reads a field
whose name has a dot into the dotted column of that name, which no query
path addresses, so dotted columns are not pushed down there.
DataFusion simplifies a short `x IN (1, NULL)` into `x = 1 OR NULL`, whose NULL is neither true nor false, so the filter was not pushed down at all. A NULL predicate now translates to a filter that matches nothing under either polarity, and a disjunction or conjunction with such a side simplifies to what the other side keeps.
…aive timestamp Arrow reads a timestamp without a time zone as wall-clock time in the zone it gains, through chrono, whose range is narrower than a BSON date's; even UTC fails for a distant date, which CAST reports as an error and TRY_CAST as NULL. Such a cast is no longer looked through.
With unnesting, a field that holds a document in some documents and a scalar in others is both a column and the prefix of others (address, address.city). Projecting both is a path collision, which MongoDB 4.4 and later refuses, so any query reading both columns failed. The prefix alone returns the whole embedded document, which unnesting flattens.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The always-false filter can match MongoDB view rows that omit _id, causing exact predicates to return incorrect results.
Review effort: Balanced
Findings: 1
What changed in this PR
Improves MongoDB correctness by aligning pushed filters and converted values with DataFusion semantics.
Changes:
- Reworks filter translation with type, null, collation, and exactness safeguards.
- Removes unsafe sort pushdown and fixes timestamp overflow conversion.
- Avoids MongoDB projection path collisions and adds extensive tests.
| File | Description |
|---|---|
core/src/mongodb/utils/expression/tests.rs |
Adds comprehensive filter translation tests. |
core/src/mongodb/utils/expression.rs |
Implements semantics-aware MongoDB filter translation. |
core/src/mongodb/utils/arrow.rs |
Prevents timestamp scaling overflow. |
core/src/mongodb/table.rs |
Integrates exact/inexact filters and removes sort pushdown. |
core/src/mongodb/connection.rs |
Adds collation handling and collision-safe projections. |
core/src/mongodb/connection_pool.rs |
Exposes configured unnest depth. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A predicate that is never true translated to `{_id: {$exists: false}}`,
which assumes every document has an `_id`. A view can project `_id`
away, and there it matched every document, so an exact filter such as
`i32 = 2.5` on an integer column returned every row. `{_id: {$in: []}}`
matches no document, a missing `_id` included.
|
Addressed #77 (review). That review had no line-comment thread. Its headline was that the semantic changes across engines warrant a human's validation. That is a call for a human reviewer, so I'm not requesting another Copilot pass. What a reviewer can check against MongoDB rather than against the code:
The two failing checks on c6cbc3a come from code this PR doesn't touch, and neither is a required check on DuckDB Tests (release) failed on both attempts, in
Python Tests failed on both attempts (jobs 108487428128 and 108496636772) in the ClickHouse setup: Against 25.8 the pool connects, and the example then stops at its own missing |
…datafusion-table-providers#77 10b7fe71d7 on spiceai-54 squash-merges spiceai/datafusion-table-providers#77 onto 14fdd18233, trunk's previous pin. Its tree is that of c6cbc3a3ca, the branch head this pull request was tested at, so the code is unchanged. The ledger row names spiceai-54 instead of the pull request's branch and drops the TEMPORARY marker, and the MongoDB patch row records the view without `_id` that its guard now queries.
… and guard it with a round-trip suite Pins spiceai/datafusion-table-providers#77, which pushes a MongoDB filter down only where MongoDB selects the rows SQL keeps from the converted documents, stops reporting MongoDB's sort order as the SQL one, projects no path beside one of its prefixes, and reads a date too distant for a microsecond or nanosecond column as NULL. The pin is marked TEMPORARY in docs/dev/fork_patches.md until that pull request merges. Adds a differential round-trip suite over a collection of null, missing, mistyped, array, ObjectId, symbol, NaN, signed-zero and distant values, a case-insensitive collated collection and view, unnested and dotted fields, and a JSON catch-all, each compared with an Arrow acceleration of the same collection.
…datafusion-table-providers#77 10b7fe71d7 on spiceai-54 squash-merges spiceai/datafusion-table-providers#77 onto 14fdd18233, trunk's previous pin. Its tree is that of c6cbc3a3ca, the branch head this pull request was tested at, so the code is unchanged. The ledger row names spiceai-54 instead of the pull request's branch and drops the TEMPORARY marker, and the MongoDB patch row records the view without `_id` that its guard now queries.

Summary
MongoDB filter pushdown selected different rows from the ones SQL keeps, because it translated SQL operators into MongoDB operators one for one. MongoDB and SQL disagree on how those operators treat the documents the connector converts into rows:
$ne,$nin,$notand$normatch a document whose field is null or missing, where SQL's three-valued logic makes the predicate NULL, soage <> 30returned the rows with noage.{f: {$gt: "6"}}never matches the number 7. The conversion, though, renders a non-string value into aUtf8column: anObjectIdas its hex string, a document as JSON, a symbol asSymbol("x"), anything else as its display string.int,longanddouble, while the conversion keeps only the BSON types the column's Arrow type accepts, and rounds alongbeyond 2^53 into anf64column.-0.0equal to0.0, which Arrow orders apart.$ninand the range operators.TRY_CASTthen yields NULL), and a cast of a timestamp without a time zone into another zone moves the instant. The translator looked through both.\as the escape, the wayDataFusionevaluates LIKE.DataFusionremoved its own sort.Each comparison is now guarded by the BSON types its column accepts. Arrays are excluded wherever the conversion nulls them. A negation is pushed down to the comparisons (De Morgan, with every operator negated as three-valued logic requires) instead of being wrapped in
$nor. Each filter reportsExactonly where its document selects exactly the rows the predicate keeps; otherwise it reportsInexact, selects a superset, andDataFusionfilters again.The table reads its collection's collation once. A query that orders or excludes strings names the simple collation on a collated collection. On a view, whose collation cannot be replaced, or where the collation cannot be read, only equality,
IN, LIKE and null tests are pushed down, since a collation can only widen$inand does not apply to$regex. A query that needs no ordering keeps the collection's collation, so its indexes stay usable.The sort-pushdown override is removed. Exact sort pushdown through per-type segmented queries can follow separately.
Two conversion fixes come with it, because the translator has to agree with them:
datewhose milliseconds overflow a microsecond or nanosecond column is now NULL, as Arrow's safe cast makes it. Before, it wrapped to an instant centuries away: a year-2262 date read as 1677-09-21. A column declaredtimestampin Spice is nanoseconds, so this was reachable.A NULL predicate, which is what
DataFusionleaves ofx IN (1, NULL), now translates to a filter that matches nothing under either polarity, so such filters are pushed down. That filter is{_id: {$in: []}}, which matches no document, including on a view that projects_idaway.With unnesting, a field that holds a document in some documents and a scalar in others is both a column and the prefix of others (
address,address.city). The projection named both, which MongoDB 4.4 and later refuse (Path collision at address.city remaining portion city), soSELECT *over such a collection failed. It now names only the prefix, whose embedded document unnesting flattens.Breaking changes
Four public items of the
mongodbmodule change:utils::expression::expr_to_mongo_filter(expr)is removed. It translated a predicate without the column types or the collection's collation. The query that selects the rows SQL keeps depends on both, so no translation under that signature can be correct. Usetranslate_filter(expr, schema, scope), which also reports whether the document is exact.utils::expression::combine_exprs_with_and(exprs)is removed. It isdatafusion::logical_expr::utils::conjunction(exprs.iter().cloned()).connection::MongoDBConnection::query_arrowno longer takessort_doc, and takescollation: Option<&Collation>afterfilters_doc. MongoDB's sort order is not SQL's, so no sort is sent. Passconnection.string_comparison(collection).await.collation_for(filters_doc), which is the simple collation on a collated collection when the filter orders or excludes strings, andNoneotherwise.table::MongoDBExec::newtakesfilter_scope: &FilterScopeandstrings: StringComparisonafterfilters, which the translation needs.spiceai/spiceai calls none of them. It imports
MongoDBTableFactory,connection_pool::MongoDBConnectionPool,Error,projection::project_bson_document,utils::arrow::mongo_docs_to_arrowandutils::unnest, whose signatures do not change.Evidence
The full differential runs are in spiceai/spiceai#14419, which pins this branch. Each query runs against the federated dataset and against an Arrow acceleration of the same collection, which
DataFusionevaluates locally, onmongo:8.0:d62271b7a2)s <> 'x',NOT IN,=)collation=[simple]TRY_CAST(t AS TIMESTAMP) IS NULLwith a year-2262 date[], not[1, 3])SELECT *withunnest_depth: 1whereaddressis a document and a stringPath collision at address.cityFor example, on trunk:
On this branch the same query returns the truth rows, with
filters=[{"$or":[{"s":{"$gt":"Z"}},…]}] collation=[simple].A cast that adds a time zone to a naive timestamp column is no longer looked through. Compared with a literal, such a cast never reaches the provider, because
DataFusion's unwrap-cast rewrite moves it to the literal. Tested asIS NULL, though,arrow_cast(t, 'Timestamp(Millisecond, Some("UTC"))') IS NULLover a year-275760 date returned[]federated. Local evaluation of the same rows fails withCast error: Cannot cast timezone to different timezone, because Arrow converts through chrono, whose range is narrower than a BSON date's.Tests
cargo test -p datafusion-table-providers --lib --no-default-features --features mongodb mongodb::: 164 passed.cargo clippy -p datafusion-table-providers --no-default-features --features mongodb -- -D warningsreports nothing in the MongoDB module. Codex reviewed the translator adversarially twice. Each finding was reproduced against MongoDB before it was fixed, and one it predicted (a zone-adding cast compared with a literal) did not reproduce, as described above. spiceai/spiceai#14419 addscrates/runtime/tests/mongo/pushdown_roundtrip.rs, which runs every case above as a differential test against a MongoDB container, along with filters that match nothing over a view that projects_idaway.