[Caideyipi]

Description

import-data and export-data can silently produce incorrect output or report success after an import fails. For example, a tree query containing interleaved columns from two devices currently exports only the first device and assigns it a value from the other device. Multiple custom table queries also overwrite the same output file and can use the wrong destination table.

This change fixes the data tools by:

• Preserving the original column indices for every tree device, omitting absent values, and splitting SQL files by the number of generated INSERT statements.
• Giving each custom table query its own output files and deriving the destination table with the existing relational SQL parser. SQL/TsFile queries with multiple source relations or a different database fail before writing. Table and column identifiers are quoted.
• Escaping SQL text literals and emitting SQL NULL for missing values.
• Honoring typed CSV export headers and retaining the original CSV header spelling when importing table data with case-insensitive names and types.
• Clearing CSV batches only after successful insertion and propagating exhausted retries, worker errors, and archive errors to a nonzero process exit code.
• Archiving each imported TsFile under its relative filename with its .resource and .mods sidecars, and reporting file-operation failures accurately.
• Preserving the Java process exit code in the Windows import/export launchers.

Validation

• mvn -o test -pl iotdb-client/cli -Dtest=DataToolRegressionTest,ImportTsFileOperationTest,WriteDataFileTest -Ddevelocity.off=true: 21 tests passed, including 18 new regression tests.
• mvn -o test-compile -DskipTests -Ddevelocity.off=true: full 53-module reactor passed.
• mvn -o test-compile -P with-zh-locale -DskipTests -Ddevelocity.off=true: full 53-module Chinese-locale reactor passed.
• Windows launcher probes: both scripts returned Java exit code 17 with inherited ret_code unset, 0, and 23.
• git diff --check passed.

No live-cluster integration tests were run.

This PR has:

• been self-reviewed.
• added regression tests for the corrected behavior.

[CritasWang]

I reviewed commit e5db31f1a860c19ba1a872f0b222e0612f5bb746 and reproduced two issues that should be addressed before merging:

1. [P1] Archive filename collisions can silently discard a source file.

   In AbstractImportData.java, lines 166–180, replacing every path separator with _ is not an injective mapping. For example, input/a/b.tsfile and input/a_b.tsfile both become <successDir>/a_b.tsfile. The subsequent file operations use REPLACE_EXISTING, so the second archive operation silently overwrites the first. With -os mv, both original paths disappear, but only one file remains in the archive, and no error is propagated. A local regression test expecting both files to survive instead found only one. Please preserve the relative directory structure, or use collision-safe names and reject unintended overwrites.

2. [P2] Typed CSV export breaks round trips for column names containing spaces.

   This PR starts honoring typed CSV headers, but ImportDataTable.java, lines 395–401 still removes all whitespace from the column name when parsing a typed header. For a legal column named value with space, -dt true exports value with space(DOUBLE), which the importer turns into valuewithspace. Tablet.addValue then throws an IllegalArgumentException because that measurement does not exist. I reproduced this by exporting and importing the same row: the untyped CSV imported successfully, while the typed CSV failed. Please strip only the type suffix and preserve whitespace within the identifier.

Validation was performed in an isolated checkout: the PR's existing 21 targeted tests passed, and two additional regression tests reproduced the issues above. The tests used the locally cached TsFile 2.4.1-261009-SNAPSHOT because 2.4.1-260915-SNAPSHOT was unavailable locally. No live-cluster integration tests were run.

[CritasWang]

Thanks for working on this. I went through the full diff at e5db31f together with the surrounding code. The tree multi-device SQL export, the per-query output files and destination-table derivation, literal escaping and NULL handling, the typed CSV headers, and the archival target fix all look correct to me. The 21 tests in DataToolRegressionTest, ImportTsFileOperationTest and WriteDataFileTest pass locally, and I reproduced the import-side issues below with small unit probes.

I'd like the first inline comment addressed before merging: a failed CSV batch now ends the whole worker thread, so the files queued behind it are skipped and never reported. The second is about the exit-code coverage that is still incomplete. The rest is minor.

Follow-ups outside this diff

