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]

Reply via email to