[PR] [KYUUBI #7786] Give HiveDialect its own colspecs dict [kyuubi]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas opened a new pull request, #7787:
URL: https://github.com/apache/kyuubi/pull/7787

   ### Why are the changes needed?
   
   `python/pyhive/sqlalchemy_hive.py`'s `HiveDialect` never defines its own `colspecs`, so it inherits `sqlalchemy.engine.default.DefaultDialect.colspecs` directly, the same dict object, not a copy:
   
   ```python
   >>> from sqlalchemy.engine.default import DefaultDialect
   >>> from pyhive.sqlalchemy_hive import HiveDialect
   >>> HiveDialect.colspecs is DefaultDialect.colspecs
   True
   ```
   
   Every SQLAlchemy built-in dialect (`postgresql`, `mysql`, etc.) defines its own `colspecs` dict, even when there's nothing to override yet, specifically to avoid this. Mutating a dialect's `colspecs` in 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 own `colspecs`, not just Hive.
   
   This isn't hypothetical: Apache Superset's Databricks engine spec subclasses `HiveDialect` for Databricks support and patches `HiveDialect.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 `HiveDialect` its own `colspecs = {}` and confirmed the isolation directly, same check as in the linked issue:
   
   ```python
   >>> HiveDialect.colspecs is DefaultDialect.colspecs
   False
   >>> HiveDialect.colspecs['marker'] = 'hive-only'
   >>> DefaultDialect.colspecs
   {}
   ```
   
   `colspecs` was and remains empty for this dialect; it's just no longer the same object as `DefaultDialect`'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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [PR] [KYUUBI #7786] Give HiveDialect its own colspecs dict [kyuubi]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas commented on PR #7787:
URL: https://github.com/apache/kyuubi/pull/7787#issuecomment-6193793060

   One more question while this is open: is there any intention to publish `python/pyhive` as its own installable package now that it lives here? Right now `pip install pyhive` still pulls dropbox's original, and this copy's own `setup.py` still points back at `github.com/dropbox/PyHive`.
   
   The reason I'm asking: apache/superset worked around the shared-`colspecs` bug on its own side (apache/superset#44707) rather than waiting on a fix here or at dropbox, specifically because there wasn't a maintained release either of us could pin to. We'd genuinely like to `pip install` straight from this repo once there's a real release, and happy to help with packaging/release work if that'd move things along.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org