[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

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. DataRegion.split() latches 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.

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.

The two changes cover different ingresses: the sort covers the engine's own INSERT INTO ... SELECT / SELECT INTO path, while the per-row re-check keeps the classification correct for any tablet that reaches DataRegion.insertTablet(), including the session insertTablet API.

Testing

InsertTabletStatementGeneratorTest (new, 8 cases):

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;
4. a full batch of mixed types is sorted;
5. a tag column moves together with its row;
6. the generator is reusable after reset();
7. duplicate timestamps keep their relative order;
8. an aligned tablet is sorted.

DataRegionTest:

1. testOutOfOrderTabletKeepsSequenceSpaceNonOverlapped — insert 10..39, close the file (lastFlushTime = 39), insert the tablet [100, 15], then assert the sequence space has no overlap (RepairDataFileScanUtil.checkTimePartitionHasOverlap) and that 15 landed in the unsequence space;
2. testOutOfOrderTabletWithMultipleInversionsKeepsSequenceSpaceNonOverlapped — same with several descending steps in one tablet.

They fail on the unpatched code (unseqFiles expected 1, was 0) and pass with the patch.

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

TreeIntoOperatorTest also passes.

Out of scope

• The session insertTablet API. It documents that the batch must be sorted by the client (InsertTabletNode#times); the per-row re-check keeps the sequence/unsequence classification right for unsorted tablets, but the other assumptions in the table above still hold only for sorted input.
• Files that already exist on disk. This change prevents the overlapping state from being created; it does not change how such files are read. START REPAIR DATA moves an overlapping sequence file to the unsequence space.

---

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.12%. Comparing base (28c5028) to head (0ed3860).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #19035      +/-   ##
============================================
+ Coverage     47.97%   48.12%   +0.14%     
- Complexity      604      621      +17     
============================================
  Files          5572     5572              
  Lines        409698   410324     +626     
  Branches      54009    54140     +131     
============================================
+ Hits         196558   197461     +903     
+ Misses       213140   212863     -277     

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

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?

[shuwenwei]

I would like to keep this PR scoped to the INSERT INTO ingress only.

The invariant violation originates because the internal INSERT INTO path bypasses the sorting performed by the Session API and constructs an InsertTabletStatement whose times are not sorted, although InsertTabletStatement documents that this invariant must hold. The minimal fix is therefore to restore that invariant inside InsertTabletStatementGenerator before the statement leaves the INSERT INTO path.

I do not think this PR should add defensive behavior to the general storage write path or to the query read path. Please remove the DataRegion.split() changes, the SeriesScanUtil changes, and the tests introduced solely for those defensive paths. Keep the tablet sorting in InsertTabletStatementGenerator, its unit tests, and an end-to-end INSERT INTO ... SELECT regression test proving that this path no longer produces incorrect results.

Once INSERT INTO generates a compliant sorted statement, it will no longer create the invalid sequence-file layout. Existing corrupted files and unsorted data from other ingress paths are separate concerns and should be handled in dedicated changes rather than partially masked in this PR.

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability 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