hussein-awala commented on code in PR #32630:
URL: https://github.com/apache/airflow/pull/32630#discussion_r1264652989
##########
airflow/api_connexion/parameters.py:
##########
@@ -125,3 +126,9 @@ def apply_sorting(
else:
order_by = f"{lstriped_orderby} asc"
return query.order_by(text(order_by))
+
+
+def get_query_count(query_stmt: sqlalchemy.sql.selectable.Select, session:
sqlalchemy.orm.Session) -> int:
Review Comment:
> Also, is the order_by reset necessary?
TBH, I am not sure how sqlalchemy processes this and generates the query,
but according to the documentation, it just select from the subquery: `<query>
FROM (<subquery>) AS subquery`, so if we compare this format with and without
order by:
```
airflow=# EXPLAIN SELECT COUNT(1) FROM (SELECT * FROM dag_run WHERE
state='failed' ORDER BY data_interval_start) AS subquery;
QUERY PLAN
----------------------------------------------------------------------
Aggregate (cost=1.04..1.05 rows=1 width=8)
-> Sort (cost=1.02..1.03 rows=1 width=1459)
Sort Key: dag_run.data_interval_start
-> Seq Scan on dag_run (cost=0.00..1.01 rows=1 width=1459)
Filter: ((state)::text = 'failed'::text)
(5 rows)
airflow=# EXPLAIN SELECT COUNT(1) FROM (SELECT * FROM dag_run WHERE
state='failed') AS subquery;
QUERY PLAN
-------------------------------------------------------------
Aggregate (cost=1.01..1.02 rows=1 width=8)
-> Seq Scan on dag_run (cost=0.00..1.01 rows=1 width=0)
Filter: ((state)::text = 'failed'::text)
(3 rows)
```
There is no many rows in my DB (fresh breeze DB), so this slight difference
could be much bigger in the big DB.
Personally I always reset the order_by when I don't need it to be safe and
to ensure a better performance. WDYT?
--
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]
