Thanks for the PR! We're going to have to think carefully whether or not we want to make this change. A few top of mind concerns that may not be reflected in the current test suite:

• Possible performance issues. For example, does DuckDB actually optimize away the subquery when not needed?
• Possible impacts on the DataCubeIndexer. We need to make sure that the static query analysis performed by the indexer still works as intended with changes to all generated mark queries.

I also need to think about any possible alternative approaches to achieving the desired functionality.

Just thinking through different options, one possibility might be to include an explicit key for window expressions. So alongside expr (for normal expressions) and agg (for aggregate expressions), we might add win or similar for window expressions. Generally I don't like growing the surface area this way if we can avoid it, but it would enable more targeted query generation if we identify any issues with using CTEs or subqueries by default.

One smaller change might be to use a chained subquery directly, rather than a CTE using with. Though in theory this should only be a syntactic change to the current PR, it is possible that this could be optimized differently depending on DB internals.

From having used this change locally for a bit, if nothing else, I do find that the sql queries logged to the console get uglier / harder to debug with this change. So I am particularly sympathetic to something that does not change the queries where it's not needed (even if they get optimized away fully, which in the few examples I've looked at using duckdb's DESCRIBE it seemed like they were).

Unfortunately, I have a poor grasp of the DataCubeIndexer so I'm not sure about the implications there -- can just lend the data point that having used this change for a few weeks, I haven't seen any wonky behavior associated with this change.

If you've thought about it more and are on-board with it I can look into adding something like your suggestion of win.