Apply filter early in TopK

[Dandandan]

Which issue does this PR close?

• Closes Optimize TopK with filter #15699

This depends on #15770

Rationale for this change

Perf changes (combined PRs - I'll rerun once #15770 is merged ):

Details

--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃        main ┃ topk-apply-filter ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │    13.34 ms │          13.29 ms │     no change │
│ QQuery 1     │    18.76 ms │          18.94 ms │     no change │
│ QQuery 2     │    55.91 ms │          57.17 ms │     no change │
│ QQuery 3     │    56.22 ms │          54.68 ms │     no change │
│ QQuery 4     │   515.18 ms │         442.01 ms │ +1.17x faster │
│ QQuery 5     │   668.61 ms │         657.18 ms │     no change │
│ QQuery 6     │    14.78 ms │          14.26 ms │     no change │
│ QQuery 7     │    21.12 ms │          21.05 ms │     no change │
│ QQuery 8     │   531.48 ms │         524.48 ms │     no change │
│ QQuery 9     │   798.76 ms │         742.72 ms │ +1.08x faster │
│ QQuery 10    │   173.24 ms │         170.70 ms │     no change │
│ QQuery 11    │   185.32 ms │         187.51 ms │     no change │
│ QQuery 12    │   739.43 ms │         696.25 ms │ +1.06x faster │
│ QQuery 13    │   961.74 ms │         869.97 ms │ +1.11x faster │
│ QQuery 14    │   631.79 ms │         590.30 ms │ +1.07x faster │
│ QQuery 15    │   594.81 ms │         537.24 ms │ +1.11x faster │
│ QQuery 16    │  1344.23 ms │        1208.84 ms │ +1.11x faster │
│ QQuery 17    │  1269.51 ms │        1152.34 ms │ +1.10x faster │
│ QQuery 18    │  2341.58 ms │        2302.53 ms │     no change │
│ QQuery 19    │    50.48 ms │          45.09 ms │ +1.12x faster │
│ QQuery 20    │   960.37 ms │         973.62 ms │     no change │
│ QQuery 21    │  1046.35 ms │        1086.33 ms │     no change │
│ QQuery 22    │  1615.74 ms │        1642.84 ms │     no change │
│ QQuery 23    │  5739.41 ms │        5630.90 ms │     no change │
│ QQuery 24    │   349.55 ms │         345.82 ms │     no change │
│ QQuery 25    │   346.15 ms │         227.88 ms │ +1.52x faster │
│ QQuery 26    │   411.18 ms │         332.30 ms │ +1.24x faster │
│ QQuery 27    │  1278.73 ms │        1266.88 ms │     no change │
│ QQuery 28    │ 10645.79 ms │       10677.74 ms │     no change │
│ QQuery 29    │   434.18 ms │         438.49 ms │     no change │
│ QQuery 30    │   559.20 ms │         571.79 ms │     no change │
│ QQuery 31    │   540.42 ms │         538.21 ms │     no change │
│ QQuery 32    │  2347.65 ms │        2413.47 ms │     no change │
│ QQuery 33    │  2798.99 ms │        2721.18 ms │     no change │
│ QQuery 34    │  2972.11 ms │        2980.84 ms │     no change │
│ QQuery 35    │   807.84 ms │         827.81 ms │     no change │
│ QQuery 36    │    81.23 ms │          84.15 ms │     no change │
│ QQuery 37    │    36.27 ms │          35.63 ms │     no change │
│ QQuery 38    │    80.34 ms │          82.46 ms │     no change │
│ QQuery 39    │   130.78 ms │         133.40 ms │     no change │
│ QQuery 40    │    27.35 ms │          28.20 ms │     no change │
│ QQuery 41    │    26.51 ms │          26.34 ms │     no change │
│ QQuery 42    │    21.86 ms │          23.00 ms │  1.05x slower │
└──────────────┴─────────────┴───────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary                ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (main)                │ 44244.29ms │
│ Total Time (topk-apply-filter)   │ 43395.86ms │
│ Average Time (main)              │  1028.94ms │
│ Average Time (topk-apply-filter) │  1009.21ms │
│ Queries Faster                   │         11 │
│ Queries Slower                   │          1 │
│ Queries with No Change           │         31 │
│ Queries with Failure             │          0 │
└──────────────────────────────────┴────────────┘


Benchmark run_topk_tpch.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃      main ┃ topk-apply-filter ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ Q1           │  28.59 ms │          16.69 ms │ +1.71x faster │
│ Q2           │  30.36 ms │          20.65 ms │ +1.47x faster │
│ Q3           │ 101.92 ms │          54.89 ms │ +1.86x faster │
│ Q4           │  27.61 ms │          22.37 ms │ +1.23x faster │
│ Q5           │  19.31 ms │          12.73 ms │ +1.52x faster │
│ Q6           │  42.45 ms │          26.39 ms │ +1.61x faster │
│ Q7           │  85.18 ms │          70.72 ms │ +1.20x faster │
│ Q8           │ 103.80 ms │          46.25 ms │ +2.24x faster │
│ Q9           │  99.54 ms │          63.53 ms │ +1.57x faster │
│ Q10          │ 141.00 ms │         100.62 ms │ +1.40x faster │
│ Q11          │  78.28 ms │          56.94 ms │ +1.37x faster │
└──────────────┴───────────┴───────────────────┴───────────────┘

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[adriangb]

Can you make a PR on our fork and we merge this into the main PR?

[adriangb]

Or you can just push to the main PR, I gave you write access to our fork :)

My one question is: how does this optimization play with filter pushdown? If a child plan accepted the filter as Exact should we then not re-filter? A related question which you've alluded to before: if no child plan accepted the filter at all should we avoid updating it?

[Dandandan]

Or you can just push to the main PR, I gave you write access to our fork :)

My one question is: how does this optimization play with filter pushdown? If a child plan accepted the filter as Exact should we then not re-filter? A related question which you've alluded to before: if no child plan accepted the filter at all should we avoid updating it?

I think avoiding running the filter twice for "exact cases" is optimal.
In practice, I am not sure if in this case it would add much overhead: converting to rows and comparing against / updating the heap / running the compaction logic will be the most expensive part by far in this scenario.
It will be hard I think to show it being much slower somewhere.

Your second question: I think if we actually always use the filter for topk, I guess it isn't really wasteful. I think theoretically we should do it just before runing the topk instead of after (to avoid running it for the last iteration without using it). But also here I think it will be hard to show any benefit.

[Dandandan]

I applied the changes to #15770 let's continue there