Repository navigation
Conversation
HiveDialect never defined its own colspecs, so it inherited sqlalchemy.engine.default.DefaultDialect.colspecs directly, the same dict object, not a copy. Mutating a dialect's colspecs in place to customize one type is a normal thing to do (Apache Superset's Databricks engine spec does exactly this), but without its own dict that mutation silently corrupted type handling for every other dialect in the process that also hadn't defined its own colspecs. Give it an empty colspecs dict, same as every SQLAlchemy built-in dialect does. Fixes apache#7786. Assisted-by: Claude:claude-sonnet-5
Member
Author
|
One more question while this is open: is there any intention to publish The reason I'm asking: apache/superset worked around the shared- |
…d to this change (BatchesResourceSuite open-batch-session/lock-timeout, ToolRegistryThreadSafetyTest concurrency race).
This branch has not been deployed
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
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.
Why are the changes needed?
python/pyhive/sqlalchemy_hive.py'sHiveDialectnever defines its owncolspecs, so it inheritssqlalchemy.engine.default.DefaultDialect.colspecsdirectly, the same dict object, not a copy:Every SQLAlchemy built-in dialect (
postgresql,mysql, etc.) defines its owncolspecsdict, even when there's nothing to override yet, specifically to avoid this. Mutating a dialect'scolspecsin place (HiveDialect.colspecs[SomeType] = ...) is a completely normal way to customize how a type renders for one dialect. Without its own dict, that mutation silently corrupts type handling for every other dialect in the same process that also hasn't defined its owncolspecs, not just Hive.This isn't hypothetical: Apache Superset's Databricks engine spec subclasses
HiveDialectfor Databricks support and patchesHiveDialect.colspecs[types.String] = ...to customize string-type handling. Because of the shared object, that one line broke type compilation for ~38 unrelated third-party dialects loaded in the same process, until Superset worked around it on its own side (apache/superset#44707).Filed the identical fix upstream at dropbox/PyHive#490 before noticing this module had already been donated here. Confirmed the bug is present in this copy too.
Fixes #7786.
How was this patch tested?
Gave
HiveDialectits owncolspecs = {}and confirmed the isolation directly, same check as in the linked issue:colspecswas and remains empty for this dialect; it's just no longer the same object asDefaultDialect's, so a mutation on one no longer leaks into the other.I don't have a live Hive/Kyuubi server available in my environment to run the full
python/test suite end to end, but this change touches only a class-level attribute with no effect on connection or query logic, and verified it above directly against this module.Was this patch assisted by generative AI tooling?
Assisted-by: Claude:claude-sonnet-5