Skip to content

[Fix] Preserve table insert errors for rejected fields - #18880

Merged
jt2594838 merged 5 commits into
apache:masterfrom
hongzhi-gao:fix/writeback-incompatible-columns
Oct 16, 2026
Merged

jt2594838 merged 5 commits into
apache:masterfrom
hongzhi-gao:fix/writeback-incompatible-columns

Conversation

@hongzhi-gao

@hongzhi-gao hongzhi-gao commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

  • Preserve the original failures as suppressed exceptions when a relational Tablet is split across leader connections. The aggregate error message stays unchanged, while callers can inspect each failure's status code.
  • When schema validation rejects every FIELD in a table insert, retain the metadata error and continue to data partition lookup and write execution. The time row remains queryable with null FIELD values, including when no TAG survives.
  • Keep tree-model all-measurement-failed writes skipped. Allow table-model time rows through DataRegion, MemTable, and WAL recovery, including the zero-measurement WAL representation.

Verification

  • mvn spotless:check -pl iotdb-core/datanode,integration-test -P with-integration-tests -q and git diff --check — passed.
  • mvn verify -DskipUTs -Dit.test=IoTDBInsertTableIT#testRejectedOnlyFieldStillWritesTime -DfailIfNoTests=false -Dfailsafe.failIfNoSpecifiedTests=false -pl integration-test -am -PTableSimpleIT -P with-integration-tests -q — passed (1 ConfigNode, 1 DataNode).
  • The same targeted IT with -PTableClusterIT — passed (1 ConfigNode, 3 DataNodes). It checks field error reporting, time-row visibility with and without TAGs, and visibility after flush.
  • mvn test -pl iotdb-core/datanode -am -Dtest=AnalyzeUtilsTest,TsFilePlanRedoerTest#testRedoRelationalInsertWithoutFieldValues,InsertNodeIsMeasurementFailedTest -Dsurefire.failIfNoSpecifiedTests=false -q — 7, 1, and 23 tests passed. The WAL test serializes, deserializes, and replays zero-measurement row and tablet nodes.
  • mvn test-compile -P with-zh-locale -DskipTests -q — full reactor passed. The default-locale reactor build also passed as part of targeted IT verification.

Review

  • Reviewed table/tree write selection, data partition lookup, MemTable insertion, WAL replay, flush visibility, and the effect on the write path. No file or RPC format changed.
  • Existing SQL validation for a request containing only time and no originally supplied FIELD remains separate from partial insertion after FIELD validation failure.

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 916ecf4. No blocking issues found in grouped Tablet exception preservation, fully rejected FIELD handling, or the Pipe heartbeat test fixture update.

Validation: compiled the changed classes against local build artifacts and ran SessionCacheLeaderTest (12), AnalyzeUtilsTest (7), and PipeMemoryManagerResizeTest (11) via JUnitCore; all 30 tests passed. Full integration tests were not run locally.

@hongzhi-gao
hongzhi-gao force-pushed the fix/writeback-incompatible-columns branch from 916ecf4 to 8d57461 Compare October 15, 2026 07:07
@hongzhi-gao

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Since commit 916ecf4, the branch has also added the table-model case where every supplied FIELD is rejected: the metadata error is preserved, and the timestamp row is still written, including when no TAG survives validation. This is covered by commit 9a7fb10.

I ran the focused IoTDBInsertTableIT#testRejectedOnlyFieldStillWritesTime locally in both 1 ConfigNode + 1 DataNode and 1 ConfigNode + 3 DataNodes configurations; both passed. It checks error reporting and time-row visibility before and after flush. The focused WAL replay unit test and default/zh compile checks also passed. The full IT suite was not run.

@jt2594838
jt2594838 merged commit e492e44 into apache:master Oct 16, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants