Release 0.7.4.1: SQLAlchemy 2 result correctness fixes and wheel publisher - #1
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.
Collaborator
Author
|
Upstream PR with the same code fixes: passren#43 |
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.
Collaborator
Author
|
Three more commits, each with tests that fail before and pass after:
Seven existing test expectations changed because they encoded the removed behaviour: |
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.
Summary
Fork release 0.7.4.1 on top of upstream 0.7.4. Fixes several SQLAlchemy 2 / SQL translation defects that returned wrong rows without raising, and adds a Jenkins publisher for immutable wheels.
The code fixes are the same commits proposed upstream in passren/PyMongoSQL (linked in a comment below). The last two commits (
ci:andchore: release 0.7.4.1) are fork-only.Fixes (each with fail-before / pass-after tests)
sqlalchemy_dialect.pyget_columns/_infer_bson_type): Decimal128 reflected asString, and a leading null asNullType. Int64 and binary values also fell through toString, and the camel-case type-map keys never matched. Test file:tests/test_sqlalchemy_reflection_types.py.helper.pyto_bson_value; Numeric/Float colspecs):decimal.Decimalbinds failed with "cannot encode object", and Numeric reads returnedbson.Decimal128. Test file:tests/test_sqlalchemy_numeric.py.visit_column,builder._strip_collection_qualifier):SELECT users.name FROM userswas read as the nested pathusers.name, so it silently returned NULL.select(table)raisedNoSuchColumnError. Test file:tests/test_sqlalchemy_qualified_columns.py.builder._build_sql_aggregate_plan,handler.py,ast.py, dialect reserved words):?parameters.IN (1, 2)compared against strings; NOT IN and NOT LIKE were misparsed.''was not unescaped.COUNT(*) AS countfailed to parse because PartiQL keywords were never quoted.tests/test_sql_grouping_filters_aliases.py._MongoUuid): reading a SQLAlchemy 2Uuidcolumn failed with'UUID' object has no attribute 'replace'. Test file:tests/test_sqlalchemy_uuid.py.bson.Int64instead ofint.--inside quoted literals: the comment stripper cut'a -- b', so the query failed to parse.Publishing (fork only)
Jenkinsfileruns the full suite against a disposablemongo:8.0pod container, builds a reproducible wheel and publishes it withci/publish_wheel.py. The upload is conditional (IfNoneMatch, never overwritten), and a retry is accepted only when the archive content is identical.0.7.4.1+pr.<n>.<sha>, normalized per PEP 440 byci/release_version.pyso the filename matches what the build backend writes. Stable0.7.4.1is published only frommaster.Test plan