-
Notifications
You must be signed in to change notification settings - Fork 29
Changes to support DLT #636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| import logging | ||
| import re | ||
| from contextlib import closing | ||
| from datetime import datetime | ||
|
|
||
| import sqlalchemy.exc | ||
| from sqlalchemy import ( | ||
|
|
@@ -692,8 +693,52 @@ def visit_big_integer(self, type_, **kw): | |
| def visit_large_binary(self, type_): | ||
| return self.visit_BLOB(type_) | ||
|
|
||
| def visit_datetime(self, type_): | ||
| return self.visit_TIMESTAMP(type_) | ||
| # --- Date/time --- | ||
|
|
||
| # Some SQLAlchemy versions dispatch DateTime() to 'DATETIME' (upper-case) | ||
| def visit_DATETIME(self, type_, **kw): | ||
| return "TIMESTAMP" | ||
|
|
||
| # Others dispatch to 'datetime' (lower-case) — keep both for safety | ||
| def visit_datetime(self, type_, **kw): | ||
| return "TIMESTAMP" | ||
|
|
||
| # If anything ever uses an explicit TIMESTAMP type, make it consistent | ||
| def visit_TIMESTAMP(self, type_, **kw): | ||
| return "TIMESTAMP" | ||
|
|
||
| # --- Strings / Text --- | ||
|
|
||
| # SA String(length) -> VARCHAR(n); String() -> CLOB (Exasol requires length) | ||
| def visit_string(self, type_, **kw): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar question. Why not call
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fully agree that this needs to be cleaned up a bit and we reuse methods if they're available/work in our case. |
||
| if type_.length: | ||
| return f"VARCHAR({int(type_.length)})" | ||
| return "CLOB" | ||
|
|
||
| # SA Text() -> Exasol CLOB (Exasol has no TEXT; VARCHAR requires a length) | ||
| def visit_text(self, type_, **kw): | ||
| return "CLOB" | ||
|
|
||
| # (optional) SA UnicodeText() -> CLOB as well | ||
| def visit_unicode_text(self, type_, **kw): | ||
| return "CLOB" | ||
|
|
||
| # --- Numeric / Decimal --- | ||
|
|
||
| # Ensure Numeric/DECIMAL always renders with an explicit scale | ||
| def visit_numeric(self, type_, **kw): | ||
| # SA may pass scale as None or -1 when only precision was given | ||
| p = type_.precision | ||
| s = 0 if (type_.scale in (None, -1)) else type_.scale | ||
| if p is not None: | ||
| return f"DECIMAL({p},{s})" | ||
| # sensible fallback if nothing provided | ||
| return "DECIMAL(18,0)" | ||
|
|
||
| # Some code paths use DECIMAL directly rather than numeric | ||
| def visit_DECIMAL(self, type_, **kw): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious as to how these were arrived to: Were the other
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is just enough to get DLT working right now, there's probably some/plenty of other cases that might still not work. |
||
| return self.visit_numeric(type_, **kw) | ||
|
|
||
|
|
||
|
|
||
| class EXAIdentifierPreparer(compiler.IdentifierPreparer): | ||
|
|
@@ -757,6 +802,41 @@ def _get_schema(sql_compiler, dialect): | |
| def should_autocommit_text(self, statement): | ||
| return AUTOCOMMIT_REGEXP.match(statement) | ||
|
|
||
| class EXATimestamp(sqltypes.TypeDecorator): | ||
| """Coerce Python datetime to a JSON-serializable wire value for pyexasol. | ||
|
|
||
| Exasol TIMESTAMP has no timezone; we format naive/UTC datetimes accordingly. | ||
| """ | ||
| impl = sqltypes.TIMESTAMP | ||
| cache_ok = True | ||
|
|
||
| def bind_processor(self, dialect): | ||
| def process(value): | ||
| if value is None: | ||
| return None | ||
| # Normal case: a Python datetime instance | ||
| if isinstance(value, datetime): | ||
| # Keep microseconds; Exasol accepts 'YYYY-MM-DD HH:MM:SS.ffffff' | ||
| return value.strftime("%Y-%m-%d %H:%M:%S.%f") | ||
| # Defensive: if a SA DateTime *type* accidentally lands here as a value | ||
| if isinstance(value, sqltypes.DateTime): | ||
| return None | ||
| return value | ||
| return process | ||
|
|
||
|
|
||
| from sqlalchemy import exc as sa_exc | ||
|
|
||
| _PYEXA_TO_SA = { | ||
| "ExaQueryError": sa_exc.ProgrammingError, | ||
| "ExaAuthError": sa_exc.OperationalError, | ||
| "ExaRequestError": sa_exc.OperationalError, | ||
| "ExaCommunicationError": sa_exc.OperationalError, | ||
| "ExaRuntimeError": sa_exc.DatabaseError, | ||
| "ExaConstraintViolationError": sa_exc.IntegrityError, | ||
| "ExaIntegrityError": sa_exc.IntegrityError, | ||
| "ExaError": sa_exc.DatabaseError, | ||
| } | ||
|
|
||
| class EXADialect(default.DefaultDialect): | ||
| name = "exasol" | ||
|
|
@@ -1199,3 +1279,33 @@ def fkey_rec(): | |
| def get_indexes(self, connection, table_name, schema=None, **kw): | ||
| """EXASolution has no explicit indexes""" | ||
| return [] | ||
|
|
||
| def type_descriptor(self, typeobj): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you try putting the ExaTimestamp in the https://github.com/exasol/sqlalchemy-exasol/blob/master/sqlalchemy_exasol/websocket.py#L91
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's try this out. |
||
| """Return a DB-specific TypeEngine for a generic SA type. | ||
|
|
||
| We wrap DateTime columns so their Python values serialize cleanly for pyexasol. | ||
| """ | ||
| if isinstance(typeobj, sqltypes.DateTime): | ||
| return EXATimestamp() | ||
| return super().type_descriptor(typeobj) | ||
|
|
||
| # leave this for true DB-API remapping (ODBC etc.) | ||
| dbapi_exception_translation_map = {} | ||
|
|
||
| def do_execute(self, cursor, statement, parameters, context=None): | ||
| # print("TRIGGERED DO EXECUTE") | ||
| try: | ||
| return super().do_execute(cursor, statement, parameters, context) | ||
| except Exception as e: | ||
| mapped = _PYEXA_TO_SA.get(e.__class__.__name__) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might be able to simplify the exceptions here, as the Python Exception case allows for more than 1 to be checked, and we could extract that to a function if other usages needed to be covered. However, it's possible you have something more in mind, as it looks like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have a look at how you'd do this this afternoon .. |
||
| if mapped: | ||
| # simplest: construct the SA exception directly | ||
| try: | ||
| raise mapped(statement, parameters, e) from e | ||
| except TypeError: | ||
| # handle minor signature differences across SA versions | ||
| try: | ||
| raise mapped(statement, parameters, e, False) from e | ||
| except TypeError: | ||
| raise mapped(str(e)) from e | ||
| raise | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional that they aren't built referring to one another? For instance, we could have visit_DATETIME as it currently is defined and then have visit_datetime call self.visit_DATETIME and pass on the values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fully agree that this needs to be cleaned up a bit and we reuse methods if they're available/work in our case.