[ColinLeeo]

Problem

Fixes #19020.

A tablet whose rows are not in ascending time order could write a row older than the device's last flush time into the sequence space. The resulting sequence TsFile overlapped the existing sequence TsFile of the same device, and plain SELECT queries then returned duplicated rows and broke the time order, silently — no exception, no log entry.

A pure-SQL reproduction (create the layout, insert an out-of-order tablet through INSERT INTO ... SELECT, then observe the wrong order) is in the issue.

The state is reachable through the engine's own SQL path: TableInsertTabletStatementGenerator builds the insert tablet in the child operator's output order and does not sort it, while the storage layer documents that it assumes the batch has been sorted by the client (InsertTabletNode.checkTTLInternal).

Root cause

Three defects combine: the engine feeds itself an unsorted tablet, the write side turns that into an overlapping file, and the read side turns the file into a wrong result.

1. The tablet reaches storage unsorted

INSERT INTO ... SELECT copies the child operator's rows verbatim, so the tablet that enters the storage layer can have any order. Several places assume the opposite:

Assumption	Where
the first row is the minimum time, the last row the maximum	TsFileProcessor updates the tsfile resource boundaries from the first/last row of each range
the sequence flag only has to be re-evaluated when it is still false	DataRegion.split()
time partitions are crossed in one direction only	InsertTabletNode.collectSplitRanges()
expired rows form a prefix	InsertTabletNode.checkTTLInternal(), used by DataRegion
the last row is the latest row	InsertTabletNode.composeLastTimeValuePair(), used for the last cache
timestamps[0] .. timestamps[rowCount - 1] is the time range	PipeInsertNodeTabletInsertionEvent.mayEventTimeOverlappedWithTimeRange()

2. Write side: the sequence flag is latched

DataRegion.split() classified the rows of a tablet by latching the sequence flag:

} else if (!isSequence) {
  if (time > lastFlushTime) {
    ...
    isSequence = true;
  }
}
// else: isSequence not changed, just move the cursor forward

Once isSequence became true, every remaining row of the device was classified as sequence regardless of its timestamp. For the tablet [100, 15] of a device whose last flushed time is 39, the row 15 was written into a new sequence TsFile, producing an overlapping sequence file.

3. Read side: overlapping sequence pages are merged too late

SeriesScanUtil expands the pages of every overlapping sequence chunk into seqPageReaders, an append-only list, and both the overlap check (firstPageOverlapped) and the page selection (getFirstPageReaderFromCachedReaders) only look at the head of that list. Once files overlap, the pages of the later file are appended after the pages of the earlier one, so a page that starts before the head is not examined until the earlier file is exhausted; by then its rows have already been returned.

With P1[10,20] P2[21,30] P3[31,39] from the first file and P4[15,100] from the second:

current page	head of list	check	outcome
P1[10,20]	P2[21,30]	20 >= 21 false	returned directly
P2[21,30]	P3[31,39]	30 >= 31 false	returned directly
P3[31,39]	P4[15,100]	39 >= 15 true	P3 and P4 merged

So P4 is merged, but only after P1 and P2 have already been emitted: rows come back twice and out of time order.

Fix

Engine ingress: sort the tablet before it is written

InsertTabletStatementGenerator.constructInsertTabletStatement() sorts the rows stably by time, permuting the value columns and the null bitmaps along with the timestamps. A tablet that is already sorted — the normal case — returns after one comparison per row and is not copied.

This restores every assumption in the table above at once, instead of patching six consumers independently.

Write side: no longer latch the sequence flag

Track whether the device's rows are still in ascending time order:

• Sorted tablets (the normal case) keep exactly the previous behaviour. The only extra cost is one time < previousTime comparison per row; no additional lastFlushTime lookups.
• Once a descending step is seen, each remaining row is classified individually against the device's last flush time, and the range is split whenever the classification changes.

For [100, 15] the split becomes seq [[0,1)] + unseq [[1,2)]: the older row goes to the unsequence space and no overlapping sequence file is created.

Alternatives considered and not taken: sorting the tablet before the insert (needs remapping results and the processedRows bitset) or sorting only in TableIntoOperator (would leave the session insertTablet API unprotected). Fixing it in the storage layer covers every ingress.

Read side: keep the sequence page readers in scan order

seqPageReaders is now kept ordered in scan order when a page is unpacked (ascending: by start time, descending: by end time); the unsequence side already uses an ordered priority queue.

