Skip to content

Fix tsfile-cli clean-build race and harden CLI validation - #836

Merged
jt2594838 merged 1 commit into
apache:developfrom
SpriCoder:fix/tsfile-cli-hardening
Jun 10, 2026
Merged

Fix tsfile-cli clean-build race and harden CLI validation#836
jt2594838 merged 1 commit into
apache:developfrom
SpriCoder:fix/tsfile-cli-hardening

Conversation

@SpriCoder

Copy link
Copy Markdown
Contributor

Summary

Build fix.tsfile_cli_obj compiles against the staged include tree (LIBRARY_INCLUDE_DIR) but had no dependency on the copy_* targets that populate it, so a clean parallel build could fail with fatal error: 'common/db_common.h' file not found — deterministically reproducible with make tsfile_cli_obj in a fresh build directory. This adds the missing add_dependencies(tsfile_cli_obj tsfile) edge (tsfile transitively depends on all header-staging targets). Include paths stay on CMAKE_CURRENT_SOURCE_DIR, which is also correct when cpp/ is embedded by an outer project via add_subdirectory. Supersedes #835.

Crash fix.sample -n <huge value> aborted via an uncaught std::length_error from reservoir.reserve(). The pre-allocation is now capped (the reservoir still grows to -n as rows actually arrive) and main gets a last-resort exception handler so e.g. std::bad_alloc reports an error instead of SIGABRT.

Validation & diagnostics.

  • --offset is rejected on non-row commands, --model on write, matching the existing "reject instead of silently ignore" policy
  • write names the specific offending read-only flag instead of read-only flags are not valid for write
  • head/cat/sample/schema now error when -d/-t does not match the file's data model instead of silently ignoring the filter (previously head -d dev1 on a table-model file printed all rows unfiltered)
  • open/create failures include the storage error code (cannot open x.tsfile: file is corrupted (code N)) instead of one lumped message
  • numeric flag values detect overflow via ERANGE (previously -n 9...9 silently clamped to LLONG_MAX)

Output correctness. Non-finite FLOAT/DOUBLE cells are emitted as null in JSON output — bare nan/inf is not valid JSON and broke jq/json.loads consumers.

Docs. README/SKILL: -m also applies to stats/count; count covers all tables (not first-table-only); note that the table format buffers all rows in memory to align columns.

Test plan

  • Clean-dir build: fresh cmake + make tsfile_cli -j8 succeeds (previously raced/failed); make tsfile_cli_obj alone also succeeds
  • CSV → tsfile round-trip: write + count/cat/stats/ls/meta/schema/head/sample regression on macOS
  • Every new validation path exercised with expected message and exit code (1 usage / 2 file)
  • sample -n 4611686018427387904 returns rows, exit 0 (previously SIGABRT)
  • JSON output (incl. NaN/Inf cells) validated with python json.loads
  • ./mvnw spotless:apply -P with-cpp produces no further changes

Build: tsfile_cli_obj compiled against the staged include tree without
depending on the copy_* targets that populate it, so a clean parallel
build could fail with "'common/db_common.h' file not found"
(deterministic via `make tsfile_cli_obj` in a fresh build dir). Add the
missing add_dependencies edge.
CLI hardening:
- reject --offset on non-row commands and --model on write; name each
read-only flag rejected by write instead of a lumped message
- error when -d/-t does not match the file's data model in
head/cat/sample/schema instead of silently ignoring the filter
- include the storage error code in open/create failure messages
- detect numeric flag overflow (ERANGE) in -n/--offset/--start/--end/--seed
- cap the sample reservoir pre-allocation so a huge -n cannot abort via
std::length_error; add a last-resort exception handler in main
- emit null for non-finite FLOAT/DOUBLE cells in JSON output (bare
nan/inf is not valid JSON)
- docs: -m also applies to stats/count, count covers all tables, note
that the table format buffers rows in memory
@jt2594838
jt2594838 merged commit 0963453 into apache:developJun 10, 2026
29 checks passed
@SpriCoder
SpriCoder deleted the fix/tsfile-cli-hardening branch June 10, 2026 10:47
Sign up for freeto 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.

2 participants

@SpriCoder@jt2594838