diff --git a/.clang-tidy b/.clang-tidy index 7cf6854bc..b917eda05 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -28,9 +28,9 @@ Checks: >- # positives for overridden 'T& operator=(T&)'. CheckOptions: - key: readability-identifier-length.IgnoredVariableNames - value: \_ - - key: readability-identifier-length.IgnoredVariableNames - value: to + value: '_|to' + - key: readability-identifier-length.IgnoredParameterNames + value: '_|to' - key: readability-identifier-naming.NamespaceCase value: lower_case - key: readability-identifier-naming.ClassCase diff --git a/src/main.test.cpp b/src/main.test.cpp index e11fd7d43..f1ad87599 100644 --- a/src/main.test.cpp +++ b/src/main.test.cpp @@ -10,6 +10,8 @@ #include "silo/common/log.h" #include "silo/common/panic.h" +namespace { + int changeCwdToTestFolder() { // Look for the test data directory (`testBaseData`) in the current directory and up to // directories above the current directory. If found, change the current working @@ -29,6 +31,8 @@ int changeCwdToTestFolder() { return 1; } +} // namespace + int main(int argc, char* argv[]) { if (auto exit = changeCwdToTestFolder()) { return exit; diff --git a/src/silo/database.cpp b/src/silo/database.cpp index 8809a9115..73e3c31e8 100644 --- a/src/silo/database.cpp +++ b/src/silo/database.cpp @@ -23,7 +23,7 @@ namespace silo { Database::Database(schema::DatabaseSchema database_schema) - : schema(database_schema), + : schema(std::move(database_schema)), table(std::make_shared(schema.getDefaultTableSchema())) {} DatabaseInfo Database::getDatabaseInfo() const { diff --git a/src/silo/database.h b/src/silo/database.h index a2d30572c..25af4d53a 100644 --- a/src/silo/database.h +++ b/src/silo/database.h @@ -1,8 +1,6 @@ #pragma once -#include #include -#include #include "silo/common/data_version.h" #include "silo/common/silo_directory.h" @@ -23,7 +21,7 @@ class Database { DataVersion data_version_ = DataVersion::mineDataVersion(); public: - Database(silo::schema::DatabaseSchema database_schema); + explicit Database(silo::schema::DatabaseSchema database_schema); virtual ~Database() = default; @@ -33,7 +31,7 @@ class Database { [[nodiscard]] virtual DatabaseInfo getDatabaseInfo() const; - virtual DataVersion::Timestamp getDataVersionTimestamp() const; + [[nodiscard]] virtual DataVersion::Timestamp getDataVersionTimestamp() const; }; } // namespace silo diff --git a/src/silo/database.test.cpp b/src/silo/database.test.cpp index 129177459..fc812861c 100644 --- a/src/silo/database.test.cpp +++ b/src/silo/database.test.cpp @@ -2,16 +2,13 @@ #include #include -#include #include #include #include "config/source/yaml_file.h" -#include "silo/append/append.h" #include "silo/append/database_inserter.h" #include "silo/append/ndjson_line_reader.h" -#include "silo/common/nucleotide_symbols.h" #include "silo/common/phylo_tree.h" #include "silo/config/preprocessing_config.h" #include "silo/database_info.h" @@ -38,7 +35,7 @@ std::shared_ptr buildTestDatabase() { ); std::map lineage_trees; - for (auto filename : config.initialization_files.getLineageDefinitionFilenames()) { + for (const auto& filename : config.initialization_files.getLineageDefinitionFilenames()) { lineage_trees[filename] = silo::common::LineageTreeAndIdMap::fromLineageDefinitionFilePath(filename); } @@ -52,10 +49,10 @@ std::shared_ptr buildTestDatabase() { auto database = std::make_shared( silo::Database{silo::initialize::Initializer::createSchemaFromConfigFiles( std::move(database_config), - std::move(reference_genomes), - std::move(lineage_trees), - std::move(phylo_tree_file), - /*without_unaligned_columns=*/false + reference_genomes, + lineage_trees, + phylo_tree_file, + /*without_unaligned_sequences=*/false )} ); std::ifstream input(input_directory / "input.ndjson"); diff --git a/src/silo/database_info.h b/src/silo/database_info.h index 87b03a134..c230fad50 100644 --- a/src/silo/database_info.h +++ b/src/silo/database_info.h @@ -1,15 +1,8 @@ #pragma once -#include -#include -#include - #include #include -#include "silo/common/format_number.h" -#include "silo/common/nucleotide_symbols.h" - namespace silo { struct DatabaseInfo { @@ -28,7 +21,7 @@ void to_json(nlohmann::json& json, const silo::DatabaseInfo& databaseInfo); template <> class [[maybe_unused]] fmt::formatter { public: - constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + static constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } [[maybe_unused]] static auto format(silo::DatabaseInfo database_info, format_context& ctx) -> decltype(ctx.out()) { return fmt::format_to(ctx.out(), "{}", nlohmann::json{database_info}.dump()); diff --git a/src/silo/query_engine/actions/action.cpp b/src/silo/query_engine/actions/action.cpp index 0167aeee6..31c82f5fe 100644 --- a/src/silo/query_engine/actions/action.cpp +++ b/src/silo/query_engine/actions/action.cpp @@ -3,9 +3,7 @@ #include #include #include -#include #include -#include #include #include @@ -28,7 +26,6 @@ #include "silo/query_engine/bad_request.h" #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/exec_node/arrow_util.h" -#include "silo/query_engine/exec_node/ndjson_sink.h" #include "silo/query_engine/exec_node/throttled_batch_reslicer.h" #include "silo/query_engine/exec_node/zstd_decompress_expression.h" #include "silo/storage/column/column_type_visitor.h" @@ -57,7 +54,7 @@ std::optional Action::getOrdering() const { using arrow::compute::SortOrder; std::vector sort_keys; - for (auto order_by_field : order_by_fields) { + for (const auto& order_by_field : order_by_fields) { auto sort_order = order_by_field.ascending ? SortOrder::Ascending : SortOrder::Descending; sort_keys.emplace_back(order_by_field.name, sort_order); } @@ -224,7 +221,8 @@ QueryPlan Action::toQueryPlan( std::string_view request_id ) { validateOrderByFields(table->schema); - auto query_plan = toQueryPlanImpl(table, partition_filters, query_options, request_id); + auto query_plan = + toQueryPlanImpl(std::move(table), std::move(partition_filters), query_options, request_id); if (!query_plan.status().ok()) { SILO_PANIC("Arrow error: {}", query_plan.status().ToString()); }; @@ -235,7 +233,7 @@ arrow::Result Action::addSortNode( arrow::acero::ExecPlan* arrow_plan, arrow::acero::ExecNode* node, const std::vector& output_fields, - const arrow::Ordering ordering, + const arrow::Ordering& ordering, std::optional /*num_rows_to_produce*/ ) { arrow::AsyncGenerator> generator; @@ -277,14 +275,14 @@ arrow::Result Action::addSortNode( namespace { -uint64_t hash64(uint64_t x, uint64_t seed) { - x ^= seed; - x ^= x >> 33; - x *= 0xff51afd7ed558ccdULL; - x ^= x >> 33; - x *= 0xc4ceb9fe1a85ec53ULL; - x ^= x >> 33; - return x; +uint64_t hash64(uint64_t value, uint64_t seed) { + value ^= seed; + value ^= value >> 33; + value *= 0xff51afd7ed558ccdULL; + value ^= value >> 33; + value *= 0xc4ceb9fe1a85ec53ULL; + value ^= value >> 33; + return value; } arrow::Result removeRandomizeColumn( @@ -341,7 +339,7 @@ arrow::Result Action::addRandomizeColumn( return std::nullopt; } - auto input_batch = maybe_input_batch.value(); + const auto& input_batch = maybe_input_batch.value(); SILO_ASSERT(!input_batch.values.empty()); auto rows_in_batch = input_batch.values.at(0).length(); SILO_ASSERT_NE(rows_in_batch, arrow::Datum::kUnknownLength); @@ -406,8 +404,8 @@ class ColumnToReferenceSequenceVisitor { public: template std::optional operator()( - const TableSchema& table_schema, - const ColumnIdentifier& column_identifier + const TableSchema& /*table_schema*/, + const ColumnIdentifier& /*column_identifier*/ ) { return std::nullopt; } @@ -419,7 +417,7 @@ std::optional ColumnToReferenceSequenceVisitor::operator( const TableSchema& table_schema, const ColumnIdentifier& column_identifier ) { - auto metadata = + auto* metadata = table_schema.getColumnMetadata>(column_identifier.name) .value(); std::string reference; @@ -435,7 +433,7 @@ std::optional ColumnToReferenceSequenceVisitor::operator( const TableSchema& table_schema, const ColumnIdentifier& column_identifier ) { - auto metadata = + auto* metadata = table_schema.getColumnMetadata>(column_identifier.name) .value(); std::string reference; @@ -451,7 +449,7 @@ std::optional ColumnToReferenceSequenceVisitor::operator( const TableSchema& table_schema, const ColumnIdentifier& column_identifier ) { - auto metadata = + auto* metadata = table_schema.getColumnMetadata(column_identifier.name) .value(); return metadata->dictionary_string; @@ -465,18 +463,17 @@ arrow::Result Action::addZstdDecompressNode( const silo::schema::TableSchema& table_schema ) const { auto output_fields = getOutputSchema(table_schema); - bool needs_decompression = - std::any_of(output_fields.begin(), output_fields.end(), [](const auto& column_identifier) { - return schema::isSequenceColumn(column_identifier.type); - }); + bool needs_decompression = std::ranges::any_of(output_fields, [](const auto& column_identifier) { + return schema::isSequenceColumn(column_identifier.type); + }); if (needs_decompression) { size_t sum_of_reference_genome_sizes = 0; std::vector column_expressions; std::vector column_names; - for (auto column : getOutputSchema(table_schema)) { + for (const auto& column : getOutputSchema(table_schema)) { if (auto reference = storage::column::visit(column.type, ColumnToReferenceSequenceVisitor{}, table_schema, column)) { - column_expressions.push_back(exec_node::ZstdDecompressExpression::Make( + column_expressions.push_back(exec_node::ZstdDecompressExpression::make( arrow::compute::field_ref(arrow::FieldRef{column.name}), reference.value() )); sum_of_reference_genome_sizes += reference.value().length(); @@ -511,7 +508,7 @@ arrow::Result Action::addZstdDecompressNode( "additional sink node to help backpressure application before zstd decompression" ); - SILO_ASSERT_GT(sum_of_reference_genome_sizes, 0u); + SILO_ASSERT_GT(sum_of_reference_genome_sizes, 0U); // We aim for 64 MB batch size to give the plan time to apply backpressure auto maximum_batch_size = @@ -520,7 +517,7 @@ arrow::Result Action::addZstdDecompressNode( // Delay delivery of large number of batches to about 100 MB per second // A batch targets 64 MB, therefore we allow 1.5 batches per second. // Therefore, we should never emit more than one resliced batch per 0.667 seconds - constexpr std::chrono::milliseconds target_batch_rate{667}; + constexpr std::chrono::milliseconds TARGET_BATCH_RATE{667}; ARROW_ASSIGN_OR_RAISE( node, @@ -531,7 +528,7 @@ arrow::Result Action::addZstdDecompressNode( arrow::acero::SourceNodeOptions{ schema_of_sequence_batches, exec_node::ThrottledBatchReslicer{ - batch_generator, maximum_batch_size, target_batch_rate, backpressure_monitor + batch_generator, maximum_batch_size, TARGET_BATCH_RATE, backpressure_monitor } } ) diff --git a/src/silo/query_engine/actions/action.h b/src/silo/query_engine/actions/action.h index 1f56f9c5c..963792403 100644 --- a/src/silo/query_engine/actions/action.h +++ b/src/silo/query_engine/actions/action.h @@ -11,7 +11,6 @@ #include "silo/config/runtime_config.h" #include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/query_engine/filter/operators/operator.h" #include "silo/query_engine/query_plan.h" #include "silo/schema/database_schema.h" #include "silo/storage/table.h" @@ -37,7 +36,7 @@ class Action { virtual ~Action() = default; // Returns the type of this action, which is used for logging the performance by Action type - virtual std::string_view getType() const = 0; + [[nodiscard]] virtual std::string_view getType() const = 0; QueryPlan toQueryPlan( std::shared_ptr table, @@ -53,9 +52,9 @@ class Action { std::optional randomize_seed ); - std::optional getOrdering() const; + [[nodiscard]] std::optional getOrdering() const; - virtual std::vector getOutputSchema( + [[nodiscard]] virtual std::vector getOutputSchema( const silo::schema::TableSchema& table_schema ) const = 0; @@ -81,7 +80,7 @@ class Action { ) const; private: - virtual arrow::Result toQueryPlanImpl( + [[nodiscard]] virtual arrow::Result toQueryPlanImpl( std::shared_ptr table, std::vector partition_filters, const config::QueryOptions& query_options, @@ -92,7 +91,7 @@ class Action { arrow::acero::ExecPlan* arrow_plan, arrow::acero::ExecNode* node, const std::vector& output_fields, - const arrow::Ordering ordering, + const arrow::Ordering& ordering, std::optional num_rows_to_produce ); diff --git a/src/silo/query_engine/actions/aggregated.cpp b/src/silo/query_engine/actions/aggregated.cpp index 8c1336bda..3a02e0b0b 100644 --- a/src/silo/query_engine/actions/aggregated.cpp +++ b/src/silo/query_engine/actions/aggregated.cpp @@ -82,9 +82,13 @@ arrow::Result Aggregated::toQueryPlanImpl( ) const { EVOBENCH_SCOPE("Aggregated", "toQueryPlanImpl"); if (group_by_fields.empty()) { - return makeAggregateWithoutGrouping(table, partition_filters, query_options, request_id); + return makeAggregateWithoutGrouping( + std::move(table), std::move(partition_filters), query_options, request_id + ); } - return makeAggregateWithGrouping(table, partition_filters, query_options, request_id); + return makeAggregateWithGrouping( + std::move(table), std::move(partition_filters), query_options, request_id + ); } arrow::Result Aggregated::makeAggregateWithoutGrouping( @@ -93,19 +97,22 @@ arrow::Result Aggregated::makeAggregateWithoutGrouping( const config::QueryOptions& /*query_options*/, std::string_view request_id ) const { + const auto& table_schema = table->schema; + std::function>()> producer = - [table, partition_filters, produced = false]( - ) mutable -> arrow::Future> { - if (produced == true) { + [table = std::move(table), + partition_filters = std::move(partition_filters), + already_produced = false]() mutable -> arrow::Future> { + if (already_produced) { std::optional result = std::nullopt; return arrow::Future{result}; } - produced = true; + already_produced = true; int32_t result_count = 0; for (const auto& partition_filter : partition_filters) { - result_count += static_cast(partition_filter.getConstReference().cardinality()); + result_count += static_cast(partition_filter.getConstReference().cardinality()); } arrow::Int32Builder result_builder{}; @@ -123,7 +130,7 @@ arrow::Result Aggregated::makeAggregateWithoutGrouping( ARROW_ASSIGN_OR_RAISE(auto arrow_plan, arrow::acero::ExecPlan::Make()); arrow::acero::SourceNodeOptions options{ - exec_node::columnsToArrowSchema(getOutputSchema(table->schema)), + exec_node::columnsToArrowSchema(getOutputSchema(table_schema)), std::move(producer), arrow::Ordering::Implicit() }; @@ -140,6 +147,8 @@ arrow::Result Aggregated::makeAggregateWithGrouping( const config::QueryOptions& query_options, std::string_view request_id ) const { + const auto& table_schema = table->schema; + ARROW_ASSIGN_OR_RAISE(auto arrow_plan, arrow::acero::ExecPlan::Make()); std::vector group_by_fields_identifiers = @@ -151,8 +160,8 @@ arrow::Result Aggregated::makeAggregateWithGrouping( exec_node::makeTableScan( arrow_plan.get(), group_by_fields_identifiers, - partition_filters, - table, + std::move(partition_filters), + std::move(table), query_options.materialization_cutoff ) ); @@ -164,8 +173,9 @@ arrow::Result Aggregated::makeAggregateWithGrouping( }; std::vector field_refs; - for (auto group_by_field : group_by_fields_identifiers) { - field_refs.emplace_back(arrow::FieldRef{group_by_field.name}); + field_refs.reserve(group_by_fields_identifiers.size()); + for (const auto& group_by_field : group_by_fields_identifiers) { + field_refs.emplace_back(group_by_field.name); } arrow::acero::AggregateNodeOptions aggregate_node_options({aggregate}, field_refs); ARROW_ASSIGN_OR_RAISE( @@ -173,11 +183,11 @@ arrow::Result Aggregated::makeAggregateWithGrouping( arrow::acero::MakeExecNode("aggregate", arrow_plan.get(), {node}, aggregate_node_options) ); - ARROW_ASSIGN_OR_RAISE(node, addOrderingNodes(arrow_plan.get(), node, table->schema)); + ARROW_ASSIGN_OR_RAISE(node, addOrderingNodes(arrow_plan.get(), node, table_schema)); ARROW_ASSIGN_OR_RAISE(node, addLimitAndOffsetNode(arrow_plan.get(), node)); - ARROW_ASSIGN_OR_RAISE(node, addZstdDecompressNode(arrow_plan.get(), node, table->schema)); + ARROW_ASSIGN_OR_RAISE(node, addZstdDecompressNode(arrow_plan.get(), node, table_schema)); return QueryPlan::makeQueryPlan(arrow_plan, node, request_id); } diff --git a/src/silo/query_engine/actions/aggregated.test.cpp b/src/silo/query_engine/actions/aggregated.test.cpp index 7ce643517..4b9c57c89 100644 --- a/src/silo/query_engine/actions/aggregated.test.cpp +++ b/src/silo/query_engine/actions/aggregated.test.cpp @@ -1,5 +1,3 @@ -#include "silo/query_engine/actions/aggregated.h" - #include #include #include @@ -8,17 +6,15 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; using boost::uuids::random_generator; nlohmann::json createData(const std::string& country, const std::string& date) { - static std::atomic_int id = 0; - const auto primary_key = id++; - std::string age = id % 2 == 0 ? "null" : fmt::format("{}", 3 * id + 4); + static std::atomic_int row_id = 0; + const auto primary_key = row_id++; + std::string age = row_id % 2 == 0 ? "null" : fmt::format("{}", (3 * row_id) + 4); float coverage = 0.9; return nlohmann::json::parse(fmt::format( diff --git a/src/silo/query_engine/actions/details.h b/src/silo/query_engine/actions/details.h index 0e2605bca..951d87caa 100644 --- a/src/silo/query_engine/actions/details.h +++ b/src/silo/query_engine/actions/details.h @@ -8,7 +8,6 @@ #include #include "silo/query_engine/actions/simple_select_action.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { @@ -18,7 +17,7 @@ class Details : public SimpleSelectAction { public: explicit Details(std::vector fields); - std::vector getOutputSchema( + [[nodiscard]] std::vector getOutputSchema( const silo::schema::TableSchema& table_schema ) const override; }; diff --git a/src/silo/query_engine/actions/details.test.cpp b/src/silo/query_engine/actions/details.test.cpp index c800c7d72..30a34db05 100644 --- a/src/silo/query_engine/actions/details.test.cpp +++ b/src/silo/query_engine/actions/details.test.cpp @@ -1,5 +1,3 @@ -#include "silo/query_engine/actions/details.h" - #include #include #include @@ -8,17 +6,14 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; -using silo::test::QueryTestData; using silo::test::QueryTestScenario; using boost::uuids::random_generator; nlohmann::json createData(const std::string& country, const std::string& date) { - static std::atomic_int id = 0; - const auto primary_key = id++; - std::string age = id % 2 == 0 ? "null" : fmt::format("{}", 3 * id + 4); + static std::atomic_int row_id = 0; + const auto primary_key = row_id++; + std::string age = row_id % 2 == 0 ? "null" : fmt::format("{}", (3 * row_id) + 4); float coverage = 0.9; return nlohmann::json::parse(fmt::format( @@ -73,7 +68,7 @@ const auto REFERENCE_GENOMES = ReferenceGenomes{ {{"gene1", "M*"}}, }; -const QueryTestData TEST_DATA{ +const silo::test::QueryTestData TEST_DATA{ .ndjson_input_data = {createData("Switzerland", "2020-01-01"), createData("Germany", "2000-03-07"), diff --git a/src/silo/query_engine/actions/fasta.cpp b/src/silo/query_engine/actions/fasta.cpp index 9088d1e94..05c0b1901 100644 --- a/src/silo/query_engine/actions/fasta.cpp +++ b/src/silo/query_engine/actions/fasta.cpp @@ -7,15 +7,7 @@ #include #include -#include "silo/common/numbers.h" -#include "silo/common/panic.h" -#include "silo/common/range.h" -#include "silo/database.h" #include "silo/query_engine/bad_request.h" -#include "silo/query_engine/copy_on_write_bitmap.h" - -using silo::common::add1; -using silo::common::Range; namespace silo::query_engine::actions { @@ -27,7 +19,7 @@ std::vector Fasta::getOutputSchema( table_schema.getColumnByType(); for (const auto& sequence_name : sequence_names) { schema::ColumnIdentifier column_identifier{ - sequence_name, schema::ColumnType::ZSTD_COMPRESSED_STRING + .name = sequence_name, .type = schema::ColumnType::ZSTD_COMPRESSED_STRING }; CHECK_SILO_QUERY( std::ranges::find(columns_in_database, column_identifier) != columns_in_database.end(), diff --git a/src/silo/query_engine/actions/fasta.h b/src/silo/query_engine/actions/fasta.h index d725b1170..332b90525 100644 --- a/src/silo/query_engine/actions/fasta.h +++ b/src/silo/query_engine/actions/fasta.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -8,7 +7,6 @@ #include "silo/query_engine/actions/simple_select_action.h" #include "silo/schema/database_schema.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { @@ -24,7 +22,7 @@ class Fasta : public SimpleSelectAction { : sequence_names(std::move(sequence_names)), additional_fields(std::move(additional_fields)) {} - std::vector getOutputSchema( + [[nodiscard]] std::vector getOutputSchema( const silo::schema::TableSchema& table_schema ) const override; }; diff --git a/src/silo/query_engine/actions/fasta_aligned.cpp b/src/silo/query_engine/actions/fasta_aligned.cpp index 6ca303f25..5eb566257 100644 --- a/src/silo/query_engine/actions/fasta_aligned.cpp +++ b/src/silo/query_engine/actions/fasta_aligned.cpp @@ -1,20 +1,14 @@ #include "silo/query_engine/actions/fasta_aligned.h" -#include -#include #include -#include #include #include #include #include -#include "silo/common/panic.h" #include "silo/query_engine/actions/action.h" #include "silo/query_engine/bad_request.h" -#include "silo/storage/column/sequence_column.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { diff --git a/src/silo/query_engine/actions/fasta_aligned.h b/src/silo/query_engine/actions/fasta_aligned.h index d0a9832c5..cf8acd1ca 100644 --- a/src/silo/query_engine/actions/fasta_aligned.h +++ b/src/silo/query_engine/actions/fasta_aligned.h @@ -6,13 +6,7 @@ #include -#include "silo/query_engine/actions/action.h" #include "silo/query_engine/actions/simple_select_action.h" -#include "silo/query_engine/bad_request.h" -#include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/query_engine/filter/expressions/expression.h" -#include "silo/query_engine/filter/operators/operator.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { @@ -26,7 +20,7 @@ class FastaAligned : public SimpleSelectAction { std::vector&& additional_fields ); - std::vector getOutputSchema( + [[nodiscard]] std::vector getOutputSchema( const silo::schema::TableSchema& table_schema ) const override; }; diff --git a/src/silo/query_engine/actions/fasta_aligned.test.cpp b/src/silo/query_engine/actions/fasta_aligned.test.cpp index f72263798..59aead0cf 100644 --- a/src/silo/query_engine/actions/fasta_aligned.test.cpp +++ b/src/silo/query_engine/actions/fasta_aligned.test.cpp @@ -1,5 +1,3 @@ -#include "silo/query_engine/actions/fasta_aligned.h" - #include #include #include @@ -8,16 +6,14 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; using boost::uuids::random_generator; nlohmann::json createDataWithNucleotideSequence(const std::string& nucleotideSequence) { - static std::atomic_int id = 0; - const auto primary_key = id++; + static std::atomic_int row_id = 0; + const auto primary_key = row_id++; return nlohmann::json::parse(fmt::format( R"( diff --git a/src/silo/query_engine/actions/insertions.cpp b/src/silo/query_engine/actions/insertions.cpp index 3f722ed2d..55ae8b6d8 100644 --- a/src/silo/query_engine/actions/insertions.cpp +++ b/src/silo/query_engine/actions/insertions.cpp @@ -60,11 +60,11 @@ void InsertionAggregation< namespace { template void validateSequenceNames( - std::shared_ptr table, + const storage::Table& table, const std::vector& sequence_names ) { for (const std::string& sequence_name : sequence_names) { - auto column = table->schema.getColumn(sequence_name); + auto column = table.schema.getColumn(sequence_name); CHECK_SILO_QUERY( column.has_value() && column.value().type == SymbolType::COLUMN_TYPE, "The database does not contain the {} sequence '{}'", @@ -78,13 +78,13 @@ void validateSequenceNames( template std::unordered_map::PrefilteredBitmaps> InsertionAggregation::preFilterBitmaps( - std::shared_ptr table, + const storage::Table& table, const std::vector& sequence_names, std::vector& bitmap_filter ) { std::unordered_map pre_filtered_bitmaps; - for (size_t i = 0; i < table->getNumberOfPartitions(); ++i) { - const storage::TablePartition& table_partition = table->getPartition(i); + for (size_t i = 0; i < table.getNumberOfPartitions(); ++i) { + const storage::TablePartition& table_partition = table.getPartition(i); for (auto& [sequence_name, sequence_column] : table_partition.columns.getColumns()) { @@ -208,20 +208,23 @@ arrow::Result InsertionAggregation::toQueryPlanImpl( std::string_view request_id ) const { EVOBENCH_SCOPE("InsertionAggregation", "toQueryPlanImpl"); - validateSequenceNames(table, sequence_names); + validateSequenceNames(*table, sequence_names); auto sequence_names_to_evaluate = sequence_names; auto output_fields = getOutputSchema(table->schema); std::function>()> producer = - [table, output_fields, partition_filters, sequence_names_to_evaluate, produced = false]( - ) mutable -> arrow::Future> { + [table, + output_fields, + partition_filters, + sequence_names_to_evaluate, + already_produced = false]() mutable -> arrow::Future> { EVOBENCH_SCOPE("InsertionAggregation", "producer"); - if (produced == true) { + if (already_produced) { std::optional result = std::nullopt; return arrow::Future{result}; } - produced = true; + already_produced = true; std::unordered_map output_builder; for (const auto& output_field : output_fields) { @@ -231,7 +234,7 @@ arrow::Result InsertionAggregation::toQueryPlanImpl( } const auto bitmaps_to_evaluate = - preFilterBitmaps(table, sequence_names_to_evaluate, partition_filters); + preFilterBitmaps(*table, sequence_names_to_evaluate, partition_filters); for (const auto& [sequence_name, prefiltered_bitmaps] : bitmaps_to_evaluate) { const auto default_sequence_name = table->schema.getDefaultSequenceName(); const bool omit_sequence_in_response = @@ -278,7 +281,7 @@ arrow::Result InsertionAggregation::toQueryPlanImpl( template std::vector InsertionAggregation::getOutputSchema( - const silo::schema::TableSchema& table_schema + const silo::schema::TableSchema& /*table_schema*/ ) const { std::vector fields; fields.emplace_back(std::string(POSITION_FIELD_NAME), schema::ColumnType::INT32); diff --git a/src/silo/query_engine/actions/insertions.h b/src/silo/query_engine/actions/insertions.h index 208f07212..288286d8d 100644 --- a/src/silo/query_engine/actions/insertions.h +++ b/src/silo/query_engine/actions/insertions.h @@ -1,7 +1,5 @@ #pragma once -#include -#include #include #include #include @@ -13,8 +11,6 @@ #include #include -#include "silo/common/aa_symbols.h" -#include "silo/common/nucleotide_symbols.h" #include "silo/query_engine/actions/action.h" #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/exec_node/json_value_type_array_builder.h" @@ -50,28 +46,28 @@ class InsertionAggregation : public Action { std:: unordered_map::PrefilteredBitmaps> static preFilterBitmaps( - std::shared_ptr table, + const storage::Table& table, const std::vector& sequence_names, std::vector& bitmap_filter ); public: - InsertionAggregation(std::vector&& sequence_names); + explicit InsertionAggregation(std::vector&& sequence_names); void validateOrderByFields(const schema::TableSchema& schema) const override; - arrow::Result toQueryPlanImpl( + [[nodiscard]] arrow::Result toQueryPlanImpl( std::shared_ptr table, std::vector partition_filters, const config::QueryOptions& query_options, std::string_view request_id ) const override; - std::vector getOutputSchema( + [[nodiscard]] std::vector getOutputSchema( const silo::schema::TableSchema& table_schema ) const override; - std::string_view getType() const override { return "InsertionAggregation"; } + [[nodiscard]] std::string_view getType() const override { return "InsertionAggregation"; } }; template diff --git a/src/silo/query_engine/actions/insertions.test.cpp b/src/silo/query_engine/actions/insertions.test.cpp index 4ebf89820..8eb5fd4be 100644 --- a/src/silo/query_engine/actions/insertions.test.cpp +++ b/src/silo/query_engine/actions/insertions.test.cpp @@ -1,5 +1,3 @@ -#include "silo/query_engine/actions/insertions.h" - #include #include #include @@ -10,8 +8,6 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; @@ -21,10 +17,10 @@ nlohmann::json createData( std::vector insertions, std::vector aa_insertions ) { - static std::atomic_int id = 0; - const auto primary_key = id++; + static std::atomic_int row_id = 0; + const auto primary_key = row_id++; - std::string country = id % 3 == 0 ? "Germany" : "Switzerland"; + std::string country = row_id % 3 == 0 ? "Germany" : "Switzerland"; for (auto& insertion : insertions) { insertion = fmt::format("\"{}\"", insertion); diff --git a/src/silo/query_engine/actions/most_recent_common_ancestor.cpp b/src/silo/query_engine/actions/most_recent_common_ancestor.cpp index 0b4b81c55..17613844f 100644 --- a/src/silo/query_engine/actions/most_recent_common_ancestor.cpp +++ b/src/silo/query_engine/actions/most_recent_common_ancestor.cpp @@ -10,24 +10,14 @@ #include #include -#include "evobench/evobench.hpp" #include "silo/common/phylo_tree.h" -#include "silo/common/tree_node_id.h" -#include "silo/config/database_config.h" -#include "silo/query_engine/actions/action.h" #include "silo/query_engine/bad_request.h" -#include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/query_engine/exec_node/arrow_util.h" #include "silo/query_engine/exec_node/json_value_type_array_builder.h" #include "silo/schema/database_schema.h" -#include "silo/storage/column_group.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { using silo::common::MRCAResponse; using silo::common::PhyloTree; -using silo::common::TreeNodeId; -using silo::schema::ColumnType; MostRecentCommonAncestor::MostRecentCommonAncestor( std::string column_name, @@ -35,22 +25,19 @@ MostRecentCommonAncestor::MostRecentCommonAncestor( ) : TreeAction(std::move(column_name), print_nodes_not_in_tree) {} -using silo::query_engine::filter::operators::Operator; - arrow::Status MostRecentCommonAncestor::addResponseToBuilder( NodeValuesResponse& all_node_ids, std::unordered_map& output_builder, - const PhyloTree& phylo_tree, - bool print_nodes_not_in_tree + const PhyloTree& phylo_tree ) const { MRCAResponse response = phylo_tree.getMRCA(all_node_ids.node_values); std::optional mrca_node = - response.mrca_node_id.transform([](const auto& id) { return id.string; }); + response.mrca_node_id.transform([](const auto& node_id) { return node_id.string; }); std::optional mrca_parent = - response.parent_id_of_mrca.transform([](const auto& id) { return id.string; }); + response.parent_id_of_mrca.transform([](const auto& node_id) { return node_id.string; }); - int32_t missing_node_count = - all_node_ids.missing_node_count + static_cast(response.not_in_tree.size()); + auto missing_node_count = + static_cast(all_node_ids.missing_node_count + response.not_in_tree.size()); if (auto builder = output_builder.find("mrcaNode"); builder != output_builder.end()) { ARROW_RETURN_NOT_OK(builder->second.insert(mrca_node)); @@ -73,7 +60,7 @@ arrow::Status MostRecentCommonAncestor::addResponseToBuilder( } std::vector MostRecentCommonAncestor::getOutputSchema( - const schema::TableSchema& table_schema + const schema::TableSchema& /*table_schema*/ ) const { auto base = makeBaseOutputSchema(); base.emplace_back("mrcaNode", schema::ColumnType::STRING); diff --git a/src/silo/query_engine/actions/most_recent_common_ancestor.h b/src/silo/query_engine/actions/most_recent_common_ancestor.h index 9cc3aa64d..e207a6006 100644 --- a/src/silo/query_engine/actions/most_recent_common_ancestor.h +++ b/src/silo/query_engine/actions/most_recent_common_ancestor.h @@ -9,8 +9,6 @@ #include #include "silo/query_engine/actions/tree_action.h" -#include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { @@ -21,16 +19,16 @@ class MostRecentCommonAncestor : public TreeAction { arrow::Status addResponseToBuilder( NodeValuesResponse& all_node_ids, std::unordered_map& output_builder, - const common::PhyloTree& phylo_tree, - bool print_nodes_not_in_tree + const common::PhyloTree& phylo_tree ) const override; - std::vector getOutputSchema(const schema::TableSchema& table_schema + [[nodiscard]] std::vector getOutputSchema( + const schema::TableSchema& table_schema ) const override; - std::string_view getType() const override { return "MostRecentCommonAncestor"; } + [[nodiscard]] std::string_view getType() const override { return "MostRecentCommonAncestor"; } - std::string_view myResultFieldName() const override { return "mrcaNode"; } + [[nodiscard]] std::string_view myResultFieldName() const override { return "mrcaNode"; } }; // NOLINTNEXTLINE(readability-identifier-naming) diff --git a/src/silo/query_engine/actions/mutations.cpp b/src/silo/query_engine/actions/mutations.cpp index 295486a57..7b26192d1 100644 --- a/src/silo/query_engine/actions/mutations.cpp +++ b/src/silo/query_engine/actions/mutations.cpp @@ -88,6 +88,7 @@ __attribute__((noinline)) void initializeCountsWithSequenceCount( uint32_t sequence_count ) { EVOBENCH_SCOPE("Mutations", "initializeCountsWithSequenceCount"); + // NOLINTNEXTLINE(modernize-loop-convert) for (uint32_t position_idx = 0; position_idx < count_per_local_reference_position.size(); ++position_idx) { count_per_local_reference_position[position_idx] += sequence_count; @@ -481,14 +482,14 @@ arrow::Result Mutations::toQueryPlanImpl( output_fields, partition_filters, sequence_names_to_evaluate, - produced = false]() mutable -> arrow::Future> { + already_produced = false]() mutable -> arrow::Future> { EVOBENCH_SCOPE("Mutations", "producer"); - if (produced) { + if (already_produced) { std::optional result = std::nullopt; return arrow::Future{result}; } - produced = true; + already_produced = true; std::unordered_map::PrefilteredBitmaps> bitmaps_to_evaluate = preFilterBitmaps(*table, partition_filters); diff --git a/src/silo/query_engine/actions/mutations.test.cpp b/src/silo/query_engine/actions/mutations.test.cpp index 9309bf383..30416df1b 100644 --- a/src/silo/query_engine/actions/mutations.test.cpp +++ b/src/silo/query_engine/actions/mutations.test.cpp @@ -1,5 +1,3 @@ -#include "silo/query_engine/actions/mutations.h" - #include #include #include @@ -8,8 +6,6 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/query_engine/actions/phylo_subtree.cpp b/src/silo/query_engine/actions/phylo_subtree.cpp index e4de063f1..dbe26a1e9 100644 --- a/src/silo/query_engine/actions/phylo_subtree.cpp +++ b/src/silo/query_engine/actions/phylo_subtree.cpp @@ -11,22 +11,13 @@ #include #include "silo/common/phylo_tree.h" -#include "silo/common/tree_node_id.h" -#include "silo/config/database_config.h" -#include "silo/query_engine/actions/action.h" #include "silo/query_engine/bad_request.h" -#include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/query_engine/exec_node/arrow_util.h" #include "silo/query_engine/exec_node/json_value_type_array_builder.h" #include "silo/schema/database_schema.h" -#include "silo/storage/column_group.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { using silo::common::NewickResponse; using silo::common::PhyloTree; -using silo::common::TreeNodeId; -using silo::schema::ColumnType; PhyloSubtree::PhyloSubtree( std::string column_name, @@ -36,18 +27,15 @@ PhyloSubtree::PhyloSubtree( : TreeAction(std::move(column_name), print_nodes_not_in_tree), contract_unary_nodes(contract_unary_nodes) {} -using silo::query_engine::filter::operators::Operator; - arrow::Status PhyloSubtree::addResponseToBuilder( NodeValuesResponse& all_node_ids, std::unordered_map& output_builder, - const PhyloTree& phylo_tree, - bool print_nodes_not_in_tree + const PhyloTree& phylo_tree ) const { NewickResponse response = phylo_tree.toNewickString(all_node_ids.node_values, contract_unary_nodes); - int32_t missing_node_count = - all_node_ids.missing_node_count + static_cast(response.not_in_tree.size()); + auto missing_node_count = + static_cast(all_node_ids.missing_node_count + response.not_in_tree.size()); if (auto builder = output_builder.find("subtreeNewick"); builder != output_builder.end()) { ARROW_RETURN_NOT_OK(builder->second.insert(response.newick_string)); @@ -64,7 +52,7 @@ arrow::Status PhyloSubtree::addResponseToBuilder( } std::vector PhyloSubtree::getOutputSchema( - const schema::TableSchema& table_schema + const schema::TableSchema& /*table_schema*/ ) const { auto base = makeBaseOutputSchema(); base.emplace_back("subtreeNewick", schema::ColumnType::STRING); diff --git a/src/silo/query_engine/actions/phylo_subtree.h b/src/silo/query_engine/actions/phylo_subtree.h index 6e884921c..d963eefcf 100644 --- a/src/silo/query_engine/actions/phylo_subtree.h +++ b/src/silo/query_engine/actions/phylo_subtree.h @@ -9,8 +9,6 @@ #include #include "silo/query_engine/actions/tree_action.h" -#include "silo/query_engine/copy_on_write_bitmap.h" -#include "silo/storage/table.h" namespace silo::query_engine::actions { @@ -22,15 +20,15 @@ class PhyloSubtree : public TreeAction { arrow::Status addResponseToBuilder( NodeValuesResponse& all_node_ids, std::unordered_map& output_builder, - const common::PhyloTree& phylo_tree, - bool print_nodes_not_in_tree + const common::PhyloTree& phylo_tree ) const override; - std::vector getOutputSchema(const schema::TableSchema& table_schema + [[nodiscard]] std::vector getOutputSchema( + const schema::TableSchema& table_schema ) const override; - std::string_view getType() const override { return "PhyloSubtree"; } - std::string_view myResultFieldName() const override { return "subtreeNewick"; } + [[nodiscard]] std::string_view getType() const override { return "PhyloSubtree"; } + [[nodiscard]] std::string_view myResultFieldName() const override { return "subtreeNewick"; } }; // NOLINTNEXTLINE(readability-identifier-naming) diff --git a/src/silo/query_engine/actions/simple_select_action.cpp b/src/silo/query_engine/actions/simple_select_action.cpp index c418b4a80..1a9387074 100644 --- a/src/silo/query_engine/actions/simple_select_action.cpp +++ b/src/silo/query_engine/actions/simple_select_action.cpp @@ -8,7 +8,7 @@ #include #include "evobench/evobench.hpp" -#include "silo/query_engine/exec_node/ndjson_sink.h" +#include "silo/query_engine/bad_request.h" #include "silo/query_engine/exec_node/table_scan.h" #include "silo/schema/database_schema.h" @@ -16,9 +16,10 @@ namespace silo::query_engine::actions { void SimpleSelectAction::validateOrderByFields(const schema::TableSchema& schema) const { auto output_schema = getOutputSchema(schema); - auto output_schema_fields = output_schema | - std::views::transform([](const auto& x) { return x.name; }) | - std::views::common; + auto output_schema_fields = + output_schema | + std::views::transform([](const auto& identifier) { return identifier.name; }) | + std::views::common; for (const OrderByField& field : order_by_fields) { CHECK_SILO_QUERY( std::ranges::find(output_schema_fields, field.name) != std::end(output_schema_fields), @@ -45,7 +46,7 @@ arrow::Result SimpleSelectAction::toQueryPlanImpl( exec_node::makeTableScan( arrow_plan.get(), getOutputSchema(table->schema), - partition_filters, + std::move(partition_filters), table, query_options.materialization_cutoff ) diff --git a/src/silo/query_engine/actions/tree_action.cpp b/src/silo/query_engine/actions/tree_action.cpp index f99e5c1a1..0bd7491b0 100644 --- a/src/silo/query_engine/actions/tree_action.cpp +++ b/src/silo/query_engine/actions/tree_action.cpp @@ -11,12 +11,10 @@ #include #include "evobench/evobench.hpp" -#include "silo/common/phylo_tree.h" -#include "silo/common/tree_node_id.h" -#include "silo/config/database_config.h" #include "silo/query_engine/actions/action.h" #include "silo/query_engine/bad_request.h" #include "silo/query_engine/copy_on_write_bitmap.h" +#include "silo/query_engine/exec_node/arrow_util.h" #include "silo/schema/database_schema.h" #include "silo/storage/column_group.h" #include "silo/storage/table.h" @@ -28,18 +26,18 @@ TreeAction::TreeAction(std::string column_name, bool print_nodes_not_in_tree) : column_name(std::move(column_name)), print_nodes_not_in_tree(print_nodes_not_in_tree) {} -using silo::query_engine::filter::operators::Operator; - -void TreeAction::validateOrderByFields(const schema::TableSchema& schema) const { +void TreeAction::validateOrderByFields(const schema::TableSchema& /*schema*/) const { std::vector allowed{myResultFieldName(), "missingNodeCount"}; if (print_nodes_not_in_tree) { - allowed.push_back("missingFromTree"); + allowed.emplace_back("missingFromTree"); } for (const auto& field : order_by_fields) { - bool ok = std::ranges::any_of(allowed, [&](std::string_view f) { return f == field.name; }); + bool is_valid_field = std::ranges::any_of(allowed, [&](std::string_view allowed_name) { + return allowed_name == field.name; + }); CHECK_SILO_QUERY( - ok, + is_valid_field, "OrderByField {} is not contained in the result of this operation. " "Allowed values are {}.", field.name, @@ -49,10 +47,10 @@ void TreeAction::validateOrderByFields(const schema::TableSchema& schema) const } NodeValuesResponse TreeAction::getNodeValues( - std::shared_ptr table, + const storage::Table& table, const std::string& column_name, std::vector& bitmap_filter -) const { +) { size_t num_rows = 0; for (const auto& filter : bitmap_filter) { num_rows += filter.getConstReference().cardinality(); @@ -60,8 +58,8 @@ NodeValuesResponse TreeAction::getNodeValues( std::unordered_set all_tree_node_ids; uint32_t num_empty = 0; all_tree_node_ids.reserve(num_rows); - for (size_t i = 0; i < table->getNumberOfPartitions(); ++i) { - const storage::TablePartition& table_partition = table->getPartition(i); + for (size_t i = 0; i < table.getNumberOfPartitions(); ++i) { + const storage::TablePartition& table_partition = table.getPartition(i); const auto& string_column = table_partition.columns.string_columns.at(column_name); CopyOnWriteBitmap& filter = bitmap_filter[i]; @@ -78,13 +76,15 @@ NodeValuesResponse TreeAction::getNodeValues( } } } - return NodeValuesResponse{std::move(all_tree_node_ids), num_empty}; + return NodeValuesResponse{ + .node_values = std::move(all_tree_node_ids), .missing_node_count = num_empty + }; } arrow::Result TreeAction::toQueryPlanImpl( std::shared_ptr table, std::vector partition_filters, - const config::QueryOptions& query_options, + const config::QueryOptions& /*query_options*/, std::string_view request_id ) const { CHECK_SILO_QUERY( @@ -111,10 +111,9 @@ arrow::Result TreeAction::toQueryPlanImpl( ); const auto& phylo_tree = optional_table_metadata.value()->phylo_tree.value(); auto output_fields = getOutputSchema(table->schema); - auto evaluated_partition_filters = partition_filters; + auto evaluated_partition_filters = std::move(partition_filters); auto column_name_to_evaluate = column_name; - auto print_missing_nodes = print_nodes_not_in_tree; std::function>()> producer = [this, @@ -123,14 +122,13 @@ arrow::Result TreeAction::toQueryPlanImpl( output_fields, evaluated_partition_filters, &phylo_tree, - produced = false, - print_missing_nodes]() mutable -> arrow::Future> { + already_produced = false]() mutable -> arrow::Future> { EVOBENCH_SCOPE("TreeAction", "producer"); - if (produced == true) { + if (already_produced) { std::optional result = std::nullopt; return arrow::Future{result}; } - produced = true; + already_produced = true; std::unordered_map output_builder; for (const auto& output_field : output_fields) { @@ -140,11 +138,9 @@ arrow::Result TreeAction::toQueryPlanImpl( } auto node_values = - this->getNodeValues(table, column_name_to_evaluate, evaluated_partition_filters); + getNodeValues(*table, column_name_to_evaluate, evaluated_partition_filters); - ARROW_RETURN_NOT_OK( - this->addResponseToBuilder(node_values, output_builder, phylo_tree, print_missing_nodes) - ); + ARROW_RETURN_NOT_OK(this->addResponseToBuilder(node_values, output_builder, phylo_tree)); // Order of result_columns is relevant as it needs to be consistent with vector in schema std::vector result_columns; diff --git a/src/silo/query_engine/actions/tree_action.h b/src/silo/query_engine/actions/tree_action.h index 06dc0780a..9638aca44 100644 --- a/src/silo/query_engine/actions/tree_action.h +++ b/src/silo/query_engine/actions/tree_action.h @@ -13,8 +13,6 @@ #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/storage/table.h" -#include -#include "silo/query_engine/exec_node/arrow_util.h" #include "silo/query_engine/exec_node/json_value_type_array_builder.h" namespace silo::query_engine::actions { @@ -31,9 +29,9 @@ class TreeAction : public Action { bool print_nodes_not_in_tree; protected: - virtual std::string_view myResultFieldName() const = 0; + [[nodiscard]] virtual std::string_view myResultFieldName() const = 0; - std::vector makeBaseOutputSchema() const { + [[nodiscard]] std::vector makeBaseOutputSchema() const { std::vector fields; fields.emplace_back("missingNodeCount", schema::ColumnType::INT32); if (print_nodes_not_in_tree) { @@ -47,17 +45,16 @@ class TreeAction : public Action { public: TreeAction(std::string column_name, bool print_nodes_not_in_tree); - NodeValuesResponse getNodeValues( - std::shared_ptr table, + static NodeValuesResponse getNodeValues( + const storage::Table& table, const std::string& column_name, std::vector& bitmap_filter - ) const; + ); virtual arrow::Status addResponseToBuilder( NodeValuesResponse& all_node_ids, std::unordered_map& output_builder, - const common::PhyloTree& phylo_tree, - bool print_nodes_not_in_tree + const common::PhyloTree& phylo_tree ) const = 0; [[nodiscard]] arrow::Result toQueryPlanImpl( diff --git a/src/silo/query_engine/exec_node/arrow_util.cpp b/src/silo/query_engine/exec_node/arrow_util.cpp index 5e903512d..ab970a8c9 100644 --- a/src/silo/query_engine/exec_node/arrow_util.cpp +++ b/src/silo/query_engine/exec_node/arrow_util.cpp @@ -2,12 +2,10 @@ namespace silo::query_engine::exec_node { -const std::shared_ptr columnTypeToArrowType(schema::ColumnType column_type) { +std::shared_ptr columnTypeToArrowType(schema::ColumnType column_type) { switch (column_type) { case schema::ColumnType::STRING: - return arrow::utf8(); case schema::ColumnType::INDEXED_STRING: - return arrow::utf8(); case schema::ColumnType::DATE: return arrow::utf8(); case schema::ColumnType::BOOL: @@ -19,9 +17,7 @@ const std::shared_ptr columnTypeToArrowType(schema::ColumnType case schema::ColumnType::FLOAT: return arrow::float64(); case schema::ColumnType::AMINO_ACID_SEQUENCE: - return arrow::binary(); case schema::ColumnType::NUCLEOTIDE_SEQUENCE: - return arrow::binary(); case schema::ColumnType::ZSTD_COMPRESSED_STRING: return arrow::binary(); } diff --git a/src/silo/query_engine/exec_node/arrow_util.h b/src/silo/query_engine/exec_node/arrow_util.h index dcd870b20..14e3e50ce 100644 --- a/src/silo/query_engine/exec_node/arrow_util.h +++ b/src/silo/query_engine/exec_node/arrow_util.h @@ -18,7 +18,7 @@ namespace silo::query_engine::exec_node { -const std::shared_ptr columnTypeToArrowType(schema::ColumnType column_type); +std::shared_ptr columnTypeToArrowType(schema::ColumnType column_type); std::shared_ptr columnsToArrowSchema( const std::vector& columns diff --git a/src/silo/query_engine/exec_node/json_value_type_array_builder.cpp b/src/silo/query_engine/exec_node/json_value_type_array_builder.cpp index c3a09a466..b7c57e726 100644 --- a/src/silo/query_engine/exec_node/json_value_type_array_builder.cpp +++ b/src/silo/query_engine/exec_node/json_value_type_array_builder.cpp @@ -6,7 +6,7 @@ namespace silo::query_engine::exec_node { -JsonValueTypeArrayBuilder::JsonValueTypeArrayBuilder(std::shared_ptr type) { +JsonValueTypeArrayBuilder::JsonValueTypeArrayBuilder(const std::shared_ptr& type) { if (type == arrow::int32()) { builder = arrow::Int32Builder{}; } else if (type == arrow::float64()) { @@ -25,8 +25,8 @@ arrow::Status JsonValueTypeArrayBuilder::insert( ) { if (!value.has_value()) { return std::visit( - [&](auto& b) { - ARROW_RETURN_NOT_OK(b.AppendNull()); + [](auto& builder) { + ARROW_RETURN_NOT_OK(builder.AppendNull()); return arrow::Status::OK(); }, builder @@ -34,19 +34,19 @@ arrow::Status JsonValueTypeArrayBuilder::insert( } return std::visit( - [&](auto&& val) { + [this](auto&& val) { using T = std::decay_t; return std::visit( - [&](auto& b) { - using B = std::decay_t; + [&val](auto& builder) { + using B = std::decay_t; if constexpr (std::is_same_v && std::is_same_v) { - ARROW_RETURN_NOT_OK(b.Append(val)); + ARROW_RETURN_NOT_OK(builder.Append(val)); } else if constexpr (std::is_same_v && std::is_same_v) { - ARROW_RETURN_NOT_OK(b.Append(val)); + ARROW_RETURN_NOT_OK(builder.Append(val)); } else if constexpr (std::is_same_v && std::is_same_v) { - ARROW_RETURN_NOT_OK(b.Append(val)); + ARROW_RETURN_NOT_OK(builder.Append(val)); } else if constexpr (std::is_same_v && std::is_same_v) { - ARROW_RETURN_NOT_OK(b.Append(val)); + ARROW_RETURN_NOT_OK(builder.Append(val)); } else { SILO_PANIC("Type mismatch between value and builder"); } @@ -60,28 +60,9 @@ arrow::Status JsonValueTypeArrayBuilder::insert( } arrow::Result JsonValueTypeArrayBuilder::toDatum() { - return std::visit( - [&](auto& b) { - using B = std::decay_t; - if constexpr (std::is_same_v) { - auto& array = get(builder); - return array.Finish(); - } else if constexpr (std::is_same_v) { - auto& array = get(builder); - return array.Finish(); - } else if constexpr (std::is_same_v) { - auto& array = get(builder); - return array.Finish(); - } else if constexpr (std::is_same_v) { - auto& array = get(builder); - return array.Finish(); - } else { - SILO_PANIC("Type mismatch between value and builder"); - } - }, - builder - ) - .Map([](auto&& x) { return arrow::Datum{x}; }); + return std::visit([](auto& builder) { return builder.Finish(); }, builder).Map([](auto&& array) { + return arrow::Datum{array}; + }); } } // namespace silo::query_engine::exec_node diff --git a/src/silo/query_engine/exec_node/json_value_type_array_builder.h b/src/silo/query_engine/exec_node/json_value_type_array_builder.h index e159e4959..7704dc85a 100644 --- a/src/silo/query_engine/exec_node/json_value_type_array_builder.h +++ b/src/silo/query_engine/exec_node/json_value_type_array_builder.h @@ -17,7 +17,7 @@ class JsonValueTypeArrayBuilder { builder; public: - JsonValueTypeArrayBuilder(std::shared_ptr type); + explicit JsonValueTypeArrayBuilder(const std::shared_ptr& type); arrow::Status insert(const std::optional>& value ); diff --git a/src/silo/query_engine/exec_node/table_scan.cpp b/src/silo/query_engine/exec_node/table_scan.cpp index 8987a54ae..74e65b125 100644 --- a/src/silo/query_engine/exec_node/table_scan.cpp +++ b/src/silo/query_engine/exec_node/table_scan.cpp @@ -1,4 +1,5 @@ #include "silo/query_engine/exec_node/table_scan.h" +#include #include #include @@ -7,7 +8,9 @@ #include #include +#include "evobench/evobench.hpp" #include "silo/query_engine/batched_bitmap_reader.h" +#include "silo/storage/column/column_type_visitor.h" namespace silo::query_engine::exec_node { @@ -73,7 +76,7 @@ arrow::Status ColumnEntryAppender::operator()::TYPE) ); - auto array = + auto* array = table_scan_node .getColumnTypeArrayBuilders>() .at(column_name); @@ -93,7 +96,7 @@ arrow::Status ColumnEntryAppender::operator()::TYPE) ); - auto array = + auto* array = table_scan_node .getColumnTypeArrayBuilders>() .at(column_name); @@ -113,11 +116,11 @@ arrow::Status ColumnEntryAppender::operator()() .at(column_name); - auto& column = + const auto& column = table_partition.columns.getColumns().at( column_name ); @@ -236,7 +239,9 @@ arrow::Result makeTableScan( std::shared_ptr table, size_t batch_size_cutoff ) { - exec_node::TableScanGenerator generator(columns, partition_filters_, table, batch_size_cutoff); + exec_node::TableScanGenerator generator( + columns, std::move(partition_filters_), std::move(table), batch_size_cutoff + ); arrow::acero::SourceNodeOptions source_node_options{ exec_node::columnsToArrowSchema(columns), generator, arrow::Ordering::Implicit() }; diff --git a/src/silo/query_engine/exec_node/table_scan.h b/src/silo/query_engine/exec_node/table_scan.h index 52000a02f..e021bf2d5 100644 --- a/src/silo/query_engine/exec_node/table_scan.h +++ b/src/silo/query_engine/exec_node/table_scan.h @@ -10,12 +10,9 @@ #include #include -#include "evobench/evobench.hpp" -#include "silo/query_engine/bad_request.h" #include "silo/query_engine/batched_bitmap_reader.h" #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/exec_node/arrow_util.h" -#include "silo/storage/column/column_type_visitor.h" #include "silo/storage/table.h" namespace silo::query_engine::exec_node { @@ -26,7 +23,7 @@ class ExecBatchBuilder { std::vector output_fields; public: - ExecBatchBuilder(std::vector output_fields); + explicit ExecBatchBuilder(std::vector output_fields); template std::map*> getColumnTypeArrayBuilders() { diff --git a/src/silo/query_engine/exec_node/throttled_batch_reslicer.cpp b/src/silo/query_engine/exec_node/throttled_batch_reslicer.cpp index c1e4e9d79..56cf91ff7 100644 --- a/src/silo/query_engine/exec_node/throttled_batch_reslicer.cpp +++ b/src/silo/query_engine/exec_node/throttled_batch_reslicer.cpp @@ -64,7 +64,7 @@ arrow::Result> ThrottledBatchReslicer::deliverSl delayForTargetBatchRate(); - int64_t chunk_size = std::min(static_cast(batch_size), remaining); + int64_t chunk_size = std::min(batch_size, remaining); arrow::ExecBatch batch = current_batch.value().Slice(offset, chunk_size); offset += chunk_size; remaining -= chunk_size; diff --git a/src/silo/query_engine/exec_node/throttled_batch_reslicer.h b/src/silo/query_engine/exec_node/throttled_batch_reslicer.h index aea914bb0..b69e924cf 100644 --- a/src/silo/query_engine/exec_node/throttled_batch_reslicer.h +++ b/src/silo/query_engine/exec_node/throttled_batch_reslicer.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -21,8 +22,8 @@ class ThrottledBatchReslicer { BackpressureMonitor* backpressure_monitor; std::optional current_batch; - size_t offset; - size_t remaining; // always >0 when current_batch != std::nullopt + int64_t offset; + int64_t remaining; // always >0 when current_batch != std::nullopt std::optional last_batch_delivered; @@ -33,7 +34,7 @@ class ThrottledBatchReslicer { std::chrono::milliseconds target_batch_rate, BackpressureMonitor* backpressure_monitor ) - : input_batches(input_batches), + : input_batches(std::move(input_batches)), batch_size(batch_size), target_batch_rate(target_batch_rate), backpressure_monitor(backpressure_monitor) { diff --git a/src/silo/query_engine/exec_node/throttled_batch_reslicer.test.cpp b/src/silo/query_engine/exec_node/throttled_batch_reslicer.test.cpp index 34fb02717..9cf65f41a 100644 --- a/src/silo/query_engine/exec_node/throttled_batch_reslicer.test.cpp +++ b/src/silo/query_engine/exec_node/throttled_batch_reslicer.test.cpp @@ -4,16 +4,17 @@ #include #include #include +#include #include #include #include -#include #include -using namespace silo::query_engine::exec_node; -using namespace arrow; -using namespace arrow::acero; -using namespace arrow::compute; +using arrow::AsyncGenerator; +using arrow::ExecBatch; +using arrow::acero::BackpressureMonitor; + +using silo::query_engine::exec_node::ThrottledBatchReslicer; // Mock BackpressureMonitor for testing class MockBackpressureMonitor : public BackpressureMonitor { @@ -35,7 +36,7 @@ class ThrottledBatchReslicerTest : public ::testing::Test { std::unique_ptr mock_backpressure_monitor; // Helper function to create a simple ExecBatch with integer data - ExecBatch CreateTestBatch(int64_t length, int32_t start_value = 0) { + static ExecBatch createTestBatch(int64_t length, int32_t start_value = 0) { auto builder = std::make_shared(); for (int32_t i = 0; i < length; ++i) { ARROW_EXPECT_OK(builder->Append(start_value + i)); @@ -47,26 +48,26 @@ class ThrottledBatchReslicerTest : public ::testing::Test { } // Helper to create async generator from vector of batches - AsyncGenerator> CreateGenerator( + static AsyncGenerator> createGenerator( std::vector> batches ) { - auto batches_wrapped = std::make_shared(batches); - auto it = std::make_sharedbegin())>(batches_wrapped->begin()); - auto end_it = batches_wrapped->end(); + auto batches_wrapped = std::make_shared(std::move(batches)); + auto iter = std::make_sharedbegin())>(batches_wrapped->begin()); + auto end_iter = batches_wrapped->end(); - return [batches_wrapped, it, end_it]() -> Future> { - if (*it == end_it) { + return [batches_wrapped, iter, end_iter]() -> arrow::Future> { + if (*iter == end_iter) { return std::optional{std::nullopt}; } - auto result = **it; - ++(*it); + auto result = **iter; + ++(*iter); return result; }; } }; TEST_F(ThrottledBatchReslicerTest, ConstructorValidation) { - auto generator = CreateGenerator({}); + auto generator = createGenerator({}); // Valid construction should not throw EXPECT_NO_THROW({ @@ -90,39 +91,39 @@ TEST_F(ThrottledBatchReslicerTest, ConstructorValidation) { } TEST_F(ThrottledBatchReslicerTest, EmptyInput) { - auto generator = CreateGenerator({std::nullopt}); + auto generator = createGenerator({std::nullopt}); ThrottledBatchReslicer reslicer( generator, 100, std::chrono::milliseconds(10), mock_backpressure_monitor.get() ); auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); EXPECT_FALSE(result.ValueOrDie().has_value()); } TEST_F(ThrottledBatchReslicerTest, EmptyBatch) { - auto empty_batch = CreateTestBatch(0); - auto generator = CreateGenerator({empty_batch, std::nullopt}); + auto empty_batch = createTestBatch(0); + auto generator = createGenerator({empty_batch, std::nullopt}); ThrottledBatchReslicer reslicer( generator, 100, std::chrono::milliseconds(10), mock_backpressure_monitor.get() ); auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); - auto batch = result.ValueOrDie(); + const auto& batch = result.ValueOrDie(); ASSERT_TRUE(batch.has_value()); EXPECT_EQ(batch->length, 0); } TEST_F(ThrottledBatchReslicerTest, BatchSmallerThanTargetSize) { - auto small_batch = CreateTestBatch(50); // Smaller than target size - auto generator = CreateGenerator({small_batch, std::nullopt}); + auto small_batch = createTestBatch(50); // Smaller than target size + auto generator = createGenerator({small_batch, std::nullopt}); ThrottledBatchReslicer reslicer( generator, @@ -147,8 +148,8 @@ TEST_F(ThrottledBatchReslicerTest, BatchSmallerThanTargetSize) { } TEST_F(ThrottledBatchReslicerTest, BatchEqualToTargetSize) { - auto exact_batch = CreateTestBatch(100); - auto generator = CreateGenerator({exact_batch, std::nullopt}); + auto exact_batch = createTestBatch(100); + auto generator = createGenerator({exact_batch, std::nullopt}); ThrottledBatchReslicer reslicer( generator, @@ -158,17 +159,17 @@ TEST_F(ThrottledBatchReslicerTest, BatchEqualToTargetSize) { ); auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); - auto batch = result.ValueOrDie(); + const auto& batch = result.ValueOrDie(); ASSERT_TRUE(batch.has_value()); EXPECT_EQ(batch->length, 100); } TEST_F(ThrottledBatchReslicerTest, BatchLargerThanTargetSize) { - auto large_batch = CreateTestBatch(250); - auto generator = CreateGenerator({large_batch, std::nullopt}); + auto large_batch = createTestBatch(250); + auto generator = createGenerator({large_batch, std::nullopt}); ThrottledBatchReslicer reslicer( generator, @@ -179,39 +180,39 @@ TEST_F(ThrottledBatchReslicerTest, BatchLargerThanTargetSize) { // First call should return first slice auto future1 = reslicer(); - auto result1 = future1.result(); + const auto& result1 = future1.result(); ASSERT_TRUE(result1.ok()); - auto batch1 = result1.ValueOrDie(); + const auto& batch1 = result1.ValueOrDie(); ASSERT_TRUE(batch1.has_value()); EXPECT_EQ(batch1->length, 100); // Second call should return second slice auto future2 = reslicer(); - auto result2 = future2.result(); + const auto& result2 = future2.result(); ASSERT_TRUE(result2.ok()); - auto batch2 = result2.ValueOrDie(); + const auto& batch2 = result2.ValueOrDie(); ASSERT_TRUE(batch2.has_value()); EXPECT_EQ(batch2->length, 100); // Third call should return remaining slice auto future3 = reslicer(); - auto result3 = future3.result(); + const auto& result3 = future3.result(); ASSERT_TRUE(result3.ok()); - auto batch3 = result3.ValueOrDie(); + const auto& batch3 = result3.ValueOrDie(); ASSERT_TRUE(batch3.has_value()); EXPECT_EQ(batch3->length, 50); // Fourth call should return nullopt (end of input) auto future4 = reslicer(); - auto result4 = future4.result(); + const auto& result4 = future4.result(); ASSERT_TRUE(result4.ok()); EXPECT_FALSE(result4.ValueOrDie().has_value()); } TEST_F(ThrottledBatchReslicerTest, MultipleBatches) { - auto batch1 = CreateTestBatch(150, 0); - auto batch2 = CreateTestBatch(75, 150); - auto generator = CreateGenerator({batch1, batch2, std::nullopt}); + auto batch1 = createTestBatch(150, 0); + auto batch2 = createTestBatch(75, 150); + auto generator = createGenerator({batch1, batch2, std::nullopt}); ThrottledBatchReslicer reslicer( generator, 100, std::chrono::milliseconds(1), mock_backpressure_monitor.get() @@ -222,9 +223,9 @@ TEST_F(ThrottledBatchReslicerTest, MultipleBatches) { // Process all batches while (true) { auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); - auto batch = result.ValueOrDie(); + const auto& batch = result.ValueOrDie(); if (!batch.has_value()) { break; @@ -238,15 +239,15 @@ TEST_F(ThrottledBatchReslicerTest, MultipleBatches) { } TEST_F(ThrottledBatchReslicerTest, ThrottlingDelay) { - auto large_batch = CreateTestBatch(200); - auto generator = CreateGenerator({large_batch, std::nullopt}); + auto large_batch = createTestBatch(200); + auto generator = createGenerator({large_batch, std::nullopt}); const auto delay = std::chrono::milliseconds(50); ThrottledBatchReslicer reslicer(generator, 100, delay, mock_backpressure_monitor.get()); // First call should not have delay (no previous batch) auto future1 = reslicer(); - auto result1 = future1.result(); + const auto& result1 = future1.result(); ASSERT_TRUE(result1.ok()); EXPECT_TRUE(result1.ValueOrDie().has_value()); @@ -254,7 +255,7 @@ TEST_F(ThrottledBatchReslicerTest, ThrottlingDelay) { // Second call should have delay auto future2 = reslicer(); - auto result2 = future2.result(); + const auto& result2 = future2.result(); ASSERT_TRUE(result2.ok()); EXPECT_TRUE(result2.ValueOrDie().has_value()); @@ -266,8 +267,8 @@ TEST_F(ThrottledBatchReslicerTest, ThrottlingDelay) { } TEST_F(ThrottledBatchReslicerTest, BackpressureMonitorLogging) { - auto batch = CreateTestBatch(50); - auto generator = CreateGenerator({batch, std::nullopt}); + auto batch = createTestBatch(50); + auto generator = createGenerator({batch, std::nullopt}); // Set up expectations for backpressure monitor calls EXPECT_CALL(*mock_backpressure_monitor, bytes_in_use()).Times(::testing::AtLeast(1)); @@ -278,7 +279,7 @@ TEST_F(ThrottledBatchReslicerTest, BackpressureMonitorLogging) { ); auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); } @@ -292,7 +293,7 @@ TEST_F(ThrottledBatchReslicerTest, DataIntegrity) { ARROW_EXPECT_OK(builder->Finish(&array)); auto batch = ExecBatch({array}, 150); - auto generator = CreateGenerator({batch, std::nullopt}); + auto generator = createGenerator({batch, std::nullopt}); ThrottledBatchReslicer reslicer( generator, 100, std::chrono::milliseconds(1), mock_backpressure_monitor.get() @@ -300,9 +301,9 @@ TEST_F(ThrottledBatchReslicerTest, DataIntegrity) { // Get first slice (0-99) auto future1 = reslicer(); - auto result1 = future1.result(); + const auto& result1 = future1.result(); ASSERT_TRUE(result1.ok()); - auto batch1 = result1.ValueOrDie(); + const auto& batch1 = result1.ValueOrDie(); ASSERT_TRUE(batch1.has_value()); EXPECT_EQ(batch1->length, 100); @@ -314,9 +315,9 @@ TEST_F(ThrottledBatchReslicerTest, DataIntegrity) { // Get second slice (100-149) auto future2 = reslicer(); - auto result2 = future2.result(); + const auto& result2 = future2.result(); ASSERT_TRUE(result2.ok()); - auto batch2 = result2.ValueOrDie(); + const auto& batch2 = result2.ValueOrDie(); ASSERT_TRUE(batch2.has_value()); EXPECT_EQ(batch2->length, 50); @@ -329,7 +330,7 @@ TEST_F(ThrottledBatchReslicerTest, DataIntegrity) { TEST_F(ThrottledBatchReslicerTest, ExceptionHandling) { // Create a generator that throws an exception - auto throwing_generator = []() -> Future> { + auto throwing_generator = []() -> arrow::Future> { throw std::runtime_error("Test exception"); }; @@ -338,7 +339,7 @@ TEST_F(ThrottledBatchReslicerTest, ExceptionHandling) { ); auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); // Should return an error status, not throw EXPECT_FALSE(result.ok()); @@ -347,8 +348,8 @@ TEST_F(ThrottledBatchReslicerTest, ExceptionHandling) { // Performance test to ensure throttling works correctly under load TEST_F(ThrottledBatchReslicerTest, PerformanceThrottling) { - auto large_batch = CreateTestBatch(1000); - auto generator = CreateGenerator({large_batch, std::nullopt}); + auto large_batch = createTestBatch(1000); + auto generator = createGenerator({large_batch, std::nullopt}); const auto delay = std::chrono::milliseconds(10); ThrottledBatchReslicer reslicer(generator, 100, delay, mock_backpressure_monitor.get()); @@ -358,9 +359,9 @@ TEST_F(ThrottledBatchReslicerTest, PerformanceThrottling) { while (true) { auto future = reslicer(); - auto result = future.result(); + const auto& result = future.result(); ASSERT_TRUE(result.ok()); - auto batch = result.ValueOrDie(); + const auto& batch = result.ValueOrDie(); if (!batch.has_value()) { break; diff --git a/src/silo/query_engine/exec_node/zstd_decompress_expression.cpp b/src/silo/query_engine/exec_node/zstd_decompress_expression.cpp index e0d5c4a49..c341d3f84 100644 --- a/src/silo/query_engine/exec_node/zstd_decompress_expression.cpp +++ b/src/silo/query_engine/exec_node/zstd_decompress_expression.cpp @@ -15,6 +15,8 @@ #include #include +#include + #include "evobench/evobench.hpp" #include "silo/common/panic.h" #include "silo/zstd/zstd_context.h" @@ -25,7 +27,7 @@ namespace silo::query_engine::exec_node { namespace { struct BinaryDecompressKernel { - static arrow::Status Exec( + static arrow::Status exec( arrow::compute::KernelContext* context, const arrow::compute::ExecSpan& input, arrow::compute::ExecResult* out @@ -86,9 +88,9 @@ struct BinaryDecompressKernel { } }; -arrow::Result RegisterCustomFunctionImpl() { +arrow::Result registerCustomFunctionImpl() { std::string function_name = "silo_zstd_decompressor"; - auto registry = arrow::compute::GetFunctionRegistry(); + auto* registry = arrow::compute::GetFunctionRegistry(); std::string summary = "Decompresses each value of zstd_compressed_binary using the scalar dictionary"; @@ -105,7 +107,7 @@ arrow::Result RegisterCustomFunctionImpl() { { arrow::compute::ScalarKernel kernel; - kernel.exec = BinaryDecompressKernel::Exec; + kernel.exec = BinaryDecompressKernel::exec; kernel.signature = arrow::compute::KernelSignature::Make({arrow::binary(), arrow::binary()}, arrow::utf8()); kernel.null_handling = arrow::compute::NullHandling::INTERSECTION; @@ -118,9 +120,9 @@ arrow::Result RegisterCustomFunctionImpl() { return function_name; } -std::string RegisterCustomFunction() { +std::string registerCustomFunction() { std::string function_name; - auto status = RegisterCustomFunctionImpl().Value(&function_name); + auto status = registerCustomFunctionImpl().Value(&function_name); if (!status.ok()) { throw std::runtime_error(fmt::format("Err: {}", status.ToString())); } @@ -128,16 +130,17 @@ std::string RegisterCustomFunction() { } } // namespace -arrow::Expression ZstdDecompressExpression::Make( +arrow::Expression ZstdDecompressExpression::make( arrow::Expression input_expression, std::string dictionary_string ) { - static std::string function_name = RegisterCustomFunction(); + static std::string function_name = registerCustomFunction(); auto dict_scalar = std::make_shared(std::move(dictionary_string)); return arrow::compute::call( - function_name, {input_expression, arrow::compute::literal(arrow::Datum(dict_scalar))} + function_name, + {std::move(input_expression), arrow::compute::literal(arrow::Datum(dict_scalar))} ); } diff --git a/src/silo/query_engine/exec_node/zstd_decompress_expression.h b/src/silo/query_engine/exec_node/zstd_decompress_expression.h index dd4cbe3e6..6889bf025 100644 --- a/src/silo/query_engine/exec_node/zstd_decompress_expression.h +++ b/src/silo/query_engine/exec_node/zstd_decompress_expression.h @@ -6,7 +6,7 @@ namespace silo::query_engine::exec_node { class ZstdDecompressExpression { public: - static arrow::compute::Expression Make( + static arrow::compute::Expression make( arrow::compute::Expression input_expression, std::string dictionary_string ); diff --git a/src/silo/query_engine/exec_node/zstd_decompress_expression.test.cpp b/src/silo/query_engine/exec_node/zstd_decompress_expression.test.cpp index 23bae402b..1b6a01a34 100644 --- a/src/silo/query_engine/exec_node/zstd_decompress_expression.test.cpp +++ b/src/silo/query_engine/exec_node/zstd_decompress_expression.test.cpp @@ -2,6 +2,8 @@ #include +#include + #include #include #include @@ -20,9 +22,10 @@ #include "silo/zstd/zstd_compressor.h" #include "silo/zstd/zstd_dictionary.h" +namespace { arrow::Result> setupTestTable( std::vector> values, - std::string dictionary_string + std::string_view dictionary_string ) { std::shared_ptr schema = arrow::schema( {arrow::field("id", arrow::int32()), @@ -34,9 +37,9 @@ arrow::Result> setupTestTable( arrow::Int32Builder id_builder; arrow::BinaryBuilder value_builder; - int32_t id = 1; + int32_t id_column_value = 1; for (auto& value : values) { - ARROW_RETURN_NOT_OK(id_builder.Append(id++)); + ARROW_RETURN_NOT_OK(id_builder.Append(id_column_value++)); if (value.has_value()) { ARROW_RETURN_NOT_OK(value_builder.Append(compressor.compress(value->data(), value->size())) ); @@ -50,22 +53,21 @@ arrow::Result> setupTestTable( } using silo::query_engine::exec_node::ZstdDecompressExpression; - std::shared_ptr runValuesThroughProjection( std::vector> values, - std::string dictionary_string + const std::string& dictionary_string ) { - auto input_table = setupTestTable(values, dictionary_string).ValueOrDie(); + auto input_table = setupTestTable(std::move(values), dictionary_string).ValueOrDie(); auto arrow_plan = arrow::acero::ExecPlan::Make().ValueOrDie(); arrow::acero::TableSourceNodeOptions source_options{input_table}; - auto node = + auto* node = arrow::acero::MakeExecNode("table_source", arrow_plan.get(), {}, source_options).ValueOrDie(); arrow::acero::ProjectNodeOptions project_options( {arrow::compute::field_ref("id"), - ZstdDecompressExpression::Make( + ZstdDecompressExpression::make( arrow::compute::field_ref("some_zstd_compressed_column"), dictionary_string )} ); @@ -107,6 +109,7 @@ void assertDecompressedStringArray( } } } +} // namespace TEST(ZstdDecompressExpression, decompressesValues) { std::vector> values = {"ACGT", "ACCT", "ACGG"}; @@ -128,12 +131,13 @@ TEST(ZstdDecompressExpression, empty) { TEST(ZstdDecompressExpression, largeSet) { std::vector> values = {}; + values.reserve(25002); for (size_t i = 0; i < 15000; ++i) { - values.push_back("ACGT"); + values.emplace_back("ACGT"); } - values.push_back(std::nullopt); + values.emplace_back(std::nullopt); for (size_t i = 0; i < 10000; ++i) { - values.push_back("AAAA"); + values.emplace_back("AAAA"); } auto result_table = runValuesThroughProjection(values, "ACGTC"); assertDecompressedStringArray(values, result_table); diff --git a/src/silo/query_engine/filter/expressions/and.test.cpp b/src/silo/query_engine/filter/expressions/and.test.cpp index 599da80a6..8e0c0369b 100644 --- a/src/silo/query_engine/filter/expressions/and.test.cpp +++ b/src/silo/query_engine/filter/expressions/and.test.cpp @@ -6,17 +6,15 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; using boost::uuids::random_generator; nlohmann::json createData(const std::string& country, const std::string& date) { - static std::atomic_int id = 0; - const auto primary_key = id++; - std::string age = id % 2 == 0 ? "null" : fmt::format("{}", 3 * id + 4); + static std::atomic_int row_id = 0; + const auto primary_key = row_id++; + std::string age = row_id % 2 == 0 ? "null" : fmt::format("{}", (3 * row_id) + 4); float coverage = 0.9; return nlohmann::json::parse(fmt::format( diff --git a/src/silo/query_engine/filter/expressions/insertion_contains.cpp b/src/silo/query_engine/filter/expressions/insertion_contains.cpp index 13e293e91..bfe03a615 100644 --- a/src/silo/query_engine/filter/expressions/insertion_contains.cpp +++ b/src/silo/query_engine/filter/expressions/insertion_contains.cpp @@ -1,8 +1,5 @@ #include "silo/query_engine/filter/expressions/insertion_contains.h" -#include -#include -#include #include #include @@ -11,14 +8,11 @@ #include "silo/common/aa_symbols.h" #include "silo/common/nucleotide_symbols.h" -#include "silo/database.h" #include "silo/query_engine/bad_request.h" #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/filter/expressions/expression.h" #include "silo/query_engine/filter/operators/bitmap_producer.h" -#include "silo/query_engine/filter/operators/empty.h" #include "silo/query_engine/filter/operators/operator.h" -#include "silo/query_engine/filter/operators/union.h" #include "silo/query_engine/query_parse_sequence_name.h" #include "silo/storage/column/insertion_index.h" #include "silo/storage/column/sequence_column.h" diff --git a/src/silo/query_engine/filter/expressions/lineage_filter.test.cpp b/src/silo/query_engine/filter/expressions/lineage_filter.test.cpp index ca317e55f..30dd37483 100644 --- a/src/silo/query_engine/filter/expressions/lineage_filter.test.cpp +++ b/src/silo/query_engine/filter/expressions/lineage_filter.test.cpp @@ -1,15 +1,11 @@ #include -#include - #include "silo/preprocessing/lineage_definition_file.h" #include "silo/test/query_fixture.test.h" namespace { using silo::ReferenceGenomes; using silo::common::LineageTreeAndIdMap; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::preprocessing::LineageDefinitionFile; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/query_engine/filter/expressions/negation.h b/src/silo/query_engine/filter/expressions/negation.h index bd24d1aa1..dd39f1393 100644 --- a/src/silo/query_engine/filter/expressions/negation.h +++ b/src/silo/query_engine/filter/expressions/negation.h @@ -5,7 +5,6 @@ #include -#include "silo/database.h" #include "silo/query_engine/filter/expressions/expression.h" #include "silo/query_engine/filter/operators/operator.h" #include "silo/storage/table_partition.h" diff --git a/src/silo/query_engine/filter/expressions/phylo_child_filter.h b/src/silo/query_engine/filter/expressions/phylo_child_filter.h index 1c2f25218..7d72169ea 100644 --- a/src/silo/query_engine/filter/expressions/phylo_child_filter.h +++ b/src/silo/query_engine/filter/expressions/phylo_child_filter.h @@ -5,7 +5,6 @@ #include -#include "silo/database.h" #include "silo/query_engine/filter/expressions/expression.h" #include "silo/query_engine/filter/operators/operator.h" #include "silo/storage/table_partition.h" diff --git a/src/silo/query_engine/filter/expressions/string_equals.h b/src/silo/query_engine/filter/expressions/string_equals.h index 4c4feab67..496967150 100644 --- a/src/silo/query_engine/filter/expressions/string_equals.h +++ b/src/silo/query_engine/filter/expressions/string_equals.h @@ -5,7 +5,6 @@ #include -#include "silo/database.h" #include "silo/query_engine/filter/expressions/expression.h" #include "silo/query_engine/filter/operators/operator.h" #include "silo/storage/table_partition.h" diff --git a/src/silo/query_engine/filter/expressions/string_search.h b/src/silo/query_engine/filter/expressions/string_search.h index ceeda9aad..0018b965b 100644 --- a/src/silo/query_engine/filter/expressions/string_search.h +++ b/src/silo/query_engine/filter/expressions/string_search.h @@ -6,7 +6,6 @@ #include #include -#include "silo/database.h" #include "silo/query_engine/filter/expressions/expression.h" #include "silo/query_engine/filter/operators/operator.h" #include "silo/storage/table_partition.h" diff --git a/src/silo/query_engine/filter/operators/bitmap_producer.h b/src/silo/query_engine/filter/operators/bitmap_producer.h index 61f2fc1af..fee4efb0f 100644 --- a/src/silo/query_engine/filter/operators/bitmap_producer.h +++ b/src/silo/query_engine/filter/operators/bitmap_producer.h @@ -20,11 +20,11 @@ class BitmapProducer : public Operator { ~BitmapProducer() noexcept override; - [[nodiscard]] virtual Type type() const override; + [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& bitmap_producer); }; diff --git a/src/silo/query_engine/filter/operators/complement.h b/src/silo/query_engine/filter/operators/complement.h index 3cfa521a7..62498b2ce 100644 --- a/src/silo/query_engine/filter/operators/complement.h +++ b/src/silo/query_engine/filter/operators/complement.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/filter/operators/operator.h" @@ -23,11 +22,11 @@ class Complement : public Operator { ~Complement() noexcept override; - [[nodiscard]] virtual Type type() const override; + [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& complement); }; diff --git a/src/silo/query_engine/filter/operators/empty.h b/src/silo/query_engine/filter/operators/empty.h index b54b59cd7..960944b34 100644 --- a/src/silo/query_engine/filter/operators/empty.h +++ b/src/silo/query_engine/filter/operators/empty.h @@ -20,9 +20,9 @@ class Empty : public Operator { [[nodiscard]] Type type() const override; - CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& empty); }; diff --git a/src/silo/query_engine/filter/operators/full.h b/src/silo/query_engine/filter/operators/full.h index f8a4d0d70..594e86076 100644 --- a/src/silo/query_engine/filter/operators/full.h +++ b/src/silo/query_engine/filter/operators/full.h @@ -19,9 +19,9 @@ class Full : public Operator { [[nodiscard]] Type type() const override; - CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& full_operator); }; diff --git a/src/silo/query_engine/filter/operators/index_scan.h b/src/silo/query_engine/filter/operators/index_scan.h index 44ba451fc..22e355ea7 100644 --- a/src/silo/query_engine/filter/operators/index_scan.h +++ b/src/silo/query_engine/filter/operators/index_scan.h @@ -36,11 +36,11 @@ class IndexScan : public Operator { ~IndexScan() noexcept override; - [[nodiscard]] virtual Type type() const override; + [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& index_scan); }; diff --git a/src/silo/query_engine/filter/operators/intersection.h b/src/silo/query_engine/filter/operators/intersection.h index 7c3fc1cbd..8a6a97654 100644 --- a/src/silo/query_engine/filter/operators/intersection.h +++ b/src/silo/query_engine/filter/operators/intersection.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/filter/operators/operator.h" @@ -33,11 +32,11 @@ class Intersection : public Operator { ~Intersection() noexcept override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; static std::unique_ptr negate(std::unique_ptr&& intersection); }; diff --git a/src/silo/query_engine/filter/operators/is_in_covered_region.cpp b/src/silo/query_engine/filter/operators/is_in_covered_region.cpp index eb4a61240..36065a138 100644 --- a/src/silo/query_engine/filter/operators/is_in_covered_region.cpp +++ b/src/silo/query_engine/filter/operators/is_in_covered_region.cpp @@ -1,7 +1,6 @@ #include "silo/query_engine/filter/operators/is_in_covered_region.h" #include -#include #include #include diff --git a/src/silo/query_engine/filter/operators/selection.test.cpp b/src/silo/query_engine/filter/operators/selection.test.cpp index ad390b653..1f831949a 100644 --- a/src/silo/query_engine/filter/operators/selection.test.cpp +++ b/src/silo/query_engine/filter/operators/selection.test.cpp @@ -14,7 +14,7 @@ using silo::storage::column::IntColumnPartition; namespace { std::pair, IntColumnPartition> makeTestColumn( - const std::vector values + const std::vector& values ) { auto metadata = std::make_shared("test"); IntColumnPartition test_column{metadata.get()}; diff --git a/src/silo/query_engine/filter/operators/threshold.h b/src/silo/query_engine/filter/operators/threshold.h index 82f1663e4..ad0eae6f6 100644 --- a/src/silo/query_engine/filter/operators/threshold.h +++ b/src/silo/query_engine/filter/operators/threshold.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/filter/operators/operator.h" @@ -29,11 +28,11 @@ class Threshold : public Operator { ~Threshold() noexcept override; - [[nodiscard]] virtual Type type() const override; + [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; static std::unique_ptr negate(std::unique_ptr&& threshold); }; diff --git a/src/silo/query_engine/filter/operators/threshold.test.cpp b/src/silo/query_engine/filter/operators/threshold.test.cpp index c810a29ec..7da8bf051 100644 --- a/src/silo/query_engine/filter/operators/threshold.test.cpp +++ b/src/silo/query_engine/filter/operators/threshold.test.cpp @@ -8,7 +8,6 @@ using silo::query_engine::CopyOnWriteBitmap; using silo::query_engine::filter::operators::IndexScan; -using silo::query_engine::filter::operators::Operator; using silo::query_engine::filter::operators::Threshold; using silo::query_engine::filter::operators::OperatorVector; diff --git a/src/silo/query_engine/filter/operators/union.h b/src/silo/query_engine/filter/operators/union.h index 8bd70a566..0abcb179c 100644 --- a/src/silo/query_engine/filter/operators/union.h +++ b/src/silo/query_engine/filter/operators/union.h @@ -3,7 +3,6 @@ #include #include #include -#include #include "silo/query_engine/copy_on_write_bitmap.h" #include "silo/query_engine/filter/operators/operator.h" @@ -28,11 +27,11 @@ class Union : public Operator { ~Union() noexcept override; - virtual std::string toString() const override; + [[nodiscard]] std::string toString() const override; [[nodiscard]] Type type() const override; - virtual CopyOnWriteBitmap evaluate() const override; + [[nodiscard]] CopyOnWriteBitmap evaluate() const override; static std::unique_ptr negate(std::unique_ptr&& union_operator); }; diff --git a/src/silo/query_engine/query.cpp b/src/silo/query_engine/query.cpp index 724268e3e..2b4545ba5 100644 --- a/src/silo/query_engine/query.cpp +++ b/src/silo/query_engine/query.cpp @@ -32,7 +32,7 @@ std::shared_ptr Query::parseQuery(const std::string& query_string) { } QueryPlan Query::toQueryPlan( - std::shared_ptr database, + const std::shared_ptr& database, const config::QueryOptions& query_options, std::string_view request_id ) const { diff --git a/src/silo/query_engine/query.h b/src/silo/query_engine/query.h index 1766db6fa..65f2151e6 100644 --- a/src/silo/query_engine/query.h +++ b/src/silo/query_engine/query.h @@ -23,7 +23,7 @@ struct Query { static std::shared_ptr parseQuery(const std::string& query_string); [[nodiscard]] QueryPlan toQueryPlan( - std::shared_ptr database, + const std::shared_ptr& database, const config::QueryOptions& query_options, std::string_view request_id ) const; diff --git a/src/silo/query_engine/query_parse_sequence_name.h b/src/silo/query_engine/query_parse_sequence_name.h index 7139556a5..879432660 100644 --- a/src/silo/query_engine/query_parse_sequence_name.h +++ b/src/silo/query_engine/query_parse_sequence_name.h @@ -3,7 +3,7 @@ #include #include -#include "silo/database.h" +#include "silo/schema/database_schema.h" namespace silo { diff --git a/src/silo/query_engine/query_plan.cpp b/src/silo/query_engine/query_plan.cpp index c541fd214..c2d8365d9 100644 --- a/src/silo/query_engine/query_plan.cpp +++ b/src/silo/query_engine/query_plan.cpp @@ -16,11 +16,14 @@ arrow::Result QueryPlan::makeQueryPlan( arrow::acero::ExecNode* root, std::string_view request_id ) { - QueryPlan query_plan{arrow_plan, request_id}; + arrow::acero::BackpressureMonitor* backpressure_monitor; + arrow::AsyncGenerator> results_generator; ARROW_ASSIGN_OR_RAISE( - query_plan.backpressure_monitor, - exec_node::createGenerator(arrow_plan.get(), root, &query_plan.results_generator) + backpressure_monitor, exec_node::createGenerator(arrow_plan.get(), root, &results_generator) ); + QueryPlan query_plan{ + std::move(arrow_plan), std::move(results_generator), backpressure_monitor, request_id + }; query_plan.results_schema = root->output_schema(); ARROW_RETURN_NOT_OK(query_plan.arrow_plan->Validate()); return query_plan; @@ -77,7 +80,7 @@ arrow::Status QueryPlan::executeAndWriteImpl( ); } } - } guard{request_id, arrow_plan}; + } guard{.request_id = request_id, .plan = arrow_plan}; while (true) { arrow::Future> future_batch = results_generator(); diff --git a/src/silo/query_engine/query_plan.h b/src/silo/query_engine/query_plan.h index b071eb4b8..e55a365b8 100644 --- a/src/silo/query_engine/query_plan.h +++ b/src/silo/query_engine/query_plan.h @@ -1,15 +1,13 @@ #pragma once -#include #include #include +#include #include #include #include -#include "silo/common/panic.h" - namespace silo::query_engine { class QueryPlan { @@ -30,8 +28,15 @@ class QueryPlan { void executeAndWrite(std::ostream* output_stream, uint64_t timeout_in_seconds); private: - QueryPlan(std::shared_ptr arrow_plan, std::string_view request_id) - : arrow_plan(arrow_plan), + QueryPlan( + std::shared_ptr arrow_plan, + arrow::AsyncGenerator> results_generator, + arrow::acero::BackpressureMonitor* backpressure_monitor, + std::string_view request_id + ) + : arrow_plan(std::move(arrow_plan)), + results_generator(std::move(results_generator)), + backpressure_monitor(backpressure_monitor), request_id(request_id) {} arrow::Status executeAndWriteImpl(std::ostream* output_stream, uint64_t timeout_in_seconds); diff --git a/src/silo/query_engine/query_plan.test.cpp b/src/silo/query_engine/query_plan.test.cpp index 8e3aa1e44..cbe0e0bbd 100644 --- a/src/silo/query_engine/query_plan.test.cpp +++ b/src/silo/query_engine/query_plan.test.cpp @@ -8,10 +8,10 @@ #include "arrow/acero/options.h" #include "arrow/builder.h" -using arrow::acero::ExecNode; -using arrow::acero::ExecPlan; using silo::query_engine::QueryPlan; +namespace { + arrow::Result> setupTestTable() { std::shared_ptr schema = arrow::schema({arrow::field("id", arrow::int32())}); @@ -22,6 +22,7 @@ arrow::Result> setupTestTable() { ARROW_ASSIGN_OR_RAISE(std::shared_ptr id_array, id_builder.Finish()); return arrow::Table::Make(schema, {id_array}); } +} // namespace TEST(QueryPlan, timesOutWhenAnInvalidPlanDoesNotFinish) { EXPECT_THAT( diff --git a/src/silo/roaring_util/bitmap_builder.h b/src/silo/roaring_util/bitmap_builder.h index f9a344ed9..398e95156 100644 --- a/src/silo/roaring_util/bitmap_builder.h +++ b/src/silo/roaring_util/bitmap_builder.h @@ -2,8 +2,6 @@ #include -#include "silo/common/panic.h" - namespace silo::roaring_util { class BitmapBuilderByContainer { diff --git a/src/silo/roaring_util/bitmap_builder.test.cpp b/src/silo/roaring_util/bitmap_builder.test.cpp index 168dad344..0f0c0e5d1 100644 --- a/src/silo/roaring_util/bitmap_builder.test.cpp +++ b/src/silo/roaring_util/bitmap_builder.test.cpp @@ -93,7 +93,7 @@ TEST_F(BitmapBuilderByContainerTest, MultipleContainersDifferentTiles) { CONTAINER_SIZE + 10, CONTAINER_SIZE + 20, CONTAINER_SIZE + 30, - 2 * CONTAINER_SIZE + 100 + (2 * CONTAINER_SIZE) + 100 }; EXPECT_EQ(result, expected); @@ -166,9 +166,9 @@ TEST_F(BitmapBuilderByContainerTest, AscendingOrder) { roaring::Roaring expected = { 0, CONTAINER_SIZE + 10, - 2 * CONTAINER_SIZE + 20, - 3 * CONTAINER_SIZE + 30, - 4 * CONTAINER_SIZE + 40 + (2 * CONTAINER_SIZE) + 20, + (3 * CONTAINER_SIZE) + 30, + (4 * CONTAINER_SIZE) + 40 }; EXPECT_EQ(result, expected); } @@ -177,6 +177,7 @@ TEST_F(BitmapBuilderByContainerTest, LargeContainer) { BitmapBuilderByContainer builder; std::vector values; + values.reserve(CONTAINER_SIZE); for (uint32_t i = 0; i < CONTAINER_SIZE; ++i) { values.push_back(i); } @@ -198,24 +199,24 @@ TEST_F(BitmapBuilderByContainerTest, MixedOperations) { BitmapBuilderByContainer builder; // Tile 0, first batch - auto [c1, t1] = createContainer({1, 2, 3}); - builder.addContainer(0, c1, t1); + auto [container_1, typecode_1] = createContainer({1, 2, 3}); + builder.addContainer(0, container_1, typecode_1); // Tile 0, second batch (merge) - auto [c2, t2] = createContainer({4, 5, 6}); - builder.addContainer(0, c2, t2); + auto [container_2, typecode_2] = createContainer({4, 5, 6}); + builder.addContainer(0, container_2, typecode_2); // Tile 0, third batch (merge) - auto [c4, t4] = createContainer({7, 8}); - builder.addContainer(0, c4, t4); + auto [container_4, typecode_4] = createContainer({7, 8}); + builder.addContainer(0, container_4, typecode_4); // Tile 1 - auto [c3, t3] = createContainer({10, 20}); - builder.addContainer(1, c3, t3); + auto [container_3, typecode_3] = createContainer({10, 20}); + builder.addContainer(1, container_3, typecode_3); // Tile 2 - auto [c5, t5] = createContainer({100}); - builder.addContainer(2, c5, t5); + auto [container_5, typecode_5] = createContainer({100}); + builder.addContainer(2, container_5, typecode_5); roaring::Roaring result = std::move(builder).getBitmap(); @@ -225,13 +226,13 @@ TEST_F(BitmapBuilderByContainerTest, MixedOperations) { EXPECT_TRUE(result.contains(8)); EXPECT_TRUE(result.contains(CONTAINER_SIZE + 10)); EXPECT_TRUE(result.contains(CONTAINER_SIZE + 20)); - EXPECT_TRUE(result.contains(2 * CONTAINER_SIZE + 100)); + EXPECT_TRUE(result.contains((2 * CONTAINER_SIZE) + 100)); - roaring::internal::container_free(c1, t1); - roaring::internal::container_free(c2, t2); - roaring::internal::container_free(c3, t3); - roaring::internal::container_free(c4, t4); - roaring::internal::container_free(c5, t5); + roaring::internal::container_free(container_1, typecode_1); + roaring::internal::container_free(container_2, typecode_2); + roaring::internal::container_free(container_3, typecode_3); + roaring::internal::container_free(container_4, typecode_4); + roaring::internal::container_free(container_5, typecode_5); } TEST_F(BitmapBuilderByContainerTest, SingleTileMultipleAdditions) { @@ -240,8 +241,9 @@ TEST_F(BitmapBuilderByContainerTest, SingleTileMultipleAdditions) { // Add multiple batches to tile 5 for (int batch = 0; batch < 10; ++batch) { std::vector values; + values.reserve(10); for (int i = 0; i < 10; ++i) { - values.push_back(batch * 10 + i); + values.push_back((batch * 10) + i); } auto [container, typecode] = createContainer(values); builder.addContainer(5, container, typecode); diff --git a/src/silo/roaring_util/subset_ranks.test.cpp b/src/silo/roaring_util/subset_ranks.test.cpp index 78a69d76a..e5823d90b 100644 --- a/src/silo/roaring_util/subset_ranks.test.cpp +++ b/src/silo/roaring_util/subset_ranks.test.cpp @@ -10,18 +10,19 @@ namespace { class RoaringSubsetRanksTest : public ::testing::Test { protected: // Helper to create a container from a vector of values - std::pair createContainer( + static std::pair createContainer( const std::vector& values ) { assert(not values.empty()); - roaring::Roaring r; + roaring::Roaring bitmap; for (uint16_t val : values) { - r.add(val); + bitmap.add(val); } - auto cloned_container = roaring::internal::container_clone( - r.roaring.high_low_container.containers[0], r.roaring.high_low_container.typecodes[0] + auto* cloned_container = roaring::internal::container_clone( + bitmap.roaring.high_low_container.containers[0], + bitmap.roaring.high_low_container.typecodes[0] ); - return {cloned_container, r.roaring.high_low_container.typecodes[0]}; + return {cloned_container, bitmap.roaring.high_low_container.typecodes[0]}; } }; diff --git a/src/silo/schema/database_schema.cpp b/src/silo/schema/database_schema.cpp index 944ae583a..462727211 100644 --- a/src/silo/schema/database_schema.cpp +++ b/src/silo/schema/database_schema.cpp @@ -17,10 +17,6 @@ #include "silo/storage/column/column_metadata.h" #include "silo/storage/column/column_type_visitor.h" -#include "silo/storage/column/indexed_string_column.h" -#include "silo/storage/column/sequence_column.h" -#include "silo/storage/column/string_column.h" -#include "silo/storage/column/zstd_compressed_string_column.h" namespace silo::schema { @@ -30,17 +26,18 @@ bool isSequenceColumn(ColumnType type) { } std::optional TableSchema::getColumn(std::string_view name) const { - auto it = std::ranges::find_if(column_metadata, [&name](const auto& metadata_pair) { + auto iter = std::ranges::find_if(column_metadata, [&name](const auto& metadata_pair) { return metadata_pair.first.name == name; }); - if (it == column_metadata.end()) { + if (iter == column_metadata.end()) { return std::nullopt; } - return it->first; + return iter->first; } std::vector TableSchema::getColumnIdentifiers() const { std::vector result; + result.reserve(column_metadata.size()); for (const auto& [column_identifier, _] : column_metadata) { result.push_back(column_identifier); } @@ -60,7 +57,10 @@ std::optional TableSchema::getDefaultSequenceName() class ColumnMetadataSaverByType { public: template - void operator()(Archive& archive, std::shared_ptr metadata) { + void operator()( + Archive& archive, + const std::shared_ptr& metadata + ) { auto typed_metadata = dynamic_cast(metadata.get()); SILO_ASSERT(typed_metadata != nullptr); archive << *typed_metadata; @@ -78,12 +78,13 @@ class ColumnMetadataLoaderByType { }; template -void TableSchema::save(Archive& archive, const unsigned int version) const { +void TableSchema::save(Archive& archive, const unsigned int /*version*/) const { archive & default_nucleotide_sequence; archive & default_aa_sequence; archive & primary_key; std::vector column_identifiers; + column_identifiers.reserve(column_metadata.size()); for (const auto& [column_identifier, _] : column_metadata) { column_identifiers.push_back(column_identifier); } @@ -97,7 +98,7 @@ void TableSchema::save(Archive& archive, const unsigned int version) const { } template -void TableSchema::load(Archive& archive, const unsigned int version) { +void TableSchema::load(Archive& archive, const unsigned int /*version*/) { archive & default_nucleotide_sequence; archive & default_aa_sequence; archive & primary_key; @@ -114,16 +115,20 @@ void TableSchema::load(Archive& archive, const unsigned int version) { } TableName::TableName(std::string_view name) { - for (char c : name) { - if (c < 'a' || c > 'z') { + for (char character : name) { + if (character < 'a' || character > 'z') { throw std::runtime_error("Table names may only contain lower-case letters"); } } this->name = name; } +namespace { + TableName default_table_name{"default"}; +} + const TableName& TableName::getDefault() { return default_table_name; } @@ -136,7 +141,7 @@ const TableSchema& DatabaseSchema::getDefaultTableSchema() const { namespace silo::schema { -void DatabaseSchema::saveToFile(const std::filesystem::path& file_path) { +void DatabaseSchema::saveToFile(const std::filesystem::path& file_path) const { std::ofstream database_schema_file{file_path, std::ios::binary}; boost::archive::binary_oarchive output_archive(database_schema_file); output_archive << tables; diff --git a/src/silo/schema/database_schema.h b/src/silo/schema/database_schema.h index 51db6577f..b0721740b 100644 --- a/src/silo/schema/database_schema.h +++ b/src/silo/schema/database_schema.h @@ -182,7 +182,7 @@ class DatabaseSchema { [[nodiscard]] const TableSchema& getDefaultTableSchema() const; static DatabaseSchema loadFromFile(const std::filesystem::path& file_path); - void saveToFile(const std::filesystem::path& file_path); + void saveToFile(const std::filesystem::path& file_path) const; }; } // namespace silo::schema diff --git a/src/silo/storage/buffer/page.h b/src/silo/storage/buffer/page.h index d94242680..a5a5285a7 100644 --- a/src/silo/storage/buffer/page.h +++ b/src/silo/storage/buffer/page.h @@ -1,7 +1,6 @@ #pragma once -#include -#include +#include #include diff --git a/src/silo/storage/column/bool_column.cpp b/src/silo/storage/column/bool_column.cpp index 83592d34d..f5c1a0d69 100644 --- a/src/silo/storage/column/bool_column.cpp +++ b/src/silo/storage/column/bool_column.cpp @@ -10,7 +10,7 @@ BoolColumnPartition::BoolColumnPartition(ColumnMetadata* metadata) : metadata(metadata) {} void BoolColumnPartition::insert(bool value) { - if (value == true) { + if (value) { true_bitmap.add(num_values++); } else { false_bitmap.add(num_values++); diff --git a/src/silo/storage/column/bool_column.h b/src/silo/storage/column/bool_column.h index 1f47e1c89..b000ed54e 100644 --- a/src/silo/storage/column/bool_column.h +++ b/src/silo/storage/column/bool_column.h @@ -1,9 +1,6 @@ #pragma once #include -#include -#include -#include #include #include @@ -19,7 +16,6 @@ class BoolColumnPartition { static constexpr schema::ColumnType TYPE = schema::ColumnType::BOOL; using value_type = bool; - public: roaring::Roaring true_bitmap; roaring::Roaring false_bitmap; roaring::Roaring null_bitmap; @@ -32,14 +28,11 @@ class BoolColumnPartition { public: explicit BoolColumnPartition(Metadata* metadata); - size_t numValues() const { return num_values; } + [[nodiscard]] size_t numValues() const { return num_values; } [[nodiscard]] bool getValue(size_t row_id) const { SILO_ASSERT(!null_bitmap.contains(row_id)); - if (true_bitmap.contains(row_id)) { - return true; - } - return false; + return true_bitmap.contains(row_id); } [[nodiscard]] bool isNull(size_t row_id) const { return null_bitmap.contains(row_id); } diff --git a/src/silo/storage/column/column.h b/src/silo/storage/column/column.h index 755eeaed9..10abedb21 100644 --- a/src/silo/storage/column/column.h +++ b/src/silo/storage/column/column.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -10,7 +11,7 @@ enum class ColumnType : uint8_t; namespace silo::storage::column { template -concept Column = requires(T t) { +concept Column = requires(T column) { typename T::Metadata; // ensure it is actually a type requires std::is_class_v || @@ -18,7 +19,7 @@ concept Column = requires(T t) { requires std::is_constructible_v; - { t.numValues() } -> std::convertible_to; + { column.numValues() } -> std::convertible_to; { T::TYPE } -> std::convertible_to; }; diff --git a/src/silo/storage/column/column_metadata.h b/src/silo/storage/column/column_metadata.h index d6763ed13..f9def6239 100644 --- a/src/silo/storage/column/column_metadata.h +++ b/src/silo/storage/column/column_metadata.h @@ -1,6 +1,9 @@ #pragma once +#include +#include #include +#include #include #include @@ -12,7 +15,7 @@ class ColumnMetadata { std::string column_name; explicit ColumnMetadata(std::string column_name) - : column_name(column_name) {} + : column_name(std::move(column_name)) {} virtual ~ColumnMetadata() = default; }; @@ -23,23 +26,23 @@ BOOST_SERIALIZATION_SPLIT_FREE(silo::storage::column::ColumnMetadata); namespace boost::serialization { template [[maybe_unused]] void save( - Archive& ar, + Archive& archive, const silo::storage::column::ColumnMetadata& object, [[maybe_unused]] const uint32_t version ) { - ar & object.column_name; + archive & object.column_name; } } // namespace boost::serialization BOOST_SERIALIZATION_SPLIT_FREE(std::shared_ptr); namespace boost::serialization { template [[maybe_unused]] void load( - Archive& ar, + Archive& archive, std::shared_ptr& object, [[maybe_unused]] const uint32_t version ) { std::string column_name; - ar & column_name; + archive & column_name; object = std::make_shared(std::move(column_name)); } } // namespace boost::serialization diff --git a/src/silo/storage/column/column_type_visitor.h b/src/silo/storage/column/column_type_visitor.h index 8191fbfb4..8d9be7f65 100644 --- a/src/silo/storage/column/column_type_visitor.h +++ b/src/silo/storage/column/column_type_visitor.h @@ -1,8 +1,5 @@ #pragma once -#include -#include - #include "silo/common/panic.h" #include "silo/schema/database_schema.h" #include "silo/storage/column/bool_column.h" diff --git a/src/silo/storage/column/date_column.h b/src/silo/storage/column/date_column.h index fc75cf663..e60e3ee92 100644 --- a/src/silo/storage/column/date_column.h +++ b/src/silo/storage/column/date_column.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -47,7 +46,7 @@ class DateColumnPartition { [[nodiscard]] const std::vector& getValues() const; - size_t numValues() const { return values.size(); } + [[nodiscard]] size_t numValues() const { return values.size(); } [[nodiscard]] bool isNull(size_t row_id) const { return values.at(row_id) == common::NULL_DATE; } [[nodiscard]] silo::common::Date getValue(size_t row_id) const { return values.at(row_id); } diff --git a/src/silo/storage/column/float_column.h b/src/silo/storage/column/float_column.h index c4d79d820..ac29b8cc6 100644 --- a/src/silo/storage/column/float_column.h +++ b/src/silo/storage/column/float_column.h @@ -2,8 +2,6 @@ #include #include -#include -#include #include #include @@ -31,11 +29,11 @@ class FloatColumnPartition { explicit FloatColumnPartition(ColumnMetadata* metadata); - size_t numValues() const { return values.size(); } + [[nodiscard]] size_t numValues() const { return values.size(); } [[nodiscard]] bool isNull(size_t row_id) const { return null_bitmap.contains(row_id); } - double getValue(size_t row_id) const { return values.at(row_id); } + [[nodiscard]] double getValue(size_t row_id) const { return values.at(row_id); } void insert(double value); diff --git a/src/silo/storage/column/float_column.test.cpp b/src/silo/storage/column/float_column.test.cpp index 485030f12..d05d99380 100644 --- a/src/silo/storage/column/float_column.test.cpp +++ b/src/silo/storage/column/float_column.test.cpp @@ -3,8 +3,6 @@ #include #include -#include "silo/preprocessing/preprocessing_exception.h" - using silo::storage::column::ColumnMetadata; using silo::storage::column::FloatColumnPartition; diff --git a/src/silo/storage/column/horizontal_coverage_index.cpp b/src/silo/storage/column/horizontal_coverage_index.cpp index 39e22e59e..454c0827e 100644 --- a/src/silo/storage/column/horizontal_coverage_index.cpp +++ b/src/silo/storage/column/horizontal_coverage_index.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include #include @@ -93,11 +92,11 @@ void HorizontalCoverageIndex::insertSequenceCoverage(std::string sequence, uint3 start_of_covered_region, end_of_covered_region_exclusive, positions_with_symbol_missing ); } -template void HorizontalCoverageIndex::insertSequenceCoverage( +template void HorizontalCoverageIndex::insertSequenceCoverage( std::string sequence, uint32_t offset ); -template void HorizontalCoverageIndex::insertSequenceCoverage( +template void HorizontalCoverageIndex::insertSequenceCoverage( std::string sequence, uint32_t offset ); diff --git a/src/silo/storage/column/horizontal_coverage_index.h b/src/silo/storage/column/horizontal_coverage_index.h index 509529081..4fc446b3e 100644 --- a/src/silo/storage/column/horizontal_coverage_index.h +++ b/src/silo/storage/column/horizontal_coverage_index.h @@ -7,6 +7,7 @@ #include #include +#include "silo/common/panic.h" #include "silo/roaring_util/bitmap_builder.h" namespace silo::storage::column { @@ -32,17 +33,17 @@ class HorizontalCoverageIndex { template void insertSequenceCoverage(std::string sequence, uint32_t offset); - template - [[nodiscard]] std::array getCoverageBitmapForPositions( + template + [[nodiscard]] std::array getCoverageBitmapForPositions( uint32_t position ) const { size_t row_count = start_end.size(); uint32_t range_start = position; - uint32_t range_end = position + BATCH_SIZE; + uint32_t range_end = position + BatchSize; using silo::roaring_util::BitmapBuilderByRange; - std::array result_builders; + std::array result_builders; for (uint32_t row_id_upper_bits = 0; row_id_upper_bits << 16 < row_count; ++row_id_upper_bits) { @@ -65,7 +66,7 @@ class HorizontalCoverageIndex { } } - std::array result; + std::array result; std::ranges::transform(result_builders, result.begin(), [](BitmapBuilderByRange& builder) { return std::move(builder).getBitmap(); }); diff --git a/src/silo/storage/column/horizontal_coverage_index.test.cpp b/src/silo/storage/column/horizontal_coverage_index.test.cpp index 95ccbdca4..2b797fcf0 100644 --- a/src/silo/storage/column/horizontal_coverage_index.test.cpp +++ b/src/silo/storage/column/horizontal_coverage_index.test.cpp @@ -53,7 +53,7 @@ TEST_F(HorizontalCoverageIndexTest, InsertSequenceWithOffset) { } TEST_F(HorizontalCoverageIndexTest, InsertEmptySequence) { - std::string sequence = ""; + std::string sequence; EXPECT_NO_THROW(index->insertSequenceCoverage(sequence, 0)); } @@ -391,7 +391,7 @@ TEST_F(HorizontalCoverageIndexTest, LongSequence) { EXPECT_EQ(index->getCoverageBitmapForPositions<1>(0).at(0), roaring::Roaring{0}); EXPECT_EQ(index->getCoverageBitmapForPositions<1>(GENOME_LENGTH / 4).at(0), roaring::Roaring{0}); EXPECT_EQ( - index->getCoverageBitmapForPositions<1>(GENOME_LENGTH / 2 - 1).at(0), roaring::Roaring{0} + index->getCoverageBitmapForPositions<1>((GENOME_LENGTH / 2) - 1).at(0), roaring::Roaring{0} ); // Verify uncovered diff --git a/src/silo/storage/column/indexed_string_column.h b/src/silo/storage/column/indexed_string_column.h index 96af97167..73b2d77d8 100644 --- a/src/silo/storage/column/indexed_string_column.h +++ b/src/silo/storage/column/indexed_string_column.h @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -26,14 +27,14 @@ class IndexedStringColumnMetadata : public ColumnMetadata { common::BidirectionalStringMap dictionary; std::optional lineage_tree; - IndexedStringColumnMetadata(std::string column_name) - : ColumnMetadata(column_name) {} + explicit IndexedStringColumnMetadata(std::string column_name) + : ColumnMetadata(std::move(column_name)) {} IndexedStringColumnMetadata( std::string column_name, silo::common::BidirectionalStringMap dictionary ) - : ColumnMetadata(column_name), + : ColumnMetadata(std::move(column_name)), dictionary(std::move(dictionary)) {} IndexedStringColumnMetadata( @@ -81,7 +82,6 @@ class IndexedStringColumnPartition { public: explicit IndexedStringColumnPartition(Metadata* metadata); - public: [[nodiscard]] std::optional filter(silo::Idx value_id) const; std::optional filter(const std::optional& value) const; @@ -103,8 +103,8 @@ class IndexedStringColumnPartition { return std::string{lookupValue(getValue(row_id))}; } - [[nodiscard]] inline std::string_view lookupValue(Idx id) const { - return metadata->dictionary.getValue(id); + [[nodiscard]] std::string_view lookupValue(Idx dict_id) const { + return metadata->dictionary.getValue(dict_id); } [[nodiscard]] std::optional getValueId(const std::string& value) const; @@ -118,13 +118,13 @@ BOOST_SERIALIZATION_SPLIT_FREE(silo::storage::column::IndexedStringColumnMetadat namespace boost::serialization { template [[maybe_unused]] void save( - Archive& ar, + Archive& archive, const silo::storage::column::IndexedStringColumnMetadata& object, [[maybe_unused]] const uint32_t version ) { - ar & object.column_name; - ar & object.dictionary; - ar & object.lineage_tree; + archive & object.column_name; + archive & object.dictionary; + archive & object.lineage_tree; } } // namespace boost::serialization @@ -132,16 +132,16 @@ BOOST_SERIALIZATION_SPLIT_FREE(std::shared_ptr [[maybe_unused]] void load( - Archive& ar, + Archive& archive, std::shared_ptr& object, [[maybe_unused]] const uint32_t version ) { std::string column_name; silo::common::BidirectionalStringMap dictionary; std::optional lineage_tree; - ar & column_name; - ar & dictionary; - ar & lineage_tree; + archive & column_name; + archive & dictionary; + archive & lineage_tree; if (lineage_tree.has_value()) { object = std::make_shared( std::move(column_name), std::move(dictionary), std::move(lineage_tree.value()) diff --git a/src/silo/storage/column/insertion_index.cpp b/src/silo/storage/column/insertion_index.cpp index 10ac66454..e257c788b 100644 --- a/src/silo/storage/column/insertion_index.cpp +++ b/src/silo/storage/column/insertion_index.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include diff --git a/src/silo/storage/column/insertion_index.h b/src/silo/storage/column/insertion_index.h index 78f3d7993..4d4ed144a 100644 --- a/src/silo/storage/column/insertion_index.h +++ b/src/silo/storage/column/insertion_index.h @@ -12,9 +12,6 @@ #include #include -#include "silo/common/aa_symbols.h" -#include "silo/common/nucleotide_symbols.h" - namespace silo::storage::insertion { template @@ -70,11 +67,13 @@ class InsertionPosition { const std::regex& search_pattern ) const; - std::unique_ptr searchWithRegex(const std::regex& regex_search_pattern) const; + [[nodiscard]] std::unique_ptr searchWithRegex( + const std::regex& regex_search_pattern + ) const; void buildThreeMerIndex(); - std::unique_ptr search(const std::string& search_pattern) const; + [[nodiscard]] std::unique_ptr search(const std::string& search_pattern) const; }; template diff --git a/src/silo/storage/column/int_column.cpp b/src/silo/storage/column/int_column.cpp index 999b952dd..43050c3e7 100644 --- a/src/silo/storage/column/int_column.cpp +++ b/src/silo/storage/column/int_column.cpp @@ -1,11 +1,7 @@ #include "silo/storage/column/int_column.h" -#include - #include -#include "silo/preprocessing/preprocessing_exception.h" - namespace silo::storage::column { IntColumnPartition::IntColumnPartition(ColumnMetadata* metadata) diff --git a/src/silo/storage/column/int_column.h b/src/silo/storage/column/int_column.h index 8419b1bfa..235e7b26b 100644 --- a/src/silo/storage/column/int_column.h +++ b/src/silo/storage/column/int_column.h @@ -1,8 +1,6 @@ #pragma once #include -#include -#include #include #include @@ -37,7 +35,7 @@ class IntColumnPartition { return values.at(row_id); } - size_t numValues() const { return values.size(); } + [[nodiscard]] size_t numValues() const { return values.size(); } void insert(int32_t value); diff --git a/src/silo/storage/column/int_column.test.cpp b/src/silo/storage/column/int_column.test.cpp index fc95ce134..95e52d609 100644 --- a/src/silo/storage/column/int_column.test.cpp +++ b/src/silo/storage/column/int_column.test.cpp @@ -3,8 +3,6 @@ #include #include -#include "silo/preprocessing/preprocessing_exception.h" - using silo::storage::column::ColumnMetadata; using silo::storage::column::IntColumnPartition; diff --git a/src/silo/storage/column/lineage_index.h b/src/silo/storage/column/lineage_index.h index bec52d9db..d45c04943 100644 --- a/src/silo/storage/column/lineage_index.h +++ b/src/silo/storage/column/lineage_index.h @@ -1,13 +1,11 @@ #pragma once -#include #include #include #include #include -#include "silo/common/bidirectional_string_map.h" #include "silo/common/lineage_tree.h" #include "silo/common/types.h" diff --git a/src/silo/storage/column/lineage_index.test.cpp b/src/silo/storage/column/lineage_index.test.cpp index 48fbfae02..91e5cb881 100644 --- a/src/silo/storage/column/lineage_index.test.cpp +++ b/src/silo/storage/column/lineage_index.test.cpp @@ -3,14 +3,10 @@ #include #include -#include "silo/storage/column/indexed_string_column.h" - using silo::Idx; using silo::common::LineageTree; using silo::common::RecombinantEdgeFollowingMode; using silo::storage::LineageIndex; -using silo::storage::column::IndexedStringColumnMetadata; -using silo::storage::column::IndexedStringColumnPartition; /* v * 1 @@ -31,7 +27,6 @@ LineageTree createDoubleDiamondLineageTree() { 6, {{1, 2}, {2, 3}, {1, 0}, {0, 3}, {3, 4}, {0, 5}, {5, 4}}, {}, {} ); } -} // namespace void assertEqualHelper(std::optional& actual, roaring::Roaring& expected) { ASSERT_EQ(actual.has_value(), !expected.isEmpty()); @@ -39,6 +34,7 @@ void assertEqualHelper(std::optional& actual, roaring:: ASSERT_EQ(*actual.value(), expected); } } +} // namespace TEST(LineageIndex, someTreeValuesCorrectBehavior) { auto lineage_tree = createDoubleDiamondLineageTree(); diff --git a/src/silo/storage/column/sequence_column.h b/src/silo/storage/column/sequence_column.h index 5bdcd9d9a..b5c2b273f 100644 --- a/src/silo/storage/column/sequence_column.h +++ b/src/silo/storage/column/sequence_column.h @@ -146,7 +146,7 @@ class SequenceColumnPartition { template <> class [[maybe_unused]] fmt::formatter { public: - constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + static constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } [[maybe_unused]] static auto format( const silo::storage::column::SequenceColumnInfo& sequence_store_info, format_context& ctx diff --git a/src/silo/storage/column/string_column.test.cpp b/src/silo/storage/column/string_column.test.cpp index 802bc0f28..6acff0c92 100644 --- a/src/silo/storage/column/string_column.test.cpp +++ b/src/silo/storage/column/string_column.test.cpp @@ -168,7 +168,7 @@ TEST(StringColumn, manyMixedValues) { test_values.reserve(50001); for (size_t i = 0; i < 50001; ++i) { if (i % 2 == 1) { - test_values.push_back("SHRT"); + test_values.emplace_back("SHRT"); } else if (i % 10000 == 0) { test_values.push_back(fmt::format("{}_VERY_VERY_LONG_STRING_{}", i, std::string(i, 'x'))); } else { diff --git a/src/silo/storage/column/vertical_sequence_index.test.cpp b/src/silo/storage/column/vertical_sequence_index.test.cpp index 51f46651c..7c6db535c 100644 --- a/src/silo/storage/column/vertical_sequence_index.test.cpp +++ b/src/silo/storage/column/vertical_sequence_index.test.cpp @@ -34,8 +34,8 @@ TEST_F(VerticalSequenceIndexTest, AddAndRetrieveSinglePosition) { std::vector sequences(5, "N"); roaring::Roaring row_ids; - for (uint32_t i = 0; i < 5; i++) { - row_ids.add(i); + for (uint32_t row_id = 0; row_id < 5; row_id++) { + row_ids.add(row_id); } index.overwriteSymbolsInSequences(sequences, row_ids); @@ -69,8 +69,8 @@ TEST_F(VerticalSequenceIndexTest, AddMultiplePositions) { std::vector sequences(num_seqs, "NNN"); roaring::Roaring row_ids; - for (uint32_t i = 0; i < num_seqs; i++) { - row_ids.add(i); + for (uint32_t row_id = 0; row_id < num_seqs; row_id++) { + row_ids.add(row_id); } index.overwriteSymbolsInSequences(sequences, row_ids); @@ -125,8 +125,8 @@ TEST_F(VerticalSequenceIndexTest, EmptySymbolMap) { std::vector sequences(5, "C"); roaring::Roaring row_ids; - for (uint32_t i = 0; i < 5; i++) { - row_ids.add(i); + for (uint32_t row_id = 0; row_id < 5; row_id++) { + row_ids.add(row_id); } index.overwriteSymbolsInSequences(sequences, row_ids); @@ -172,8 +172,8 @@ TEST_F(VerticalSequenceIndexTest, LargeNumberOfSequences) { SymbolMap> ids_0; SymbolMap> ids_1; - for (uint32_t i = 0; i < num_seqs; i++) { - ids_0[Nucleotide::Symbol::A].push_back(i); + for (uint32_t row_id = 0; row_id < num_seqs; row_id++) { + ids_0[Nucleotide::Symbol::A].push_back(row_id); } ids_1[Nucleotide::Symbol::G].push_back(0); @@ -184,8 +184,8 @@ TEST_F(VerticalSequenceIndexTest, LargeNumberOfSequences) { std::vector sequences(num_seqs, "NN"); roaring::Roaring row_ids; - for (uint32_t i = 0; i < num_seqs; i++) { - row_ids.add(i); + for (uint32_t row_id = 0; row_id < num_seqs; row_id++) { + row_ids.add(row_id); } index.overwriteSymbolsInSequences(sequences, row_ids); @@ -223,9 +223,9 @@ TEST_F(VerticalSequenceIndexTest, NonContiguousPositions) { TEST_F(VerticalSequenceIndexTest, AllDifferentNucleotideSymbols) { SymbolMap> ids; - uint32_t i = 0; + uint32_t row_id = 0; for (const auto& symbol : Nucleotide::SYMBOLS) { - ids[symbol] = {i++}; + ids[symbol] = {row_id++}; } index.addSymbolsToPositions(0, ids); @@ -235,19 +235,19 @@ TEST_F(VerticalSequenceIndexTest, AllDifferentNucleotideSymbols) { index.overwriteSymbolsInSequences(sequences, row_ids); - i = 0; + row_id = 0; for (const auto& symbol : Nucleotide::SYMBOLS) { - EXPECT_EQ(sequences[i], fmt::format("{}", Nucleotide::symbolToChar(symbol))); - ++i; + EXPECT_EQ(sequences[row_id], fmt::format("{}", Nucleotide::symbolToChar(symbol))); + ++row_id; } } TEST(AminoAcidVerticalSequenceIndexTest, AllDifferentAminoAcidSymbols) { VerticalSequenceIndex index; SymbolMap> ids; - uint32_t i = 0; + uint32_t row_id = 0; for (const auto& symbol : AminoAcid::SYMBOLS) { - ids[symbol] = {i++}; + ids[symbol] = {row_id++}; } index.addSymbolsToPositions(0, ids); @@ -257,20 +257,20 @@ TEST(AminoAcidVerticalSequenceIndexTest, AllDifferentAminoAcidSymbols) { index.overwriteSymbolsInSequences(sequences, row_ids); - i = 0; + row_id = 0; for (const auto& symbol : AminoAcid::SYMBOLS) { - EXPECT_EQ(sequences[i], fmt::format("{}", AminoAcid::symbolToChar(symbol))); - ++i; + EXPECT_EQ(sequences[row_id], fmt::format("{}", AminoAcid::symbolToChar(symbol))); + ++row_id; } } TEST_F(VerticalSequenceIndexTest, SparseRowSelection) { SymbolMap> ids; - for (uint32_t i = 0; i < 100; i++) { - if (i % 2 == 0) { - ids[Nucleotide::Symbol::B].push_back(i); + for (uint32_t row_id = 0; row_id < 100; row_id++) { + if (row_id % 2 == 0) { + ids[Nucleotide::Symbol::B].push_back(row_id); } else { - ids[Nucleotide::Symbol::Y].push_back(i); + ids[Nucleotide::Symbol::Y].push_back(row_id); } } index.addSymbolsToPositions(0, ids); @@ -278,17 +278,17 @@ TEST_F(VerticalSequenceIndexTest, SparseRowSelection) { // Select only even rows std::vector sequences(100, "N"); roaring::Roaring row_ids; - for (uint32_t i = 0; i < 100; i++) { - row_ids.add(i); + for (uint32_t row_id = 0; row_id < 100; row_id++) { + row_ids.add(row_id); } index.overwriteSymbolsInSequences(sequences, row_ids); - for (uint32_t i = 0; i < 100; i++) { - if (i % 2 == 0) { - EXPECT_EQ(sequences[i], "B"); + for (uint32_t row_id = 0; row_id < 100; row_id++) { + if (row_id % 2 == 0) { + EXPECT_EQ(sequences[row_id], "B"); } else { - EXPECT_EQ(sequences[i], "Y"); + EXPECT_EQ(sequences[row_id], "Y"); } } } @@ -533,10 +533,10 @@ TEST(splitIdsIntoBatches, ConsecutiveBatches) { auto result = splitIdsIntoBatches(input); ASSERT_EQ(result.size(), 4); - for (size_t i = 0; i < result.size(); ++i) { - EXPECT_EQ(result[i].first, i + 1); - ASSERT_EQ(result[i].second.size(), 1); - EXPECT_EQ(result[i].second[0], 0x0000); + for (size_t row_id = 0; row_id < result.size(); ++row_id) { + EXPECT_EQ(result[row_id].first, row_id + 1); + ASSERT_EQ(result[row_id].second.size(), 1); + EXPECT_EQ(result[row_id].second[0], 0x0000); } } @@ -609,9 +609,9 @@ TEST(splitIdsIntoBatches, AlternatingBatches) { auto result = splitIdsIntoBatches(input); ASSERT_EQ(result.size(), 4); - for (size_t i = 0; i < result.size(); ++i) { - EXPECT_EQ(result[i].first, i + 1); - ASSERT_EQ(result[i].second.size(), 1); - EXPECT_EQ(result[i].second[0], 0x0001); + for (size_t row_id = 0; row_id < result.size(); ++row_id) { + EXPECT_EQ(result[row_id].first, row_id + 1); + ASSERT_EQ(result[row_id].second.size(), 1); + EXPECT_EQ(result[row_id].second[0], 0x0001); } } diff --git a/src/silo/storage/column/zstd_compressed_string_column.cpp b/src/silo/storage/column/zstd_compressed_string_column.cpp index 28d902a38..86593e33c 100644 --- a/src/silo/storage/column/zstd_compressed_string_column.cpp +++ b/src/silo/storage/column/zstd_compressed_string_column.cpp @@ -21,12 +21,12 @@ void ZstdCompressedStringColumnPartition::reserve(size_t row_count) { } void ZstdCompressedStringColumnPartition::insertNull() { - values.push_back({}); + values.emplace_back(); } void ZstdCompressedStringColumnPartition::insert(std::string_view value) { auto compressed = metadata->compressor.compress(value.data(), value.size()); - values.push_back(std::string{compressed}); + values.emplace_back(compressed); } std::optional ZstdCompressedStringColumnPartition::getDecompressed(size_t row_id diff --git a/src/silo/storage/column/zstd_compressed_string_column.h b/src/silo/storage/column/zstd_compressed_string_column.h index 802f8deae..399dcecd3 100644 --- a/src/silo/storage/column/zstd_compressed_string_column.h +++ b/src/silo/storage/column/zstd_compressed_string_column.h @@ -1,8 +1,6 @@ #pragma once #include -#include -#include #include #include @@ -22,7 +20,6 @@ class ZstdCompressedStringColumnMetadata : public ColumnMetadata { ZstdDecompressor decompressor; std::string dictionary_string; - public: explicit ZstdCompressedStringColumnMetadata( std::string column_name, std::string dictionary_string @@ -49,11 +46,11 @@ class ZstdCompressedStringColumnPartition { void insertNull(); void insert(std::string_view value); - size_t numValues() const { return values.size(); } + [[nodiscard]] size_t numValues() const { return values.size(); } - std::optional getDecompressed(size_t row_id) const; + [[nodiscard]] std::optional getDecompressed(size_t row_id) const; - std::optional getCompressed(size_t row_id) const; + [[nodiscard]] std::optional getCompressed(size_t row_id) const; private: friend class boost::serialization::access; @@ -71,12 +68,12 @@ BOOST_SERIALIZATION_SPLIT_FREE(silo::storage::column::ZstdCompressedStringColumn namespace boost::serialization { template [[maybe_unused]] void save( - Archive& ar, + Archive& archive, const silo::storage::column::ZstdCompressedStringColumnMetadata& object, [[maybe_unused]] const uint32_t version ) { - ar & object.column_name; - ar & object.dictionary_string; + archive & object.column_name; + archive & object.dictionary_string; } } // namespace boost::serialization @@ -85,14 +82,14 @@ BOOST_SERIALIZATION_SPLIT_FREE(std::shared_ptr< namespace boost::serialization { template [[maybe_unused]] void load( - Archive& ar, + Archive& archive, std::shared_ptr& object, [[maybe_unused]] const uint32_t version ) { std::string column_name; std::string dictionary_string; - ar & column_name; - ar & dictionary_string; + archive & column_name; + archive & dictionary_string; object = std::make_shared( std::move(column_name), std::move(dictionary_string) ); diff --git a/src/silo/storage/column_group.h b/src/silo/storage/column_group.h index 4df1b98fe..5d317bff6 100644 --- a/src/silo/storage/column_group.h +++ b/src/silo/storage/column_group.h @@ -2,20 +2,15 @@ #include #include -#include #include -#include #include -#include #include #include #include #include "silo/common/aa_symbols.h" -#include "silo/common/json_value_type.h" #include "silo/common/nucleotide_symbols.h" -#include "silo/config/database_config.h" #include "silo/schema/database_schema.h" #include "silo/storage/column/bool_column.h" #include "silo/storage/column/date_column.h" diff --git a/src/silo/storage/reference_genomes.test.cpp b/src/silo/storage/reference_genomes.test.cpp index fff2a8b62..b5b9ed7a6 100644 --- a/src/silo/storage/reference_genomes.test.cpp +++ b/src/silo/storage/reference_genomes.test.cpp @@ -2,9 +2,6 @@ #include -#include "silo/common/aa_symbols.h" -#include "silo/common/nucleotide_symbols.h" - TEST(ReferenceGenome, readFromFile) { auto under_test = silo::ReferenceGenomes::readFromFile("testBaseData/exampleDataset/reference_genomes.json"); diff --git a/src/silo/storage/table.cpp b/src/silo/storage/table.cpp index 3298513d6..52d012dbf 100644 --- a/src/silo/storage/table.cpp +++ b/src/silo/storage/table.cpp @@ -16,7 +16,6 @@ #include #include "evobench/evobench.hpp" -#include "silo/common/fmt_formatters.h" #include "silo/persistence/exception.h" #include "silo/roaring_util/roaring_serialize.h" #include "silo/schema/duplicate_primary_key_exception.h" @@ -43,7 +42,7 @@ void Table::validatePrimaryKeyUnique() const { std::unordered_set unique_keys; unique_keys.reserve(total_rows); - for (auto& partition : partitions) { + for (const auto& partition : partitions) { auto& primary_key_column = partition->columns.string_columns.at(primary_key.name); auto num_values = primary_key_column.numValues(); for (size_t i = 0; i < num_values; ++i) { @@ -119,7 +118,7 @@ void Table::loadData(const std::filesystem::path& save_directory) { if (!file_vec.back()) { throw persistence::SaveDatabaseException( - fmt::format("Cannot open partition input file {} for loading", file) + fmt::format("Cannot open partition input file {} for loading", file.string()) ); } } diff --git a/src/silo/storage/table.h b/src/silo/storage/table.h index 9fe06a2a9..b695fda55 100644 --- a/src/silo/storage/table.h +++ b/src/silo/storage/table.h @@ -1,7 +1,6 @@ #pragma once #include "silo/schema/database_schema.h" -#include "silo/storage/column_group.h" #include "silo/storage/table_partition.h" namespace silo::storage { @@ -12,12 +11,12 @@ class Table { public: schema::TableSchema schema; - Table(schema::TableSchema schema) + explicit Table(schema::TableSchema schema) : schema(std::move(schema)) {} - size_t getNumberOfPartitions() const; + [[nodiscard]] size_t getNumberOfPartitions() const; - const TablePartition& getPartition(size_t partition_idx) const; + [[nodiscard]] const TablePartition& getPartition(size_t partition_idx) const; std::shared_ptr addPartition(); diff --git a/src/silo/storage/table_partition.h b/src/silo/storage/table_partition.h index 65d6171af..dd576d621 100644 --- a/src/silo/storage/table_partition.h +++ b/src/silo/storage/table_partition.h @@ -3,7 +3,6 @@ #include #include #include -#include #include diff --git a/src/silo/storage/vector/german_string_registry.cpp b/src/silo/storage/vector/german_string_registry.cpp index e585d7942..a921a157e 100644 --- a/src/silo/storage/vector/german_string_registry.cpp +++ b/src/silo/storage/vector/german_string_registry.cpp @@ -3,14 +3,13 @@ namespace silo::storage::vector { Idx GermanStringRegistry::insert(const silo::SiloString& silo_string) { - if (german_string_pages.empty()) { - german_string_pages.emplace_back(); - } else if (german_string_pages.back().full()) { + bool need_new_page = german_string_pages.empty() || german_string_pages.back().full(); + if (need_new_page) { german_string_pages.emplace_back(); } size_t page_id = german_string_pages.size() - 1; size_t row_in_page = german_string_pages.back().insert(silo_string); - return page_id * GermanStringPage::MAX_STRINGS_PER_PAGE + row_in_page; + return (page_id * GermanStringPage::MAX_STRINGS_PER_PAGE) + row_in_page; } SiloString GermanStringRegistry::get(Idx row_id) const { diff --git a/src/silo/storage/vector/german_string_registry.h b/src/silo/storage/vector/german_string_registry.h index dd0609b2c..6814eb3e4 100644 --- a/src/silo/storage/vector/german_string_registry.h +++ b/src/silo/storage/vector/german_string_registry.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include "silo/common/german_string.h" diff --git a/src/silo/storage/vector/variable_data_registry.cpp b/src/silo/storage/vector/variable_data_registry.cpp index 61e18c378..0c010d3f0 100644 --- a/src/silo/storage/vector/variable_data_registry.cpp +++ b/src/silo/storage/vector/variable_data_registry.cpp @@ -15,7 +15,9 @@ VariableDataRegistry::Identifier VariableDataRegistry::insert(std::string_view d if (page_id > UINT32_MAX) { SILO_PANIC("Maximum number of variable string data reached. Aborting."); } - VariableDataRegistry::Identifier id{.page_id = static_cast(page_id), .offset = offset}; + VariableDataRegistry::Identifier identifier{ + .page_id = static_cast(page_id), .offset = offset + }; offset += sizeof(size_t); if (offset == buffer::PAGE_SIZE) { @@ -34,7 +36,7 @@ VariableDataRegistry::Identifier VariableDataRegistry::insert(std::string_view d remaining_data.length() ); offset += remaining_data.length(); - return id; + return identifier; } std::memcpy( variable_data_pages.back().buffer + offset, @@ -57,21 +59,24 @@ VariableDataRegistry::DataList getDataFromPage( size_t length_on_page = std::min(length, buffer::PAGE_SIZE - offset); char* start_pointer_on_page = reinterpret_cast(page.buffer + offset); std::string_view data_on_page{start_pointer_on_page, length_on_page}; - return VariableDataRegistry::DataList{data_on_page, nullptr}; + return VariableDataRegistry::DataList{.data = data_on_page, .continuation = nullptr}; } } // namespace -VariableDataRegistry::DataList VariableDataRegistry::get(VariableDataRegistry::Identifier id +VariableDataRegistry::DataList VariableDataRegistry::get(VariableDataRegistry::Identifier identifier ) const { - auto length = *reinterpret_cast(variable_data_pages.at(id.page_id).buffer + id.offset); + auto length = *reinterpret_cast( + variable_data_pages.at(identifier.page_id).buffer + identifier.offset + ); - VariableDataRegistry::DataList ret = - getDataFromPage(variable_data_pages.at(id.page_id), id.offset + sizeof(size_t), length); + VariableDataRegistry::DataList ret = getDataFromPage( + variable_data_pages.at(identifier.page_id), identifier.offset + sizeof(size_t), length + ); length -= ret.data.length(); VariableDataRegistry::DataList* current = &ret; - auto current_page = id.page_id + 1; + auto current_page = identifier.page_id + 1; while (length > 0) { // Continuation data is always at offset 0. Length is only stored on first page and then // directly spills to the beginning of next page. Append-only data-structure diff --git a/src/silo/storage/vector/variable_data_registry.h b/src/silo/storage/vector/variable_data_registry.h index e7fb42978..9e4f9d65c 100644 --- a/src/silo/storage/vector/variable_data_registry.h +++ b/src/silo/storage/vector/variable_data_registry.h @@ -1,8 +1,8 @@ #pragma once -#include #include -#include +#include +#include #include @@ -30,7 +30,7 @@ class VariableDataRegistry { VariableDataRegistry::Identifier insert(std::string_view data); - DataList get(VariableDataRegistry::Identifier id) const; + [[nodiscard]] DataList get(VariableDataRegistry::Identifier identifier) const; template [[maybe_unused]] void serialize(Archive& archive, const uint32_t /* version */) { diff --git a/src/silo/test/amino_acid_insertion_contains.test.cpp b/src/silo/test/amino_acid_insertion_contains.test.cpp index 0e9f8c9f0..77d8c3163 100644 --- a/src/silo/test/amino_acid_insertion_contains.test.cpp +++ b/src/silo/test/amino_acid_insertion_contains.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/amino_acid_symbol_equals.test.cpp b/src/silo/test/amino_acid_symbol_equals.test.cpp index 098a62fe1..06a7e1ac9 100644 --- a/src/silo/test/amino_acid_symbol_equals.test.cpp +++ b/src/silo/test/amino_acid_symbol_equals.test.cpp @@ -3,8 +3,6 @@ #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/date_between.test.cpp b/src/silo/test/date_between.test.cpp index 55a39b35b..3308d3bf9 100644 --- a/src/silo/test/date_between.test.cpp +++ b/src/silo/test/date_between.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/default_sequence.test.cpp b/src/silo/test/default_sequence.test.cpp index 499b3a184..1afb8b422 100644 --- a/src/silo/test/default_sequence.test.cpp +++ b/src/silo/test/default_sequence.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/fasta.test.cpp b/src/silo/test/fasta.test.cpp index 92f4d1922..c6c7bf1fd 100644 --- a/src/silo/test/fasta.test.cpp +++ b/src/silo/test/fasta.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/float_equals_and_between.test.cpp b/src/silo/test/float_equals_and_between.test.cpp index dcca56c5e..a9ba24459 100644 --- a/src/silo/test/float_equals_and_between.test.cpp +++ b/src/silo/test/float_equals_and_between.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::negateFilter; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/has_mutation.test.cpp b/src/silo/test/has_mutation.test.cpp index 0e8e5d844..f8f2c5c20 100644 --- a/src/silo/test/has_mutation.test.cpp +++ b/src/silo/test/has_mutation.test.cpp @@ -6,8 +6,6 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/insertion_contains.test.cpp b/src/silo/test/insertion_contains.test.cpp index b645bc39a..301b67144 100644 --- a/src/silo/test/insertion_contains.test.cpp +++ b/src/silo/test/insertion_contains.test.cpp @@ -1,12 +1,8 @@ #include -#include - #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/int_equals_and_between.test.cpp b/src/silo/test/int_equals_and_between.test.cpp index aec87f728..5cd184606 100644 --- a/src/silo/test/int_equals_and_between.test.cpp +++ b/src/silo/test/int_equals_and_between.test.cpp @@ -1,13 +1,9 @@ #include -#include - #include "silo/test/query_fixture.test.h" namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::negateFilter; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/nucleotide_symbol_equals.test.cpp b/src/silo/test/nucleotide_symbol_equals.test.cpp index dad9deaa9..78c0ffb90 100644 --- a/src/silo/test/nucleotide_symbol_equals.test.cpp +++ b/src/silo/test/nucleotide_symbol_equals.test.cpp @@ -6,8 +6,6 @@ namespace { using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/query_fixture.test.h b/src/silo/test/query_fixture.test.h index 037aa1610..e15bc3eb8 100644 --- a/src/silo/test/query_fixture.test.h +++ b/src/silo/test/query_fixture.test.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -11,13 +10,10 @@ #include #include "silo/append/database_inserter.h" -#include "silo/common/fmt_formatters.h" #include "silo/common/lineage_tree.h" #include "silo/common/phylo_tree.h" #include "silo/config/database_config.h" -#include "silo/config/preprocessing_config.h" #include "silo/database.h" -#include "silo/database_info.h" #include "silo/initialize/initializer.h" #include "silo/query_engine/query.h" #include "silo/query_engine/query_plan.h" @@ -52,7 +48,7 @@ namespace silo::test { ); \ \ TEST_P(TEST_SUITE_NAME##FixtureAlias, testQuery) { \ - const auto scenario = GetParam(); \ + const auto& scenario = GetParam(); \ runTest(scenario); \ }; @@ -87,9 +83,9 @@ class QueryTestFixture : public ::testing::TestWithParam { auto database = std::make_shared( Database{silo::initialize::Initializer::createSchemaFromConfigFiles( silo::config::DatabaseConfig::getValidatedConfig(test_data.database_config), - std::move(test_data.reference_genomes), - std::move(test_data.lineage_trees), - std::move(test_data.phylo_tree_file), + test_data.reference_genomes, + test_data.lineage_trees, + test_data.phylo_tree_file, test_data.without_unaligned_sequences )} ); @@ -106,7 +102,7 @@ class QueryTestFixture : public ::testing::TestWithParam { shared_database = database; } - void runTest(silo::test::QueryTestScenario scenario) { + void runTest(const silo::test::QueryTestScenario& scenario) { if (!shared_database) { FAIL() << "There was an error when setting up the test suite. Database not initialized."; } @@ -131,7 +127,7 @@ class QueryTestFixture : public ::testing::TestWithParam { std::string line; while (std::getline(buffer, line)) { auto line_object = nlohmann::json::parse(line); - std::cout << line_object.dump() << std::endl; + std::cout << line_object.dump() << '\n'; actual_ndjson_result_as_array.push_back(line_object); } ASSERT_EQ(actual_ndjson_result_as_array, scenario.expected_query_result); @@ -139,6 +135,6 @@ class QueryTestFixture : public ::testing::TestWithParam { } }; -nlohmann::json negateFilter(const nlohmann::json& filter); +nlohmann::json negateFilter(const nlohmann::json& query); } // namespace silo::test diff --git a/src/silo/test/randomize.test.cpp b/src/silo/test/randomize.test.cpp index fc364aa5c..565cd0295 100644 --- a/src/silo/test/randomize.test.cpp +++ b/src/silo/test/randomize.test.cpp @@ -1,15 +1,11 @@ #include -#include - #include "silo/test/query_fixture.test.h" namespace { using nlohmann::json; using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/test/string_search.test.cpp b/src/silo/test/string_search.test.cpp index eb502799b..75aa96924 100644 --- a/src/silo/test/string_search.test.cpp +++ b/src/silo/test/string_search.test.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -7,8 +6,6 @@ #include "silo/test/query_fixture.test.h" using silo::ReferenceGenomes; -using silo::config::DatabaseConfig; -using silo::config::ValueType; using silo::test::QueryTestData; using silo::test::QueryTestScenario; diff --git a/src/silo/zstd/zstd_compressor.cpp b/src/silo/zstd/zstd_compressor.cpp index 82f8cd7fe..10cfc7108 100644 --- a/src/silo/zstd/zstd_compressor.cpp +++ b/src/silo/zstd/zstd_compressor.cpp @@ -8,8 +8,7 @@ namespace silo { ZstdCompressor::ZstdCompressor(std::shared_ptr dictionary) - : buffer(), - dictionary(std::move(dictionary)) {} + : dictionary(std::move(dictionary)) {} std::string_view ZstdCompressor::compress(const char* input_data, size_t input_size) { size_t size_bound = ZSTD_compressBound(input_size); diff --git a/src/silo/zstd/zstd_compressor.h b/src/silo/zstd/zstd_compressor.h index 6921ab605..d4080c983 100644 --- a/src/silo/zstd/zstd_compressor.h +++ b/src/silo/zstd/zstd_compressor.h @@ -17,9 +17,9 @@ class ZstdCompressor { std::shared_ptr dictionary; ZstdCContext zstd_context; + public: ZstdCompressor() = delete; - public: explicit ZstdCompressor(std::shared_ptr dictionary); std::string_view compress(const char* input_data, size_t input_size); diff --git a/src/silo/zstd/zstd_context.cpp b/src/silo/zstd/zstd_context.cpp index ceb716aa7..489309f37 100644 --- a/src/silo/zstd/zstd_context.cpp +++ b/src/silo/zstd/zstd_context.cpp @@ -1,6 +1,5 @@ #include "silo/zstd/zstd_context.h" -#include #include namespace silo { diff --git a/src/silo/zstd/zstd_decompressor.cpp b/src/silo/zstd/zstd_decompressor.cpp index 1659261a9..9596427e2 100644 --- a/src/silo/zstd/zstd_decompressor.cpp +++ b/src/silo/zstd/zstd_decompressor.cpp @@ -11,7 +11,7 @@ namespace silo { ZstdDecompressor::ZstdDecompressor(std::shared_ptr zstd_dictionary) - : zstd_dictionary(zstd_dictionary) {} + : zstd_dictionary(std::move(zstd_dictionary)) {} void ZstdDecompressor::decompress(const std::string& input, std::string& buffer) { decompress(input.data(), input.size(), buffer); diff --git a/src/silo/zstd/zstd_decompressor.h b/src/silo/zstd/zstd_decompressor.h index 29e0220cf..0a2b0f672 100644 --- a/src/silo/zstd/zstd_decompressor.h +++ b/src/silo/zstd/zstd_decompressor.h @@ -2,7 +2,6 @@ #include #include -#include #include diff --git a/src/silo/zstd/zstd_dictionary.h b/src/silo/zstd/zstd_dictionary.h index 549f2ddbc..e94469b1c 100644 --- a/src/silo/zstd/zstd_dictionary.h +++ b/src/silo/zstd/zstd_dictionary.h @@ -33,7 +33,7 @@ class ZstdDDictionary final { ZSTD_DDict* value; public: - ZstdDDictionary(std::string_view data); + explicit ZstdDDictionary(std::string_view data); ZstdDDictionary(const ZstdDDictionary& other) = delete; ZstdDDictionary& operator=(const ZstdDDictionary& other) = delete;