sfirke commented on code in PR #34583: URL: https://github.com/apache/superset/pull/34583#discussion_r2261051204
########## superset/db_engine_specs/mssql.py: ########## @@ -163,6 +164,33 @@ def extract_error_message(cls, ex: Exception) -> str: ) return f"{cls.engine} error: {cls._extract_error_message(ex)}" + @classmethod + def adjust_query_for_offset(cls, qry: Select) -> Select: + """ + Modify SQLAlchemy query to add ORDER BY when OFFSET is present. + + MSSQL requires an ORDER BY clause when using OFFSET. If no ORDER BY + is present but OFFSET is specified, we add a default ORDER BY clause. + """ + # Check if this query has OFFSET but no ORDER BY by inspecting query structure + try: + # Try to access the internal query structure safely + has_offset = getattr(qry, "_offset", None) is not None + has_order_by = bool(getattr(qry, "_order_by_clauses", ())) + + if has_offset and not has_order_by: + if qry.selected_columns: + first_column = list(qry.selected_columns)[0] + qry = qry.order_by(first_column) + else: Review Comment: This was AI's idea. If someone feels confident that columns will always be available in this context - seems likely to me - we could remove the if/else and this backup casting of 1. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org