refactor: Remove never_skip_nulls param from window node def

[TrevorBergeron]

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

[tswast]

Is this a breaking change? The coalesce means that it's never return NULL, only True, right?

[TrevorBergeron]

Its breaking in terms of expression semantics changing such that null guarding isn't part of window op definition. However, its not breaking in that behavior from user-facing api surfaces is preserved (these snapshot tests build trees at low-level so do change in behavior)

[tswast]

I think these low-level tests are in the "change detector test" category and provide more harm than value. https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html

Let's remove them. I think there is utility in golden SQL tests, but only when it reflects the end user APIs.

CC @chelsea-lin @jialuoo

[chelsea-lin]

The value of this approach depends on how we define the end-user APIs and the change detector test. If we assume users only care about query results, the 'golden SQL' might seem unnecessary. However, user feedback indicates otherwise; users frequently inspect the generated SQL via job IDs. Consequently, query readiness and performance are significant priorities. The golden SQL demonstrates that window operation performance improves without additional NULL checking. Although the final results remain the same, this performance optimization is meaningful to the end user.

[tswast]

This is a textbook example of a change detector test.

Trevor extracted logic out of an op and put it in a rewriter. Such changes shouldn't affect our golden SQL tests.

[tswast]

The golden SQL demonstrates that window operation performance improves without additional NULL checking

The semantics changed, not just the performance. These ops no longer ever return NULL. That is a big red flag for a refactor PR.

[chelsea-lin]

yeah, there are no cases from public APIs to apply a window_spec to the AllOps. Hence, we are going to raise an error so that golden tests shouldn't test the unused codes. #2285