bitflicker64 opened a new pull request, #3458:
URL: https://github.com/apache/hugegraph/pull/3458
## Purpose of the PR
Found while scanning `hugegraph-server/hugegraph-core` for small,
test-backed fixes. No issue was filed; each fix comes with a unit test that
fails on master.
- `MergeIterator.close()` closed the merged OLAP iterators but never called
`super.close()`, so the origin iterator stayed open. `RocksDBStore.query` wraps
the main table iterator in a `MergeIterator` when a query reads OLAP
properties; a query closed early (limit, paging) left that native RocksDB
iterator open.
- `ObjectIntMappingFactory` turns a non-positive hash code into its
negation, but `-Integer.MIN_VALUE` is still `Integer.MIN_VALUE`. An id such as
`IdGenerator.of(1L << 31)` (whose `hashCode()` is `Integer.MIN_VALUE`) trips
the `assert code > 0`. With assertions off it gets a negative code, and the
records negate it again to `Integer.MIN_VALUE`, which is `IntMap.NULL_VALUE`,
so the path record drops that parent link.
- `EtcdMetaDriver.etcdMetaDriverBuilder(Object... endpoints)` cast the
varargs array to `String[]` or `URI[]`. A plain varargs call creates an
`Object[]`, so `etcdMetaDriverBuilder("http://a:2379";, "http://b:2379";)` threw
`ClassCastException`. The URI branch also checked every endpoint after the
first against `String`, so two URIs were always rejected with `Invalid
endpoint`. No in-tree caller uses the etcd driver today (all use PD), so this
only affects direct callers.
## Main Changes
- `MergeIterator.close()`: close the merged iterators, then call
`super.close()` in a `finally` block. Double close is safe on this path: the
RocksDB iterator close is guarded by a `closed` flag, and the origin may
already close itself when exhausted.
- `ObjectIntMappingFactory`: map `Integer.MIN_VALUE` to 1, as 0 already is.
Collisions go through the existing probing loop. Codes live only in memory for
one traversal.
- `EtcdMetaDriver.etcdMetaDriverBuilder`: copy the endpoints into a typed
array with `Arrays.copyOf`, and check URI endpoints against `URI`. The message
for mixed types now matches the String branch (`Inconsistent endpoint ...`);
the exception type is unchanged.
- Tests go into classes already listed in `UnitTestSuite`, because
`UnitTestSuite.java` is changed by several open PRs: `QueryResultsTest` (it
already has a closeable counting iterator), `ObjectIntMappingTest` and
`EtcdMetaDriverTest` (it needs the package-private constructor).
No public or protected signature, stored format, config key or lock changes.
## 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:
- New tests:
`QueryResultsTest.testMergeIteratorCloseClosesOriginIterator`,
`ObjectIntMappingTest.testObjectWithMinValueHashCode`, and
`EtcdMetaDriverTest.testBuilderAcceptsStringEndpoints`,
`testBuilderAcceptsUriEndpoints`, `testBuilderRejectsMixedEndpoints`.
- Ran on Linux x86_64, OpenJDK 17.0.20.1, Maven 3.8.7, 2026-11-07. The
runs used commit `9a2f8bba5`, which has the same tree as this PR's head (only
the commit message was reworded).
- Before the fix (production files checked out from master, new tests
kept):
`mvn -ntp -B test -pl hugegraph-server/hugegraph-test -am -P unit-test
-Deditorconfig.skip=true -Dmaven.javadoc.skip=true
-Dtest='ObjectIntMappingTest,QueryResultsTest,EtcdMetaDriverTest'
-Dsurefire.failIfNoSpecifiedTests=false`
All 5 new tests fail: the origin iterator is closed 0 times,
`object2Code` hits the `AssertionError` at `ObjectIntMappingFactory.java:63`,
the String endpoints throw `ClassCastException` (`[Ljava.lang.Object;` to
`[Ljava.lang.String;`), two URIs throw `Invalid endpoint`, and mixed endpoints
throw `ClassCastException` instead of `IllegalArgumentException`.
- With the fix:
`mvn -ntp -B test -pl hugegraph-server/hugegraph-test -am -P unit-test
-Deditorconfig.skip=true -Dmaven.javadoc.skip=true`
Result: BUILD SUCCESS, `UnitTestSuite` ran 1431 tests, 0 failures, 0
errors, 8 skipped. The Surefire XML lists all 5 new testcases as passed.
- `-Deditorconfig.skip=true` is there because master fails the
editorconfig check on `helm/hugegraph/docs/rag.md` (missing final newline), a
file this PR does not touch.
## 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.
--
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]