• import-tsfile still has the archival bug this PR fixes for import-data. ImportTsFileBase#processingFile still computes isSuccess ? successDir : failDir + File.separator + ..., so every successfully loaded file is archived to the success directory path itself. With -os mv and a fresh (empty) success directory, this deletes source files. The first loaded TsFile replaces the empty directory, and each later one replaces the previous file (REPLACE_EXISTING). All of them have already been moved out of the source directory, so only the last one survives on disk. It is the same logic, so it could reuse the new processingFile, either here or in a quick follow-up.
• export-tsfile.bat, schema/export-schema.bat and schema/import-schema.bat still have ENDLOCAL and EXIT /B %ret_code% on separate lines. That is the same exit-code problem fixed here for export-data.bat / import-data.bat.

Nits

• The three new CliMessages constants sit between the private constructor and the "Additional auto-collected messages" separator. Their names and hashes are correct; only the placement looks accidental.
• In the tree non-aligned SQL export, processedRows now counts INSERT statements rather than result rows, so the progress number means something slightly different.

Checked, no action needed

• '' escaping is valid for both the tree and the relational lexer; neither treats \ as an escape.
• Unquoted NULL is accepted by the tree INSERT grammar and becomes a real null on the server (CommonUtils.parseValue).
• identifierValue lower-cases names the same way the server canonicalizes them (QualifiedName).
• iotdb-relational-grammar was already on the CLI classpath through node-commons, so the packaged CLI does not change.

[CritasWang]

This catch wraps the whole while (... pollFromQueue() != null) loop. executeWithRetry now throws IllegalStateException, but importFromCsvFile only catches IOException (plus IllegalPathException in the tree variant). As a result, a single failed batch ends this worker thread.

The files still in the queue are then not imported, not counted in loadFileFailedNum, and not mentioned in the output:

• With -tn 1, or once every thread has hit a failure, all remaining files are skipped.
• With the default 8 threads, which files get imported depends on how many threads have already exited.

For the tree align-by-device path this is also a behaviour change, because it used to log the StatementExecutionException and continue.

Repro: set threadNum = 1 and queue a_bad.csv (its batch throws), then b_good.csv. init() returns false, but b_good.csv is still in ImportDataScanTool's queue afterwards and was never inserted.

Suggestion: move the try/catch inside the loop body, for both the TsFile loop and the CSV/SQL loop. Each failure is then handled per file: mark the import as failed, increment loadFileFailedNum, print the file name, and continue with the next file.

Probe test (fits into DataToolRegressionTest; fails today)

@Test
public void testFailedCsvDoesNotSkipRemainingFiles() throws Exception {
  Path bad = directory.resolve("a_bad.csv");
  Path good = directory.resolve("b_good.csv");
  Files.writeString(bad, "Time,root.a.d1.s1(INT32)\n1,11\n");
  Files.writeString(good, "Time,root.a.d2.s1(INT32)\n1,22\n");
  SessionPool pool = mock(SessionPool.class);
  set(ImportDataTree.class, "sessionPool", pool);
  doAnswer(
          invocation -> {
            if (((List<?>) invocation.getArgument(0)).contains("root.a.d1")) {
              throw new StatementExecutionException("type mismatch");
            }
            return null;
          })
      .when(pool)
      .insertRecords(any(), any(), any(), any(), any());
  ImportDataScanTool.putToQueue(bad.toString());
  ImportDataScanTool.putToQueue(good.toString());
  assertFalse(AbstractImportData.init(new ImportDataTree())); // threadNum = 1 in setUp()
  assertNull(ImportDataScanTool.pollFromQueue()); // fails today: b_good.csv is still queued
}

[Caideyipi]

Addressed in 240cfd502cce. Both worker loops now catch exceptions per file, record that file's failure, and continue draining the queue. Added single-worker regressions for tree align-by-time, tree align-by-device, and table CSV imports: the first batch fails while the next file is still inserted.

[CritasWang]

importFailed is private and is only set inside AbstractImportData: on worker exceptions, archival failures and failed TsFile loads. These paths still print "Import completely!" and exit with 0:

• .sql input. Failed statements in ImportDataTree#importFromSqlFile / ImportDataTable#importFromSqlFile are only written to the .failed file. In a probe where every statement throws StatementExecutionException, init() still returns true.
• CSV files rejected as a whole. This covers an empty file, a first column that is not Time, a wrong extension, and an IOException while reading, i.e. the early returns and the catch in both importFromCsvFile implementations. A probe with the header ts,root.a.d1.s1 returns true.

