GH-38792: [C++][Gandiva] Optimize cache

[likun666661]

Rationale for this change

In Gandiva, it has been observed that the tool caches compiled expressions. However, when using the expression cache, Gandiva takes into account the schema, expr->ToString, and specific parameters. Nevertheless, in llvm_generator.cc, Gandiva still traverses the expressions to generate LLVM IR code and then produces specific JIT functions. Considering that llvm_generator traverses expressions and calculates the memory start positions of fields involved in the computation, we can further enhance Gandiva's caching mechanism.

Specifically, we can exclude the influence of the schema. Simultaneously, we can also exclude the field names' information involved in the expression computation. For example, for a schema {f0:int32, f1:int32} with an expression add(f0, f1), we can cache it initially. Then, for a schema {a0:int32, a1:int32, a2:int32} with an expression add(a1, a2), it should not require recompilation; it can directly reuse the JIT function generated by the expression {f0:int32, f1:int32}, add(f0, f1).

What changes are included in this PR?

This PR primarily alters the construction of the cache key and makes modifications to the schema validation in both the projector and filter. It also adjusts the validation of fields in expr_validator. Additionally, a ToCacheKeyString function has been added to each node. This PR mainly focuses on these areas.

Are these changes tested?

Yes,these changes are tested.

Are there any user-facing changes?

No.

#38792

• Closes: [C++] [Gandiva] Optimize Gandiva Cache #38792

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - How to contribute patches

[github-actions]

⚠️ GitHub issue #38433 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #38434 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #38790 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #38792 has been automatically assigned in GitHub to PR creator.

[pitrou]

@js8544

[js8544]

Thanks for your contribution! The proposal sounds good: caching at node level is indeed better. I have a couple of questions regarding the implementation:

1. Could you resolve the conflicts so that we can initiate the CI checks?
2. I see that schema checks are completely removed. Will invalid execution happen, e.g. Projector of one type executed on a RecordBatch of another type? If so, is it possible to keep the checks on types and only skip the checks on names?

[likun666661]

Thanks for your contribution! The proposal sounds good: caching at node level is indeed better. I have a couple of questions regarding the implementation:

1. Could you resolve the conflicts so that we can initiate the CI checks?
2. I see that schema checks are completely removed. Will invalid execution happen, e.g. Projector of one type executed on a RecordBatch of another type? If so, is it possible to keep the checks on types and only skip the checks on names?

1. I will do this later.
2. For second，gandiva expr validator will check the tree expr. So , when in function node ,it will check the fieldnode type and thus it will checks on types and only skip the checks on names.

[likun666661]

Thanks for your contribution! The proposal sounds good: caching at node level is indeed better. I have a couple of questions regarding the implementation:

1. Could you resolve the conflicts so that we can initiate the CI checks?
2. I see that schema checks are completely removed. Will invalid execution happen, e.g. Projector of one type executed on a RecordBatch of another type? If so, is it possible to keep the checks on types and only skip the checks on names?

I add some check for invalid execution happen as you said. Thanks for the advice.

[js8544]

@kou Could you help approve the CI workflows? Thanks!

[kou]

Approved!

[likun666661]

I implemented type validation for calculations within Gandiva. All of my local tests passed successfully before I add the type validation check. However, it appears that some existing tests in Gandiva have fields with types that may not necessarily match the data types. In such cases, my type validation is blocking them. As a result, a few Gandiva tests did not pass on the CI. Could you please approve another round of CI testing? @kou

[kou]

Approved!
FYI: You can run CI on your fork by enabling GitHub Actions on your fork.

[thisisnic]

Thank you for your contribution. Unfortunately, this
pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label
or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you
do not have repository permissions to reopen the PR, please tag a maintainer.

[lriggs]

@likun666661 Are you interested in finishing this still? If not Im happy to take it on.