sameerpandey17 commented on code in PR #75635: URL: https://github.com/apache/airflow/pull/75635#discussion_r4381353867
########## airflow-core/src/airflow/api_fastapi/common/parameters/search.py: ########## Review Comment: thanks for your question Benchmark results (Postgres 16, 1M-row table with a `varchar(250)` primary key, prefixes in 5 equal-sized groups, warm cache, `EXPLAIN (ANALYZE, BUFFERS)`; database default collation `en_US.utf8`, and the same table with `cs-CZ-x-icu`): | prefix | matching rows | before (bounded range) | after (this PR) | |---|---|---|---| | broad (`dag`, `etl`, `ml`, `sync`, ~20% of rows each) | ~200k | parallel seq scan, ~42–60 ms | parallel seq scan, ~39–71 ms | | selective `etl_a` | ~12k | bitmap index scan, ~10–12 ms | parallel seq scan, ~45–51 ms | | selective `report_0` | ~12k | bitmap index scan, ~10 ms | parallel seq scan, ~42–46 ms | So the concern is confirmed: for selective prefixes the planner used the primary-key index before, and without the upper bound it falls back to a sequential scan, about 4–5x slower on 1M rows. Broad prefixes were already sequential scans, so no change there. Limits: synthetic single-column table, Postgres only (MySQL and SQLite not measured), count query only, so real tables such as `task_instance` may differ. Since the bug only affects collations with contractions while this change affects all Postgres users, I'm no longer sure this is the right trade-off. Options I see: (1) keep the old bounded range and document the limitation, (2) drop the upper bound only where the collation is known to be affected, (3) something else you'd prefer. I'm happy to rework this PR in whichever direction you want, or close it if (1) is the answer. -- 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]