Could we add something like protected static void markImportFailed() and call it from those branches? If that is out of scope on purpose, it would help to state in the description which failure modes now produce a non-zero exit code.

Whether row-level conversion failures that only go to .failed should also fail the run is a separate product decision, so I'm not suggesting that here.

[Caideyipi]

Addressed in 240cfd502cce. Shared failure markers now cover failed SQL statements, table SQL database mismatches, SQL read errors, and CSV files rejected for an empty file, invalid time header, unsupported extension, or read/path errors. Failed SQL files are counted once as failures instead of successes. Added coverage in both models. The existing row-level conversion/.failed policy is unchanged.

[CritasWang]

Minor / FYI: because this counts tableName nodes, it also rejects single-table queries that mention the table more than once, for example select * from t where v in (select v from t) or a CTE over t. Both fail with "requires a query from a single table". Failing closed seems right here, since simple de-duplication would also let self-joins through. It may be worth spelling out the limitation in the docs or help text, e.g. that joins, subqueries and CTEs are not supported for -ft sql/tsfile custom queries.

[Caideyipi]

Updated the -q help in 240cfd502cce for both English and Chinese: table SQL/TsFile custom queries require one table without joins, subqueries, or CTEs. The help also now reflects that -q supports SQL/TsFile export, replacing the obsolete CSV-only description.

[Caideyipi]

Applied both findings in faa5d5e5ed0.

1. TsFile archival now preserves the source's relative directory structure, so a/b.tsfile and a_b.tsfile remain distinct in both success and failure archives. It checks the main file and existing sidecar destinations before processing the group, rejects occupied targets, and no longer uses REPLACE_EXISTING for copies, moves, or hard-link fallbacks. Archival errors propagate to the import result.
2. Table CSV header parsing now removes only the trailing type annotation and preserves whitespace and embedded parentheses in the column name.

Added regression coverage for the colliding paths under copy/hard-link/move operations, existing main-file and sidecar conflicts without source-file loss, and typed/untyped CSV export-import round trips with a column containing spaces and parentheses.

Validation: Spotless and CLI Checkstyle passed. mvn -o test -pl iotdb-client/cli -Dtest=DataToolRegressionTest passed all 21 tests, and the same command with -P with-zh-locale also passed all 21 tests. These runs used the PR's declared TsFile 2.4.1-260915-SNAPSHOT.

Full-reactor checks remain incomplete due to local memory limits: the English build failed with a JVM native-memory allocation error in Thrift compilation; the Chinese retry was stopped during DataNode Checkstyle with its capped 512 MB heap full. No live-cluster integration tests were run.

[Caideyipi]

Applied the remaining review feedback in 240cfd502cce:

• Worker exceptions are handled per file in both queues, so a failed CSV batch no longer skips the files behind it. SQL statement/read failures and whole-file CSV rejections now fail the run and update file counts.
• import-tsfile now shares the collision-safe TsFile/sidecar operations with import-data. Both success and failure archives preserve relative directories, reject occupied main/sidecar targets before moving the group, and propagate archival errors to the processing result.
• Fixed return-code propagation in export-tsfile.bat, schema/export-schema.bat, and schema/import-schema.bat.
• Documented table SQL/TsFile custom-query limitations in both locales, grouped the new message constants before the constructor, and restored result-row counting for non-aligned tree SQL export progress.

Validation: Spotless and CLI Checkstyle passed. mvn -o test -pl iotdb-client/cli -Dtest=DataToolRegressionTest,ImportTsFileOperationTest,WriteDataFileTest passed all 31 tests, and the same command with -P with-zh-locale also passed all 31. These runs used the declared TsFile 2.4.1-260915-SNAPSHOT. The 51 English/Chinese message keys and format specifiers match. All five Windows launchers touched by this PR correctly returned a Java probe's exit code 7 despite a caller-side ret_code=91.

Full-reactor mvn -o test-compile -DskipTests and mvn -o test-compile -DskipTests -P with-zh-locale both passed all 53 modules, with Maven capped at a 1 GB heap.

No live-cluster integration tests were run. Review threads are left open for re-review.