Columns with identifiers that don't match the requirements of `GenericDialect` aren't pushed down through window and aggregate operators

[masonh22]

Describe the bug

In the push-down filter optimization for window operators, we create a set of all the columns that appear in the window "partition by" expression and check whether all columns in a filter are contained in the set of "partition by" columns. If so, that filter can be pushed down past the window operator.

The problem is with how columns are extracted from the "partition by" expression. Currently, the code calls Expr::schema_name() to serialize the column name to a string, then parses it into a column using Column::from_qualified_name(). The problem is that if Column::from_qualified_name() gets a column name that can't be parsed by the rules of GenericDialect, it throws the whole string into the name field, ignoring the relation.

For example, if we have a partition column that looks something like Column { relation: Some(Bare { table: "test" }), name: "$a", .. }, the window push down rule will extract this into Column { relation: None, name: "test.$a", .. }. Then, the logic will falsely identify that a filter containing only the column "test.$a" can't be pushed down because it's represented differently than the column we extracted.

To Reproduce

Create a table with field names that start with a '$', create a logical plan with a window and filter operation that should be pushed down, and inspect the optimized plan.

Expected behavior

The filter should be pushed down.

Additional context

No response

[masonh22]

One thing to note which I haven't tested is that the Aggregate operator may have the same issue as it does the same thing when extracting the group_by columns:

// We can push down Predicate which in groupby_expr.
let group_expr_columns = agg
    .group_expr
    .iter()
    .map(|e| Ok(Column::from_qualified_name(e.schema_name().to_string())))
    .collect::<Result<HashSet<_>>>()?;

[Omega359]

The current behaviour of Column::from_qualified_name seems a bit of a footgun to me.

[masonh22]

Update: this does affect Aggregate operators as well. This may also be more prevalent than I initially suspected. In some dataframe tests that use aggregates and filters, filters weren't being pushed down because the relation is "?table?", which doesn't get parsed correctly by Column::from_qualified_name().