Fix TVList iterator state across memtable page switches

[shuwenwei]

Summary

Fix two related TVList iterator state issues:

• Preserve a prepared duplicate-timestamp group when switching fake pages without moving the aligned iterator cursor.
• Clear stale prepared state after non-aligned batch reads, so a later point reader cannot reuse an unprepared index.

The PR also adds focused ASC and DESC regression tests for both aligned and non-aligned paths.

Root Causes

1. Aligned point reader across an empty/effectively empty page

TVListIterator.skipToCurrentTimeRangeStartPosition() previously invalidated probeNext unconditionally, even when the binary search did not move the iterator cursor.

For aligned TVLists, a duplicate-timestamp group can be prepared before the current page boundary. selectedIndices then contains the per-column value rows for that group, while index is left at the last physical row of the group.

If a following fake page does not contain that prepared timestamp, the binary search can return the current index. The old code then cleared probeNext, so the next probe recomputed the group from its last physical row and lost earlier non-null values.

2. Non-aligned batch-to-point transition

Non-aligned TVListIterator.nextBatch() advances index while consuming a batch but did not clear probeNext before returning. A later point reader could therefore observe:

index has advanced
probeNext is still true

That state violates the iterator invariant. The point path skips prepareNext() and directly consumes the current index, potentially returning an old duplicate value or a deleted point instead of the normalized latest value.

Complete Trigger Flow

Preconditions:

1. One single TVList has multiple physical rows that require iterator normalization, such as duplicate timestamps or points hidden by deletion.
2. A MemChunk is represented by multiple fake page wrappers sharing one MemPointIterator.
3. A page is consumed through the batch path and a later page is consumed through the point/overlap path.
4. The transition includes a page whose cursor does not move, or whose current timestamp is already inside the next page range.

Aligned sequence:

1. The first page reaches its boundary and calls hasNextTimeValuePair().
2. prepareNext() scans the next duplicate-timestamp group beyond the current page boundary.
3. selectedIndices keeps the correct non-null row for each measurement, while index ends at the last physical row of the duplicate group.
4. The current page returns false but leaves the prepared group with probeNext = true.
5. setCurrentPageTimeRange() switches to an empty or effectively empty page.
6. skipToCurrentTimeRangeStartPosition() finds that the prepared timestamp is outside the page and the cursor does not move.
7. The old implementation cleared probeNext despite the cursor remaining on the prepared group.
8. The next probe recomputes the group from its final physical row and loses values stored in earlier rows.
9. A later page then emits null for the missing measurement at that timestamp.

Example:

Physical rows:
  time=1:   s0=1,  s1=null
  time=100: s0=2,  s1=null
  time=100: s0=null, s1=20

Expected merged rows:
  time=1:   [1, null]
  time=100: [2, 20]

Without the fix:
  time=100: [null, 20]

Non-aligned sequence:

1. Batch consumption prepares and emits the first page.
2. The batch loop advances index to the first physical row of the next timestamp group and stops at the page boundary.
3. nextBatch() returns without clearing probeNext.
4. The next page is empty or the current timestamp is already inside its range, so the stale flag is preserved.
5. The point reader skips prepareNext() and consumes the unprepared index directly.
6. Duplicate versions or deleted points can be returned.

Example:

Physical rows:
  time=1:   value=1
  time=100: value=2
  time=100: value=3

Expected point result for time=100:
  [3]

Without the non-aligned fix:
  [2, 3]

A deletion variant can return a deleted point because the deletion check in skipDeletedOrTimeNotSatisfiedRows() is bypassed.

Fix

Aligned point state

Preserve probeNext when the cursor does not move:

• If newIndex > index, the cursor advanced and the prepared state is invalidated.
• If newIndex == index, the prepared duplicate-timestamp group remains valid for a later page.

Non-aligned batch state

After TVListIterator.nextBatch() builds and returns its block, clear probeNext. This matches the existing aligned batch behavior and ensures that any later point read performs prepareNext() from the current physical index.

Tests

Added focused regression tests for:

• Aligned ASC page switching: [1,33], [34,66], [67,100]
• Aligned DESC page switching: [67,100], [34,66], [1,33]
• Non-aligned ASC batch-to-point duplicate timestamps
• Non-aligned DESC batch-to-point deletion handling

The aligned ASC test reproduces the pre-fix Expected: 2 / Actual: null failure. The non-aligned tests reproduce duplicate or deleted data when stale probeNext is preserved.

These are iterator-level regression tests. They verify the state transitions directly and do not cover the complete SeriesScanUtil -> LazyMemVersionPageReader -> PriorityMergeReader integration path.

Test Scope and Realistic Repro Conditions

The regression tests intentionally use minimal iterator fixtures and explicitly supply fake-page time ranges so that the state transitions are deterministic. They do not claim that a few physical rows would naturally produce multiple fake pages.

In a real memtable, AlignedReadOnlyMemChunk derives the fake-page count from the physical row count:

F = 10 * maxNumberOfPointsInPage
pageNum = min(100, max(1, physicalRowCount / F))

