Skip to content

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

Merged
CurtHagenlocher merged 3 commits into
apache:mainfrom
vthemelis:constant-col-lookup
Mar 31, 2026
Merged

GH-301: [C#] Use an index lookup for O(1) field index access#300
CurtHagenlocher merged 3 commits into
apache:mainfrom
vthemelis:constant-col-lookup

Conversation

@vthemelis
Copy link
Copy Markdown
Contributor

@vthemelis vthemelis commented Mar 30, 2026

Closes #301.

Ports the optimization from the closed PR at apache/arrow#44633 into the new .NET-specific repository.

The original PR was closed on November 18, 2025 with the note that the C# implementation had moved to a new repository.

This version keeps the current arrow-dotnet behavior intact:

  • GetFieldIndex(..., comparer: null) and the default path now use a cached CurrentCulture index lookup for the common case.
  • Missing fields still return -1.
  • Duplicate field names still return the first match.
  • Non-default comparers still fall back to the existing linear scan.

I also added dedicated schema tests covering:

  • null, Ordinal, OrdinalIgnoreCase, and CurrentCulture comparers
  • duplicate-name lookup returning the first match
  • missing-name behavior for each comparer

Local verification:

  • dotnet build test/Apache.Arrow.Tests/Apache.Arrow.Tests.csproj
  • DOTNET_ROLL_FORWARD=Major DOTNET_ROLL_FORWARD_TO_PRERELEASE=1 dotnet test test/Apache.Arrow.Tests/Apache.Arrow.Tests.csproj --no-restore --logger 'console;verbosity=minimal'

@vthemelis vthemelis changed the title GH-44501: [C#] Use an index lookup for O(1) field index access GH-301: [C#] Use an index lookup for O(1) field index access Mar 30, 2026
Copy link
Copy Markdown
Contributor

@adamreeve adamreeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me thanks @vthemelis

From apache/arrow#44633 there were a couple of issues raised:

  • This doesn't support lookup of multiple indexes for the same name. I think this is OK, the PR just speeds up the existing API without changing behaviour (the first matching index is returned). If there's a need to support lookup with multiple results that could be a separate change.
  • Use of StringComparer.CurrentCulture. I think it's correct to use this here to avoid changing behaviour. If we want to switch to the Ordinal comparer being the default, that should be a separate change.

@CurtHagenlocher CurtHagenlocher merged commit fc49cf3 into apache:main Mar 31, 2026
14 checks passed
@vthemelis vthemelis deleted the constant-col-lookup branch March 31, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use an index lookup for O(1) field index access

3 participants