vaidikgarg-bit opened a new pull request, #3424:
URL: https://github.com/apache/hugegraph/pull/3424

   ## Purpose of the PR
   
   - close #3415
   
   `AggregativeQueryObserver.readBatchData` calls `hasNext()` on the result 
iterator before it checks the batch size, then calls it once more after the 
loop to decide whether the query is finished. `MapLimitIterator`, 
`InAccurateUnionFilterIterator` and `InAccurateIntersectionIterator` start 
every `hasNext()` with `current = null` and pull the next unseen row. The 
second probe throws away a row that is already recorded in the dedup set or 
bitmap, so it never comes back. With a batch size of 1 and three distinct rows, 
a `LIMIT_DEDUP` query returns two.
   
   ## Main Changes
   
   - `hasNext()` in the three iterators returns `true` while a row is pending 
instead of reading past it
   - `next()` throws `NoSuchElementException` once the iterator is exhausted, 
matching `UnionFilterIterator`
   - after `close()` the iterators stop probing the source and `isValid()` 
returns `false`; the dedup set or bitmap is cleared even if closing the source 
throws
   - `FilterIteratorContractTest` now covers the three iterators: a pending row 
survives repeated probes, no row is lost across single-record batches, and the 
existing close checks run against them too
   
   The `next()` and `close()` changes are there because the existing contract 
tests loop over every filter kind, so adding these iterators to the test means 
they have to meet the same contract `UnionFilterIterator` already does.
   
   ## 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:
       - `FilterIteratorContractTest`: on master the new batch tests fail with 
`expected:<3> but was:<2>` and the pending-row and close tests fail; with this 
change every test in the class passes
       - `store-server-test` (189), `store-core-test` (141, 1 skipped) and 
`store-rocksdb-test` (19) pass locally
   
   ## 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]

Reply via email to