Naturally forming the three-page sequence needed by the aligned scenario requires substantially more rows, approximately at least 3 * F physical rows, plus an intermediate page that is empty or effectively empty for the query. The production path also needs to enter the point-reader/overlap path, such as when a disk page overlaps the memtable pages.

No Maven build, formatting, or test command was run in this environment after the fixes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.82%. Comparing base (d21169f) to head (82cf6fe).
⚠️ Report is 46 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18647      +/-   ##
============================================
+ Coverage     42.19%   42.82%   +0.63%     
- Complexity      414      442      +28     
============================================
  Files          5411     5451      +40     
  Lines        390362   395425    +5063     
  Branches      51116    51805     +689     
============================================
+ Hits         164698   169358    +4660     
- Misses       225664   226067     +403     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JackieTien97]

Requesting changes for the non-aligned batch-to-point regression described inline. Preserving the prepared aligned group is appropriate, but the shared base-class change also preserves stale probeNext state left by non-aligned nextBatch().

Validation against 1a15a21:

• A clean reactor build and AlignedTVListIteratorTest / NonAlignedTVListIteratorTest passed: 25 tests, no failures.
• Three additional mixed batch/point checks (duplicate timestamps, ASC deletions, DESC deletions) passed with the parent implementation and failed with this implementation.
• A separate check using 300,000 physical rows, naturally generated fake pages, LazyMemVersionPageReader, and PriorityMergeReader reproduced the duplicate output. This check explicitly selects batch reading for the first two pages and point reading for the last page; it is not a full SQL integration test.
• Both new aligned regression tests failed with the parent implementation and passed with this PR.

Please also invalidate the non-aligned prepared state after batch consumption and add regression coverage for mixed batch/point reads.

[JackieTien97]

[P1] Invalidate non-aligned batch state before preserving probeNext

newIndex == index does not guarantee that probeNext still describes the current record. Non-aligned TVListIterator.nextBatch() advances index directly but does not clear probeNext before returning. With this change, switching through an empty fake page preserves that stale true, so subsequent point reads skip prepareNext() and can return duplicate timestamps or deleted points. This matters when an earlier non-overlapping page is read in batches and a later overlapping page is consumed through the point-reader path.

For example, consider a single non-aligned LongTVList with these physical records, in write order:

index  time  value
0      1     1
1      100   2
2      100   3

Read [1,33] in batches, visit the empty page [34,66], then read [67,100] as points:

1. The first hasNextBatch() prepares index 0 and sets probeNext = true.
2. nextBatch() emits (1,1), advances to index 1, and stops because time 100 exceeds the first page boundary. It leaves probeNext = true, although index 1 has not been prepared for point reading.
3. For the empty page, binary search returns index 1 again. Previously, this branch cleared probeNext, allowing hasNextBatch() to prepare the time-100 group and advance to its latest record, index 2. After this change, preparation is skipped and index 1 remains selected.
4. When the last page is selected, time 100 is already inside its range, so the early return leaves the stale flag untouched. The first point read emits (100,2); the next read also emits (100,3).

Expected last-page output: [(100,3)]
Parent implementation:    [(100,3)]
This PR:                  [(100,2), (100,3)]

The same stale flag bypasses deletion checks. My ASC and DESC deletion variants both passed with the parent implementation and returned an extra deleted point with this PR.

Please clear probeNext after non-aligned nextBatch() advances the cursor, as the aligned batch implementation already does, and cover the batch-to-point transition while retaining these aligned regressions.

Minimal JUnit reproducer (imports omitted)

@Test
public void testNonAlignedBatchToPointAfterEmptyPage() throws Exception {
  LongTVList list = LongTVList.newList();
  list.putLong(1, 1);
  list.putLong(100, 2);
  list.putLong(100, 3);

  MemPointIterator iterator =
      list.iterator(
          Ordering.ASC,
          list.rowCount(),
          null,
          Collections.emptyList(),
          0,
          TSEncoding.PLAIN,
          1024,
          null);

  iterator.setCurrentPageTimeRange(new TimeRange(1, 33));
  int firstPageRows = 0;
  while (iterator.hasNextBatch()) {
    firstPageRows += iterator.nextBatch().getPositionCount();
  }
  Assert.assertEquals(1, firstPageRows);

  iterator.setCurrentPageTimeRange(new TimeRange(34, 66));
  Assert.assertFalse(iterator.hasNextBatch());

  iterator.setCurrentPageTimeRange(new TimeRange(67, 100));
  List<String> result = new ArrayList<>();
  while (iterator.hasNextTimeValuePair()) {
    TimeValuePair pair = iterator.nextTimeValuePair();
    result.add(pair.getTimestamp() + ":" + pair.getValue().getLong());
  }
  Assert.assertEquals(Collections.singletonList("100:3"), result);
}

This minimal test supplies the page ranges explicitly. I also reproduced it with 300,000 physical rows: 299,998 records at time 1 followed by (100,2) and (100,3). ReadOnlyMemChunk naturally generated [1,33], [34,66], and [67,100]; the same batch/batch/point sequence through LazyMemVersionPageReader and PriorityMergeReader returned [100:2, 100:3] on this PR and [100:3] with the parent implementation.