private void addSeqPageReader(IVersionPageReader pageReader) {
  ListIterator<IVersionPageReader> iterator = seqPageReaders.listIterator(seqPageReaders.size());
  while (iterator.hasPrevious()) {
    if (!isBeforeInScanOrder(pageReader, iterator.previous())) {
      iterator.next();
      break;
    }
  }
  iterator.add(pageReader);
}

For non-overlapping files every insert lands at the tail, so the iteration order is unchanged and the only cost is one comparison per page. This keeps query results correct for overlapping sequence files that already exist on disk.

Testing

InsertTabletStatementGeneratorTest (new):

1. rows [100, 15] come out as [15, 100] with the values following their rows;
2. a null value moves together with its row, including the null bitmap;
3. an already sorted tablet is left untouched.

Write side — DataRegionTest#testOutOfOrderTabletKeepsSequenceSpaceNonOverlapped:

1. insert 10..39 for a device and close the file (last flushed time = 39);
2. insert the out-of-order tablet [100, 15] for the same device;
3. assert that the sequence space has no overlap (RepairDataFileScanUtil.checkTimePartitionHasOverlap) and that 15 landed in the unsequence space.

It fails on the unpatched code (unseqFiles expected 1, was 0) and passes with the patch.

Read side — OverlappingSeqChunkScanTest writes overlapping sequence files directly and covers:

1. a page nested inside an earlier page (f1 [10,19][20,29][30,39], f2 [25,27]);
2. a page spanning beyond the whole earlier file (f1 [10,20][21,30][31,39], f2 [15 ... 100]);
3. an overlap fully contained in the first page (f1 [10,19][20,29], f2 [12,13,14]);

each asserted to be strictly ordered and duplicate-free, in both scan directions where applicable. All three fail on the unpatched code — case 1 returns 33 points instead of 30, with 25 26 27 repeated after 29.

IoTDBInsertQueryIT#testInsertIntoQueryWithOutOfOrderTabletDoesNotOverlapSequenceFiles reproduces the situation end-to-end through SQL and asserts the rows come back in time order.

TreeIntoOperatorTest, SeriesScanUtilCancellationTest, SeriesAggregationScanOperatorTest and AlignedSeriesScanOperatorTest also pass.

Out of scope

