Retrospective review: changes in 0.7.4.1-0.7.4.2 (merged without review) - #3
Merged
Merged
Conversation
…eflection get_columns typed each field from the first sampled value, and _infer_bson_type had no branch for Decimal128, Int64 or binary values, so they fell through to String. A field whose first sampled document held a null reflected as NullType even when later documents held values. The type map lookup also lowercased its key, so the camel-case objectId and binData entries could never match. Type a field from its first non-null sampled value, recognise Decimal128 (DECIMAL), Int64 (BigInteger) and Binary/bytes (LargeBinary), and look the BSON type up without changing its case.
The dialect declares supports_native_decimal, so SQLAlchemy passes decimal.Decimal parameters straight to the DBAPI and installs no Numeric result processor. PyMongo cannot encode decimal.Decimal, so binding one failed with "cannot encode object", and reads returned bson.Decimal128 instead of the decimal.Decimal a Numeric column promises. Encode bound decimal.Decimal values as Decimal128 when placeholders are replaced, and give Numeric and Float columns result processors that convert Decimal128 before the usual Numeric handling.
SQLAlchemy qualifies every table-bound column (SELECT users.name FROM users), and the compiler only dropped the qualifier for names starting with an underscore. The translator reads a dotted name as an embedded-document path, so users.name looked for a field name inside a field users: projections silently returned NULL, filters matched nothing, and select(table) raised NoSuchColumnError. Render columns without a table qualifier in the SQLAlchemy compiler, and resolve collection-qualified references in hand-written SQL to the field before building the plan. Other dotted names keep their nested-path meaning.
Several constructs were translated into MongoDB queries that ran without
error but returned wrong rows:
- GROUP BY was ignored: the generated $group always used _id: null, so
SELECT flag, COUNT(*) ... GROUP BY flag returned one global count and
dropped the grouped column. ORDER BY, OFFSET and LIMIT were also
dropped from aggregate queries, and ? placeholders in their WHERE
clause were never replaced.
- IN wrapped every literal in quotes, so IN (1, 2) compared numbers with
the strings '1' and '2', and a quoted value containing a comma was
split in two. NOT IN and NOT LIKE were read as a field named <x>NOT.
- LIKE did not escape regex metacharacters, and a SQL-escaped quote
('O''Brien') was kept doubled.
- The SQLAlchemy dialect never quoted PartiQL keywords, so a label such
as COUNT(*) AS count failed to parse, and a quoted alias kept its
quotes in the result description.
Group on the GROUP BY keys and project the SELECT list in order, apply
ORDER BY/OFFSET/LIMIT and parameters inside the generated pipeline, keep
IN literal types, support NOT IN and NOT LIKE, escape LIKE patterns,
unescape doubled quotes, quote PartiQL keywords in the dialect and
unquote quoted aliases and ORDER BY keys. Columns that are neither
grouped nor aggregated, and HAVING, now raise instead of being dropped.
Superset-mode subqueries that aggregate are run as aggregates.
The dialect did not declare native UUID support, so SQLAlchemy 2's Uuid type used its string-based processors. PyMongo returns uuid.UUID (or a subtype-4 Binary), and reading a Uuid column failed with "'UUID' object has no attribute 'replace'". Declare native UUID support and map Uuid to a type that binds standard subtype-4 binaries and reads uuid.UUID, subtype-4 Binary or string values. Legacy subtype 3 is returned unchanged because its byte order depends on the driver that wrote it.
The command responses the DBAPI decodes carry 64-bit integers as bson.Int64, so Integer and BigInteger columns returned that subclass rather than the int SQLAlchemy promises. Convert it in the Integer result processor.
The preprocessor cut every line at the first "--", including one inside a string literal or quoted identifier, so WHERE name = 'a -- b' failed to parse. Strip a line comment only when it starts outside quotes.
Jenkins runs the full test suite against a disposable MongoDB in the build pod, builds a reproducible wheel and uploads it without ever overwriting an existing artifact; a retry is accepted only when the stored archive has identical content. Pull-request builds publish <version>+pr.<number>.<sha>, normalized per PEP 440 so the filename matches the one the build backend writes; stable versions are published only from master.
Fork release on top of 0.7.4 carrying the reflection, Decimal, qualified-column and GROUP BY/IN/LIKE/alias fixes.
setuptools_scm runs git while resolving build requirements, and git refuses a workspace owned by a different uid (dubious ownership). Mark the checkout as a safe directory through the environment for the build step.
The CI image's git predates GIT_CONFIG_COUNT, so the environment override was ignored. Add the workspace to safe.directory in the ephemeral pod's global config instead.
WHERE clauses were translated by splitting getText() output, which has no whitespace. NOT a = 1 became a filter on a field named "NOTa" (no rows), NOT (a = 1 OR b = 2) produced no filter at all (every row), and an operand that could not be translated was silently dropped from an AND, or the whole clause fell back to a $text search. <>, NOT IN and NOT LIKE also matched documents where the field was NULL or missing, which SQL never returns. Translate WHERE over the parse tree. Each predicate yields the filter of documents for which it is TRUE and the filter for which it is FALSE (a NULL or missing operand is in neither). NOT swaps them and AND/OR combine them by De Morgan's laws, so NOT a = 1 excludes NULLs as in SQL. A bare boolean field (WHERE flag / WHERE NOT flag) is supported. A predicate on anything but a field path, or a LIKE with a bound pattern, now raises instead of matching the wrong rows; the SQLAlchemy dialect renders LIKE patterns inline so Core like() keeps working. "= NULL" keeps its existing IS NULL meaning. DELETE and UPDATE use the same translation, and a WHERE clause that cannot be translated now fails the statement: it previously became an empty filter and matched every document.
…slated The FROM handler used the whole table reference text as the collection name, so FROM users AS u read a collection named "usersASu" and returned no rows, and u.name was read as an embedded path. Joins and, outside superset mode, subqueries were treated the same way and silently returned nothing. Read the collection and its alias from the parse tree, resolve alias-qualified references (u.name, including in GROUP BY and the ordered SELECT list) to the field, and raise NotSupportedError for joins, subqueries in standard mode and AT/BY bindings. Collection- qualified GROUP BY keys are also resolved now; they grouped on a missing nested path before.
… stage Superset-mode subqueries load the MongoDB rows into an in-memory SQLite table. SQLite has no boolean type, so boolean columns came back as 1/0, and a single NULL in a column made the whole column TEXT, returning numbers and booleans as strings. Declare boolean columns with a private type that is converted back to bool when a query selects the column (expressions such as SUM stay numeric), and let NULL values fit any column type when inferring the schema.
Release 0.7.4.1: SQLAlchemy 2 result correctness fixes and wheel publisher
Result rows carried bson.Decimal128 and, in command responses, bson.Int64 values. DB API consumers expect decimal.Decimal and int; Decimal128 in particular cannot be summed or serialised by most libraries. Convert both, including inside embedded documents and arrays.
WHERE predicates recovered the field name by searching the predicate's
concatenated token text for IN(, LIKE, ISNULL and similar, so a field
whose name contains one of them was truncated:
"dislikes IS NULL" filtered on "dis" and "unlike = 5" became a regex on
"un". SELECT returned other rows, and DELETE and UPDATE removed or
rewrote documents that did not match. The same text search rejected
quoted field names with spaces, hyphens or non-ASCII characters,
reversed comparisons (5 < age), and string literals containing IN( or
LIKE.
Read each predicate from its parse-tree node instead: the field is the
path on one side of the operator (either side), the value the literal,
parameter or value function on the other. Also:
- LIKE: fold concatenated string literals ('%' || 'ab' || '%', as
SQLAlchemy renders contains/startswith/endswith) and honour ESCAPE
(like(..., escape=...), autoescape). ESCAPE is re-attached when the
grammar lets it absorb the rest of the WHERE clause.
- Parameters: a bound parameter is a marker in the translated filter, so
a string literal '?' is compared as a value, and a parameter the
statement does not use raises instead of being ignored.
- LIMIT/OFFSET must be integer literals (the dialect renders them
inline); LIMIT ? was dropped and returned every row.
- COUNT/SUM/AVG/MIN/MAX(DISTINCT x) are computed from the set of
distinct non-NULL values; COUNT(DISTINCT x) returned 0.
- HAVING is translated into a $match after grouping, including
aggregates that are not in the SELECT list.
- SELECT expressions other than fields and aggregates raise instead of
returning a NULL column.
- A fractional literal no double represents exactly (0.1) is compared
as a double against double fields and exactly (Decimal128) against
other numeric types.
- Generated pipelines use extended JSON so Decimal128 and date literals
survive.
Fork release carrying the parse-tree predicate translation, LIKE concatenation/ESCAPE, DISTINCT aggregates, HAVING, literal LIMIT/OFFSET and BSON number conversion fixes.
- UPDATE SET values are read from the parse tree like WHERE values, so SET x = '?' stores the string and SET x = ? binds a parameter; SET and WHERE parameters are bound together in statement order. - LIMIT and OFFSET accept a bound parameter (validated as a non-negative integer at execution) instead of being dropped, which returned every row. Other non-integer values raise. - LIMIT 0 returns no rows: MongoDB reads limit 0 as "no limit", and $limit: 0 is invalid in a pipeline. Tests that used a quoted '?' as a WHERE or SET parameter now use the documented unquoted ?; a quoted '?' is a string literal.
…late ILIKE
The dialect rendered LIKE patterns as inline literals so the translator
could turn them into a regex while parsing. SQLAlchemy's positional
compilation then rewrites any %(name)s inside such a literal as a
parameter (contains('%(k)s') failed with KeyError, other patterns could
be corrupted).
Render LIKE patterns as bound parameters again and translate them when
parameters are bound: a pattern built from literals and parameters
('%' || ? || '%' ESCAPE '/', as SQLAlchemy renders contains, startswith
and endswith, including autoescape) becomes the regex at execution. A
non-string pattern parameter raises. lower(col) LIKE lower(pattern), as
SQLAlchemy renders ilike(), becomes a case-insensitive regex.
SQLAlchemy 2.0 renders every bind as %(name)s and then converts the whole compiled statement to qmark with a regular expression. Under literal_binds that also rewrites the text of an inline string literal, so x = '%(k)s' was sent as x = '?'. The compiler now masks quoted literals and identifiers while the markers are converted. limit_clause passed literal_binds twice when the statement was compiled with literal_binds (as Apache Superset compiles chart queries), raising TypeError for any query with a LIMIT.
DATE_TRUNC('<unit>', field) in a projection or GROUP BY is translated to
$dateTrunc (week-ending units add six days with $dateAdd). Units: second,
minute, hour, day, week, week_monday, month, quarter, year,
week_ending_saturday, week_ending_sunday; truncation is in UTC. An unknown
unit or a non-field argument raises instead of dropping the column.
The superset-mode SQLite stage registers the same DATE_TRUNC and
STR_TO_DATETIME, stores datetimes as fixed-width UTC text and returns
datetime values for them.
Decimal128 columns were stored in the SQLite stage as doubles or text, so
SUM lost digits and ORDER BY sorted text. They are now stored as REAL plus
an exact text copy, and queries are rewritten (sqlglot, the new
"superset" extra) so the column, SUM/AVG/MIN/MAX, GROUP BY, ORDER BY and
comparisons with numeric literals are evaluated with Decimal arithmetic in
Decimal128 precision (34 digits). Other uses of such a column raise
NotSupportedError instead of computing with doubles.
SUM over a group whose values are all NULL or missing returned 0 ($sum of no numbers); SQL returns NULL. SUM(DISTINCT) likewise. An aggregate without GROUP BY over no input rows returned no row; SQL returns one row (COUNT 0, other aggregates NULL), which is what a chart showing a total expects. The $group is wrapped in $facet so the empty input yields that row; HAVING, LIMIT and grouped queries are unchanged.
When a virtual dataset's own query matched no documents the SQLite stage created no table, so the outer query failed with "no such table" instead of returning COUNT 0 or no rows. The table is now created from the subquery's columns.
A grouped query ordered by an aggregate that is not in the SELECT list (Apache Superset's series-limit pre-query: GROUP BY the series, ORDER BY the limit metric) raised. The aggregate is now computed as a hidden output, like HAVING's, and removed after $sort. HAVING raised KeyError when the SELECT list had a DATE_TRUNC column.
Release 0.7.4.2: read predicates from the parse tree; LIKE, DISTINCT, HAVING and paging fixes
Collaborator
Author
|
@fitzee this was merged before review because the SQLAlchemy 2 release couldn't wait. Could you review it after the fact? Anything you find will be fixed in a follow-up PR. |
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
1 participant
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.
Retrospective review only — do not merge
#1 and #2 were merged without submitted reviews, and 0.7.4.2 was published from that history. This PR makes the combined released changes reviewable; it is not a new release or a claim that they are already approved. Please review before #5276 treats this release as reviewed. Do not merge this comparison into its historical base or master.
Exact boundary and artifact
review/base-before-pr-1: 8812bfc, upstream 0.7.4, the commit before Release 0.7.4.1: SQLAlchemy 2 result correctness fixes and wheel publisher #1.review/release-0.7.4.2: bbe04c0, the master release commit merging Release 0.7.4.2: read predicates from the parse tree; LIKE, DISTINCT, HAVING and paging fixes #2, with__version__ = "0.7.4.2".2339f0eand every Release 0.7.4.2: read predicates from the parse tree; LIKE, DISTINCT, HAVING and paging fixes #2 commit through5bc8d46; not merely the intermediate version-bump commit6989e45.Changes in #1 / 0.7.4.1
Changes in #2 / 0.7.4.2
dislikes IS NULLandunlike = 5, quoted/non-ASCII fields, reversed comparisons and operator-like literal text.?or%(name)s; support literal-bound LIMIT and honor LIMIT 0.Fresh live evidence — this exact head, 2026-09-29
996 passed, 8 skipped with Python 3.11.15, SQLAlchemy 2.0.52, PyMongo 4.18.2, pandas 3.0.6, against one disposable MongoDB 8.0.32 container (
mongo:8.0, loopback-only port 27029).The credentials above are throwaway local test-fixture credentials. The replay used the unchanged release tree, seeded its users/products/orders/time-series/views, and ran the complete test directory. Scope includes live parse-tree DML predicates, qualified columns, NULL/NOT semantics, type reflection and round trips, decimal/time-grain/literal-bind behavior, grouping/aliases and Superset's SQLite mode. Eight skips remain skips, not passes. No Atlas/TLS/production-image qualification is claimed by this run.
Historical fail-before evidence (not relabeled as today's run)
No Shell pins or manifest status are changed here, and no merge or stable publication is performed.