Skip to content

Support FilterExec with embedded projections in WindowTopN optimization #21596

Description

@SubhamSinghal

Is your feature request related to a problem or challenge?

Discussion thread

The WindowTopN physical optimizer rule currently skips FilterExec nodes that have an embedded projection (filter.projection().is_some()). This happens when ProjectionPushdown runs before WindowTopN and embeds a projection into the FilterExec.

// Don't handle filters with projections
if filter.projection().is_some() {
    return None;
} 

Describe the solution you'd like

Extend WindowTopN to handle FilterExec with embedded projections. This would require:

  1. Extracting the projection mapping from FilterExec
  2. Resolving the window column index through the projection (mapping projected column indices back to the original schema)
  3. Carrying the projection forward into the replacement plan (either re-wrapping PartitionedTopKExec with a ProjectionExec, or embedding the projection)

Describe alternatives you've considered

No response

Additional context

No response

Activity

  1. SubhamSinghal commented on Aug 1, 2026

    @SubhamSinghal
    ContributorAuthor

    PR: #23599

  2. zhuqi-lucas commented on Oct 9, 2026

    @zhuqi-lucas
    Contributor

    One correction to the premise here, found while attempting this in #23599.

    This happens when ProjectionPushdown runs before WindowTopN and embeds a projection into the FilterExec.

    That isn't true of the default rule list. In datafusion/physical-optimizer/src/optimizer.rs, WindowTopN is at position 117 and ProjectionPushdown at 142 — WindowTopN runs first, so a FilterExec reaching it never carries an embedded projection. The guard it hits (filter.projection().is_some()) is unreachable from a stock pipeline, and try_swapping_with_projection (driven by ProjectionPushdown) is the only producer of embedded filter projections.

    The shape does occur in pipelines that re-run projection pushdown earlier, which is where this came from. But that means the capability can't be exercised by sqllogictest as it stands: every test I wrote for it either passed on main unchanged, or needed a hand-built non-default rule order, so nothing keeps it honest as the rule list evolves.

    Per the discussion in #23599, the projection handling is dropped there for that reason (the PR is now just a bail-out guard for FilterExec::fetch). #26152 tracks running the slt corpus under an alternative optimizer pipeline, which is what would make this testable — worth treating as a prerequisite rather than writing the handling first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions