Revert "[GLUTEN-8966][VL] Propagate HashAggregate's ignoreNullKeys when possible"

[zml1206]

What changes are proposed in this pull request?

Reverts #8967.

Spark optimize rule InferFiltersFromConstraints will extract filter IsNotNull, rule PushDownPredicates will push down IsNotNull through aggregate. So the data processed by aggregate is already filtered, adding ignoreNullKeys will incur additional computational cost for velox.

How was this patch tested?

[github-actions]

Run Gluten Clickhouse CI on x86

[github-actions]

Run Gluten Clickhouse CI on x86

[zml1206]

@zhztheplayer @WangGuangxin cc @jackylee-ch @jinchengchenghh

[beliefer]

It seems good to me.

[jackylee-ch]

Thanks for clarifying. I just tested the test case added in #8967 on vanilla Spark — isNotNull is added after the file scan. So we shouldn’t worry about ignoreNullKeys right now.

[jackylee-ch]

@WangGuangxin Are there any other cases that shows we require the ignoreNullKeys? If not, we might revert this PR.

[zhztheplayer]

@beliefer @zml1206 I noticed this PR and #10811 are for similar purposes. Maybe it's a coincidence?

[zml1206]

@beliefer @zml1206 I noticed this PR and #10811 are for similar purposes. Maybe it's a coincidence?

Yes, the purpose is the same, but I think it is more appropriate to remove it. Even if there are other cases, it would be more appropriate to optimize it in the spark optimizer.

[beliefer]

@zhztheplayer @zml1206 Whether reverting or modifying, I think it's okay.

[WangGuangxin]

@beliefer @zml1206 Get. I'm ok to revert it

[jackylee-ch]

Thanks for your great work!

[zml1206]

Thanks for review @jackylee-ch @zhztheplayer @WangGuangxin @beliefer , Merge into main.