Repository navigation
Fix #3606: Avoid allocations in ODataPath equality - #3608
Conversation
Replace the indexed LINQ query with a direct segment loop while preserving null validation, length checks, ordered comparison, and first-mismatch short-circuiting. Add coverage for empty paths, null input, multiple equivalent segments, and mismatches at the first, middle, and last positions.
|
@RuneH-MSFT, could you please review this fix for #3606? |
|
/AzurePipelines run |
There was a problem hiding this comment.
Pull request overview
This PR optimizes ODataPath equality checks by replacing an indexed LINQ predicate with a simple indexed for loop to avoid iterator/allocation overhead while preserving existing behavior (null validation, length checks, ordered comparison, and short-circuiting). It also expands unit test coverage to ensure equality behaves correctly across empty paths, null inputs, multi-segment paths, and mismatches at various positions.
Changes:
- Replace the LINQ-based segment comparison in
ODataPath.Equals(ODataPath other)with a direct indexed loop and early return on first mismatch. - Add unit tests covering empty-path equality, null argument behavior, multi-segment equality, and mismatches at first/middle/last segment positions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Microsoft.OData.Core/UriParser/SemanticAst/ODataPath.cs |
Replaces LINQ-based equality evaluation with an allocation-free indexed loop while keeping existing validation and semantics. |
test/UnitTests/Microsoft.OData.Core.Tests/UriParser/SemanticAst/ODataPathTests.cs |
Adds targeted tests to validate equality behavior for empty paths, null input, multi-segment paths, and mismatch positions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
RuneH-MSFT
left a comment
RuneH-MSFT
left a comment
There was a problem hiding this comment.
@xuzhg - This looks good. Thank you!
Replace the indexed LINQ query with a direct segment loop while preserving null validation, length checks, ordered comparison, and first-mismatch short-circuiting.
Add coverage for empty paths, null input, multiple equivalent segments, and mismatches at the first, middle, and last positions.
Issues
This pull request fixes #xxx.
Description
Briefly describe the changes of this pull request.
Checklist (Uncheck if it is not completed)
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.
Repository notes
Team members can start a CI build by adding a comment with the text
/AzurePipelines runto a PR. A bot may respond indicating that there is no pipeline associated with the pull request. This can be ignored if the build is triggered.Team members should not trigger a build this way for pull requests coming from forked repositories. They should instead trigger the build manually by setting the "branch" to
refs/pull/{prId}/mergewhere{prId}is the ID of the PR.