vaidikgarg-bit opened a new pull request, #3404:
URL: https://github.com/apache/hugegraph/pull/3404
## Purpose of the PR
- close #3396
`RocksDBSession.BackendColumn` compares name and value bytes in `equals()`
but had no `hashCode()`, so it used identity hashing. A RocksDB scan allocates
a new column per row, so the hash-based iterators in `hg-store-core` never
matched byte-equal rows held in separate objects. With 100 keys read twice, the
intersection with `size = 2` returned 0 rows instead of 100 and the union below
its 10,000-entry sort threshold returned 200. Those are the issue's numbers,
and the new tests reproduce them on `master`.
## Main Changes
- `RocksDBSession.BackendColumn.hashCode()` returns `31 *
Arrays.hashCode(name) + Arrays.hashCode(value)`, the same arrays `equals()`
compares (`Bytes.equals` is `Arrays.equals`, and both handle null arrays
consistently).
- New `FilterIteratorColumnEqualityTest` in `hg-store-test`, registered in
`CoreSuiteTest`. Every row is built from fresh arrays, as the issue asks. It
covers:
- equal hash codes for byte-equal columns, and distinct `HashSet` entries
for the same name with different values;
- `IntersectionFilterIterator` with `size = 2`;
- `UnionFilterIterator` below its sort threshold;
- `MapLimitIterator`.
### Behaviour changes reviewers should know about
- `MapLimitIterator` (the LIMIT_DEDUP and PRECISE_DEDUP-with-limit paths in
`BusinessHandlerImpl`) now drops byte-equal copies. Before, its
`ConcurrentHashSet` kept all of them.
- The in-memory intersection now counts byte-equal rows together, as the
sorted `ReduceIterator` path does. So it also has the limitation the class
comments already describe: a row repeated inside one source counts toward
`size`, so with `size = 2` an element that appears twice in one source and not
in the other is returned. This PR doesn't change that rule. Fixing it needs
per-source counting, which I'd treat as a separate issue.
- No in-tree caller reaches these iterators today. `StoreQueryParam` fixes
`dedupOption` to `NONE`, and the server's `query.dedup_option` setting is never
passed to the store, so only a gRPC client that sets `dedup_option` gets here.
### Out of scope
- Equality compares name and value, while the sorted union and intersection
paths group by name only. With the in-tree INDEX_SCAN (`lookupBack = true`),
`scanIndexToElementId` sets `value` to the owner id, which is the same in every
source, so the two rules agree there. They can differ when `lookup = false`,
where `value` is the raw index value. This PR keeps both rules and doesn't
claim they're equivalent; the same-name/different-value test pins the equality
rule.
- The SortShuffle path of the union (more than 10,000 entries) is #3397.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- `mvn test -pl hugegraph-store/hg-store-test -am -P store-core-test
-Dtest=FilterIteratorColumnEqualityTest
-Dsurefire.failIfNoSpecifiedTests=false`: 4 of the 5 tests fail on `master`
(hash codes differ, the intersection returns no keys, union and limit dedup
return keys twice) and all 5 pass with this change. The iterator tests compare
the exact set of returned keys.
- `mvn test -pl hugegraph-store/hg-store-test -am -P store-core-test`:
139 tests, 0 failures (1 skipped, `IpUtilTest.testConfiguredLinkLocalStays`, an
environment assumption).
- `mvn test -pl hugegraph-store/hg-store-test -am -P
store-rocksdb-test`: 19 tests, 0 failures.
- `mvn editorconfig:check -pl hugegraph-store/hg-store-test -am` and
`mvn apache-rat:check` pass.
## Does this PR potentially affect the following parts?
- [ ] Dependencies ([add/update
license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
info &
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
- [ ] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
- [x] Nope
## Documentation Status
- [ ] `Doc - TODO`: required documentation is pending; complete it before
merging.
- [ ] `Doc - Done`: documentation is included here or linked below.
- [x] `Doc - No Need`: no user-visible documentation is affected.
Documentation files in this PR or paired hugegraph-doc PR:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
