rebenitez1802 commented on code in PR #42899:
URL: https://github.com/apache/superset/pull/42899#discussion_r4414399779
##########
superset/db_engine_specs/presto.py:
##########
@@ -167,6 +167,13 @@ class PrestoBaseEngineSpec(BaseEngineSpec,
metaclass=ABCMeta):
supports_dynamic_schema = True
supports_catalog = supports_dynamic_catalog =
supports_cross_catalog_queries = True
+ # Presto and Trino require OFFSET to appear before LIMIT in the SQL
grammar.
+ # Superset normalizes compiled SQL accordingly so pagination works even
when
+ # the connection resolves to a driver whose dialect emits the ANSI
+ # ``LIMIT ... OFFSET`` ordering (e.g. PyHive). See
+ # ``BaseEngineSpec.apply_offset_before_limit``.
+ offset_before_limit = True
Review Comment:
Still open after the master merge. A correction to my example above: with
sqlglot 30.24 (#45704) the backslash value no longer changes. Page 2 now
renders `'C:usersdata'`, which Spark reads the same as page 1's
`'C:\users\data'`.
The re-render still breaks page 2 on Hive, though:
- `named_struct('x', f.a, 'y', d.q).x` becomes `STRUCT(f.a, d.q).x`. Hive
names those fields `col1`/`col2`, so `.x` fails.
- The `/*+ MAPJOIN(d) */` hint is still dropped.
Hive and Spark already accept `LIMIT … OFFSET`, so they get the rewrite with
no benefit. The fix is the same: `offset_before_limit = False` on
`HiveEngineSpec`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]