From e500dc22e4fc06f1eb3606a71dc688d318d9fa02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Tue, 17 Oct 2023 14:14:58 +0200 Subject: [PATCH 1/2] Respect default format when using http_write_exception_in_output_format --- src/Formats/FormatFactory.cpp | 2 +- src/Interpreters/executeQuery.cpp | 49 +++++++++++++++++-- ...default_format_on_http_exception.reference | 25 ++++++++++ ...99_use_default_format_on_http_exception.sh | 33 +++++++++++++ 4 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 tests/queries/0_stateless/02899_use_default_format_on_http_exception.reference create mode 100755 tests/queries/0_stateless/02899_use_default_format_on_http_exception.sh diff --git a/src/Formats/FormatFactory.cpp b/src/Formats/FormatFactory.cpp index 9b3913244550..2713b7cb35fd 100644 --- a/src/Formats/FormatFactory.cpp +++ b/src/Formats/FormatFactory.cpp @@ -410,7 +410,7 @@ std::unique_ptr FormatFactory::wrapReadBufferIfNeeded( static void addExistingProgressToOutputFormat(OutputFormatPtr format, ContextPtr context) { - auto element_id = context->getProcessListElement(); + auto element_id = context->getProcessListElementSafe(); if (element_id) { /// While preparing the query there might have been progress (for example in subscalar subqueries) so add it here diff --git a/src/Interpreters/executeQuery.cpp b/src/Interpreters/executeQuery.cpp index b96e05c1f9ad..cda2d70714ab 100644 --- a/src/Interpreters/executeQuery.cpp +++ b/src/Interpreters/executeQuery.cpp @@ -1332,7 +1332,46 @@ void executeQuery( BlockIO streams; OutputFormatPtr output_format; - std::tie(ast, streams) = executeQueryImpl(begin, end, context, false, QueryProcessingStage::Complete, &istr); + auto update_format_for_exception_if_needed = [&]() + { + if (!output_format) + { + try + { + String format_name = context->getDefaultFormat(); + output_format = FormatFactory::instance().getOutputFormat(format_name, ostr, {}, context, output_format_settings); + if (output_format) + { + /// Force an update of the headers before we start writing + result_details.content_type = output_format->getContentType(); + result_details.format = format_name; + set_result_details(result_details); + set_result_details = nullptr; + } + } + catch (const DB::Exception & e) + { + /// Ignore this exception and report the original one + LOG_WARNING(&Poco::Logger::get("executeQuery"), getExceptionMessageAndPattern(e, true)); + } + } + }; + + try + { + std::tie(ast, streams) = executeQueryImpl(begin, end, context, false, QueryProcessingStage::Complete, &istr); + } + catch (...) + { + if (handle_exception_in_output_format) + { + update_format_for_exception_if_needed(); + if (output_format) + handle_exception_in_output_format(*output_format); + } + throw; + } + auto & pipeline = streams.pipeline; std::unique_ptr compressed_buffer; @@ -1426,8 +1465,12 @@ void executeQuery( } catch (...) { - if (handle_exception_in_output_format && output_format) - handle_exception_in_output_format(*output_format); + if (handle_exception_in_output_format) + { + update_format_for_exception_if_needed(); + if (output_format) + handle_exception_in_output_format(*output_format); + } streams.onException(); throw; } diff --git a/tests/queries/0_stateless/02899_use_default_format_on_http_exception.reference b/tests/queries/0_stateless/02899_use_default_format_on_http_exception.reference new file mode 100644 index 000000000000..a943df067644 --- /dev/null +++ b/tests/queries/0_stateless/02899_use_default_format_on_http_exception.reference @@ -0,0 +1,25 @@ +INSERT WITH default_format=JSON +Content-Type:application/json;charset=UTF-8 +"exception":"Code:62. + +INSERT WITH default_format=XML +Content-Type:application/xml;charset=UTF-8 +Code:62.DB::Ex---tion: + +INSERT WITH default_format=BADFORMAT +Content-Type:text/plain;charset=UTF-8 +X-ClickHouse-Ex---tion-Code:62 +Code:62.DB::Ex---tion: + +INSERT WITH X-ClickHouse-Format: JSON +Content-Type:application/json;charset=UTF-8 +"exception":"Code:62. + +INSERT WITH X-ClickHouse-Format: XML +Content-Type:application/xml;charset=UTF-8 +Code:62.DB::Ex---tion: + +INSERT WITH X-ClickHouse-Format: BADFORMAT +Content-Type:text/plain;charset=UTF-8 +X-ClickHouse-Ex---tion-Code:62 +Code:62.DB::Ex---tion: diff --git a/tests/queries/0_stateless/02899_use_default_format_on_http_exception.sh b/tests/queries/0_stateless/02899_use_default_format_on_http_exception.sh new file mode 100755 index 000000000000..f92ab7db4fb0 --- /dev/null +++ b/tests/queries/0_stateless/02899_use_default_format_on_http_exception.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +CH_URL="$CLICKHOUSE_URL&http_write_exception_in_output_format=1" + +echo "INSERT WITH default_format=JSON" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}&default_format=JSON" -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' +echo "" +echo "INSERT WITH default_format=XML" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}&default_format=XML" -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' +echo "" +echo "INSERT WITH default_format=BADFORMAT" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}&default_format=BADFORMAT" -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' + + +echo "" +echo "INSERT WITH X-ClickHouse-Format: JSON" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}" -H 'X-ClickHouse-Format: JSON' -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' +echo "" +echo "INSERT WITH X-ClickHouse-Format: XML" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}" -H 'X-ClickHouse-Format: XML' -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' +echo "" +echo "INSERT WITH X-ClickHouse-Format: BADFORMAT" +echo "INSERT INTO system.numbers Select * from numbers(10);" \ + | ${CLICKHOUSE_CURL} -sS "${CH_URL}" -H 'X-ClickHouse-Format: BADFORMAT' -i | grep 'xception\|Content-Type' | sed 's/Exception/Ex---tion/' | awk '{ print $1 $2 $3 }' From b4a4c1e02e9c7417bc942ffe0400a41cfb52d78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Tue, 17 Oct 2023 16:02:06 +0200 Subject: [PATCH 2/2] Only change headers if necessary --- src/Interpreters/executeQuery.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/executeQuery.cpp b/src/Interpreters/executeQuery.cpp index cda2d70714ab..734e4e68776c 100644 --- a/src/Interpreters/executeQuery.cpp +++ b/src/Interpreters/executeQuery.cpp @@ -1340,7 +1340,7 @@ void executeQuery( { String format_name = context->getDefaultFormat(); output_format = FormatFactory::instance().getOutputFormat(format_name, ostr, {}, context, output_format_settings); - if (output_format) + if (output_format && output_format->supportsWritingException()) { /// Force an update of the headers before we start writing result_details.content_type = output_format->getContentType();