Skip to content

fix(graph): compare integral store sort fields across number types - #114

Merged
yuluo-yx merged 1 commit into
agentic-ai-java:mainfrom
dvd233:fix/graph-integral-store-sorting
Oct 7, 2026
Merged

yuluo-yx merged 1 commit into
agentic-ai-java:mainfrom
dvd233:fix/graph-integral-store-sorting

Conversation

@dvd233

@dvd233 dvd233 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

Fix sorting Store fields whose values have different standard integral Java types.

For example, writing 1L and 2147483648L to FileSystemStore and reading them back produces an Integer and a Long under Jackson's default untyped deserialization. Sorting that field currently throws ClassCastException. Values beyond the signed-long range can also become BigInteger.

This is a minimal ARGI port of alibaba/spring-ai-alibaba#5001, following the contribution-location suggestion. The original PR remains separate.

Does this pull request fix one issue?

NONE in this repository.

Describe how you did it

  • Compare mixed Byte, Short, Integer, Long, and exact BigInteger values through BigInteger, without floating-point conversion or narrowing large integers.
  • Preserve same-class comparison, custom-number dispatch, null/missing-value ordering, secondary sort fields, and unrelated comparable behavior.
  • Keep the current locale-independent search and filesystem-directory fixes intact.
  • Add regression coverage for both sort directions, JSON round trips, values beyond 2^53 and signed-long range, cross-type equalities, comparator contracts, and unaffected comparables.

The production change is 14 added lines with private helpers only. No public API, dependency, serialized format, schema, or default serializer setting changes.

Describe how to verify it

./mvnw -B -ntp -pl argi-graph-core -Dspotless.apply.skip=true -Dtest=BaseStoreIntegralSortingTest test
./mvnw -B -ntp -pl argi-graph-core -Dspotless.apply.skip=true -Dtest=BaseStoreIntegralSortingTest,MemoryStoreTest,FileSystemStoreTest,DatabaseStoreTest,DatabaseStoreAllDialectTest,StoreIntegrationTest,GraphStoreIntegrationTest,StoreSearchLocaleTest test
./mvnw -B -ntp -pl argi-graph-core -Dspotless.apply.skip=true checkstyle:check spotless:check

Source-bound native validation passed on e3a102ae0c56b74591814f96453498d09185d67a, based on ARGI e24b9988ae0be6126f2bf927ea94ff9b989691ee, using Maven 3.10.0 and Temurin Java 17.0.20.1:

  • Unchanged baseline plus the new regression: exactly 12 intended ClassCastException errors and 3 passing controls, with zero assertion failures or skipped cases.
  • Candidate regression: 15/15 passed.
  • Selected Store suite: 79/79 passed across 8 classes, with no failures, errors, or skips.
  • Checkstyle: 0 violations.
  • Read-only Spotless: 293 Java files clean, 0 needing changes, 0 skipped by caching. Lifecycle spotless:apply was disabled.

The runner checked the exact source commit, tree, parent, and changed-file hashes before and after each phase. Native baseline and candidate regression runs rebuilt 205 production and 88 test sources. The selected tests ran offline with IP egress disabled after dependency warm-up.

Local make lint also passed. Full reactor tests, Java 21, live external-service integration, current license-eye 0.9, Extensions, and aggregate compatibility verification are not claimed here.

Special notes for reviews

Prepared with AI assistance, including automated tests and independent AI-assisted reviews. No human-review attestation is made. Please pay particular attention to the intentionally narrow integral whitelist and preservation of existing fallback semantics.

The own-fork validation harness is on a separate seven-file validation branch and is not part of this two-file PR. The selected Store suite includes the current locale and filesystem controls, local H2 tests, mocked SQL-dialect tests, and Store/graph integration tests.

@yuluo-yx yuluo-yx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM thanks

@yuluo-yx
yuluo-yx merged commit 317e6ba into agentic-ai-java:main Oct 7, 2026
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.

2 participants