enable supports_filter_during_aggregation for Generic dialect

[goldmedal]

• Related to Enable supports_filter_during_aggregation for the Generic Dialect datafusion#15719

[goldmedal]

@alamb, What do you think about this? I think this change is related to the default behavior of DataFusion.

[alamb]

Makes sense to me @goldmedal -- I think the intent is that the GenericDialect should support as many SQL constructs as possible

The only thing I think this PR needs is a test and it should be good to go

[goldmedal]

The only thing I think this PR needs is a test and it should be good to go

oops , I forgot to mention the tests. I found it will be covered by the original case:

datafusion-sqlparser-rs/tests/sqlparser_common.rs

Lines 11644 to 11655 in 81d8909

	fn test_selective_aggregation() {
	let sql = concat!(
	"SELECT ",
	"ARRAY_AGG(name) FILTER (WHERE name IS NOT NULL), ",
	"ARRAY_AGG(name) FILTER (WHERE name LIKE 'a%') AS agg2 ",
	"FROM region"
	);
	assert_eq!(
	all_dialects_where(|d| d.supports_filter_during_aggregation())
	.verified_only_select(sql)
	.projection,
	vec![

I guess it's enough?

[alamb]

oops , I forgot to mention the tests. I found it will be covered by the original case:

I was thinking a test that would prevent against regressions in a future refactor -- namely a test that will fail if this code change is reverted,

I think as it stands, if the code change is reverted all the tests will still pass 🤔

[goldmedal]

oops , I forgot to mention the tests. I found it will be covered by the original case:

I was thinking a test that would prevent against regressions in a future refactor -- namely a test that will fail if this code change is reverted,

I think as it stands, if the code change is reverted all the tests will still pass 🤔

I see. Make sense. I'll add another test to protect it.

[goldmedal]

I list all the dialects expected to support filtering during aggregation.

[alamb]

Thank you @goldmedal

FYI @iffyio