Skip pre hash computation for join when input is table scan

[feilong-liu]

Description

Skip hash generation for a join, when the input is table scan, and the hash is on a single big int and is not reused later.

Motivation and Context

We observed in production query that, hash precomputation actually hurts performance (both cpu and latency) for the case described above. Hence add an option to disable hash precomputation for it.

Impact

CPU and latency improvement for the targeted queries.

Test Plan

Existing unit tests and verifier test

Contributor checklist

• Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Add a session property to skip hash precomputation for join when the input is table scan, and the hash is on a single big int and is not reused later. It's controlled by session property `skip_hash_generation_for_join_with_table_scan_input` and default to not enabled.

[vivek-bharathan]

Please add tests showing plan changes

[vivek-bharathan]

I'm curious why we would ever add this hash computation when the parent does not require it. I.e. wouldn't this check simply always be
"return hashComputation.isPresent() && !parentPreference.getHashes().contains(hashComputation.get());"

[feilong-liu]

This optimization is based on our observation, where TableScan below join is significantly than ScanProject (here project is for hash generation) for big int join key. Do not observe the same for other cases.

[mlyublena]

what about other operators like filter/project on top of table scan or values

[feilong-liu]

In verifier suite, didn't observe same performance improvement for these cases, hence limit to the specific case where I see most significant performance improvement here.

[vivek-bharathan]

This was similar to my question above.
If you look at this comment in the code, it seems to suggest that aggregations in general perform better for BIGINT's without the generated hash. I suspect the same principle applies to joins. I wonder if we are special casing this too much in adding the TableScanNode check.
Would it be possible to share the benchmarks you are seeing this behavior on?

[feilong-liu]

If you look at this comment in the code, it seems to suggest that aggregations in general perform better for BIGINT's without the generated hash.

I see, this is because we have custom group by hash BigintGroupByHash for group by on single big int column, which does not utilize existing pre-computed hash. Not sure if join will see the same pattern, but currently we do not have specialized join hash implementation for bigint like group by. Hence what applied to group by may not be available to join here.

Would it be possible to share the benchmarks you are seeing this behavior on?

The benchmarks is based on production queries and cannot be shared.
But the queries which improve most is the queries which have table scan as input source of join. The plan changed from join <- ScanProject to join <- TableScan, and the biggest savings are from changing TableScan to ScanProject, especially when the input table is huge. And this is why I want to specialize for this case in this optimization here.

[vivek-bharathan]

Fair enough. We can always relax this constraint in the future if needed

[feilong-liu]

Please add tests showing plan changes

Added unit plan test

[vivek-bharathan]

Fair enough. We can always relax this constraint in the future if needed