fix: apply filter before flat mapping in logical planner

[purplefox]

Description

This PR changes the logic plan so that any filter node gets applied before a flat map node.

Testing done

Non functional change but I've added a new QTT test for a table function with a where clause anyway.

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[big-andy-coates]

Thanks @purplefox

It would probably be worth adding a unit test along the lines of shouldNotInvokeUdtfsForFilteredRows or similar somewhere so that we've got a test that fails should someone put this back the wrong way. Maybe create a unit test for the LogicalPlanner if one doesn't already exist.

[purplefox]

It would probably be worth adding a unit test along the lines of shouldNotInvokeUdtfsForFilteredRows or similar somewhere so that we've got a test that fails should someone put this back the wrong way. Maybe create a unit test for the LogicalPlanner if one doesn't already exist.

Hey @big-andy-coates - tbh I am not a big fan of unit tests constraining details of the implementation, especially non functional detail; the tests become very brittle and they can force a straight jacket around the implementation making it hard to refactor. Let the implementation be free, and constrain the interface not the implementation! (Don't get me started on fine grained unit tests and TDD, but I think you can infer my opinion on that already ;) )

For this kind of thing, I'd say the right way of checking behaviour is via a performance regression test (which I know we don't have yet). If the performance does regress after someone changes the order then we'll catch it. But maybe the performance doesn't regress after someone changes it, in which case it's OK to re-order it, and perhaps our assumptions about what is slow and what is not were wrong in the first place.