Fix equality for identifier sets of different sizes

[ItsM0rty]

Identifiers::operator== uses the three-iterator overload of std::equal. It treats a matching left-hand prefix as equal, so an empty set compares equal to any set. If the left-hand range is longer, the comparison can overrun the right-hand range; the checked-iterator build detects this invalid access. This operator is used by the pickup-and-delivery initial-solution invariant comparing all orders against assigned and unassigned orders.

This uses the underlying sets' equality operator, which checks both size and contents.

The new standalone C++17 test checks every pair of subsets for signed and unsigned identifiers, plus extreme values. Expected results come from the subset masks. The CI job runs the test in optimized and checked-iterator builds without needing a database.

Validation:

• The original code fails the regression on Windows and Linux. A separate checked-iterator probe also catches the longer-left-hand-range case.
• All 2,048 subset comparisons pass with the fix using Windows GCC 16.2, Linux GCC 13.3, and Clang 18.1.3. I tested optimized and checked-iterator builds, plus ASan/UBSan on Linux.
• cpplint and whitespace checks pass, and the workflow YAML parses.

I haven't built the full extension or run the database-backed pgTAP suite.

Summary by CodeRabbit

• Bug Fixes

  • Identifier sets with different numbers of elements are now correctly recognized as unequal.

• Tests

  • Added coverage for equality comparisons across signed and unsigned identifier sets.
  • Automated checks now compile and run the tests in optimized and debug configurations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 0a132248-137a-481f-baa2-4d8cfbcf6902

📥 Commits

Reviewing files that changed from the base of the PR and between 2572819 and bdeffa7.

📒 Files selected for processing (3)

• .github/workflows/check-files.yml
• include/cpp_common/identifiers.hpp
• tools/testers/identifiers_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Identifiers<T>::operator== now compares underlying sets directly. A new test checks all subset-mask pairs for signed and unsigned values. A GitHub Actions job builds and runs the test in optimized and checked-iterator configurations.

Changes

Identifier Set Equality

Layer / File(s)	Summary
Set equality implementation include/cpp_common/identifiers.hpp	Identifiers<T>::operator== now uses std::set equality, which compares set sizes as well as elements.
Equality validation tools/testers/identifiers_test.cpp, .github/workflows/check-files.yml	The test checks all pairs of subsets for signed and unsigned values. The workflow builds and runs it in optimized and checked-iterator configurations with warnings treated as errors.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to bdeff

The equality fix has no identified merge-blocking issue. Merge after normal checks.

Security architecture risk: ⚪ Minimal · up to bdeff

The change restores complete-set equality without adding production entrypoints, privileges, or state transitions. The standalone regression test runs under the workflow’s existing read-only permissions. No material security risk was identified in the changed code.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed production behavior is bounded to existing identifier comparisons and their callers. The supplied downstream example is an in-process order-partition assertion; the PR adds no production caller or additional authority around that comparison.

Trust Boundaries and Controls

• observed — The new test executes repository-controlled code under existing push and pull_request triggers with contents: read permissions. It uses a hosted runner, has no explicit secret inputs or elevated deployment permissions, and takes no external runtime input. Its helper functions have anonymous-namespace scope rather than forming production entrypoints.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: correcting equality for identifier sets with different sizes.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each set with care,
Five signed values hop in the air.
Five unsigned join the testing run,
Two thousand checks are counted, done.
The build hops through both modes with cheer.}

_{Comment @coderabbitai help to get the list of available commands.}