• The session insertTablet API. It documents that the batch must be sorted by the client (InsertTabletNode#times); a client that violates that contract can still reach the storage layer with an unsorted tablet.
• Files that already exist on disk. The read-side change covers pages that are discovered, but a sequence resource whose data range starts before files that were registered earlier is still discovered in file-name order. That state is not produced by the write path after this fix; handling it for historical data or externally imported files is left out of this change.

---

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.02%. Comparing base (a927dd6) to head (7f12bc2).

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19035      +/-   ##
============================================
+ Coverage     47.97%   48.02%   +0.04%     
  Complexity      604      604              
============================================
  Files          5571     5571              
  Lines        409682   409732      +50     
  Branches      54009    54024      +15     
============================================
+ Hits         196560   196779     +219     
+ Misses       213122   212953     -169     

☔ 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.

[sonarqubecloud]

Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[JackieTien97]

Thanks for the detailed root-cause analysis and the regression tests. I left a few inline comments, mostly about the cost the new sort adds to INSERT INTO ... SELECT and about how much of the problem the storage-layer guard actually covers. The numbers below were measured locally on the PR head (7f12bc27af; JDK 21, warmed-up JIT, 10,000-row batches).

[JackieTien97]

Array.get / Array.set box every primitive element and make a native call per element, so the unsorted path is much more expensive than it needs to be. Measured constructInsertTabletStatement() on the PR head (10,000-row batch, full-batch path):

columns	already sorted	unsorted (this PR)	same algorithm, typed copy loops
1 tag + 10 DOUBLE	0.004 ms	14.0 ms	0.56 ms
1 tag + 50 DOUBLE	0.004 ms	64.0 ms	2.19 ms

The sort runs synchronously on the INTO operator's driver thread before the write is submitted, so it doesn't overlap with the write, and every batch coming from ORDER BY time DESC, a join, or a multi-device table scan takes this path.

Could the copy switch on the column's TSDataType and use typed loops instead? The Integer[] index sort itself is cheap (the typed variant above still uses it); the reflective copy dominates. If you also want to drop the boxing, Session#sortedIndex has a stable primitive merge sort with a fast path for strictly descending input.

[JackieTien97]

For the table model one generator covers the whole target table, so a tablet usually holds several devices. The child operator emits them grouped by device, each device already in ascending time order, which is what the storage layer actually relies on: split, the device time index and the last cache all work per device range. A global stable sort by time interleaves those devices, so splitByDevice goes from one segment per device to up to one segment per row.

DataRegion#insertTablet alone, 10,000-row relational tablets, device-grouped (what master writes) vs. time-interleaved (what this PR writes):

devices × rows per device	device segments	grouped	interleaved
1000 × 10	1000 → 10000	2.4 ms	6.8–7.9 ms
100 × 100	100 → 10000	1.45 ms	6.06 ms
10 × 1000	10 → 10000	1.22 ms	5.6 ms

The coordinator side (collectSplitRanges / generateOneSplitList) and updateLastCache also scale with the number of segments and are not included above. Downsampling statements such as INSERT INTO agg SELECT date_bin(1h, time), tag, avg(s) FROM src GROUP BY 1, tag hit this on every batch.

If the goal is only the out-of-order rows of a single device, sorting only when some device's rows go backwards (or sorting by (device, time)) would keep the grouping. There is a trade-off worth spelling out, though. From reading the code (not verified with a test), two consumers depend on a globally sorted tablet:

• PipeInsertNodeTabletInsertionEvent#mayEventTimeOverlappedWithTimeRange only looks at the first and last timestamps.
• The relational TTL check marks expired rows individually, but splitAndInsert writes with noFailure = true, which passes null results to the memtable, so those rows are not skipped.

Both look wrong on master for device-grouped tablets, and the global sort happens to fix them. Could the PR either keep the device grouping and handle those two consumers, or keep the global sort and call out the extra write cost?

[JackieTien97]

This branch only moves rows that are not newer than the last flushed time into the unsequence space. Rows that are all newer than it, but out of order among themselves, still land in one sequence range, and TsFileProcessor#insertTablet takes the device start time from the first row of each range, so the file's time index is still wrong.

Reproduced on the PR head: write 10..39 for a device and close the file (last flushed time 39), then write the unsorted tablet [100, 50] and close again. The second sequence file is indexed [100, 100] while its chunk statistics are [50, 100] with 2 points, and no unsequence file is created. A query with WHERE time <= 60 is then pruned by the resource time index and misses the row at 50.

Not every ingress sorts before reaching this point: the REST handlers (StatementConstructionHandler under rest/protocol/table/v1 and rest/protocol/v2) and MQTT (MPPPublishHandler) build the InsertTabletStatement as received. For those paths the TTL prefix, the forward-only partition detection in collectSplitRanges / getTimePartitionSlots, and the last-row last cache are unchanged as well. So "Fixing it in the storage layer covers every ingress" in the description doesn't quite hold, and the description still lists sorting as an alternative not taken, while a later commit sorts in the generator.

Would it be simpler to do the O(n) sortedness check, and the sort when needed, once at a common point for every InsertTabletStatement (e.g. during analysis, before the time partition slots are computed)? That would cover INTO, REST and MQTT in one place, and this branch could remain a safety net.

[JackieTien97]

The datanode already has this: new PipeTableModelTabletEventSorter(statement).sortByTimestampIfNecessary() (used by PipeTransferTabletRawReqV2) does the same sortedness check, a stable sort by time, and a typed reorder of the values and bitmaps, including the handling of how DATE columns are stored. Could this reuse it, or move the core of PipeInsertEventSorter into a shared utility so the query engine doesn't depend on the pipe package? Otherwise the two copies have to stay consistent on stability, bitmaps and DATE columns.

[JackieTien97]

Minor: once timeOrdered is false, every remaining row calls getLastFlushTime(...) (a HashMap<Long, …> lookup with boxing plus a device lookup) under the write lock. split is called per device range and the partition only changes in the first branch, so the value is constant until the partition changes. It could be looked up once per partition and reused, which would also help the existing !isSequence prefix. This only affects unsorted input.

[JackieTien97]

Minor: when rowCount != rowLimit, the arrays are first truncated by the copy above, and sortRowsByTimeIfNeeded() then allocates every column again. While an unsorted batch is sorted, two copies of the times, columns and bitmaps are alive, plus the boxed Integer[] index and the int[] / long[] buffers. None of that is in ramBytesUsed(), and the row limit derived from into_operation_buffer_size_in_byte assumes a single copy. For wide tables, applying the permutation while truncating would do both in one copy.

[JackieTien97]

The 10-point page is only needed so this test can fail on the unfixed code, but the cluster is shared, so testSimple and the other existing tests in this class now also run with 10-point pages, which changes their storage layout and coverage. Would a separate IT class for the new test be better, or a comment explaining why the whole class needs this setting?