From d36f2bf66bafb76fd2b7e8260a5324122e0621af Mon Sep 17 00:00:00 2001 From: Sergei Trifonov Date: Fri, 14 Jan 2022 15:15:02 +0300 Subject: [PATCH 1/2] do not construct std::string if there is no error --- src/Core/Block.cpp | 22 ++++++++++---------- src/Core/Block.h | 4 ++-- src/Processors/Formats/Impl/NativeFormat.cpp | 2 +- src/Processors/Port.cpp | 2 +- src/Processors/QueryPlan/WindowStep.cpp | 2 +- src/QueryPipeline/Pipe.cpp | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Core/Block.cpp b/src/Core/Block.cpp index 85eb62642207..39dafad2db6a 100644 --- a/src/Core/Block.cpp +++ b/src/Core/Block.cpp @@ -38,14 +38,14 @@ static ReturnType onError(const std::string & message [[maybe_unused]], int code template static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, const ColumnWithTypeAndName & expected, - const std::string & context_description, bool allow_materialize, int code) + const char * context_description, bool allow_materialize, int code) { if (actual.name != expected.name) - return onError("Block structure mismatch in " + context_description + " stream: different names of columns:\n" + return onError("Block structure mismatch in " + std::string(context_description) + " stream: different names of columns:\n" + actual.dumpStructure() + "\n" + expected.dumpStructure(), code); if (!actual.type->equals(*expected.type)) - return onError("Block structure mismatch in " + context_description + " stream: different types:\n" + return onError("Block structure mismatch in " + std::string(context_description) + " stream: different types:\n" + actual.dumpStructure() + "\n" + expected.dumpStructure(), code); if (!actual.column || !expected.column) @@ -66,7 +66,7 @@ static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, con } if (actual_column->getName() != expected.column->getName()) - return onError("Block structure mismatch in " + context_description + " stream: different columns:\n" + return onError("Block structure mismatch in " + std::string(context_description) + " stream: different columns:\n" + actual.dumpStructure() + "\n" + expected.dumpStructure(), code); if (isColumnConst(*actual.column) && isColumnConst(*expected.column)) @@ -75,7 +75,7 @@ static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, con Field expected_value = assert_cast(*expected.column).getField(); if (actual_value != expected_value) - return onError("Block structure mismatch in " + context_description + " stream: different values of constants, actual: " + return onError("Block structure mismatch in " + std::string(context_description) + " stream: different values of constants, actual: " + applyVisitor(FieldVisitorToString(), actual_value) + ", expected: " + applyVisitor(FieldVisitorToString(), expected_value), code); } @@ -85,11 +85,11 @@ static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, con template -static ReturnType checkBlockStructure(const Block & lhs, const Block & rhs, const std::string & context_description, bool allow_materialize) +static ReturnType checkBlockStructure(const Block & lhs, const Block & rhs, const char * context_description, bool allow_materialize) { size_t columns = rhs.columns(); if (lhs.columns() != columns) - return onError("Block structure mismatch in " + context_description + " stream: different number of columns:\n" + return onError("Block structure mismatch in " + std::string(context_description) + " stream: different number of columns:\n" + lhs.dumpStructure() + "\n" + rhs.dumpStructure(), ErrorCodes::LOGICAL_ERROR); for (size_t i = 0; i < columns; ++i) @@ -604,11 +604,11 @@ Names Block::getDataTypeNames() const bool blocksHaveEqualStructure(const Block & lhs, const Block & rhs) { - return checkBlockStructure(lhs, rhs, {}, false); + return checkBlockStructure(lhs, rhs, "", false); } -void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const std::string & context_description) +void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const char * context_description) { checkBlockStructure(lhs, rhs, context_description, false); } @@ -616,11 +616,11 @@ void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const bool isCompatibleHeader(const Block & actual, const Block & desired) { - return checkBlockStructure(actual, desired, {}, true); + return checkBlockStructure(actual, desired, "", true); } -void assertCompatibleHeader(const Block & actual, const Block & desired, const std::string & context_description) +void assertCompatibleHeader(const Block & actual, const Block & desired, const char * context_description) { checkBlockStructure(actual, desired, context_description, true); } diff --git a/src/Core/Block.h b/src/Core/Block.h index cad29dea7e6f..ff756714d10a 100644 --- a/src/Core/Block.h +++ b/src/Core/Block.h @@ -182,13 +182,13 @@ using ExtraBlockPtr = std::shared_ptr; bool blocksHaveEqualStructure(const Block & lhs, const Block & rhs); /// Throw exception when blocks are different. -void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const std::string & context_description); +void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const char * context_description); /// Actual header is compatible to desired if block have equal structure except constants. /// It is allowed when column from actual header is constant, but in desired is not. /// If both columns are constant, it is checked that they have the same value. bool isCompatibleHeader(const Block & actual, const Block & desired); -void assertCompatibleHeader(const Block & actual, const Block & desired, const std::string & context_description); +void assertCompatibleHeader(const Block & actual, const Block & desired, const char * context_description); /// Calculate difference in structure of blocks and write description into output strings. NOTE It doesn't compare values of constant columns. void getBlocksDifference(const Block & lhs, const Block & rhs, std::string & out_lhs_diff, std::string & out_rhs_diff); diff --git a/src/Processors/Formats/Impl/NativeFormat.cpp b/src/Processors/Formats/Impl/NativeFormat.cpp index 19e2ede6b654..8f4121133786 100644 --- a/src/Processors/Formats/Impl/NativeFormat.cpp +++ b/src/Processors/Formats/Impl/NativeFormat.cpp @@ -33,7 +33,7 @@ class NativeInputFormat final : public IInputFormat if (!block) return {}; - assertBlocksHaveEqualStructure(getPort().getHeader(), block, getName()); + assertBlocksHaveEqualStructure(getPort().getHeader(), block, getName().c_str()); block.checkNumberOfRows(); size_t num_rows = block.rows(); diff --git a/src/Processors/Port.cpp b/src/Processors/Port.cpp index 0a6026b27f2e..62c262cb5b2d 100644 --- a/src/Processors/Port.cpp +++ b/src/Processors/Port.cpp @@ -16,7 +16,7 @@ void connect(OutputPort & output, InputPort & input) auto out_name = output.getProcessor().getName(); auto in_name = input.getProcessor().getName(); - assertCompatibleHeader(output.getHeader(), input.getHeader(), " function connect between " + out_name + " and " + in_name); + assertCompatibleHeader(output.getHeader(), input.getHeader(), (" function connect between " + out_name + " and " + in_name).c_str()); input.output_port = &output; output.input_port = &input; diff --git a/src/Processors/QueryPlan/WindowStep.cpp b/src/Processors/QueryPlan/WindowStep.cpp index cd4bb5f67307..4153dc16dcd0 100644 --- a/src/Processors/QueryPlan/WindowStep.cpp +++ b/src/Processors/QueryPlan/WindowStep.cpp @@ -77,7 +77,7 @@ void WindowStep::transformPipeline(QueryPipelineBuilder & pipeline, const BuildQ }); assertBlocksHaveEqualStructure(pipeline.getHeader(), output_stream->header, - "WindowStep transform for '" + window_description.window_name + "'"); + ("WindowStep transform for '" + window_description.window_name + "'").c_str()); } void WindowStep::describeActions(FormatSettings & settings) const diff --git a/src/QueryPipeline/Pipe.cpp b/src/QueryPipeline/Pipe.cpp index 6cef7cc28bd5..e81491de8f36 100644 --- a/src/QueryPipeline/Pipe.cpp +++ b/src/QueryPipeline/Pipe.cpp @@ -135,7 +135,7 @@ Pipe::Pipe(ProcessorPtr source, OutputPort * output, OutputPort * totals, Output if (!port) return; - assertBlocksHaveEqualStructure(header, port->getHeader(), name); + assertBlocksHaveEqualStructure(header, port->getHeader(), name.c_str()); ++num_specified_ports; From f70c6320cfbb5b34171e66d46a8881da45501ce3 Mon Sep 17 00:00:00 2001 From: Sergei Trifonov Date: Fri, 14 Jan 2022 18:41:41 +0300 Subject: [PATCH 2/2] use std::string_view instead of raw pointers --- src/Core/Block.cpp | 8 ++++---- src/Core/Block.h | 4 ++-- src/Processors/Formats/Impl/NativeFormat.cpp | 2 +- src/Processors/Port.cpp | 2 +- src/Processors/QueryPlan/WindowStep.cpp | 2 +- src/QueryPipeline/Pipe.cpp | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Core/Block.cpp b/src/Core/Block.cpp index 39dafad2db6a..07db1a1fafab 100644 --- a/src/Core/Block.cpp +++ b/src/Core/Block.cpp @@ -38,7 +38,7 @@ static ReturnType onError(const std::string & message [[maybe_unused]], int code template static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, const ColumnWithTypeAndName & expected, - const char * context_description, bool allow_materialize, int code) + std::string_view context_description, bool allow_materialize, int code) { if (actual.name != expected.name) return onError("Block structure mismatch in " + std::string(context_description) + " stream: different names of columns:\n" @@ -85,7 +85,7 @@ static ReturnType checkColumnStructure(const ColumnWithTypeAndName & actual, con template -static ReturnType checkBlockStructure(const Block & lhs, const Block & rhs, const char * context_description, bool allow_materialize) +static ReturnType checkBlockStructure(const Block & lhs, const Block & rhs, std::string_view context_description, bool allow_materialize) { size_t columns = rhs.columns(); if (lhs.columns() != columns) @@ -608,7 +608,7 @@ bool blocksHaveEqualStructure(const Block & lhs, const Block & rhs) } -void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const char * context_description) +void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, std::string_view context_description) { checkBlockStructure(lhs, rhs, context_description, false); } @@ -620,7 +620,7 @@ bool isCompatibleHeader(const Block & actual, const Block & desired) } -void assertCompatibleHeader(const Block & actual, const Block & desired, const char * context_description) +void assertCompatibleHeader(const Block & actual, const Block & desired, std::string_view context_description) { checkBlockStructure(actual, desired, context_description, true); } diff --git a/src/Core/Block.h b/src/Core/Block.h index ff756714d10a..efa5ce7c3264 100644 --- a/src/Core/Block.h +++ b/src/Core/Block.h @@ -182,13 +182,13 @@ using ExtraBlockPtr = std::shared_ptr; bool blocksHaveEqualStructure(const Block & lhs, const Block & rhs); /// Throw exception when blocks are different. -void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, const char * context_description); +void assertBlocksHaveEqualStructure(const Block & lhs, const Block & rhs, std::string_view context_description); /// Actual header is compatible to desired if block have equal structure except constants. /// It is allowed when column from actual header is constant, but in desired is not. /// If both columns are constant, it is checked that they have the same value. bool isCompatibleHeader(const Block & actual, const Block & desired); -void assertCompatibleHeader(const Block & actual, const Block & desired, const char * context_description); +void assertCompatibleHeader(const Block & actual, const Block & desired, std::string_view context_description); /// Calculate difference in structure of blocks and write description into output strings. NOTE It doesn't compare values of constant columns. void getBlocksDifference(const Block & lhs, const Block & rhs, std::string & out_lhs_diff, std::string & out_rhs_diff); diff --git a/src/Processors/Formats/Impl/NativeFormat.cpp b/src/Processors/Formats/Impl/NativeFormat.cpp index 8f4121133786..19e2ede6b654 100644 --- a/src/Processors/Formats/Impl/NativeFormat.cpp +++ b/src/Processors/Formats/Impl/NativeFormat.cpp @@ -33,7 +33,7 @@ class NativeInputFormat final : public IInputFormat if (!block) return {}; - assertBlocksHaveEqualStructure(getPort().getHeader(), block, getName().c_str()); + assertBlocksHaveEqualStructure(getPort().getHeader(), block, getName()); block.checkNumberOfRows(); size_t num_rows = block.rows(); diff --git a/src/Processors/Port.cpp b/src/Processors/Port.cpp index 62c262cb5b2d..0a6026b27f2e 100644 --- a/src/Processors/Port.cpp +++ b/src/Processors/Port.cpp @@ -16,7 +16,7 @@ void connect(OutputPort & output, InputPort & input) auto out_name = output.getProcessor().getName(); auto in_name = input.getProcessor().getName(); - assertCompatibleHeader(output.getHeader(), input.getHeader(), (" function connect between " + out_name + " and " + in_name).c_str()); + assertCompatibleHeader(output.getHeader(), input.getHeader(), " function connect between " + out_name + " and " + in_name); input.output_port = &output; output.input_port = &input; diff --git a/src/Processors/QueryPlan/WindowStep.cpp b/src/Processors/QueryPlan/WindowStep.cpp index 4153dc16dcd0..cd4bb5f67307 100644 --- a/src/Processors/QueryPlan/WindowStep.cpp +++ b/src/Processors/QueryPlan/WindowStep.cpp @@ -77,7 +77,7 @@ void WindowStep::transformPipeline(QueryPipelineBuilder & pipeline, const BuildQ }); assertBlocksHaveEqualStructure(pipeline.getHeader(), output_stream->header, - ("WindowStep transform for '" + window_description.window_name + "'").c_str()); + "WindowStep transform for '" + window_description.window_name + "'"); } void WindowStep::describeActions(FormatSettings & settings) const diff --git a/src/QueryPipeline/Pipe.cpp b/src/QueryPipeline/Pipe.cpp index e81491de8f36..6cef7cc28bd5 100644 --- a/src/QueryPipeline/Pipe.cpp +++ b/src/QueryPipeline/Pipe.cpp @@ -135,7 +135,7 @@ Pipe::Pipe(ProcessorPtr source, OutputPort * output, OutputPort * totals, Output if (!port) return; - assertBlocksHaveEqualStructure(header, port->getHeader(), name.c_str()); + assertBlocksHaveEqualStructure(header, port->getHeader(), name); ++num_specified_ports;