GH-44501: [C#] Use an index lookup for O(1) field index access

[vthemelis]

Fixes #44501

Rationale for this change

This should speed up the reasonably common operation of looking up a column via name in Schema or RecordBatch.

I would argue that the fact that this operation was previously O(N) (now O(1)) is unintuitive and could easily lead to accidental performance woes.

Note that I would like to also replace the existing Lookups with signature string -> Field but unfortunately those use StringComparer.Default instead of StringComparer.CurrentCulture (which I'm using to make the changed public function backwards compatible). Not sure if this is intentional.

What changes are included in this PR?

This PR simply memoizes the index lookup of the fields. It should not break any existing behaviour.

It also fixes a recent regression about the behaviour of the changed function in the case of using a custom comparator and have no matches in the schema.

Are these changes tested?

All code paths changed are covered by new unit tests.

Are there any user-facing changes?

Yes, the lookup function throws an exception when there are no matches fuels rather than returning -1. This was a regression introduces O(days) ago.

• GitHub Issue: [C#] Column(string) method in RecordBatch is linear to the number of columns #44501

[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 - Contributing Overview

[github-actions]

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

[vthemelis]

@CurtHagenlocher it looks like my tests are failing in CI but I can't replicate this locally.

[vthemelis]

Oh, I see the issue. The CI does a merge into main first and this has changed since my PR: #44576

The PR above has changed the lookup behaviour when the field is not matched.

CC @georgevanburgh

[vthemelis]

@CurtHagenlocher and @georgevanburgh I've reverted to the original behaviour (throwing an InvalidOperationException) for now. Let me know if you prefer keeping the return -1 regression.

[CurtHagenlocher]

I'm a bit tied up with work but will look at this by Friday.

[vthemelis]

I'm a bit tied up with work but will look at this by Friday.

Thank you very much!