sha174n commented on PR #42598: URL: https://github.com/apache/superset/pull/42598#issuecomment-5368859562
Reviewed the full change plus the thread history and the approach holds up: gating the default row cap so it only applies to single-source statements fixes the drop-rows-before-join problem while keeping the cap for plain single-table reads and honoring pushed-down limits via min(). Detecting the multi-source case by parsing (count_referenced_tables) rather than string matching is the right call, and routing it through _check_script_length is a good touch. One coverage note: in the added regression test the join key is a primary key on both sides, so each inner lookup only returns one row; a case with multiple inner matches per outer row would exercise the deferred-rescan path more directly. And the remaining open point (each source read with no limit on the multi-source path) reads as the correct tradeoff here, but a separate configurable ceiling for large multi-source reads would be a sensible follow-up. -- 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]
