From 72853ea14a03dadc9beb56fff52329c5a9fdd205 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Sat, 10 Oct 2026 11:21:20 +0800 Subject: [PATCH] fix(cpp): reject UTF-8 name collisions before CLI output --- cpp/test/tools/command_e2e_test.cc | 14 +- cpp/test/tools/output_format_test.cc | 35 +- cpp/test/tools/utf8_collision_e2e_test.cc | 438 ++++++++++++++++++++++ cpp/tools/commands/cmd_count.cc | 16 + cpp/tools/commands/cmd_export.cc | 19 + cpp/tools/commands/cmd_ls.cc | 7 + cpp/tools/commands/cmd_schema.cc | 7 + cpp/tools/commands/cmd_stats.cc | 23 +- cpp/tools/commands/commands.h | 5 + cpp/tools/commands/row_query.cc | 205 +++++----- cpp/tools/format/output_format.cc | 47 ++- cpp/tools/format/output_format.h | 21 ++ 12 files changed, 718 insertions(+), 119 deletions(-) create mode 100644 cpp/test/tools/utf8_collision_e2e_test.cc diff --git a/cpp/test/tools/command_e2e_test.cc b/cpp/test/tools/command_e2e_test.cc index e9f8cde71..e3af86ab1 100644 --- a/cpp/test/tools/command_e2e_test.cc +++ b/cpp/test/tools/command_e2e_test.cc @@ -1815,9 +1815,9 @@ TEST(CliE2E, Utf8JsonKeyCollisionsFailWithoutPublishingOutput) { args.insert(args.end() - 1, {"--start", "0", "-n", "0"}); } std::ostringstream out, err; - EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 3) << err.str(); + EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 2) << err.str(); EXPECT_TRUE(out.str().empty()); - EXPECT_NE(err.str().find("duplicate NDJSON column name"), + EXPECT_NE(err.str().find("duplicate column name"), std::string::npos); EXPECT_TRUE(tsfile_cli::is_valid_utf8(err.str())); } @@ -1837,10 +1837,9 @@ TEST(CliE2E, Utf8JsonKeyCollisionsFailWithoutPublishingOutput) { args.insert(args.end() - 1, "--force"); } std::ostringstream out, err; - EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 3) << err.str(); + EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 2) << err.str(); EXPECT_TRUE(out.str().empty()); - EXPECT_NE(err.str().find("duplicate NDJSON column name"), - std::string::npos); + EXPECT_NE(err.str().find("duplicate column name"), std::string::npos); std::ifstream target(output.c_str(), std::ios::binary); if (existing) { ASSERT_TRUE(target.is_open()); @@ -1897,10 +1896,9 @@ TEST(CliE2E, TableUtf8JsonKeyCollisionsFailInRowsAndStats) { std::ostringstream out, err; EXPECT_EQ(tsfile_cli::run_cli( {command, "-t", table, "-f", "ndjson", path}, out, err), - 3); + 2); EXPECT_TRUE(out.str().empty()); - EXPECT_NE(err.str().find("duplicate NDJSON column name"), - std::string::npos); + EXPECT_NE(err.str().find("duplicate column name"), std::string::npos); EXPECT_TRUE(tsfile_cli::is_valid_utf8(err.str())); } std::remove(path.c_str()); diff --git a/cpp/test/tools/output_format_test.cc b/cpp/test/tools/output_format_test.cc index 59c8cbe29..aaf910691 100644 --- a/cpp/test/tools/output_format_test.cc +++ b/cpp/test/tools/output_format_test.cc @@ -346,29 +346,36 @@ TEST(RowWriterTest, PreservesNonUtf8BlobBytesAsHex) { } } -TEST(RowWriterTest, RejectsJsonKeyCollisionsAfterUtf8Replacement) { +TEST(RowWriterTest, RejectsColumnCollisionsAfterUtf8Replacement) { const std::vector> headers = { {"bad\xff", "bad\xfe"}, {"bad\xff", "bad\xef\xbf\xbd"}, + {"same", "same"}, }; for (const auto& header : headers) { - for (bool no_header : {false, true}) { - std::ostringstream out; - RowWriter writer(out, OutputFormat::kJson, header, - {common::INT64, common::INT64}, no_header); - EXPECT_FALSE(writer.write({"42", "7"}, {false, false})); - EXPECT_FALSE(writer.finish()); - EXPECT_TRUE(out.str().empty()); + for (OutputFormat format : {OutputFormat::kJson, OutputFormat::kCsv, + OutputFormat::kTable, OutputFormat::kTsv}) { + for (bool no_header : {false, true}) { + std::ostringstream out; + RowWriter writer(out, format, header, + {common::INT64, common::INT64}, no_header); + EXPECT_FALSE(writer.write({"42", "7"}, {false, false})); + EXPECT_FALSE(writer.finish()); + EXPECT_TRUE(out.str().empty()); + } } } } -TEST(RowWriterTest, RejectsJsonKeyCollisionsWithoutRows) { - std::ostringstream out; - RowWriter writer(out, OutputFormat::kJson, {"bad\xff", "bad\xfe"}, - {common::STRING, common::STRING}, false); - EXPECT_FALSE(writer.finish()); - EXPECT_TRUE(out.str().empty()); +TEST(RowWriterTest, RejectsColumnCollisionsWithoutRows) { + for (OutputFormat format : {OutputFormat::kJson, OutputFormat::kCsv, + OutputFormat::kTable, OutputFormat::kTsv}) { + std::ostringstream out; + RowWriter writer(out, format, {"bad\xff", "bad\xfe"}, + {common::STRING, common::STRING}, false); + EXPECT_FALSE(writer.finish()); + EXPECT_TRUE(out.str().empty()); + } } TEST(RowWriterTest, CsvEscapesLeadingBackslashesOnlyInTextValues) { diff --git a/cpp/test/tools/utf8_collision_e2e_test.cc b/cpp/test/tools/utf8_collision_e2e_test.cc new file mode 100644 index 000000000..cedca663b --- /dev/null +++ b/cpp/test/tools/utf8_collision_e2e_test.cc @@ -0,0 +1,438 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * License); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include +#include + +#include +#include +#include +#include +#include + +#include "cli/run_cli.h" +#include "cli_test_util.h" +#include "format/input_format.h" + +namespace { + +const std::vector kFormats = {"csv", "table", "ndjson"}; +const std::vector kNames = {"bad\xff", "bad\xfe", + "bad\xef\xbf\xbd"}; + +struct TreeDefinition { + std::string device; + std::vector fields; +}; + +struct TableDefinition { + std::string table; + std::vector tags; + std::vector fields; +}; + +class CliUtf8CollisionTest : public ::testing::Test { + protected: + void SetUp() override { storage::libtsfile_init(); } + + void TearDown() override { + for (const auto& path : files_) std::remove(path.c_str()); + for (const auto& path : directories_) { + for (const auto& name : + {"_manifest.json", "0001.csv", "0002.csv", "0001.ndjson", + "0002.ndjson", "0001.txt", "0002.txt"}) { + std::remove((path + "/" + name).c_str()); + } +#ifdef _WIN32 + _rmdir(path.c_str()); +#else + rmdir(path.c_str()); +#endif + } + } + + std::string temp_file(const std::string& extension = ".tsfile") { + std::string path = + tsfile_cli_test::unique_temp_path("utf8_collision", extension); + files_.push_back(path); + return path; + } + + std::string temp_directory() { + std::string path = + tsfile_cli_test::unique_temp_path("utf8_collision_export", ""); + directories_.push_back(path); + return path; + } + + bool exists(const std::string& path) { + struct stat st; + return stat(path.c_str(), &st) == 0; + } + + void write_tree(const std::string& path, + const std::vector& definitions, + bool aligned = false) { + storage::WriteFile file; + int flags = O_WRONLY | O_CREAT | O_TRUNC; +#ifdef _WIN32 + flags |= O_BINARY; +#endif + ASSERT_EQ(file.create(path, flags, 0666), common::E_OK); + storage::TsFileTreeWriter writer(&file); + for (const auto& definition : definitions) { + std::string device = definition.device; + std::vector schemas; + for (const auto& field : definition.fields) { + schemas.push_back(new storage::MeasurementSchema( + field, common::INT64, common::PLAIN, common::UNCOMPRESSED)); + if (!aligned) { + ASSERT_EQ( + writer.register_timeseries(device, schemas.back()), + common::E_OK); + } + } + if (aligned) { + ASSERT_EQ(writer.register_timeseries(device, schemas), + common::E_OK); + } + storage::TsRecord record(device, 1000); + for (const auto& field : definition.fields) { + record.add_point(field, static_cast(42)); + } + ASSERT_EQ(writer.write(record), common::E_OK); + if (!aligned) { + for (auto* schema : schemas) delete schema; + } + } + ASSERT_EQ(writer.flush(), common::E_OK); + ASSERT_EQ(writer.close(), common::E_OK); + } + + void write_tables(const std::string& path, + const std::vector& definitions, + int rows = 2) { + storage::TsFileWriter writer; + ASSERT_EQ(writer.open(path), common::E_OK); + for (const auto& definition : definitions) { + std::vector columns; + std::vector names; + std::vector types; + std::vector categories; + for (const auto& tag : definition.tags) { + columns.push_back(common::ColumnSchema( + tag, common::STRING, common::UNCOMPRESSED, common::PLAIN, + common::ColumnCategory::TAG)); + names.push_back(tag); + types.push_back(common::STRING); + categories.push_back(common::ColumnCategory::TAG); + } + for (const auto& field : definition.fields) { + columns.push_back(common::ColumnSchema( + field, common::INT64, common::UNCOMPRESSED, common::PLAIN, + common::ColumnCategory::FIELD)); + names.push_back(field); + types.push_back(common::INT64); + categories.push_back(common::ColumnCategory::FIELD); + } + ASSERT_EQ( + writer.register_table(std::make_shared( + definition.table, columns)), + common::E_OK); + if (rows == 0) continue; + storage::Tablet tablet(definition.table, names, types, categories, + rows); + for (int row = 0; row < rows; ++row) { + tablet.add_timestamp(row, static_cast(1000 + row)); + for (const auto& tag : definition.tags) { + tablet.add_value(row, tag, row == 0 ? "first" : "second"); + } + for (const auto& field : definition.fields) { + tablet.add_value(row, field, static_cast(42)); + } + } + ASSERT_EQ(writer.write_table(tablet), common::E_OK); + } + ASSERT_EQ(writer.flush(), common::E_OK); + ASSERT_EQ(writer.close(), common::E_OK); + } + + void expect_collision(const std::vector& args) { + std::ostringstream out, err; + EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 2) << err.str(); + EXPECT_TRUE(out.str().empty()) << out.str(); + EXPECT_NE(err.str().find("after UTF-8 replacement"), std::string::npos) + << err.str(); + EXPECT_TRUE(tsfile_cli::is_valid_utf8(err.str())); + } + + void expect_success(const std::vector& args) { + std::ostringstream out, err; + EXPECT_EQ(tsfile_cli::run_cli(args, out, err), 0) << err.str(); + EXPECT_TRUE(err.str().empty()) << err.str(); + EXPECT_TRUE(tsfile_cli::is_valid_utf8(out.str())); + } + + private: + std::vector files_; + std::vector directories_; +}; + +TEST_F(CliUtf8CollisionTest, TreeColumnsFailBeforeOutputAcrossFormats) { + for (bool aligned : {false, true}) { + const std::string path = temp_file(); + write_tree(path, {{"root.d", kNames}}, aligned); + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + for (const auto& command : + {"schema", "count", "stats", "head", "cat"}) { + SCOPED_TRACE(std::string(command) + " " + format + + " aligned=" + std::to_string(aligned)); + expect_collision({command, "-f", format, path}); + } + expect_success({"ls", "-f", format, path}); + } + // Sketch deliberately follows printSketch without collision checking. + expect_success({"sketch", path}); + } +} + +TEST_F(CliUtf8CollisionTest, TableFieldsAndTagsFailBeforeOutputAcrossFormats) { + const std::vector definitions = { + {"t", {"id"}, kNames}, + {"t", kNames, {"value"}}, + {"t", {kNames[0]}, {kNames[1]}}, + {"t", {"id"}, {kNames[0], kNames[2]}}}; + for (const auto& definition : definitions) { + for (int rows : {0, 2}) { + const std::string path = temp_file(); + write_tables(path, {definition}, rows); + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + for (const auto& command : + {"schema", "count", "stats", "head", "cat"}) { + SCOPED_TRACE(std::string(command) + " " + format + + " rows=" + std::to_string(rows)); + expect_collision({command, "-f", format, path}); + } + expect_success({"ls", "-f", format, path}); + } + } + } +} + +TEST_F(CliUtf8CollisionTest, ObjectCollisionsDoNotRequireMatchingColumnNames) { + for (bool table : {false, true}) { + for (size_t second : {size_t(1), size_t(2)}) { + const std::string path = temp_file(); + if (table) { + write_tables(path, {{kNames[0], {"id"}, {"left"}}, + {kNames[second], {"id"}, {"right"}}}); + } else { + write_tree( + path, {{kNames[0], {"left"}}, {kNames[second], {"right"}}}); + } + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + for (const auto& command : {"ls", "schema", "count", "stats"}) { + SCOPED_TRACE(std::string(command) + " " + format); + expect_collision({command, "-f", format, path}); + if (std::string(command) != "ls") { + expect_success({command, table ? "-t" : "-d", kNames[0], + "-f", format, path}); + expect_success( + {command, "-m", "left", "-f", format, path}); + } + } + } + } + } +} + +TEST_F(CliUtf8CollisionTest, ProjectionAndSeparateColumnScopesRemainValid) { + for (bool table : {false, true}) { + const std::string path = temp_file(); + const std::string scoped = temp_file(); + if (table) { + write_tables(path, {{"a", {"id"}, kNames}}); + write_tables(scoped, {{"a", {"id"}, {kNames[0], "value"}}, + {"b", {"id"}, {kNames[1], "value"}}}); + } else { + write_tree(path, {{"a", kNames}}); + write_tree(scoped, {{"a", {kNames[0], "value"}}, + {"b", {kNames[1], "value"}}}); + } + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + for (const auto& command : + {"schema", "count", "stats", "head", "cat"}) { + expect_success({command, "-m", kNames[0], "-f", format, path}); + if (std::string(command) != "head" && + std::string(command) != "cat") { + expect_success({command, "-f", format, scoped}); + } + } + const std::string output = temp_file(".out"); + expect_success({"export", table ? "-t" : "-d", "a", "-m", kNames[0], + "--type", format, "-o", output, path}); + EXPECT_TRUE(exists(output)); + const std::string dir = temp_directory(); + expect_success({"export", table ? "-t" : "-d", "a", + table ? "-t" : "-d", "b", "--type", format, + "--output-dir", dir, scoped}); + EXPECT_TRUE(exists(dir + "/_manifest.json")); + } + } +} + +TEST_F(CliUtf8CollisionTest, StatsChecksMergedTagHeadersAcrossTables) { + const std::string path = temp_file(); + write_tables( + path, {{"a", {kNames[0]}, {"value"}}, {"b", {kNames[1]}, {"value"}}}); + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + expect_collision({"stats", "-f", format, path}); + for (const auto& command : {"schema", "count"}) { + expect_success({command, "-f", format, path}); + } + expect_success({"stats", "-t", "a", "-f", format, path}); + } +} + +TEST_F(CliUtf8CollisionTest, EmptyRowWindowsCannotHideSelectedNameCollisions) { + const std::vector> windows = { + {"-n", "0"}, + {"--start", "0", "-n", "0"}, + {"--start", "2000"}, + {"--end", "0"}, + {"--offset", "1"}}; + for (bool table : {false, true}) { + const std::string path = temp_file(); + if (table) + write_tables(path, {{"t", {"id"}, kNames}}); + else + write_tree(path, {{"root.d", kNames}}); + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + for (const auto& window : windows) { + for (const auto& command : {"head", "cat", "export"}) { + SCOPED_TRACE(std::string(command) + " " + format); + std::vector args = {command}; + args.insert(args.end(), window.begin(), window.end()); + const std::string output = temp_file(".out"); + if (std::string(command) == "export") { + args.insert( + args.end(), + {table ? "-t" : "-d", table ? "t" : "root.d", + "--type", format, "-o", output}); + } else { + args.insert(args.end(), {"-f", format}); + } + args.push_back(path); + expect_collision(args); + EXPECT_FALSE(exists(output)); + } + } + } + } +} + +TEST_F(CliUtf8CollisionTest, ExportCollisionPreservesExistingTargets) { + for (bool table : {false, true}) { + const std::string path = temp_file(); + if (table) + write_tables(path, {{"t", {"id"}, kNames}}); + else + write_tree(path, {{"root.d", kNames}}); + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + const std::string output = temp_file(".out"); + expect_collision({"export", table ? "-t" : "-d", + table ? "t" : "root.d", "--type", format, "-o", + output, path}); + EXPECT_FALSE(exists(output)); + { + std::ofstream file(output.c_str(), std::ios::binary); + file << "keep existing output\n"; + } + expect_collision({"export", table ? "-t" : "-d", + table ? "t" : "root.d", "--type", format, "-o", + output, path}); + expect_collision({"export", table ? "-t" : "-d", + table ? "t" : "root.d", "--type", format, "-o", + output + "/unavailable", path}); + expect_collision({"export", table ? "-t" : "-d", + table ? "t" : "root.d", "--type", format, + "--force", "-o", output, path}); + std::ifstream file(output.c_str(), std::ios::binary); + std::ostringstream content; + content << file.rdbuf(); + EXPECT_EQ(content.str(), "keep existing output\n"); + } + } +} + +TEST_F(CliUtf8CollisionTest, + MultiExportChecksEveryObjectBeforeCreatingDirectory) { + for (bool table : {false, true}) { + for (bool object_collision : {false, true}) { + const std::string first = object_collision ? kNames[0] : "a"; + const std::string second = object_collision ? kNames[1] : "b"; + const std::string path = temp_file(); + if (table) { + write_tables( + path, {{first, {"id"}, {"left"}}, + {second, + {"id"}, + object_collision ? std::vector{"right"} + : kNames}}); + } else { + write_tree(path, + {{first, {"left"}}, + {second, object_collision + ? std::vector{"right"} + : kNames}}); + } + ASSERT_FALSE(HasFatalFailure()); + for (const auto& format : kFormats) { + const std::string dir = temp_directory(); + expect_collision({"export", table ? "-t" : "-d", first, + table ? "-t" : "-d", second, "--type", format, + "--output-dir", dir, path}); + EXPECT_FALSE(exists(dir)); + // A collision takes precedence over output-path errors as + // well, proving it is checked before publication is prepared. + const std::string occupied = temp_file(".out"); + { + std::ofstream file(occupied.c_str()); + file << "keep existing output\n"; + } + expect_collision({"export", table ? "-t" : "-d", first, + table ? "-t" : "-d", second, "--type", format, + "--output-dir", occupied, path}); + std::ifstream file(occupied.c_str()); + std::ostringstream content; + content << file.rdbuf(); + EXPECT_EQ(content.str(), "keep existing output\n"); + } + } + } +} + +} // namespace diff --git a/cpp/tools/commands/cmd_count.cc b/cpp/tools/commands/cmd_count.cc index e3ff9212f..ac3e7a2ce 100644 --- a/cpp/tools/commands/cmd_count.cc +++ b/cpp/tools/commands/cmd_count.cc @@ -308,6 +308,15 @@ int cmd_count(const ParsedArgs& args, storage::TsFileReader& reader, return ret; } } + OutputNameValidator validator; + for (const TableCountSummary& summary : summaries) { + for (const CountColumn& column : summary.columns) { + if (!validator.add_column(summary.table_name, column.name)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } + } for (const TableCountSummary& summary : summaries) { for (const CountColumn& c : summary.columns) { long long null_count = summary.row_count - c.non_null_count; @@ -344,6 +353,13 @@ int cmd_count(const ParsedArgs& args, storage::TsFileReader& reader, if (collect_ret != kExitOk) { return collect_ret; } + OutputNameValidator validator; + for (const SeriesStatRow& row : rows) { + if (!validator.add_column(row.target, row.measurement)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } for (const SeriesStatRow& row : rows) { const long long null_count = row.row_count - row.count; if (!w.write( diff --git a/cpp/tools/commands/cmd_export.cc b/cpp/tools/commands/cmd_export.cc index 9a7a1c7ee..0646550e9 100644 --- a/cpp/tools/commands/cmd_export.cc +++ b/cpp/tools/commands/cmd_export.cc @@ -261,6 +261,23 @@ int validate_multi_export_objects(const ParsedArgs& args, } } } + OutputNameValidator validator; + const bool table_mode = !args.tables.empty(); + const std::vector& objects = + table_mode ? args.tables : args.devices; + for (const std::string& object : objects) { + const std::string name = + table_mode ? storage::to_lower(object) : object; + if (!validator.add_object(name)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + ParsedArgs scope = args; + scope.table = table_mode ? name : ""; + scope.device = table_mode ? "" : name; + const int code = validate_row_query_names(scope, reader, err); + if (code != kExitOk) return code; + } return kExitOk; } @@ -331,6 +348,8 @@ int cmd_export(const ParsedArgs& args, storage::TsFileReader& reader, ParsedArgs query = args; query.command = "cat"; + const int code = validate_row_query_names(query, reader, err); + if (code != kExitOk) return code; long long rows = 0; return stream_query_to_file(query, reader, fmt, args.output, args.force, rows, err); diff --git a/cpp/tools/commands/cmd_ls.cc b/cpp/tools/commands/cmd_ls.cc index 8ee523f45..a5db54b92 100644 --- a/cpp/tools/commands/cmd_ls.cc +++ b/cpp/tools/commands/cmd_ls.cc @@ -83,6 +83,13 @@ int cmd_ls(const ParsedArgs& args, storage::TsFileReader& reader, } } + OutputNameValidator validator; + for (const std::string& name : names) { + if (!validator.add_object(name)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } const std::string model = table_model ? "table" : "tree"; RowWriter w(out, fmt, {"model", "object"}, {common::STRING, common::STRING}, false); diff --git a/cpp/tools/commands/cmd_schema.cc b/cpp/tools/commands/cmd_schema.cc index 8238436e5..f0d1fbd6c 100644 --- a/cpp/tools/commands/cmd_schema.cc +++ b/cpp/tools/commands/cmd_schema.cc @@ -211,6 +211,13 @@ int cmd_schema(const ParsedArgs& args, storage::TsFileReader& reader, } } + OutputNameValidator validator; + for (const SchemaRow& row : rows) { + if (!validator.add_column(row.cells[1], row.cells[2])) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } RowWriter w(out, fmt, {"model", "object", "column", "category", "data_type", "encoding", "compression"}, diff --git a/cpp/tools/commands/cmd_stats.cc b/cpp/tools/commands/cmd_stats.cc index e84d7101d..a9f22842c 100644 --- a/cpp/tools/commands/cmd_stats.cc +++ b/cpp/tools/commands/cmd_stats.cc @@ -520,6 +520,20 @@ int cmd_table_stats(const ParsedArgs& args, storage::TsFileReader& reader, } } + OutputNameValidator validator; + for (const TableStatsSummary& summary : summaries) { + if (summary.field_indexes.empty()) continue; + const std::string& object = summary.schema->get_table_name(); + std::vector indexes = summary.tag_indexes; + indexes.insert(indexes.end(), summary.field_indexes.begin(), + summary.field_indexes.end()); + for (size_t index : indexes) { + if (!validator.add_column(object, summary.columns[index].name)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } + } std::vector headers = {"model", "object"}; std::vector types = {common::STRING, common::STRING}; for (const std::string& tag : union_tags) { @@ -537,7 +551,7 @@ int cmd_table_stats(const ParsedArgs& args, storage::TsFileReader& reader, RowWriter w(out, fmt, headers, types, args.no_header); if (!w.error().empty()) { err << "Error: " << w.error() << "\n"; - return kExitRuntime; + return kExitFile; } for (const TableStatsSummary& summary : summaries) { std::map local_tag_positions; @@ -682,6 +696,13 @@ int cmd_stats(const ParsedArgs& args, storage::TsFileReader& reader, if (collect_ret != kExitOk) { return collect_ret; } + OutputNameValidator validator; + for (const SeriesStatRow& row : rows) { + if (!validator.add_column(row.target, row.measurement)) { + err << "Error: " << validator.error() << "\n"; + return kExitFile; + } + } for (const SeriesStatRow& row : rows) { const long long null_count = row.row_count - row.count; std::vector cells = { diff --git a/cpp/tools/commands/commands.h b/cpp/tools/commands/commands.h index cd4ae9630..d20ae6a34 100644 --- a/cpp/tools/commands/commands.h +++ b/cpp/tools/commands/commands.h @@ -55,6 +55,11 @@ int build_table_tag_filter(const ParsedArgs& args, const std::string& table_name, std::ostream& err, std::unique_ptr& ret_filter); +// Resolve the projected schema and reject name collisions without scanning +// rows or preparing output files. Export calls this for every object first. +int validate_row_query_names(const ParsedArgs& args, + storage::TsFileReader& reader, std::ostream& err); + int run_row_query(const ParsedArgs& args, storage::TsFileReader& reader, OutputFormat fmt, std::ostream& out, std::ostream& err, long long offset, long long limit, diff --git a/cpp/tools/commands/row_query.cc b/cpp/tools/commands/row_query.cc index 0c606c924..d35ae3121 100644 --- a/cpp/tools/commands/row_query.cc +++ b/cpp/tools/commands/row_query.cc @@ -150,6 +150,96 @@ int resolve_tree_paths(const ParsedArgs& args, storage::TsFileReader& reader, return kExitOk; } +struct RowQuerySelection { + bool table_model = false; + std::string object; + // Table column names or fully qualified tree paths, as required by query. + std::vector columns; +}; + +int resolve_row_query_selection(const ParsedArgs& args, + storage::TsFileReader& reader, + RowQuerySelection& selection, + std::ostream& err) { + int ret = resolve_table_model(args, reader, selection.table_model, err); + if (ret != kExitOk) return ret; + if (selection.table_model) { + selection.object = args.table; + if (selection.object.empty()) { + std::vector> schemas; + const int schemas_ret = reader.get_all_table_schemas(schemas); + if (schemas_ret != common::E_OK) { + err << "Error: failed to read table schemas: " + << error_code_message(schemas_ret) << "\n"; + return kExitFile; + } + if (schemas.empty() || !schemas[0]) { + err << "Error: no table found in file\n"; + return kExitRuntime; + } + if (schemas.size() != 1) { + err << "Error: head/cat requires -t/--table when the file " + "contains multiple tables\n"; + return kExitUsage; + } + selection.object = schemas[0]->get_table_name(); + } + std::shared_ptr schema; + const int schema_ret = + reader.get_table_schema(selection.object, schema); + if (schema_ret != common::E_OK) { + if (schema_ret == common::E_TABLE_NOT_EXIST) { + err << "Error: table '" << selection.object + << "' does not exist\n"; + return kExitUsage; + } + err << "Error: failed to read schema for table '" + << selection.object << "': " << error_code_message(schema_ret) + << "\n"; + return kExitFile; + } + ret = resolve_table_fields(args, schema, selection.columns, err); + } else { + if (args.has_tag_filter) { + err << "Error: tag filter flags are only valid for table model\n"; + return kExitUsage; + } + ParsedArgs effective_args = args; + if (effective_args.device.empty()) { + auto devices = reader.get_all_device_ids(); + if (devices.empty() || !devices[0]) { + err << "Error: no device found in file\n"; + return kExitRuntime; + } + if (devices.size() != 1) { + err << "Error: head/cat requires -d/--device when the file " + "contains multiple devices\n"; + return kExitUsage; + } + effective_args.device = devices[0]->get_device_name(); + } + selection.object = effective_args.device; + ret = + resolve_tree_paths(effective_args, reader, selection.columns, err); + } + if (ret != kExitOk) return ret; + + // Validate the selected schema before time/row filters can remove columns + // from the reader's result metadata, including entirely empty results. + std::vector header = {"time"}; + for (const std::string& column : selection.columns) { + header.push_back(selection.table_model + ? column + : column.substr(selection.object.size() + 1)); + } + const std::string error = validate_column_names(header); + if (!error.empty()) { + err << "Error: " << error << "\n"; + return kExitFile; + } + return kExitOk; +} + } // namespace int build_table_tag_filter(const ParsedArgs& args, @@ -276,6 +366,12 @@ std::vector collect_tree_query_paths( return paths; } +int validate_row_query_names(const ParsedArgs& args, + storage::TsFileReader& reader, std::ostream& err) { + RowQuerySelection selection; + return resolve_row_query_selection(args, reader, selection, err); +} + int run_row_query(const ParsedArgs& args, storage::TsFileReader& reader, OutputFormat fmt, std::ostream& out, std::ostream& err, long long offset, long long limit, long long* emitted_rows) { @@ -284,100 +380,35 @@ int run_row_query(const ParsedArgs& args, storage::TsFileReader& reader, const int64_t end = args.has_end ? static_cast(args.end) : std::numeric_limits::max(); + RowQuerySelection selection; + const int selection_ret = + resolve_row_query_selection(args, reader, selection, err); + if (selection_ret != kExitOk) return selection_ret; + storage::ResultSet* rs = nullptr; int qret = 0; const bool push_down = can_push_down_row_window(args, offset, limit); + const std::string tree_device_prefix = + selection.table_model ? "" : selection.object; std::unique_ptr tag_filter; - std::string tree_device_prefix; - - bool table_model = false; - const int model_ret = resolve_table_model(args, reader, table_model, err); - if (model_ret != kExitOk) { - return model_ret; - } - if (table_model) { - std::string table_name = args.table; - if (table_name.empty()) { - std::vector> schemas; - const int schemas_ret = reader.get_all_table_schemas(schemas); - if (schemas_ret != common::E_OK) { - err << "Error: failed to read table schemas: " - << error_code_message(schemas_ret) << "\n"; - return kExitFile; - } - if (schemas.empty() || !schemas[0]) { - err << "Error: no table found in file\n"; - return kExitRuntime; - } - if (schemas.size() != 1) { - err << "Error: head/cat requires -t/--table when the file " - "contains multiple tables\n"; - return kExitUsage; - } - table_name = schemas[0]->get_table_name(); - } - std::shared_ptr table_schema; - const int schema_ret = - reader.get_table_schema(table_name, table_schema); - if (schema_ret != common::E_OK) { - if (schema_ret == common::E_TABLE_NOT_EXIST) { - err << "Error: table '" << table_name << "' does not exist\n"; - return kExitUsage; - } - err << "Error: failed to read schema for table '" << table_name - << "': " << error_code_message(schema_ret) << "\n"; - return kExitFile; - } - std::vector cols; - int selection_ret = resolve_table_fields(args, table_schema, cols, err); - if (selection_ret != kExitOk) { - return selection_ret; - } - int filter_ret = - build_table_tag_filter(args, reader, table_name, err, tag_filter); - if (filter_ret != kExitOk) { - return filter_ret; - } + if (selection.table_model) { + const int filter_ret = build_table_tag_filter( + args, reader, selection.object, err, tag_filter); + if (filter_ret != kExitOk) return filter_ret; if (push_down) { - qret = reader.queryByRow( - table_name, cols, to_reader_row_bound(offset), - to_reader_row_bound(limit), rs, tag_filter.get()); + qret = reader.queryByRow(selection.object, selection.columns, + to_reader_row_bound(offset), + to_reader_row_bound(limit), rs, + tag_filter.get()); } else { - qret = reader.query(table_name, cols, start, end, rs, - tag_filter.get()); + qret = reader.query(selection.object, selection.columns, start, end, + rs, tag_filter.get()); } + } else if (push_down) { + qret = reader.queryByRow(selection.columns, to_reader_row_bound(offset), + to_reader_row_bound(limit), rs); } else { - if (args.has_tag_filter) { - err << "Error: tag filter flags are only valid for table model\n"; - return kExitUsage; - } - ParsedArgs effective_args = args; - if (effective_args.device.empty()) { - auto devices = reader.get_all_device_ids(); - if (devices.empty() || !devices[0]) { - err << "Error: no device found in file\n"; - return kExitRuntime; - } - if (devices.size() != 1) { - err << "Error: head/cat requires -d/--device when the file " - "contains multiple devices\n"; - return kExitUsage; - } - effective_args.device = devices[0]->get_device_name(); - } - tree_device_prefix = effective_args.device; - std::vector paths; - int selection_ret = - resolve_tree_paths(effective_args, reader, paths, err); - if (selection_ret != kExitOk) { - return selection_ret; - } - if (push_down) { - qret = reader.queryByRow(paths, to_reader_row_bound(offset), - to_reader_row_bound(limit), rs); - } else { - qret = reader.query(paths, start, end, rs); - } + qret = reader.query(selection.columns, start, end, rs); } if (qret != 0 || rs == nullptr) { @@ -404,7 +435,7 @@ int run_row_query(const ParsedArgs& args, storage::TsFileReader& reader, reader.destroy_query_data_set(rs); if (!output_error.empty()) { err << "Error: " << output_error << "\n"; - return kExitRuntime; + return kExitFile; } if (wret == common::E_OK) { const std::string bytes = staged.str(); diff --git a/cpp/tools/format/output_format.cc b/cpp/tools/format/output_format.cc index 50eee688a..696511456 100644 --- a/cpp/tools/format/output_format.cc +++ b/cpp/tools/format/output_format.cc @@ -358,6 +358,41 @@ std::string blob_hex(const std::string& cell) { } // namespace +std::string validate_column_names(const std::vector& names) { + std::set seen; + for (const std::string& name : names) { + const std::string display = replace_invalid_utf8(name); + if (!seen.insert(display).second) { + return "duplicate column name after UTF-8 replacement: \"" + + json_escape(display) + "\""; + } + } + return ""; +} + +bool OutputNameValidator::add_name(std::map& names, + const std::string& name, const char* kind) { + if (!error_.empty()) return false; + const std::string display = replace_invalid_utf8(name); + const auto entry = names.emplace(display, name); + if (!entry.second && entry.first->second != name) { + error_ = std::string("duplicate ") + kind + + " name after UTF-8 replacement: \"" + json_escape(display) + + "\""; + return false; + } + return true; +} + +bool OutputNameValidator::add_object(const std::string& name) { + return add_name(objects_, name, "object"); +} + +bool OutputNameValidator::add_column(const std::string& object, + const std::string& name) { + return add_object(object) && add_name(columns_[object], name, "column"); +} + RowWriter::RowWriter(std::ostream& out, OutputFormat fmt, std::vector header, std::vector types, bool no_header) @@ -367,18 +402,12 @@ RowWriter::RowWriter(std::ostream& out, OutputFormat fmt, types_(std::move(types)), no_header_(no_header), table_widths_(header_.size(), 0) { + error_ = validate_column_names(header_); + if (!error_.empty()) return; if (fmt_ == OutputFormat::kJson) { - std::set keys; json_keys_.reserve(header_.size()); for (const std::string& name : header_) { - std::string key = json_escape(name); - if (!keys.insert(key).second) { - error_ = - "duplicate NDJSON column name after UTF-8 replacement: \"" + - key + "\""; - return; - } - json_keys_.push_back(std::move(key)); + json_keys_.push_back(json_escape(name)); } } if (fmt_ == OutputFormat::kTable && !no_header_) { diff --git a/cpp/tools/format/output_format.h b/cpp/tools/format/output_format.h index 02d005b41..ca871ef32 100644 --- a/cpp/tools/format/output_format.h +++ b/cpp/tools/format/output_format.h @@ -21,6 +21,7 @@ #define TSFILE_CLI_OUTPUT_FORMAT_H #include +#include #include #include #include @@ -48,6 +49,26 @@ std::string csv_escape(const std::string& field); std::string json_escape(const std::string& s); std::string table_escape(const std::string& s); +// Headers must remain unique even when the result has no rows. +std::string validate_column_names(const std::vector& names); + +// Metadata repeats object/column names across rows. Only distinct raw names +// that render identically conflict; columns belong to their original object. +class OutputNameValidator { + public: + bool add_object(const std::string& name); + bool add_column(const std::string& object, const std::string& name); + const std::string& error() const { return error_; } + + private: + bool add_name(std::map& names, + const std::string& name, const char* kind); + + std::map objects_; + std::map> columns_; + std::string error_; +}; + class RowWriter { public: RowWriter(std::ostream& out, OutputFormat fmt,