[pepijnve]

Which issue does this PR close?

• Closes CommonSubexprEliminate + leaf-expression pushdown duplicate a UDF call wrapped in get_field #25329.

Rationale for this change

What changes are included in this PR?

• Don't allow movement of keep-in-place expressions in filter pushdown
• Avoid duplication of keep-in-place expressions in during leaf expression extraction

What is the testing strategy for this PR?

To be completed

Are there any user-facing changes?

No

[pepijnve]

@adriangb if you have a moment, I could use your input on this one. I got myself into an infinite in push_extraction_pairs when the double projection with the deferred extractions is created. This had me wondering if I put the 'don't duplicate' guard in the wrong place in this optimiser. Since you seem to be the original author of this code, you might have some ideas on how to achieve the same effect in a more appropriate way.

[pepijnve]

There are still some test failures which is why this is still marked draft.

For this particular case, I'm wondering if the expected output is actually an improvement made by the optimiser or not. The arithmetic isn't super expensive, but we are evaluating it twice for all rows and we're not really saving any work in the scan.

────────────┬───────────────────────────────────────────────────────────────────
    1       │-Projection: b * Int32(3) AS a, test.c
    2       │-  Projection: test.a * Int32(2) + test.c AS b, test.c
    3       │-    TableScan: test, full_filters=[(test.a * Int32(2) + test.c) * Int32(3) = Int64(1)]
          1 │+Filter: a = Int64(1)
          2 │+  Projection: b * Int32(3) AS a, test.c
          3 │+    Projection: test.a * Int32(2) + test.c AS b, test.c
          4 │+      TableScan: test
────────────┴───────────────────────────────────────────────────────────────────

The number of expressions being marked as 'keep in place' is probably a bit too broad at the moment though. I'm concerned that this change will prevent too many filter pushdown opportunities as can be seen in this failure.

────────────┬───────────────────────────────────────────────────────────────────
    1     1 │ OpaqueRequirementsExtension
    2     2 │   Sort: t.a ASC NULLS FIRST, t.ts ASC NULLS FIRST
    3       │-    Projection: t.a, CAST(t.ts AS Timestamp(ms, "UTC")) AS ts
    4       │-      Filter: __common_expr_3 > TimestampMillisecond(1000, Some("UTC")) AND __common_expr_3 < TimestampMillisecond(2000, Some("UTC"))
    5       │-        Projection: CAST(t.ts AS Timestamp(ms, "UTC")) AS __common_expr_3, t.a, t.ts
    6       │-          TableScan: t projection=[a, ts], partial_filters=[CAST(t.ts AS Timestamp(ms, "UTC")) > TimestampMillisecond(1000, Some("UTC")), CAST(t.ts AS Timestamp(ms, "UTC")) < TimestampMillisecond(2000, Some("UTC"))]
          3 │+    Filter: t.ts > TimestampMillisecond(1000, Some("UTC")) AND t.ts < TimestampMillisecond(2000, Some("UTC"))
          4 │+      Projection: t.a, CAST(t.ts AS Timestamp(ms, "UTC")) AS ts
          5 │+        TableScan: t projection=[a, ts]
────────────┴───────────────────────────────────────────────────────────────────