fix(sqlalchemy): add literal_processor to TIMESTAMP type#609
fix(sqlalchemy): add literal_processor to TIMESTAMP type#609q8webmaster wants to merge 1 commit into
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Q8Webmaster.
|
SQLAlchemy's DateTime.literal_processor() returns None for Trino's TIMESTAMP type, causing a CompileError when literal_binds=True is used and a datetime value appears in a WHERE clause (e.g. Superset's select_star partition filter). Add a literal_processor to TIMESTAMP that emits a Trino-compatible TIMESTAMP 'YYYY-MM-DD HH:MM:SS[.mmm]' literal.
q8webmaster
force-pushed
the
fix/timestamp-literal-processor
branch
from
60ae3e8 to
1f79e64
Compare
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| if value.microsecond: | ||
| ts += f".{value.microsecond // 1000:03d}" | ||
| return f"TIMESTAMP '{ts}'" | ||
| return str(value) |
There was a problem hiding this comment.
sql injection I think with something like literal("x'; DROP TABLE t; --", TIMESTAMP())
There was a problem hiding this comment.
also correctness issues in case of syntax errors: WHERE ts = 2026-06-17 would parse as integer arithmetic instead of correctly
| def literal_processor(self, dialect): | ||
| def process(value): | ||
| if isinstance(value, datetime.datetime): | ||
| ts = value.strftime("%Y-%m-%d %H:%M:%S") |
There was a problem hiding this comment.
did you intentionally drop the zone? A zone-aware value would now be treated with session zone in Trino side. Maybe see Cursor._format_prepared_param for some ideas.
| if isinstance(value, datetime.datetime): | ||
| ts = value.strftime("%Y-%m-%d %H:%M:%S") | ||
| if value.microsecond: | ||
| ts += f".{value.microsecond // 1000:03d}" |
There was a problem hiding this comment.
truncates precision. equality predicates may no longer match.
Also TIMESTAMP(precision=6) would ignore the specified precision.
| assert str(query) == 'SELECT try_cast("table".id as VARCHAR) AS id \nFROM "table"' | ||
|
|
||
|
|
||
| def test_timestamp_literal_processor(dialect): |
There was a problem hiding this comment.
add more tests:
- tz aware values
- higher precision values
- non-datetime values
hashhar
left a comment
hashhar
left a comment
There was a problem hiding this comment.
Also do we need to do the same for TIME?
2 participants
Problem
TIMESTAMP.literal_processor()is not implemented, causing aCompileErrorwhenliteral_binds=Trueis used with adatetime.datetimevalue in a WHERE clause:Concrete trigger: Apache Superset's
select_star()builds a sample query withWHERE <partition_col> = <latest_partition_value>usingliteral_binds=True. Anytable with a TIMESTAMP partition column hits this on the table metadata endpoint.
Fix
Add
literal_processortoTIMESTAMPemitting a Trino-compatibleTIMESTAMP 'YYYY-MM-DD HH:MM:SS[.mmm]'literal.Tests
Two unit tests added to
tests/unit/sqlalchemy/test_compiler.py:TIMESTAMP '2026-06-17 09:57:43.244'TIMESTAMP '2026-06-17 09:57:43'