diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index cd516ba9674..66ba8547894 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -643,7 +643,7 @@ jobs: # shellcheck disable=SC2046 docker rm -f $(docker ps -a -q) ||: sudo rm -fr "$TEMP_PATH" "$CACHES_PATH" - BuilderBinTidy: + BuilderBinClangTidy: needs: [DockerHubPush] runs-on: [self-hosted, builder] steps: @@ -1011,7 +1011,7 @@ jobs: - BuilderBinFreeBSD # - BuilderBinGCC - BuilderBinPPC64 - - BuilderBinTidy + - BuilderBinClangTidy - BuilderDebSplitted runs-on: [self-hosted, style-checker] steps: diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 79d54d77f06..9cd8fd6f49d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -707,7 +707,7 @@ jobs: # shellcheck disable=SC2046 docker rm -f $(docker ps -a -q) ||: sudo rm -fr "$TEMP_PATH" "$CACHES_PATH" - BuilderBinTidy: + BuilderBinClangTidy: needs: [DockerHubPush, FastTest] runs-on: [self-hosted, builder] steps: @@ -1065,7 +1065,7 @@ jobs: - BuilderBinFreeBSD # - BuilderBinGCC - BuilderBinPPC64 - - BuilderBinTidy + - BuilderBinClangTidy - BuilderDebSplitted runs-on: [self-hosted, style-checker] if: ${{ success() || failure() }} diff --git a/base/base/BorrowedObjectPool.h b/base/base/BorrowedObjectPool.h index 10dcc41d862..bb4c9cd1c21 100644 --- a/base/base/BorrowedObjectPool.h +++ b/base/base/BorrowedObjectPool.h @@ -89,7 +89,7 @@ public: inline void returnObject(T && object_to_return) { { - std::lock_guard lock(objects_mutex); + std::lock_guard lock(objects_mutex); objects.emplace_back(std::move(object_to_return)); --borrowed_objects_size; @@ -107,14 +107,14 @@ public: /// Allocated objects size by the pool. If allocatedObjectsSize == maxSize then pool is full. inline size_t allocatedObjectsSize() const { - std::unique_lock lock(objects_mutex); + std::lock_guard lock(objects_mutex); return allocated_objects_size; } /// Returns allocatedObjectsSize == maxSize inline bool isFull() const { - std::unique_lock lock(objects_mutex); + std::lock_guard lock(objects_mutex); return allocated_objects_size == max_size; } @@ -122,7 +122,7 @@ public: /// Then client will wait during borrowObject function call. inline size_t borrowedObjectsSize() const { - std::unique_lock lock(objects_mutex); + std::lock_guard lock(objects_mutex); return borrowed_objects_size; } diff --git a/base/base/argsToConfig.h b/base/base/argsToConfig.h index 134eed64fd2..9b7b44b7b7f 100644 --- a/base/base/argsToConfig.h +++ b/base/base/argsToConfig.h @@ -4,7 +4,7 @@ namespace Poco::Util { -class LayeredConfiguration; +class LayeredConfiguration; // NOLINT(cppcoreguidelines-virtual-class-destructor) } /// Import extra command line arguments to configuration. These are command line arguments after --. diff --git a/base/base/defines.h b/base/base/defines.h index 3959e690d71..a707e965675 100644 --- a/base/base/defines.h +++ b/base/base/defines.h @@ -124,21 +124,37 @@ #endif #endif -// Macros for Clang Thread Safety Analysis (TSA). They can be safely ignored by other compilers. -// Feel free to extend, but please stay close to https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader +/// Macros for Clang Thread Safety Analysis (TSA). They can be safely ignored by other compilers. +/// Feel free to extend, but please stay close to https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader #if defined(__clang__) -# define TSA_GUARDED_BY(...) __attribute__((guarded_by(__VA_ARGS__))) // data is protected by given capability -# define TSA_PT_GUARDED_BY(...) __attribute__((pt_guarded_by(__VA_ARGS__))) // pointed-to data is protected by the given capability -# define TSA_REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__))) // thread needs exclusive possession of given capability -# define TSA_REQUIRES_SHARED(...) __attribute__((requires_shared_capability(__VA_ARGS__))) // thread needs shared possession of given capability -# define TSA_ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__))) // annotated lock must be locked after given lock -# define TSA_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) // disable TSA for a function +# define TSA_GUARDED_BY(...) __attribute__((guarded_by(__VA_ARGS__))) /// data is protected by given capability +# define TSA_PT_GUARDED_BY(...) __attribute__((pt_guarded_by(__VA_ARGS__))) /// pointed-to data is protected by the given capability +# define TSA_REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__))) /// thread needs exclusive possession of given capability +# define TSA_REQUIRES_SHARED(...) __attribute__((requires_shared_capability(__VA_ARGS__))) /// thread needs shared possession of given capability +# define TSA_ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__))) /// annotated lock must be locked after given lock +# define TSA_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) /// disable TSA for a function + +/// Macros for suppressing TSA warnings for specific reads/writes (instead of suppressing it for the whole function) +/// Consider adding a comment before using these macros. +# define TSA_SUPPRESS_WARNING_FOR_READ(x) [&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> const auto & { return (x); }() +# define TSA_SUPPRESS_WARNING_FOR_WRITE(x) [&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> auto & { return (x); }() + +/// This macro is useful when only one thread writes to a member +/// and you want to read this member from the same thread without locking a mutex. +/// It's safe (because no concurrent writes are possible), but TSA generates a warning. +/// (Seems like there's no way to verify it, but it makes sense to distinguish it from TSA_SUPPRESS_WARNING_FOR_READ for readability) +# define TSA_READ_ONE_THREAD(x) TSA_SUPPRESS_WARNING_FOR_READ(x) + #else # define TSA_GUARDED_BY(...) # define TSA_PT_GUARDED_BY(...) # define TSA_REQUIRES(...) # define TSA_REQUIRES_SHARED(...) # define TSA_NO_THREAD_SAFETY_ANALYSIS + +# define TSA_SUPPRESS_WARNING_FOR_READ(x) +# define TSA_SUPPRESS_WARNING_FOR_WRITE(x) +# define TSA_READ_ONE_THREAD(x) #endif /// A template function for suppressing warnings about unused variables or function results. diff --git a/base/base/demangle.h b/base/base/demangle.h index 3c20920ab7c..ddca264ecab 100644 --- a/base/base/demangle.h +++ b/base/base/demangle.h @@ -27,6 +27,6 @@ struct FreeingDeleter } }; -typedef std::unique_ptr DemangleResult; +using DemangleResult = std::unique_ptr; DemangleResult tryDemangle(const char * name); diff --git a/base/base/strong_typedef.h b/base/base/strong_typedef.h index 0c2e9ca7e8e..c9ea30b73fd 100644 --- a/base/base/strong_typedef.h +++ b/base/base/strong_typedef.h @@ -23,10 +23,10 @@ public: constexpr StrongTypedef(): t() {} constexpr StrongTypedef(const Self &) = default; - constexpr StrongTypedef(Self &&) = default; + constexpr StrongTypedef(Self &&) noexcept(std::is_nothrow_move_constructible_v) = default; Self & operator=(const Self &) = default; - Self & operator=(Self &&) = default; + Self & operator=(Self &&) noexcept(std::is_nothrow_move_assignable_v)= default; template ::type> Self & operator=(const T & rhs) { t = rhs; return *this;} diff --git a/base/base/time.h b/base/base/time.h index d0b8e94a9a5..b029c9e3d42 100644 --- a/base/base/time.h +++ b/base/base/time.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #if defined (OS_DARWIN) || defined (OS_SUNOS) # define CLOCK_MONOTONIC_COARSE CLOCK_MONOTONIC diff --git a/base/base/unaligned.h b/base/base/unaligned.h index ca73298adfb..013aa5274e9 100644 --- a/base/base/unaligned.h +++ b/base/base/unaligned.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include diff --git a/base/base/wide_integer.h b/base/base/wide_integer.h index de349633723..ffd30460c03 100644 --- a/base/base/wide_integer.h +++ b/base/base/wide_integer.h @@ -27,6 +27,8 @@ #include #include +// NOLINTBEGIN(*) + namespace wide { template @@ -257,4 +259,7 @@ struct hash>; } +// NOLINTEND(*) + #include "wide_integer_impl.h" + diff --git a/base/base/wide_integer_impl.h b/base/base/wide_integer_impl.h index e469e1683c8..5e32b286871 100644 --- a/base/base/wide_integer_impl.h +++ b/base/base/wide_integer_impl.h @@ -15,6 +15,8 @@ #include #include +// NOLINTBEGIN(*) + /// Use same extended double for all platforms #if (LDBL_MANT_DIG == 64) #define CONSTEXPR_FROM_DOUBLE constexpr @@ -1478,3 +1480,5 @@ struct hash> }; } + +// NOLINTEND(*) diff --git a/base/pcg-random/pcg_extras.hpp b/base/pcg-random/pcg_extras.hpp index 39c91c4ecfa..f5ba4d48849 100644 --- a/base/pcg-random/pcg_extras.hpp +++ b/base/pcg-random/pcg_extras.hpp @@ -90,6 +90,7 @@ #define PCG_EMULATED_128BIT_MATH 1 #endif +// NOLINTBEGIN(*) namespace pcg_extras { @@ -552,4 +553,6 @@ std::ostream& operator<<(std::ostream& out, printable_typename) { } // namespace pcg_extras +// NOLINTEND(*) + #endif // PCG_EXTRAS_HPP_INCLUDED diff --git a/base/pcg-random/pcg_random.hpp b/base/pcg-random/pcg_random.hpp index d9d3519a4cf..94e43e1007b 100644 --- a/base/pcg-random/pcg_random.hpp +++ b/base/pcg-random/pcg_random.hpp @@ -113,6 +113,8 @@ #include "pcg_extras.hpp" +// NOLINTBEGIN(*) + namespace DB { struct PcgSerializer; @@ -1777,4 +1779,6 @@ typedef pcg_engines::ext_oneseq_xsh_rs_64_32<14,32,true> pcg32_k16384_fast; #pragma warning(default:4146) #endif +// NOLINTEND(*) + #endif // PCG_RAND_HPP_INCLUDED diff --git a/base/widechar_width/widechar_width.h b/base/widechar_width/widechar_width.h index 3007a112886..4927b67eb91 100644 --- a/base/widechar_width/widechar_width.h +++ b/base/widechar_width/widechar_width.h @@ -16,6 +16,8 @@ #include #include +// NOLINTBEGIN(*) + /* Special width values */ enum { widechar_nonprint = -1, // The character is not printable. @@ -518,4 +520,6 @@ inline int widechar_wcwidth(wchar_t c) { return 1; } +// NOLINTEND(*) + #endif // WIDECHAR_WIDTH_H diff --git a/contrib/libgsasl-cmake/CMakeLists.txt b/contrib/libgsasl-cmake/CMakeLists.txt index 3cf087c2f4c..f0c2395a831 100644 --- a/contrib/libgsasl-cmake/CMakeLists.txt +++ b/contrib/libgsasl-cmake/CMakeLists.txt @@ -113,4 +113,8 @@ if (TARGET ch_contrib::krb5) target_compile_definitions(_gsasl PRIVATE HAVE_GSSAPI_H=1 USE_GSSAPI=1) endif() +if (TARGET OpenSSL::SSL) + target_link_libraries(_gsasl PRIVATE OpenSSL::Crypto OpenSSL::SSL) +endif() + add_library(ch_contrib::gsasl ALIAS _gsasl) diff --git a/contrib/poco b/contrib/poco index de35b9fd72b..0e32cb42db7 160000 --- a/contrib/poco +++ b/contrib/poco @@ -1 +1 @@ -Subproject commit de35b9fd72b57127abdc3a5beaf0e320d767e356 +Subproject commit 0e32cb42db76ddaa76848470219056908053b676 diff --git a/contrib/poco-cmake/XML/CMakeLists.txt b/contrib/poco-cmake/XML/CMakeLists.txt index 45100f11eb7..97e655a0f04 100644 --- a/contrib/poco-cmake/XML/CMakeLists.txt +++ b/contrib/poco-cmake/XML/CMakeLists.txt @@ -11,6 +11,7 @@ add_library (_poco_xml_expat ${SRCS_EXPAT}) add_library (Poco::XML::Expat ALIAS _poco_xml_expat) target_include_directories (_poco_xml_expat PUBLIC "${LIBRARY_DIR}/XML/include") +target_include_directories (_poco_xml_expat PRIVATE "${LIBRARY_DIR}/Foundation/include") # Poco::XML diff --git a/docker/test/integration/runner/Dockerfile b/docker/test/integration/runner/Dockerfile index 80a2158b17d..d6bd458a01b 100644 --- a/docker/test/integration/runner/Dockerfile +++ b/docker/test/integration/runner/Dockerfile @@ -3,6 +3,7 @@ FROM ubuntu:20.04 # ARG for quick switch to a given ubuntu mirror ARG apt_archive="http://archive.ubuntu.com" + RUN sed -i "s|http://archive.ubuntu.com|$apt_archive|g" /etc/apt/sources.list RUN apt-get update \ diff --git a/docker/test/integration/runner/dockerd-entrypoint.sh b/docker/test/integration/runner/dockerd-entrypoint.sh index 0cb25d12a9f..bcaa064fe4f 100755 --- a/docker/test/integration/runner/dockerd-entrypoint.sh +++ b/docker/test/integration/runner/dockerd-entrypoint.sh @@ -30,8 +30,8 @@ set -e # cleanup for retry run if volume is not recreated # shellcheck disable=SC2046 { - docker kill $(docker ps -aq) || true - docker rm $(docker ps -aq) || true + docker ps -aq | xargs -r docker kill || true + docker ps -aq | xargs -r docker rm || true } echo "Start tests" diff --git a/docker/test/stateful/run.sh b/docker/test/stateful/run.sh index 5f55bb9fa21..d77978c904b 100755 --- a/docker/test/stateful/run.sh +++ b/docker/test/stateful/run.sh @@ -120,6 +120,10 @@ function run_tests() ADDITIONAL_OPTIONS+=('--replicated-database') fi + if [[ -n "$USE_DATABASE_ORDINARY" ]] && [[ "$USE_DATABASE_ORDINARY" -eq 1 ]]; then + ADDITIONAL_OPTIONS+=('--db-engine=Ordinary') + fi + set +e clickhouse-test -j 2 --testname --shard --zookeeper --check-zookeeper-session --no-stateless --hung-check --print-time \ --skip 00168_parallel_processing_on_replicas "${ADDITIONAL_OPTIONS[@]}" \ diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index 52bf8a60669..075f588cae3 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -115,6 +115,10 @@ function run_tests() ADDITIONAL_OPTIONS+=("$RUN_BY_HASH_TOTAL") fi + if [[ -n "$USE_DATABASE_ORDINARY" ]] && [[ "$USE_DATABASE_ORDINARY" -eq 1 ]]; then + ADDITIONAL_OPTIONS+=('--db-engine=Ordinary') + fi + set +e clickhouse-test --testname --shard --zookeeper --check-zookeeper-session --hung-check --print-time \ --test-runs "$NUM_TRIES" "${ADDITIONAL_OPTIONS[@]}" 2>&1 \ diff --git a/docker/test/stress/run.sh b/docker/test/stress/run.sh index c73784c4ef1..119ea04080f 100755 --- a/docker/test/stress/run.sh +++ b/docker/test/stress/run.sh @@ -215,7 +215,7 @@ start clickhouse-client --query "SELECT 'Server successfully started', 'OK'" >> /test_output/test_results.tsv \ || (echo -e 'Server failed to start (see application_errors.txt and clickhouse-server.clean.log)\tFAIL' >> /test_output/test_results.tsv \ - && grep -Fa ".*Application" /var/log/clickhouse-server/clickhouse-server.log > /test_output/application_errors.txt) + && grep -a ".*Application" /var/log/clickhouse-server/clickhouse-server.log > /test_output/application_errors.txt) [ -f /var/log/clickhouse-server/clickhouse-server.log ] || echo -e "Server log does not exist\tFAIL" [ -f /var/log/clickhouse-server/stderr.log ] || echo -e "Stderr log does not exist\tFAIL" @@ -284,6 +284,11 @@ then rm -rf /var/lib/clickhouse/* + # Make BC check more funny by forcing Ordinary engine for system database + # New version will try to convert it to Atomic on startup + mkdir /var/lib/clickhouse/metadata + echo "ATTACH DATABASE system ENGINE=Ordinary" > /var/lib/clickhouse/metadata/system.sql + # Install previous release packages install_packages previous_release_package_folder @@ -313,7 +318,7 @@ then start 500 clickhouse-client --query "SELECT 'Backward compatibility check: Server successfully started', 'OK'" >> /test_output/test_results.tsv \ || (echo -e 'Backward compatibility check: Server failed to start\tFAIL' >> /test_output/test_results.tsv \ - && grep -Fa ".*Application" /var/log/clickhouse-server/clickhouse-server.log >> /test_output/bc_check_application_errors.txt) + && grep -a ".*Application" /var/log/clickhouse-server/clickhouse-server.log >> /test_output/bc_check_application_errors.txt) clickhouse-client --query="SELECT 'Server version: ', version()" @@ -343,7 +348,7 @@ then -e "UNFINISHED" \ -e "Renaming unexpected part" \ -e "PART_IS_TEMPORARILY_LOCKED" \ - -e "and a merge is impossible: we didn't find smaller parts" \ + -e "and a merge is impossible: we didn't find" \ -e "found in queue and some source parts for it was lost" \ -e "is lost forever." \ /var/log/clickhouse-server/clickhouse-server.backward.clean.log | zgrep -Fa "" > /test_output/bc_check_error_messages.txt \ diff --git a/docker/test/util/process_functional_tests_result.py b/docker/test/util/process_functional_tests_result.py index fdd193e9c7f..647989e8421 100755 --- a/docker/test/util/process_functional_tests_result.py +++ b/docker/test/util/process_functional_tests_result.py @@ -86,7 +86,7 @@ def process_test_log(log_path): test_end = True test_results = [ - (test[0], test[1], test[2], "".join(test[3])) for test in test_results + (test[0], test[1], test[2], "".join(test[3]))[:4096] for test in test_results ] return ( diff --git a/docs/en/development/architecture.md b/docs/en/development/architecture.md index 6da92d96335..b2f1f2448f8 100644 --- a/docs/en/development/architecture.md +++ b/docs/en/development/architecture.md @@ -223,7 +223,7 @@ Replication in ClickHouse can be configured on a per-table basis. You could have Replication is implemented in the `ReplicatedMergeTree` storage engine. The path in `ZooKeeper` is specified as a parameter for the storage engine. All tables with the same path in `ZooKeeper` become replicas of each other: they synchronize their data and maintain consistency. Replicas can be added and removed dynamically simply by creating or dropping a table. -Replication uses an asynchronous multi-master scheme. You can insert data into any replica that has a session with `ZooKeeper`, and data is replicated to all other replicas asynchronously. Because ClickHouse does not support UPDATEs, replication is conflict-free. As there is no quorum acknowledgment of inserts, just-inserted data might be lost if one node fails. +Replication uses an asynchronous multi-master scheme. You can insert data into any replica that has a session with `ZooKeeper`, and data is replicated to all other replicas asynchronously. Because ClickHouse does not support UPDATEs, replication is conflict-free. As there is no quorum acknowledgment of inserts by default, just-inserted data might be lost if one node fails. The insert quorum can be enabled using `insert_quorum` setting. Metadata for replication is stored in ZooKeeper. There is a replication log that lists what actions to do. Actions are: get part; merge parts; drop a partition, and so on. Each replica copies the replication log to its queue and then executes the actions from the queue. For example, on insertion, the “get the part” action is created in the log, and every replica downloads that part. Merges are coordinated between replicas to get byte-identical results. All parts are merged in the same way on all replicas. One of the leaders initiates a new merge first and writes “merge parts” actions to the log. Multiple replicas (or all) can be leaders at the same time. A replica can be prevented from becoming a leader using the `merge_tree` setting `replicated_can_become_leader`. The leaders are responsible for scheduling background merges. diff --git a/docs/en/sql-reference/functions/geo/index.md b/docs/en/sql-reference/functions/geo/index.md index f76c3a3f731..ea43b3ef96c 100644 --- a/docs/en/sql-reference/functions/geo/index.md +++ b/docs/en/sql-reference/functions/geo/index.md @@ -3,6 +3,74 @@ sidebar_label: Geo sidebar_position: 62 --- -# Geo Functions +# Geo Functions + +## Geographical Coordinates Functions + +- [greatCircleDistance](./coordinates.md#greatCircleDistance) +- [geoDistance](./coordinates.md#geoDistance) +- [greatCircleAngle](./coordinates.md#greatCircleAngle) +- [pointInEllipses](./coordinates.md#pointInEllipses) +- [pointInPolygon](./coordinates.md#pointInPolygon) + +## Geohash Functions +- [geohashEncode](./geohash.md#geohashEncode) +- [geohashDecode](./geohash.md#geohashDecode) +- [geohashesInBox](./geohash.md#geohashesInBox) + +## H3 Indexes Functions + +- [h3IsValid](./h3#h3IsValid) +- [h3GetResolution](./h3#h3GetResolution) +- [h3EdgeAngle](./h3#h3EdgeAngle) +- [h3EdgeLengthM​](./h3#h3EdgeLengthM​) +- [h3EdgeLengthKm] (./h3#h3EdgeLengthKm) +- [geoToH3](./h3#geoToH3) +- [h3ToGeo](./h3#h3ToGeo) +- [h3ToGeoBoundary](./h3#h3ToGeoBoundary) +- [h3kRing](./h3#h3kRing) +- [h3GetBaseCell](./h3#h3GetBaseCell) +- [h3HexAreaM2](./h3#h3HexAreaM2) +- [h3HexAreaKm2](./h3#h3HexAreaKm2) +- [h3IndexesAreNeighbors](./h3#h3IndexesAreNeighbors) +- [h3ToChildren](./h3#h3ToChildren) +- [h3ToParent](./h3#h3ToParent) +- [h3ToString](./h3#h3ToString) +- [stringToH3](./h3#stringToH3) +- [h3GetResolution](./h3#h3GetResolution) +- [h3IsResClassIII](./h3#h3IsResClassIII) +- [h3IsPentagon](./h3#h3IsPentagon) +- [h3GetFaces](./h3#h3GetFaces) +- [h3CellAreaM2](./h3#h3CellAreaM2) +- [h3CellAreaRads2](./h3#h3CellAreaRads2) +- [h3ToCenterChild](./h3#h3ToCenterChild) +- [h3ExactEdgeLengthM](./h3#h3ExactEdgeLengthM) +- [h3ExactEdgeLengthKm](./h3#h3ExactEdgeLengthKm) +- [h3ExactEdgeLengthRads](./h3#h3ExactEdgeLengthRads) +- [h3NumHexagons](./h3#h3NumHexagons) +- [h3Line](./h3#h3Line) +- [h3Distance](./h3#h3Distance) +- [h3HexRing](./h3#h3HexRing) +- [h3GetUnidirectionalEdge](./h3#h3GetUnidirectionalEdge) +- [h3UnidirectionalEdgeIsValid](./h3#h3UnidirectionalEdgeIsValid) +- [h3GetOriginIndexFromUnidirectionalEdge](./h3#h3GetOriginIndexFromUnidirectionalEdge) +- [h3GetDestinationIndexFromUnidirectionalEdge](./h3#h3GetDestinationIndexFromUnidirectionalEdge) +- [h3GetIndexesFromUnidirectionalEdge](./h3#h3GetIndexesFromUnidirectionalEdge) +- [h3GetUnidirectionalEdgesFromHexagon](./h3#h3GetUnidirectionalEdgesFromHexagon) +- [h3GetUnidirectionalEdgeBoundary](./h3#h3GetUnidirectionalEdgeBoundary) + +## S2 Index Functions + +- [geoToS2](./s2#geoToS2) +- [s2ToGeo](./s2#s2ToGeo) +- [s2GetNeighbors](./s2#s2GetNeighbors) +- [s2CellsIntersect](./s2#s2CellsIntersect) +- [s2CapContains](./s2#s2CapContains) +- [s2CapUnion](./s2#s2CapUnion) +- [s2RectAdd](./s2#s2RectAdd) +- [s2RectContains](./s2#s2RectContains) +- [s2RectUinion](./s2#s2RectUinion) +- [s2RectIntersection](./s2#s2RectIntersection) + [Original article](https://clickhouse.com/docs/en/sql-reference/functions/geo/) diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 476725c5627..621c40c31bf 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1488,6 +1488,7 @@ int Server::main(const std::vector & /*args*/) /// We load temporary database first, because projections need it. database_catalog.initializeAndLoadTemporaryDatabase(); loadMetadataSystem(global_context); + maybeConvertOrdinaryDatabaseToAtomic(global_context, DatabaseCatalog::instance().getSystemDatabase()); /// After attaching system databases we can initialize system log. global_context->initializeSystemLogs(); global_context->setSystemZooKeeperLogAfterInitializationIfNeeded(); diff --git a/src/Access/KerberosInit.cpp b/src/Access/KerberosInit.cpp index ace03a5e0b5..1bba82bc46f 100644 --- a/src/Access/KerberosInit.cpp +++ b/src/Access/KerberosInit.cpp @@ -223,7 +223,7 @@ void kerberosInit(const String & keytab_file, const String & principal, const St { // Using mutex to prevent cache file corruptions static std::mutex kinit_mtx; - std::unique_lock lck(kinit_mtx); + std::lock_guard lck(kinit_mtx); KerberosInit k_init; k_init.init(keytab_file, principal, cache_name); } diff --git a/src/Common/Allocator.h b/src/Common/Allocator.h index 1e6e76bee91..06ccbed4064 100644 --- a/src/Common/Allocator.h +++ b/src/Common/Allocator.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #ifdef NDEBUG #define ALLOCATOR_ASLR 0 diff --git a/src/Common/Arena.h b/src/Common/Arena.h index 4c7ee458ee4..b706f3b3413 100644 --- a/src/Common/Arena.h +++ b/src/Common/Arena.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/Common/CurrentMetrics.h b/src/Common/CurrentMetrics.h index 7508c26b0ae..c184ee1e7f2 100644 --- a/src/Common/CurrentMetrics.h +++ b/src/Common/CurrentMetrics.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/Common/Exception.h b/src/Common/Exception.h index 253dbe6d65c..9d59fb4d7b5 100644 --- a/src/Common/Exception.h +++ b/src/Common/Exception.h @@ -130,7 +130,7 @@ public: int getLineNumber() const { return line_number; } void setLineNumber(int line_number_) { line_number = line_number_;} - const String getFileName() const { return file_name; } + String getFileName() const { return file_name; } void setFileName(const String & file_name_) { file_name = file_name_; } Exception * clone() const override { return new ParsingException(*this); } diff --git a/src/Common/FileCacheSettings.h b/src/Common/FileCacheSettings.h index 1d8b613bedd..c04cc915486 100644 --- a/src/Common/FileCacheSettings.h +++ b/src/Common/FileCacheSettings.h @@ -2,7 +2,7 @@ #include -namespace Poco { namespace Util { class AbstractConfiguration; } } +namespace Poco { namespace Util { class AbstractConfiguration; } } // NOLINT(cppcoreguidelines-virtual-class-destructor) namespace DB { diff --git a/src/Common/IFactoryWithAliases.h b/src/Common/IFactoryWithAliases.h index 458adc4522f..8feac867dd2 100644 --- a/src/Common/IFactoryWithAliases.h +++ b/src/Common/IFactoryWithAliases.h @@ -27,9 +27,9 @@ protected: String getAliasToOrName(const String & name) const { - if (aliases.count(name)) + if (aliases.contains(name)) return aliases.at(name); - else if (String name_lowercase = Poco::toLower(name); case_insensitive_aliases.count(name_lowercase)) + else if (String name_lowercase = Poco::toLower(name); case_insensitive_aliases.contains(name_lowercase)) return case_insensitive_aliases.at(name_lowercase); else return name; @@ -108,7 +108,7 @@ public: bool isAlias(const String & name) const { - return aliases.count(name) || case_insensitive_aliases.count(name); + return aliases.count(name) || case_insensitive_aliases.contains(name); } bool hasNameOrAlias(const String & name) const @@ -125,7 +125,7 @@ public: return name; } - virtual ~IFactoryWithAliases() override = default; + ~IFactoryWithAliases() override = default; private: using InnerMap = std::unordered_map; // name -> creator diff --git a/src/Common/LocalDate.h b/src/Common/LocalDate.h index 5d070e77647..dc36f92bebf 100644 --- a/src/Common/LocalDate.h +++ b/src/Common/LocalDate.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/Common/OvercommitTracker.cpp b/src/Common/OvercommitTracker.cpp index 4faed833428..0dea6589adc 100644 --- a/src/Common/OvercommitTracker.cpp +++ b/src/Common/OvercommitTracker.cpp @@ -137,7 +137,7 @@ void OvercommitTracker::onQueryStop(MemoryTracker * tracker) { DENY_ALLOCATIONS_IN_SCOPE; - std::unique_lock lk(overcommit_m); + std::lock_guard lk(overcommit_m); if (picked_tracker == tracker) { LOG_DEBUG_SAFE(getLogger(), "Picked query stopped"); diff --git a/src/Common/PODArray.h b/src/Common/PODArray.h index e4c1969383b..44be28eab75 100644 --- a/src/Common/PODArray.h +++ b/src/Common/PODArray.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/Common/PoolBase.h b/src/Common/PoolBase.h index 4b4e2c5cfa7..dac126fb5d7 100644 --- a/src/Common/PoolBase.h +++ b/src/Common/PoolBase.h @@ -55,7 +55,7 @@ private: explicit PoolEntryHelper(PooledObject & data_) : data(data_) { data.in_use = true; } ~PoolEntryHelper() { - std::unique_lock lock(data.pool.mutex); + std::lock_guard lock(data.pool.mutex); data.in_use = false; data.pool.available.notify_one(); } @@ -163,7 +163,7 @@ public: inline size_t size() { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); return items.size(); } diff --git a/src/Common/ProfileEvents.h b/src/Common/ProfileEvents.h index 14d134a14a0..6eebb75c5ca 100644 --- a/src/Common/ProfileEvents.h +++ b/src/Common/ProfileEvents.h @@ -4,7 +4,7 @@ #include "base/types.h" #include #include -#include +#include /** Implements global counters for various events happening in the application * - for high level profiling. diff --git a/src/Common/StackTrace.h b/src/Common/StackTrace.h index 33c408e6d8a..84a2e9d1f7f 100644 --- a/src/Common/StackTrace.h +++ b/src/Common/StackTrace.h @@ -7,7 +7,7 @@ #include #include #include -#include +#include #ifdef OS_DARWIN // ucontext is not available without _XOPEN_SOURCE diff --git a/src/Common/StringSearcher.h b/src/Common/StringSearcher.h index b556ace75a7..a82115a9923 100644 --- a/src/Common/StringSearcher.h +++ b/src/Common/StringSearcher.h @@ -7,8 +7,8 @@ #include #include #include -#include -#include +#include +#include #ifdef __SSE2__ #include diff --git a/src/Common/SystemLogBase.cpp b/src/Common/SystemLogBase.cpp index 7128d00714c..67aedbd5670 100644 --- a/src/Common/SystemLogBase.cpp +++ b/src/Common/SystemLogBase.cpp @@ -139,7 +139,7 @@ void SystemLogBase::flush(bool force) uint64_t this_thread_requested_offset; { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (is_shutdown) return; diff --git a/src/Common/ThreadPool.cpp b/src/Common/ThreadPool.cpp index a76037ae5cf..3f5091af0c9 100644 --- a/src/Common/ThreadPool.cpp +++ b/src/Common/ThreadPool.cpp @@ -209,7 +209,7 @@ template void ThreadPoolImpl::finalize() { { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); shutdown = true; } @@ -224,14 +224,14 @@ void ThreadPoolImpl::finalize() template size_t ThreadPoolImpl::active() const { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); return scheduled_jobs; } template bool ThreadPoolImpl::finished() const { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); return shutdown; } @@ -290,7 +290,7 @@ void ThreadPoolImpl::worker(typename std::list::iterator thread_ job = {}; { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (!first_exception) first_exception = std::current_exception(); // NOLINT if (shutdown_on_exception) @@ -305,7 +305,7 @@ void ThreadPoolImpl::worker(typename std::list::iterator thread_ } { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); --scheduled_jobs; if (threads.size() > scheduled_jobs + max_free_threads) diff --git a/src/Common/formatIPv6.h b/src/Common/formatIPv6.h index d6efeed17e6..83b9d6e9fb1 100644 --- a/src/Common/formatIPv6.h +++ b/src/Common/formatIPv6.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include #include diff --git a/src/Common/memcpySmall.h b/src/Common/memcpySmall.h index 6cadc19262f..4f38095c7f1 100644 --- a/src/Common/memcpySmall.h +++ b/src/Common/memcpySmall.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #ifdef __SSE2__ # include diff --git a/src/Coordination/KeeperServer.cpp b/src/Coordination/KeeperServer.cpp index d4c188fe8d9..8a46d8ee296 100644 --- a/src/Coordination/KeeperServer.cpp +++ b/src/Coordination/KeeperServer.cpp @@ -576,7 +576,7 @@ nuraft::cb_func::ReturnCode KeeperServer::callbackFunc(nuraft::cb_func::Type typ auto set_initialized = [this]() { - std::unique_lock lock(initialized_mutex); + std::lock_guard lock(initialized_mutex); initialized_flag = true; initialized_cv.notify_all(); }; diff --git a/src/Core/Field.h b/src/Core/Field.h index 2a925a9e2a6..4948ec4ae61 100644 --- a/src/Core/Field.h +++ b/src/Core/Field.h @@ -54,7 +54,7 @@ DEFINE_FIELD_VECTOR(Map); /// TODO: use map instead of vector. #undef DEFINE_FIELD_VECTOR -using FieldMap = std::map, AllocatorWithMemoryTracking>>; +using FieldMap = std::map, AllocatorWithMemoryTracking>>; #define DEFINE_FIELD_MAP(X) \ struct X : public FieldMap \ diff --git a/src/Core/PostgreSQL/Connection.h b/src/Core/PostgreSQL/Connection.h index 8c5609dc66b..97ce3c152d5 100644 --- a/src/Core/PostgreSQL/Connection.h +++ b/src/Core/PostgreSQL/Connection.h @@ -32,7 +32,7 @@ struct ConnectionInfo class Connection : private boost::noncopyable { public: - Connection(const ConnectionInfo & connection_info_, bool replication_ = false, size_t num_tries = 3); + explicit Connection(const ConnectionInfo & connection_info_, bool replication_ = false, size_t num_tries = 3); void execWithRetry(const std::function & exec); diff --git a/src/Core/PostgreSQL/PoolWithFailover.h b/src/Core/PostgreSQL/PoolWithFailover.h index 600e12fb53a..4e3a17b5e9c 100644 --- a/src/Core/PostgreSQL/PoolWithFailover.h +++ b/src/Core/PostgreSQL/PoolWithFailover.h @@ -25,13 +25,13 @@ public: static constexpr inline auto POSTGRESQL_POOL_WAIT_TIMEOUT = 5000; static constexpr inline auto POSTGRESQL_POOL_WITH_FAILOVER_DEFAULT_MAX_TRIES = 5; - PoolWithFailover( + explicit PoolWithFailover( const DB::ExternalDataSourcesConfigurationByPriority & configurations_by_priority, size_t pool_size = POSTGRESQL_POOL_DEFAULT_SIZE, size_t pool_wait_timeout = POSTGRESQL_POOL_WAIT_TIMEOUT, size_t max_tries_ = POSTGRESQL_POOL_WITH_FAILOVER_DEFAULT_MAX_TRIES); - PoolWithFailover( + explicit PoolWithFailover( const DB::StoragePostgreSQLConfiguration & configuration, size_t pool_size = POSTGRESQL_POOL_DEFAULT_SIZE, size_t pool_wait_timeout = POSTGRESQL_POOL_WAIT_TIMEOUT, diff --git a/src/Core/Settings.h b/src/Core/Settings.h index f33b087e571..a4a19c280d3 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -445,7 +445,6 @@ static constexpr UInt64 operator""_GiB(unsigned long long value) M(Seconds, wait_for_window_view_fire_signal_timeout, 10, "Timeout for waiting for window view fire signal in event time processing", 0) \ M(UInt64, min_free_disk_space_for_temporary_data, 0, "The minimum disk space to keep while writing temporary data used in external sorting and aggregation.", 0) \ \ - M(DefaultDatabaseEngine, default_database_engine, DefaultDatabaseEngine::Atomic, "Default database engine.", 0) \ M(DefaultTableEngine, default_table_engine, DefaultTableEngine::None, "Default table engine used when ENGINE is not set in CREATE statement.",0) \ M(Bool, show_table_uuid_in_table_create_query_if_not_nil, false, "For tables in databases with Engine=Atomic show UUID of the table in its CREATE query.", 0) \ M(Bool, database_atomic_wait_for_drop_and_detach_synchronously, false, "When executing DROP or DETACH TABLE in Atomic database, wait for table data to be finally dropped or detached.", 0) \ @@ -591,6 +590,8 @@ static constexpr UInt64 operator""_GiB(unsigned long long value) M(Bool, use_local_cache_for_remote_storage, true, "Use local cache for remote storage like HDFS or S3, it's used for remote table engine only", 0) \ \ M(Bool, allow_unrestricted_reads_from_keeper, false, "Allow unrestricted (without condition on path) reads from system.zookeeper table, can be handy, but is not safe for zookeeper", 0) \ + M(Bool, allow_deprecated_database_ordinary, false, "Allow to create databases with deprecated Ordinary engine", 0) \ + M(Bool, allow_deprecated_syntax_for_merge_tree, false, "Allow to create *MergeTree tables with deprecated engine definition syntax", 0) \ \ /** Experimental functions */ \ M(Bool, allow_experimental_funnel_functions, false, "Enable experimental functions for funnel analysis.", 0) \ @@ -639,6 +640,7 @@ static constexpr UInt64 operator""_GiB(unsigned long long value) MAKE_OBSOLETE(M, UInt64, background_schedule_pool_size, 128) \ MAKE_OBSOLETE(M, UInt64, background_message_broker_schedule_pool_size, 16) \ MAKE_OBSOLETE(M, UInt64, background_distributed_schedule_pool_size, 16) \ + MAKE_OBSOLETE(M, DefaultDatabaseEngine, default_database_engine, DefaultDatabaseEngine::Atomic) \ /** The section above is for obsolete settings. Do not add anything there. */ diff --git a/src/Daemon/BaseDaemon.cpp b/src/Daemon/BaseDaemon.cpp index e731787a5c1..445aa4463bd 100644 --- a/src/Daemon/BaseDaemon.cpp +++ b/src/Daemon/BaseDaemon.cpp @@ -925,7 +925,7 @@ void BaseDaemon::handleSignal(int signal_id) signal_id == SIGQUIT || signal_id == SIGTERM) { - std::unique_lock lock(signal_handler_mutex); + std::lock_guard lock(signal_handler_mutex); { ++terminate_signals_counter; sigint_signals_counter += signal_id == SIGINT; diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index a4fa1fa267b..92dae025dae 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -73,7 +73,7 @@ String DatabaseAtomic::getTableDataPath(const ASTCreateQuery & query) const void DatabaseAtomic::drop(ContextPtr) { - assert(tables.empty()); + assert(TSA_SUPPRESS_WARNING_FOR_READ(tables).empty()); try { fs::remove(path_to_metadata_symlink); @@ -90,40 +90,40 @@ void DatabaseAtomic::attachTable(ContextPtr /* context_ */, const String & name, { assert(relative_table_path != data_path && !relative_table_path.empty()); DetachedTables not_in_use; - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); not_in_use = cleanupDetachedTables(); auto table_id = table->getStorageID(); assertDetachedTableNotInUse(table_id.uuid); - DatabaseOrdinary::attachTableUnlocked(name, table, lock); + DatabaseOrdinary::attachTableUnlocked(name, table); table_name_to_path.emplace(std::make_pair(name, relative_table_path)); } StoragePtr DatabaseAtomic::detachTable(ContextPtr /* context */, const String & name) { DetachedTables not_in_use; - std::unique_lock lock(mutex); - auto table = DatabaseOrdinary::detachTableUnlocked(name, lock); + std::lock_guard lock(mutex); + auto table = DatabaseOrdinary::detachTableUnlocked(name); table_name_to_path.erase(name); detached_tables.emplace(table->getStorageID().uuid, table); not_in_use = cleanupDetachedTables(); //-V1001 return table; } -void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_name, bool no_delay) +void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_name, bool sync) { auto table = tryGetTable(table_name, local_context); /// Remove the inner table (if any) to avoid deadlock /// (due to attempt to execute DROP from the worker thread) if (table) - table->dropInnerTableIfAny(no_delay, local_context); + table->dropInnerTableIfAny(sync, local_context); else throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", - backQuote(database_name), backQuote(table_name)); + backQuote(getDatabaseName()), backQuote(table_name)); String table_metadata_path = getObjectMetadataPath(table_name); String table_metadata_path_drop; { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); table_metadata_path_drop = DatabaseCatalog::instance().getPathForDroppedMetadata(table->getStorageID()); auto txn = local_context->getZooKeeperMetadataTransaction(); if (txn && !local_context->isInternalSubquery()) @@ -136,7 +136,7 @@ void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_na /// TODO better detection and recovery fs::rename(table_metadata_path, table_metadata_path_drop); /// Mark table as dropped - DatabaseOrdinary::detachTableUnlocked(table_name, lock); /// Should never throw + DatabaseOrdinary::detachTableUnlocked(table_name); /// Should never throw table_name_to_path.erase(table_name); } @@ -145,11 +145,12 @@ void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_na /// Notify DatabaseCatalog that table was dropped. It will remove table data in background. /// Cleanup is performed outside of database to allow easily DROP DATABASE without waiting for cleanup to complete. - DatabaseCatalog::instance().enqueueDroppedTableCleanup(table->getStorageID(), table, table_metadata_path_drop, no_delay); + DatabaseCatalog::instance().enqueueDroppedTableCleanup(table->getStorageID(), table, table_metadata_path_drop, sync); } void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_name, IDatabase & to_database, const String & to_table_name, bool exchange, bool dictionary) + TSA_NO_THREAD_SAFETY_ANALYSIS /// TSA does not support conditional locking { if (typeid(*this) != typeid(to_database)) { @@ -173,7 +174,7 @@ void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_ String old_metadata_path = getObjectMetadataPath(table_name); String new_metadata_path = to_database.getObjectMetadataPath(to_table_name); - auto detach = [](DatabaseAtomic & db, const String & table_name_, bool has_symlink) + auto detach = [](DatabaseAtomic & db, const String & table_name_, bool has_symlink) TSA_REQUIRES(db.mutex) { auto it = db.table_name_to_path.find(table_name_); String table_data_path_saved; @@ -188,7 +189,7 @@ void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_ return table_data_path_saved; }; - auto attach = [](DatabaseAtomic & db, const String & table_name_, const String & table_data_path_, const StoragePtr & table_) + auto attach = [](DatabaseAtomic & db, const String & table_name_, const String & table_data_path_, const StoragePtr & table_) TSA_REQUIRES(db.mutex) { db.tables.emplace(table_name_, table_); if (table_data_path_.empty()) @@ -229,9 +230,9 @@ void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_ } if (!exchange) - other_db.checkMetadataFilenameAvailabilityUnlocked(to_table_name, inside_database ? db_lock : other_db_lock); + other_db.checkMetadataFilenameAvailabilityUnlocked(to_table_name); - StoragePtr table = getTableUnlocked(table_name, db_lock); + StoragePtr table = getTableUnlocked(table_name); if (dictionary && !table->isDictionary()) throw Exception(ErrorCodes::INCORRECT_QUERY, "Use RENAME/EXCHANGE TABLE (instead of RENAME/EXCHANGE DICTIONARY) for tables"); @@ -244,7 +245,7 @@ void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_ StorageID other_table_new_id = StorageID::createEmpty(); if (exchange) { - other_table = other_db.getTableUnlocked(to_table_name, other_db_lock); + other_table = other_db.getTableUnlocked(to_table_name); if (dictionary && !other_table->isDictionary()) throw Exception(ErrorCodes::INCORRECT_QUERY, "Use RENAME/EXCHANGE TABLE (instead of RENAME/EXCHANGE DICTIONARY) for tables"); other_table_new_id = {database_name, table_name, other_table->getStorageID().uuid}; @@ -294,7 +295,7 @@ void DatabaseAtomic::commitCreateTable(const ASTCreateQuery & query, const Stora auto table_data_path = getTableDataPath(query); try { - std::unique_lock lock{mutex}; + std::lock_guard lock{mutex}; if (query.getDatabase() != database_name) throw Exception(ErrorCodes::UNKNOWN_DATABASE, "Database was renamed to `{}`, cannot create table in `{}`", database_name, query.getDatabase()); @@ -312,7 +313,7 @@ void DatabaseAtomic::commitCreateTable(const ASTCreateQuery & query, const Stora /// It throws if `table_metadata_path` already exists (it's possible if table was detached) renameNoReplace(table_metadata_tmp_path, table_metadata_path); /// Commit point (a sort of) - attachTableUnlocked(query.getTable(), table, lock); /// Should never throw + attachTableUnlocked(query.getTable(), table); /// Should never throw table_name_to_path.emplace(query.getTable(), table_data_path); } catch (...) @@ -330,8 +331,8 @@ void DatabaseAtomic::commitAlterTable(const StorageID & table_id, const String & bool check_file_exists = true; SCOPE_EXIT({ std::error_code code; if (check_file_exists) std::filesystem::remove(table_metadata_tmp_path, code); }); - std::unique_lock lock{mutex}; - auto actual_table_id = getTableUnlocked(table_id.table_name, lock)->getStorageID(); + std::lock_guard lock{mutex}; + auto actual_table_id = getTableUnlocked(table_id.table_name)->getStorageID(); if (table_id.uuid != actual_table_id.uuid) throw Exception("Cannot alter table because it was renamed", ErrorCodes::CANNOT_ASSIGN_ALTER); @@ -363,7 +364,7 @@ void DatabaseAtomic::assertDetachedTableNotInUse(const UUID & uuid) void DatabaseAtomic::setDetachedTableNotInUseForce(const UUID & uuid) { - std::unique_lock lock{mutex}; + std::lock_guard lock{mutex}; detached_tables.erase(uuid); } diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index b748e53244d..6cb2226a7f8 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -35,7 +35,7 @@ public: bool exchange, bool dictionary) override; - void dropTable(ContextPtr context, const String & table_name, bool no_delay) override; + void dropTable(ContextPtr context, const String & table_name, bool sync) override; void attachTable(ContextPtr context, const String & name, const StoragePtr & table, const String & relative_table_path) override; StoragePtr detachTable(ContextPtr context, const String & name) override; @@ -70,9 +70,9 @@ protected: void commitCreateTable(const ASTCreateQuery & query, const StoragePtr & table, const String & table_metadata_tmp_path, const String & table_metadata_path, ContextPtr query_context) override; - void assertDetachedTableNotInUse(const UUID & uuid); + void assertDetachedTableNotInUse(const UUID & uuid) TSA_REQUIRES(mutex); using DetachedTables = std::unordered_map; - [[nodiscard]] DetachedTables cleanupDetachedTables(); + [[nodiscard]] DetachedTables cleanupDetachedTables() TSA_REQUIRES(mutex); void tryCreateMetadataSymlink(); @@ -80,9 +80,9 @@ protected: //TODO store path in DatabaseWithOwnTables::tables using NameToPathMap = std::unordered_map; - NameToPathMap table_name_to_path; + NameToPathMap table_name_to_path TSA_GUARDED_BY(mutex); - DetachedTables detached_tables; + DetachedTables detached_tables TSA_GUARDED_BY(mutex); String path_to_table_symlinks; String path_to_metadata_symlink; const UUID db_uuid; diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 5c7c1dedf9b..af82d382063 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -62,36 +62,19 @@ namespace ErrorCodes DatabasePtr DatabaseFactory::get(const ASTCreateQuery & create, const String & metadata_path, ContextPtr context) { - bool created = false; + /// Creates store/xxx/ for Atomic + fs::create_directories(fs::path(metadata_path).parent_path()); - try - { - /// Creates store/xxx/ for Atomic - fs::create_directories(fs::path(metadata_path).parent_path()); + DatabasePtr impl = getImpl(create, metadata_path, context); - /// Before 20.7 it's possible that .sql metadata file does not exist for some old database. - /// In this case Ordinary database is created on server startup if the corresponding metadata directory exists. - /// So we should remove metadata directory if database creation failed. - /// TODO remove this code - created = fs::create_directory(metadata_path); + if (impl && context->hasQueryContext() && context->getSettingsRef().log_queries) + context->getQueryContext()->addQueryFactoriesInfo(Context::QueryLogFactories::Database, impl->getEngineName()); - DatabasePtr impl = getImpl(create, metadata_path, context); + /// Attach database metadata + if (impl && create.comment) + impl->setDatabaseComment(create.comment->as()->value.safeGet()); - if (impl && context->hasQueryContext() && context->getSettingsRef().log_queries) - context->getQueryContext()->addQueryFactoriesInfo(Context::QueryLogFactories::Database, impl->getEngineName()); - - // Attach database metadata - if (impl && create.comment) - impl->setDatabaseComment(create.comment->as()->value.safeGet()); - - return impl; - } - catch (...) - { - if (created && fs::exists(metadata_path)) - fs::remove_all(metadata_path); - throw; - } + return impl; } template @@ -139,8 +122,14 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine `{}` cannot have table overrides", engine_name); if (engine_name == "Ordinary") + { + if (!create.attach && !context->getSettingsRef().allow_deprecated_database_ordinary) + throw Exception(ErrorCodes::UNKNOWN_DATABASE_ENGINE, + "Ordinary database engine is deprecated (see also allow_deprecated_database_ordinary setting)"); return std::make_shared(database_name, metadata_path, context); - else if (engine_name == "Atomic") + } + + if (engine_name == "Atomic") return std::make_shared(database_name, metadata_path, uuid, context); else if (engine_name == "Memory") return std::make_shared(database_name, context); diff --git a/src/Databases/DatabaseLazy.cpp b/src/Databases/DatabaseLazy.cpp index b024c73d578..3a1b3009878 100644 --- a/src/Databases/DatabaseLazy.cpp +++ b/src/Databases/DatabaseLazy.cpp @@ -77,10 +77,10 @@ void DatabaseLazy::createTable( void DatabaseLazy::dropTable( ContextPtr local_context, const String & table_name, - bool no_delay) + bool sync) { SCOPE_EXIT_MEMORY_SAFE({ clearExpiredTables(); }); - DatabaseOnDisk::dropTable(local_context, table_name, no_delay); + DatabaseOnDisk::dropTable(local_context, table_name, sync); } void DatabaseLazy::renameTable( @@ -158,6 +158,7 @@ DatabaseTablesIteratorPtr DatabaseLazy::getTablesIterator(ContextPtr, const Filt bool DatabaseLazy::empty() const { + std::lock_guard lock(mutex); return tables_cache.empty(); } diff --git a/src/Databases/DatabaseLazy.h b/src/Databases/DatabaseLazy.h index 3a7d7b14be1..d3c3ed2843b 100644 --- a/src/Databases/DatabaseLazy.h +++ b/src/Databases/DatabaseLazy.h @@ -37,7 +37,7 @@ public: void dropTable( ContextPtr context, const String & table_name, - bool no_delay) override; + bool sync) override; void renameTable( ContextPtr context, @@ -102,8 +102,8 @@ private: const time_t expiration_time; /// TODO use DatabaseWithOwnTablesBase::tables - mutable TablesCache tables_cache; - mutable CacheExpirationQueue cache_expiration_queue; + mutable TablesCache tables_cache TSA_GUARDED_BY(mutex); + mutable CacheExpirationQueue cache_expiration_queue TSA_GUARDED_BY(mutex); StoragePtr loadTable(const String & table_name) const; diff --git a/src/Databases/DatabaseMemory.cpp b/src/Databases/DatabaseMemory.cpp index 6df5b70c827..5268252731f 100644 --- a/src/Databases/DatabaseMemory.cpp +++ b/src/Databases/DatabaseMemory.cpp @@ -32,8 +32,8 @@ void DatabaseMemory::createTable( const StoragePtr & table, const ASTPtr & query) { - std::unique_lock lock{mutex}; - attachTableUnlocked(table_name, table, lock); + std::lock_guard lock{mutex}; + attachTableUnlocked(table_name, table); /// Clean the query from temporary flags. ASTPtr query_to_store = query; @@ -52,23 +52,24 @@ void DatabaseMemory::createTable( void DatabaseMemory::dropTable( ContextPtr /*context*/, const String & table_name, - bool /*no_delay*/) + bool /*sync*/) { - std::unique_lock lock{mutex}; - auto table = detachTableUnlocked(table_name, lock); + StoragePtr table; + { + std::lock_guard lock{mutex}; + table = detachTableUnlocked(table_name); + } try { /// Remove table without lock since: /// - it does not require it /// - it may cause lock-order-inversion if underlying storage need to /// resolve tables (like StorageLiveView) - SCOPE_EXIT(lock.lock()); - lock.unlock(); table->drop(); if (table->storesDataOnDisk()) { - assert(database_name != DatabaseCatalog::TEMPORARY_DATABASE); + assert(getDatabaseName() != DatabaseCatalog::TEMPORARY_DATABASE); fs::path table_data_dir{getTableDataPath(table_name)}; if (fs::exists(table_data_dir)) fs::remove_all(table_data_dir); @@ -76,10 +77,13 @@ void DatabaseMemory::dropTable( } catch (...) { + std::lock_guard lock{mutex}; assert(database_name != DatabaseCatalog::TEMPORARY_DATABASE); - attachTableUnlocked(table_name, table, lock); + attachTableUnlocked(table_name, table); throw; } + + std::lock_guard lock{mutex}; table->is_dropped = true; create_queries.erase(table_name); UUID table_uuid = table->getStorageID().uuid; diff --git a/src/Databases/DatabaseMemory.h b/src/Databases/DatabaseMemory.h index b854d9be1f3..eef9f306343 100644 --- a/src/Databases/DatabaseMemory.h +++ b/src/Databases/DatabaseMemory.h @@ -32,7 +32,7 @@ public: void dropTable( ContextPtr context, const String & table_name, - bool no_delay) override; + bool sync) override; ASTPtr getCreateTableQueryImpl(const String & name, ContextPtr context, bool throw_on_error) const override; ASTPtr getCreateDatabaseQuery() const override; @@ -51,9 +51,9 @@ public: void alterTable(ContextPtr local_context, const StorageID & table_id, const StorageInMemoryMetadata & metadata) override; private: - String data_path; + const String data_path; using NameToASTCreate = std::unordered_map; - NameToASTCreate create_queries; + NameToASTCreate create_queries TSA_GUARDED_BY(mutex); }; } diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 64bc9a4a094..26ea3b81e3a 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -281,7 +281,7 @@ void DatabaseOnDisk::detachTablePermanently(ContextPtr query_context, const Stri } } -void DatabaseOnDisk::dropTable(ContextPtr local_context, const String & table_name, bool /*no_delay*/) +void DatabaseOnDisk::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/) { String table_metadata_path = getObjectMetadataPath(table_name); String table_metadata_path_drop = table_metadata_path + drop_suffix; @@ -321,11 +321,11 @@ void DatabaseOnDisk::dropTable(ContextPtr local_context, const String & table_na void DatabaseOnDisk::checkMetadataFilenameAvailability(const String & to_table_name) const { - std::unique_lock lock(mutex); - checkMetadataFilenameAvailabilityUnlocked(to_table_name, lock); + std::lock_guard lock(mutex); + checkMetadataFilenameAvailabilityUnlocked(to_table_name); } -void DatabaseOnDisk::checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name, std::unique_lock &) const +void DatabaseOnDisk::checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name) const { String table_metadata_path = getObjectMetadataPath(to_table_name); @@ -503,7 +503,7 @@ ASTPtr DatabaseOnDisk::getCreateDatabaseQuery() const void DatabaseOnDisk::drop(ContextPtr local_context) { - assert(tables.empty()); + assert(TSA_SUPPRESS_WARNING_FOR_READ(tables).empty()); if (local_context->getSettingsRef().force_remove_data_recursively_on_drop) { fs::remove_all(local_context->getPath() + getDataPath()); @@ -725,8 +725,6 @@ ASTPtr DatabaseOnDisk::getCreateQueryFromStorage(const String & table_name, cons void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr query_context) { - std::lock_guard lock(modify_settings_mutex); - auto create_query = getCreateDatabaseQuery()->clone(); auto * create = create_query->as(); auto * settings = create->storage->settings; @@ -759,7 +757,7 @@ void DatabaseOnDisk::modifySettingsMetadata(const SettingsChanges & settings_cha writeChar('\n', statement_buf); String statement = statement_buf.str(); - String database_name_escaped = escapeForFileName(database_name); + String database_name_escaped = escapeForFileName(TSA_SUPPRESS_WARNING_FOR_READ(database_name)); /// FIXME fs::path metadata_root_path = fs::canonical(query_context->getGlobalContext()->getPath()); fs::path metadata_file_tmp_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql.tmp"); fs::path metadata_file_path = fs::path(metadata_root_path) / "metadata" / (database_name_escaped + ".sql"); diff --git a/src/Databases/DatabaseOnDisk.h b/src/Databases/DatabaseOnDisk.h index a118c8da678..90aba6be169 100644 --- a/src/Databases/DatabaseOnDisk.h +++ b/src/Databases/DatabaseOnDisk.h @@ -43,7 +43,7 @@ public: void dropTable( ContextPtr context, const String & table_name, - bool no_delay) override; + bool sync) override; void renameTable( ContextPtr context, @@ -70,7 +70,7 @@ public: /// will throw when the table we want to attach already exists (in active / detached / detached permanently form) void checkMetadataFilenameAvailability(const String & to_table_name) const override; - void checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name, std::unique_lock &) const; + void checkMetadataFilenameAvailabilityUnlocked(const String & to_table_name) const TSA_REQUIRES(mutex); void modifySettingsMetadata(const SettingsChanges & settings_changes, ContextPtr query_context); @@ -99,9 +99,6 @@ protected: const String metadata_path; const String data_path; - - /// For alter settings. - std::mutex modify_settings_mutex; }; } diff --git a/src/Databases/DatabaseOrdinary.cpp b/src/Databases/DatabaseOrdinary.cpp index 2b88fbbfcf7..18b70222382 100644 --- a/src/Databases/DatabaseOrdinary.cpp +++ b/src/Databases/DatabaseOrdinary.cpp @@ -174,7 +174,8 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables if (ast) { auto * create_query = ast->as(); - create_query->setDatabase(database_name); + /// NOTE No concurrent writes are possible during database loading + create_query->setDatabase(TSA_SUPPRESS_WARNING_FOR_READ(database_name)); /// Even if we don't load the table we can still mark the uuid of it as taken. if (create_query->uuid != UUIDHelpers::Nil) @@ -201,7 +202,7 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables return; } - QualifiedTableName qualified_name{database_name, create_query->getTable()}; + QualifiedTableName qualified_name{TSA_SUPPRESS_WARNING_FOR_READ(database_name), create_query->getTable()}; TableNamesSet loading_dependencies = getDependenciesSetFromCreateQuery(getContext(), qualified_name, ast); std::lock_guard lock{metadata.mutex}; @@ -234,12 +235,12 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables size_t tables_in_database = objects_in_database - dictionaries_in_database; LOG_INFO(log, "Metadata processed, database {} has {} tables and {} dictionaries in total.", - database_name, tables_in_database, dictionaries_in_database); + TSA_SUPPRESS_WARNING_FOR_READ(database_name), tables_in_database, dictionaries_in_database); } void DatabaseOrdinary::loadTableFromMetadata(ContextMutablePtr local_context, const String & file_path, const QualifiedTableName & name, const ASTPtr & ast, bool force_restore) { - assert(name.database == database_name); + assert(name.database == TSA_SUPPRESS_WARNING_FOR_READ(database_name)); const auto & create_query = ast->as(); tryAttachTable( @@ -255,7 +256,8 @@ void DatabaseOrdinary::startupTables(ThreadPool & thread_pool, bool /*force_rest { LOG_INFO(log, "Starting up tables."); - const size_t total_tables = tables.size(); + /// NOTE No concurrent writes are possible during database loading + const size_t total_tables = TSA_SUPPRESS_WARNING_FOR_READ(tables).size(); if (!total_tables) return; @@ -271,7 +273,7 @@ void DatabaseOrdinary::startupTables(ThreadPool & thread_pool, bool /*force_rest try { - for (const auto & table : tables) + for (const auto & table : TSA_SUPPRESS_WARNING_FOR_READ(tables)) thread_pool.scheduleOrThrowOnError([&]() { startup_one_table(table.second); }); } catch (...) diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 5c701c8d90c..5a22eeaf570 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -148,7 +148,7 @@ ClusterPtr DatabaseReplicated::getClusterImpl() const if (hosts.empty()) throw Exception(ErrorCodes::NO_ACTIVE_REPLICAS, "No replicas of database {} found. " "It's possible if the first replica is not fully created yet " - "or if the last replica was just dropped or due to logical error", database_name); + "or if the last replica was just dropped or due to logical error", zookeeper_path); Int32 cversion = stat.cversion; ::sort(hosts.begin(), hosts.end()); @@ -213,7 +213,7 @@ ClusterPtr DatabaseReplicated::getClusterImpl() const treat_local_port_as_remote, cluster_auth_info.cluster_secure_connection, /*priority=*/1, - database_name, + TSA_SUPPRESS_WARNING_FOR_READ(database_name), /// FIXME cluster_auth_info.cluster_secret); } @@ -588,7 +588,7 @@ void DatabaseReplicated::recoverLostReplica(const ZooKeeperPtr & current_zookeep query_context->makeQueryContext(); query_context->getClientInfo().query_kind = ClientInfo::QueryKind::SECONDARY_QUERY; query_context->getClientInfo().is_replicated_database_internal = true; - query_context->setCurrentDatabase(database_name); + query_context->setCurrentDatabase(getDatabaseName()); query_context->setCurrentQueryId(""); auto txn = std::make_shared(current_zookeeper, zookeeper_path, false, ""); query_context->initZooKeeperMetadataTransaction(txn); @@ -608,6 +608,7 @@ void DatabaseReplicated::recoverLostReplica(const ZooKeeperPtr & current_zookeep /// and make possible creation of new table with the same UUID. String query = fmt::format("CREATE DATABASE IF NOT EXISTS {} ENGINE=Ordinary", backQuoteIfNeed(to_db_name)); auto query_context = Context::createCopy(getContext()); + query_context->setSetting("allow_deprecated_database_ordinary", 1); executeQuery(query, query_context, true); /// But we want to avoid discarding UUID of ReplicatedMergeTree tables, because it will not work @@ -811,7 +812,7 @@ void DatabaseReplicated::shutdown() } -void DatabaseReplicated::dropTable(ContextPtr local_context, const String & table_name, bool no_delay) +void DatabaseReplicated::dropTable(ContextPtr local_context, const String & table_name, bool sync) { auto txn = local_context->getZooKeeperMetadataTransaction(); assert(!ddl_worker->isCurrentlyActive() || txn || startsWith(table_name, ".inner_id.")); @@ -820,7 +821,7 @@ void DatabaseReplicated::dropTable(ContextPtr local_context, const String & tabl String metadata_zk_path = zookeeper_path + "/metadata/" + escapeForFileName(table_name); txn->addOp(zkutil::makeRemoveRequest(metadata_zk_path, -1)); } - DatabaseAtomic::dropTable(local_context, table_name, no_delay); + DatabaseAtomic::dropTable(local_context, table_name, sync); } void DatabaseReplicated::renameTable(ContextPtr local_context, const String & table_name, IDatabase & to_database, diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index 45a9d12981c..3aa2aa378b7 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -30,7 +30,7 @@ public: String getEngineName() const override { return "Replicated"; } /// If current query is initial, then the following methods add metadata updating ZooKeeper operations to current ZooKeeperMetadataTransaction. - void dropTable(ContextPtr, const String & table_name, bool no_delay) override; + void dropTable(ContextPtr, const String & table_name, bool sync) override; void renameTable(ContextPtr context, const String & table_name, IDatabase & to_database, const String & to_table_name, bool exchange, bool dictionary) override; void commitCreateTable(const ASTCreateQuery & query, const StoragePtr & table, diff --git a/src/Databases/DatabasesCommon.cpp b/src/Databases/DatabasesCommon.cpp index 13cd841cc6e..5dd17789e60 100644 --- a/src/Databases/DatabasesCommon.cpp +++ b/src/Databases/DatabasesCommon.cpp @@ -218,11 +218,11 @@ bool DatabaseWithOwnTablesBase::empty() const StoragePtr DatabaseWithOwnTablesBase::detachTable(ContextPtr /* context_ */, const String & table_name) { - std::unique_lock lock(mutex); - return detachTableUnlocked(table_name, lock); + std::lock_guard lock(mutex); + return detachTableUnlocked(table_name); } -StoragePtr DatabaseWithOwnTablesBase::detachTableUnlocked(const String & table_name, std::unique_lock &) +StoragePtr DatabaseWithOwnTablesBase::detachTableUnlocked(const String & table_name) { StoragePtr res; @@ -245,11 +245,11 @@ StoragePtr DatabaseWithOwnTablesBase::detachTableUnlocked(const String & table_n void DatabaseWithOwnTablesBase::attachTable(ContextPtr /* context_ */, const String & table_name, const StoragePtr & table, const String &) { - std::unique_lock lock(mutex); - attachTableUnlocked(table_name, table, lock); + std::lock_guard lock(mutex); + attachTableUnlocked(table_name, table); } -void DatabaseWithOwnTablesBase::attachTableUnlocked(const String & table_name, const StoragePtr & table, std::unique_lock &) +void DatabaseWithOwnTablesBase::attachTableUnlocked(const String & table_name, const StoragePtr & table) { auto table_id = table->getStorageID(); if (table_id.database_name != database_name) @@ -313,7 +313,7 @@ DatabaseWithOwnTablesBase::~DatabaseWithOwnTablesBase() } } -StoragePtr DatabaseWithOwnTablesBase::getTableUnlocked(const String & table_name, std::unique_lock &) const +StoragePtr DatabaseWithOwnTablesBase::getTableUnlocked(const String & table_name) const { auto it = tables.find(table_name); if (it != tables.end()) diff --git a/src/Databases/DatabasesCommon.h b/src/Databases/DatabasesCommon.h index fcaa4af88bb..c960d295529 100644 --- a/src/Databases/DatabasesCommon.h +++ b/src/Databases/DatabasesCommon.h @@ -45,14 +45,14 @@ public: ~DatabaseWithOwnTablesBase() override; protected: - Tables tables; + Tables tables TSA_GUARDED_BY(mutex); Poco::Logger * log; DatabaseWithOwnTablesBase(const String & name_, const String & logger, ContextPtr context); - void attachTableUnlocked(const String & table_name, const StoragePtr & table, std::unique_lock & lock); - StoragePtr detachTableUnlocked(const String & table_name, std::unique_lock & lock); - StoragePtr getTableUnlocked(const String & table_name, std::unique_lock & lock) const; + void attachTableUnlocked(const String & table_name, const StoragePtr & table) TSA_REQUIRES(mutex); + StoragePtr detachTableUnlocked(const String & table_name) TSA_REQUIRES(mutex); + StoragePtr getTableUnlocked(const String & table_name) const TSA_REQUIRES(mutex); }; } diff --git a/src/Databases/IDatabase.cpp b/src/Databases/IDatabase.cpp index 1d5695188b7..3adba0d85c8 100644 --- a/src/Databases/IDatabase.cpp +++ b/src/Databases/IDatabase.cpp @@ -19,7 +19,7 @@ StoragePtr IDatabase::getTable(const String & name, ContextPtr context) const { if (auto storage = tryGetTable(name, context)) return storage; - throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", backQuoteIfNeed(database_name), backQuoteIfNeed(name)); + throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name)); } ASTPtr IDatabase::getCreateDatabaseQueryForBackup() const diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index a81df0a389a..2223d657f7f 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -198,7 +198,7 @@ public: virtual void dropTable( /// NOLINT ContextPtr /*context*/, const String & /*name*/, - [[maybe_unused]] bool no_delay = false) + [[maybe_unused]] bool sync = false) { throw Exception("There is no DROP TABLE query for Database" + getEngineName(), ErrorCodes::NOT_IMPLEMENTED); } @@ -356,8 +356,8 @@ protected: } mutable std::mutex mutex; - String database_name; - String comment; + String database_name TSA_GUARDED_BY(mutex); + String comment TSA_GUARDED_BY(mutex); }; using DatabasePtr = std::shared_ptr; diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 13f55eab9e2..230a0b4d4a4 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -39,7 +39,7 @@ DatabaseMaterializedMySQL::DatabaseMaterializedMySQL( void DatabaseMaterializedMySQL::rethrowExceptionIfNeeded() const { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (!settings->allows_query_when_mysql_lost && exception) { @@ -59,7 +59,7 @@ void DatabaseMaterializedMySQL::rethrowExceptionIfNeeded() const void DatabaseMaterializedMySQL::setException(const std::exception_ptr & exception_) { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); exception = exception_; } @@ -80,10 +80,10 @@ void DatabaseMaterializedMySQL::createTable(ContextPtr context_, const String & DatabaseAtomic::createTable(context_, name, table, query); } -void DatabaseMaterializedMySQL::dropTable(ContextPtr context_, const String & name, bool no_delay) +void DatabaseMaterializedMySQL::dropTable(ContextPtr context_, const String & name, bool sync) { checkIsInternalQuery(context_, "DROP TABLE"); - DatabaseAtomic::dropTable(context_, name, no_delay); + DatabaseAtomic::dropTable(context_, name, sync); } void DatabaseMaterializedMySQL::attachTable(ContextPtr context_, const String & name, const StoragePtr & table, const String & relative_table_path) diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.h b/src/Databases/MySQL/DatabaseMaterializedMySQL.h index 32686784f2a..a6810f29d87 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.h +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.h @@ -52,7 +52,7 @@ public: void createTable(ContextPtr context_, const String & name, const StoragePtr & table, const ASTPtr & query) override; - void dropTable(ContextPtr context_, const String & name, bool no_delay) override; + void dropTable(ContextPtr context_, const String & name, bool sync) override; void attachTable(ContextPtr context_, const String & name, const StoragePtr & table, const String & relative_table_path) override; diff --git a/src/Databases/MySQL/DatabaseMySQL.cpp b/src/Databases/MySQL/DatabaseMySQL.cpp index 58be682bd73..95098ba9cbd 100644 --- a/src/Databases/MySQL/DatabaseMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMySQL.cpp @@ -447,7 +447,7 @@ void DatabaseMySQL::detachTablePermanently(ContextPtr, const String & table_name table_iter->second.second->is_dropped = true; } -void DatabaseMySQL::dropTable(ContextPtr local_context, const String & table_name, bool /*no_delay*/) +void DatabaseMySQL::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/) { detachTablePermanently(local_context, table_name); } diff --git a/src/Databases/MySQL/DatabaseMySQL.h b/src/Databases/MySQL/DatabaseMySQL.h index 1ee090ecd52..542cd65c1f1 100644 --- a/src/Databases/MySQL/DatabaseMySQL.h +++ b/src/Databases/MySQL/DatabaseMySQL.h @@ -82,7 +82,7 @@ public: void detachTablePermanently(ContextPtr context, const String & table_name) override; - void dropTable(ContextPtr context, const String & table_name, bool no_delay) override; + void dropTable(ContextPtr context, const String & table_name, bool sync) override; void attachTable(ContextPtr context, const String & table_name, const StoragePtr & storage, const String & relative_table_path) override; @@ -109,15 +109,15 @@ private: void cleanOutdatedTables(); - void fetchTablesIntoLocalCache(ContextPtr context) const; + void fetchTablesIntoLocalCache(ContextPtr context) const TSA_REQUIRES(mutex); std::map fetchTablesWithModificationTime(ContextPtr local_context) const; std::map fetchTablesColumnsList(const std::vector & tables_name, ContextPtr context) const; - void destroyLocalCacheExtraTables(const std::map & tables_with_modification_time) const; + void destroyLocalCacheExtraTables(const std::map & tables_with_modification_time) const TSA_REQUIRES(mutex); - void fetchLatestTablesStructureIntoCache(const std::map & tables_modification_time, ContextPtr context) const; + void fetchLatestTablesStructureIntoCache(const std::map & tables_modification_time, ContextPtr context) const TSA_REQUIRES(mutex); ThreadFromGlobalPool thread; }; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index db184342a97..8b85d1b9a63 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -63,9 +63,9 @@ void DatabaseMaterializedPostgreSQL::startSynchronization() return; replication_handler = std::make_unique( - /* replication_identifier */database_name, + /* replication_identifier */ TSA_SUPPRESS_WARNING_FOR_READ(database_name), /// FIXME remote_database_name, - database_name, + TSA_SUPPRESS_WARNING_FOR_READ(database_name), /// FIXME connection_info, getContext(), is_attach, @@ -99,7 +99,8 @@ void DatabaseMaterializedPostgreSQL::startSynchronization() else { /// Nested table does not exist and will be created by replication thread. - storage = std::make_shared(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + /// FIXME TSA + storage = std::make_shared(StorageID(TSA_SUPPRESS_WARNING_FOR_READ(database_name), table_name), getContext(), remote_database_name, table_name); } /// Cache MaterializedPostgreSQL wrapper over nested table. @@ -210,7 +211,8 @@ ASTPtr DatabaseMaterializedPostgreSQL::getCreateTableQueryImpl(const String & ta std::lock_guard lock(handler_mutex); - auto storage = std::make_shared(StorageID(database_name, table_name), getContext(), remote_database_name, table_name); + /// FIXME TSA + auto storage = std::make_shared(StorageID(TSA_SUPPRESS_WARNING_FOR_READ(database_name), table_name), getContext(), remote_database_name, table_name); auto ast_storage = replication_handler->getCreateNestedTableQuery(storage.get(), table_name); assert_cast(ast_storage.get())->uuid = UUIDHelpers::generateV4(); return ast_storage; @@ -234,7 +236,7 @@ ASTPtr DatabaseMaterializedPostgreSQL::createAlterSettingsQuery(const SettingCha auto * alter = query->as(); alter->alter_object = ASTAlterQuery::AlterObjectType::DATABASE; - alter->setDatabase(database_name); + alter->setDatabase(TSA_SUPPRESS_WARNING_FOR_READ(database_name)); /// FIXME alter->set(alter->command_list, command_list); return query; @@ -390,10 +392,10 @@ void DatabaseMaterializedPostgreSQL::stopReplication() } -void DatabaseMaterializedPostgreSQL::dropTable(ContextPtr local_context, const String & table_name, bool no_delay) +void DatabaseMaterializedPostgreSQL::dropTable(ContextPtr local_context, const String & table_name, bool sync) { /// Modify context into nested_context and pass query to Atomic database. - DatabaseAtomic::dropTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, no_delay); + DatabaseAtomic::dropTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, sync); } diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index 08420f4ba5e..ac2bcedca60 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -55,7 +55,7 @@ public: StoragePtr detachTable(ContextPtr context, const String & table_name) override; - void dropTable(ContextPtr local_context, const String & name, bool no_delay) override; + void dropTable(ContextPtr local_context, const String & name, bool sync) override; void drop(ContextPtr local_context) override; diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp index 1bcc203beb9..c4b815c0c9f 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp @@ -264,7 +264,7 @@ void DatabasePostgreSQL::createTable(ContextPtr local_context, const String & ta } -void DatabasePostgreSQL::dropTable(ContextPtr, const String & table_name, bool /* no_delay */) +void DatabasePostgreSQL::dropTable(ContextPtr, const String & table_name, bool /* sync */) { std::lock_guard lock{mutex}; @@ -369,7 +369,11 @@ ASTPtr DatabasePostgreSQL::getCreateDatabaseQuery() const ASTPtr DatabasePostgreSQL::getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const { - auto storage = fetchTable(table_name, local_context, false); + StoragePtr storage; + { + std::lock_guard lock{mutex}; + storage = fetchTable(table_name, local_context, false); + } if (!storage) { if (throw_on_error) diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.h b/src/Databases/PostgreSQL/DatabasePostgreSQL.h index 3397dcc8076..fe4dff2ca93 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.h @@ -53,7 +53,7 @@ public: StoragePtr tryGetTable(const String & name, ContextPtr context) const override; void createTable(ContextPtr, const String & table_name, const StoragePtr & storage, const ASTPtr & create_query) override; - void dropTable(ContextPtr, const String & table_name, bool no_delay) override; + void dropTable(ContextPtr, const String & table_name, bool sync) override; void attachTable(ContextPtr context, const String & table_name, const StoragePtr & storage, const String & relative_table_path) override; StoragePtr detachTable(ContextPtr context, const String & table_name) override; @@ -81,7 +81,7 @@ private: bool checkPostgresTable(const String & table_name) const; - StoragePtr fetchTable(const String & table_name, ContextPtr context, bool table_checked) const; + StoragePtr fetchTable(const String & table_name, ContextPtr context, bool table_checked) const TSA_REQUIRES(mutex); void removeOutdatedTables(); diff --git a/src/Databases/SQLite/DatabaseSQLite.cpp b/src/Databases/SQLite/DatabaseSQLite.cpp index 7a4844e4d69..44a392ce1f2 100644 --- a/src/Databases/SQLite/DatabaseSQLite.cpp +++ b/src/Databases/SQLite/DatabaseSQLite.cpp @@ -173,12 +173,16 @@ ASTPtr DatabaseSQLite::getCreateDatabaseQuery() const ASTPtr DatabaseSQLite::getCreateTableQueryImpl(const String & table_name, ContextPtr local_context, bool throw_on_error) const { - auto storage = fetchTable(table_name, local_context, false); + StoragePtr storage; + { + std::lock_guard lock(mutex); + storage = fetchTable(table_name, local_context, false); + } if (!storage) { if (throw_on_error) throw Exception(ErrorCodes::UNKNOWN_TABLE, "SQLite table {}.{} does not exist", - database_name, table_name); + getDatabaseName(), table_name); return nullptr; } auto table_storage_define = database_engine_define->clone(); diff --git a/src/Databases/SQLite/DatabaseSQLite.h b/src/Databases/SQLite/DatabaseSQLite.h index c8df79d0f6a..8f0c9b4d720 100644 --- a/src/Databases/SQLite/DatabaseSQLite.h +++ b/src/Databases/SQLite/DatabaseSQLite.h @@ -54,9 +54,9 @@ private: bool checkSQLiteTable(const String & table_name) const; - NameSet fetchTablesList() const; + NameSet fetchTablesList() const TSA_REQUIRES(mutex); - StoragePtr fetchTable(const String & table_name, ContextPtr context, bool table_checked) const; + StoragePtr fetchTable(const String & table_name, ContextPtr context, bool table_checked) const TSA_REQUIRES(mutex); }; diff --git a/src/Databases/TablesLoader.h b/src/Databases/TablesLoader.h index a14a28c487a..189906df6ff 100644 --- a/src/Databases/TablesLoader.h +++ b/src/Databases/TablesLoader.h @@ -12,7 +12,7 @@ namespace Poco { - class Logger; + class Logger; // NOLINT(cppcoreguidelines-virtual-class-destructor) } class AtomicStopwatch; diff --git a/src/Dictionaries/CacheDictionaryUpdateQueue.cpp b/src/Dictionaries/CacheDictionaryUpdateQueue.cpp index 2077f846f09..aee1f0de2f6 100644 --- a/src/Dictionaries/CacheDictionaryUpdateQueue.cpp +++ b/src/Dictionaries/CacheDictionaryUpdateQueue.cpp @@ -135,14 +135,14 @@ void CacheDictionaryUpdateQueue::updateThreadFunction() /// Notify thread about finished updating the bunch of ids /// where their own ids were included. - std::unique_lock lock(update_mutex); + std::lock_guard lock(update_mutex); unit_to_update->is_done = true; is_update_finished.notify_all(); } catch (...) { - std::unique_lock lock(update_mutex); + std::lock_guard lock(update_mutex); unit_to_update->current_exception = std::current_exception(); // NOLINT(bugprone-throw-keyword-missing) is_update_finished.notify_all(); diff --git a/src/Dictionaries/RedisDictionarySource.h b/src/Dictionaries/RedisDictionarySource.h index bf745a7bb41..26f5ab2a613 100644 --- a/src/Dictionaries/RedisDictionarySource.h +++ b/src/Dictionaries/RedisDictionarySource.h @@ -8,11 +8,6 @@ namespace Poco { - namespace Util - { - class AbstractConfiguration; - } - namespace Redis { class Client; diff --git a/src/Disks/DiskCacheWrapper.cpp b/src/Disks/DiskCacheWrapper.cpp index 8e355f70432..45c98224966 100644 --- a/src/Disks/DiskCacheWrapper.cpp +++ b/src/Disks/DiskCacheWrapper.cpp @@ -90,7 +90,7 @@ DiskCacheWrapper::DiskCacheWrapper( std::shared_ptr DiskCacheWrapper::acquireDownloadMetadata(const String & path) const { - std::unique_lock lock{mutex}; + std::lock_guard lock{mutex}; auto it = file_downloads.find(path); if (it != file_downloads.end()) @@ -101,7 +101,7 @@ std::shared_ptr DiskCacheWrapper::acquireDownloadMetadata( new FileDownloadMetadata, [this, path] (FileDownloadMetadata * p) { - std::unique_lock erase_lock{mutex}; + std::lock_guard erase_lock{mutex}; file_downloads.erase(path); delete p; }); diff --git a/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.cpp b/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.cpp index 32fd285dcdb..c25d1d7470c 100644 --- a/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.cpp +++ b/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.cpp @@ -113,7 +113,7 @@ std::unique_ptr AzureObjectStorage::writeObject( /// NO return std::make_unique(std::move(buffer), std::move(finalize_callback), path); } -void AzureObjectStorage::listPrefix(const std::string & path, PathsWithSize & children) const +void AzureObjectStorage::listPrefix(const std::string & path, RelativePathsWithSize & children) const { auto client_ptr = client.get(); diff --git a/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h b/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h index 559be0ad257..ab7d2b28508 100644 --- a/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h +++ b/src/Disks/ObjectStorages/AzureBlobStorage/AzureObjectStorage.h @@ -73,7 +73,7 @@ public: size_t buf_size = DBMS_DEFAULT_BUFFER_SIZE, const WriteSettings & write_settings = {}) override; - void listPrefix(const std::string & path, PathsWithSize & children) const override; + void listPrefix(const std::string & path, RelativePathsWithSize & children) const override; /// Remove file. Throws exception if file doesn't exists or it's a directory. void removeObject(const std::string & path) override; diff --git a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp index 4564e84316d..a2c0c8abd36 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp +++ b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp @@ -51,7 +51,7 @@ void DiskObjectStorageMetadata::deserialize(ReadBuffer & buf) remote_fs_object_path = remote_fs_object_path.substr(remote_fs_root_path.size()); } assertChar('\n', buf); - storage_objects[i].relative_path = remote_fs_object_path; + storage_objects[i].path = remote_fs_object_path; storage_objects[i].bytes_size = remote_fs_object_size; } diff --git a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.h b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.h index adebbcb952d..0f2a2a5507d 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.h +++ b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.h @@ -12,17 +12,6 @@ namespace DB struct DiskObjectStorageMetadata { private: - struct RelativePathWithSize - { - String relative_path; - size_t bytes_size; - - RelativePathWithSize() = default; - - RelativePathWithSize(const String & relative_path_, size_t bytes_size_) - : relative_path(relative_path_), bytes_size(bytes_size_) {} - }; - /// Metadata file version. static constexpr uint32_t VERSION_ABSOLUTE_PATHS = 1; static constexpr uint32_t VERSION_RELATIVE_PATHS = 2; @@ -31,7 +20,7 @@ private: const std::string & common_metadata_path; /// Relative paths of blobs. - std::vector storage_objects; + RelativePathsWithSize storage_objects; /// URI const std::string & remote_fs_root_path; @@ -71,7 +60,7 @@ public: return remote_fs_root_path; } - std::vector getBlobsRelativePaths() const + RelativePathsWithSize getBlobsRelativePaths() const { return storage_objects; } diff --git a/src/Disks/ObjectStorages/DiskObjectStorageRemoteMetadataRestoreHelper.cpp b/src/Disks/ObjectStorages/DiskObjectStorageRemoteMetadataRestoreHelper.cpp index a8140e8954e..c9dbb5de078 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorageRemoteMetadataRestoreHelper.cpp +++ b/src/Disks/ObjectStorages/DiskObjectStorageRemoteMetadataRestoreHelper.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -379,7 +380,7 @@ void DiskObjectStorageRemoteMetadataRestoreHelper::restoreFiles(IObjectStorage * return true; }; - PathsWithSize children; + RelativePathsWithSize children; source_object_storage->listPrefix(restore_information.source_path, children); restore_files(children); @@ -523,7 +524,7 @@ void DiskObjectStorageRemoteMetadataRestoreHelper::restoreFileOperations(IObject return true; }; - PathsWithSize children; + RelativePathsWithSize children; source_object_storage->listPrefix(restore_information.source_path + "operations/", children); restore_file_operations(children); diff --git a/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.cpp b/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.cpp index 82c700e1a63..bedd1a83df1 100644 --- a/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.cpp +++ b/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.cpp @@ -81,7 +81,7 @@ std::unique_ptr HDFSObjectStorage::writeObject( /// NOL } -void HDFSObjectStorage::listPrefix(const std::string & path, PathsWithSize & children) const +void HDFSObjectStorage::listPrefix(const std::string & path, RelativePathsWithSize & children) const { const size_t begin_of_path = path.find('/', path.find("//") + 2); int32_t num_entries; diff --git a/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h b/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h index 28f553906ea..69878568548 100644 --- a/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h +++ b/src/Disks/ObjectStorages/HDFS/HDFSObjectStorage.h @@ -75,7 +75,7 @@ public: size_t buf_size = DBMS_DEFAULT_BUFFER_SIZE, const WriteSettings & write_settings = {}) override; - void listPrefix(const std::string & path, PathsWithSize & children) const override; + void listPrefix(const std::string & path, RelativePathsWithSize & children) const override; /// Remove file. Throws exception if file doesn't exists or it's a directory. void removeObject(const std::string & path) override; diff --git a/src/Disks/ObjectStorages/IObjectStorage.h b/src/Disks/ObjectStorages/IObjectStorage.h index ad8d5d2530e..7532f2a3267 100644 --- a/src/Disks/ObjectStorages/IObjectStorage.h +++ b/src/Disks/ObjectStorages/IObjectStorage.h @@ -41,6 +41,7 @@ struct PathWithSize /// List of paths with their sizes using PathsWithSize = std::vector; +using RelativePathsWithSize = PathsWithSize; struct ObjectMetadata { @@ -65,7 +66,7 @@ public: virtual bool exists(const std::string & path) const = 0; /// List on prefix, return children (relative paths) with their sizes. - virtual void listPrefix(const std::string & path, PathsWithSize & children) const = 0; + virtual void listPrefix(const std::string & path, RelativePathsWithSize & children) const = 0; /// Get object metadata if supported. It should be possible to receive /// at least size of object diff --git a/src/Disks/ObjectStorages/MetadataStorageFromDisk.cpp b/src/Disks/ObjectStorages/MetadataStorageFromDisk.cpp index 32e6fe5834d..22502a6e2d1 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromDisk.cpp +++ b/src/Disks/ObjectStorages/MetadataStorageFromDisk.cpp @@ -56,7 +56,7 @@ void MetadataStorageFromDiskTransaction::commit() toString(state), toString(MetadataFromDiskTransactionState::PREPARING)); { - std::unique_lock lock(metadata_storage.metadata_mutex); + std::lock_guard lock(metadata_storage.metadata_mutex); for (size_t i = 0; i < operations.size(); ++i) { try diff --git a/src/Disks/ObjectStorages/S3/ProxyResolverConfiguration.cpp b/src/Disks/ObjectStorages/S3/ProxyResolverConfiguration.cpp index 983e36489e6..109ccf0eba7 100644 --- a/src/Disks/ObjectStorages/S3/ProxyResolverConfiguration.cpp +++ b/src/Disks/ObjectStorages/S3/ProxyResolverConfiguration.cpp @@ -28,7 +28,7 @@ ClientConfigurationPerRequest ProxyResolverConfiguration::getConfiguration(const { LOG_DEBUG(&Poco::Logger::get("AWSClient"), "Obtain proxy using resolver: {}", endpoint.toString()); - std::unique_lock lock(cache_mutex); + std::lock_guard lock(cache_mutex); std::chrono::time_point now = std::chrono::system_clock::now(); @@ -110,7 +110,7 @@ void ProxyResolverConfiguration::errorReport(const ClientConfigurationPerRequest if (config.proxy_host.empty()) return; - std::unique_lock lock(cache_mutex); + std::lock_guard lock(cache_mutex); if (!cache_ttl.count() || !cache_valid) return; diff --git a/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp b/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp index 58bd29d2d73..9236cde6e93 100644 --- a/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp +++ b/src/Disks/ObjectStorages/S3/S3ObjectStorage.cpp @@ -195,7 +195,7 @@ std::unique_ptr S3ObjectStorage::writeObject( /// NOLIN return std::make_unique(std::move(s3_buffer), std::move(finalize_callback), path); } -void S3ObjectStorage::listPrefix(const std::string & path, PathsWithSize & children) const +void S3ObjectStorage::listPrefix(const std::string & path, RelativePathsWithSize & children) const { auto settings_ptr = s3_settings.get(); auto client_ptr = client.get(); diff --git a/src/Disks/ObjectStorages/S3/S3ObjectStorage.h b/src/Disks/ObjectStorages/S3/S3ObjectStorage.h index 5c53ea1f894..5d4300bffd3 100644 --- a/src/Disks/ObjectStorages/S3/S3ObjectStorage.h +++ b/src/Disks/ObjectStorages/S3/S3ObjectStorage.h @@ -80,7 +80,7 @@ public: size_t buf_size = DBMS_DEFAULT_BUFFER_SIZE, const WriteSettings & write_settings = {}) override; - void listPrefix(const std::string & path, PathsWithSize & children) const override; + void listPrefix(const std::string & path, RelativePathsWithSize & children) const override; /// Remove file. Throws exception if file doesn't exist or it's a directory. void removeObject(const std::string & path) override; diff --git a/src/Formats/FormatSettings.h b/src/Formats/FormatSettings.h index 7e0ce001405..0032aea57e4 100644 --- a/src/Formats/FormatSettings.h +++ b/src/Formats/FormatSettings.h @@ -36,7 +36,7 @@ struct FormatSettings bool seekable_read = true; UInt64 max_rows_to_read_for_schema_inference = 100; - String column_names_for_schema_inference = ""; + String column_names_for_schema_inference; enum class DateTimeInputFormat { diff --git a/src/Functions/FunctionsConversion.h b/src/Functions/FunctionsConversion.h index 038bb9bf3df..e0c42401207 100644 --- a/src/Functions/FunctionsConversion.h +++ b/src/Functions/FunctionsConversion.h @@ -205,7 +205,7 @@ struct ConvertImpl if constexpr (std::is_same_v != std::is_same_v) { - throw Exception("Conversion between numeric types and UUID is not supported", ErrorCodes::NOT_IMPLEMENTED); + throw Exception("Conversion between numeric types and UUID is not supported. Probably the passed UUID is unquoted", ErrorCodes::NOT_IMPLEMENTED); } else { diff --git a/src/IO/BufferWithOwnMemory.h b/src/IO/BufferWithOwnMemory.h index 0d571d6ae7c..479e0e88fcc 100644 --- a/src/IO/BufferWithOwnMemory.h +++ b/src/IO/BufferWithOwnMemory.h @@ -181,7 +181,7 @@ public: } private: - void nextImpl() override final + void nextImpl() final { const size_t prev_size = Base::position() - memory.data(); memory.resize(2 * prev_size + 1); diff --git a/src/IO/HTTPCommon.cpp b/src/IO/HTTPCommon.cpp index 7ed2f343209..b31ee08eaf5 100644 --- a/src/IO/HTTPCommon.cpp +++ b/src/IO/HTTPCommon.cpp @@ -212,7 +212,7 @@ namespace size_t max_connections_per_endpoint, bool resolve_host = true) { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); const std::string & host = uri.getHost(); UInt16 port = uri.getPort(); bool https = isHTTPS(uri); diff --git a/src/IO/S3Common.cpp b/src/IO/S3Common.cpp index a58732287e9..cea03277c91 100644 --- a/src/IO/S3Common.cpp +++ b/src/IO/S3Common.cpp @@ -148,7 +148,7 @@ public: { String credentials_string; { - std::unique_lock locker(token_mutex); + std::lock_guard locker(token_mutex); LOG_TRACE(logger, "Getting default credentials for EC2 instance."); auto result = GetResourceWithAWSWebServiceResult(endpoint.c_str(), EC2_SECURITY_CREDENTIALS_RESOURCE, nullptr); @@ -194,7 +194,7 @@ public: String new_token; { - std::unique_lock locker(token_mutex); + std::lock_guard locker(token_mutex); Aws::StringStream ss; ss << endpoint << EC2_IMDS_TOKEN_RESOURCE; diff --git a/src/IO/WriteBuffer.h b/src/IO/WriteBuffer.h index bd2a628355e..1fc21e1ac17 100644 --- a/src/IO/WriteBuffer.h +++ b/src/IO/WriteBuffer.h @@ -1,11 +1,10 @@ #pragma once #include -#include #include #include #include -#include +#include #include #include diff --git a/src/IO/WriteBufferFromVector.h b/src/IO/WriteBufferFromVector.h index d74b366b8e2..521acb6c8d6 100644 --- a/src/IO/WriteBufferFromVector.h +++ b/src/IO/WriteBufferFromVector.h @@ -64,7 +64,7 @@ public: } private: - void finalizeImpl() override final + void finalizeImpl() override { vector.resize( ((position() - reinterpret_cast(vector.data())) /// NOLINT diff --git a/src/Interpreters/AsynchronousInsertQueue.cpp b/src/Interpreters/AsynchronousInsertQueue.cpp index fd8f252d139..ae39c7035dd 100644 --- a/src/Interpreters/AsynchronousInsertQueue.cpp +++ b/src/Interpreters/AsynchronousInsertQueue.cpp @@ -215,7 +215,7 @@ void AsynchronousInsertQueue::push(ASTPtr query, ContextPtr query_context) } } - std::unique_lock write_lock(rwlock); + std::lock_guard write_lock(rwlock); auto it = queue.emplace(key, std::make_shared()).first; pushImpl(std::move(entry), it); } @@ -343,7 +343,7 @@ void AsynchronousInsertQueue::cleanup() if (!keys_to_remove.empty()) { - std::unique_lock write_lock(rwlock); + std::lock_guard write_lock(rwlock); size_t total_removed = 0; for (const auto & key : keys_to_remove) diff --git a/src/Interpreters/AsynchronousInsertQueue.h b/src/Interpreters/AsynchronousInsertQueue.h index db3cb3049fd..8a4e8dad8dd 100644 --- a/src/Interpreters/AsynchronousInsertQueue.h +++ b/src/Interpreters/AsynchronousInsertQueue.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include diff --git a/src/Interpreters/DDLWorker.h b/src/Interpreters/DDLWorker.h index dc93b602a29..5dc6d4acbe5 100644 --- a/src/Interpreters/DDLWorker.h +++ b/src/Interpreters/DDLWorker.h @@ -124,7 +124,7 @@ protected: std::string queue_dir; /// dir with queue of queries mutable std::mutex zookeeper_mutex; - ZooKeeperPtr current_zookeeper; + ZooKeeperPtr current_zookeeper TSA_GUARDED_BY(zookeeper_mutex); /// Save state of executed task to avoid duplicate execution on ZK error std::optional last_skipped_entry_name; diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index 13340221f38..a0579b813db 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -205,7 +205,10 @@ void DatabaseCatalog::shutdownImpl() for (auto & database : current_databases) database.second->shutdown(); - tables_marked_dropped.clear(); + { + std::lock_guard lock(tables_marked_dropped_mutex); + tables_marked_dropped.clear(); + } std::lock_guard lock(databases_mutex); for (const auto & db : databases) @@ -223,6 +226,7 @@ void DatabaseCatalog::shutdownImpl() auto & table = mapping.second.second; return db || table; }; + std::lock_guard map_lock{elem.mutex}; auto it = std::find_if(elem.map.begin(), elem.map.end(), not_empty_mapping); return it != elem.map.end(); }) == uuid_map.end()); @@ -689,7 +693,8 @@ DatabaseCatalog::updateDependency(const StorageID & old_from, const StorageID & DDLGuardPtr DatabaseCatalog::getDDLGuard(const String & database, const String & table) { std::unique_lock lock(ddl_guards_mutex); - auto db_guard_iter = ddl_guards.try_emplace(database).first; + /// TSA does not support unique_lock + auto db_guard_iter = TSA_SUPPRESS_WARNING_FOR_WRITE(ddl_guards).try_emplace(database).first; DatabaseGuard & db_guard = db_guard_iter->second; return std::make_unique(db_guard.first, db_guard.second, std::move(lock), table, database); } @@ -698,7 +703,7 @@ std::unique_lock DatabaseCatalog::getExclusiveDDLGuardForData { DDLGuards::iterator db_guard_iter; { - std::unique_lock lock(ddl_guards_mutex); + std::lock_guard lock(ddl_guards_mutex); db_guard_iter = ddl_guards.try_emplace(database).first; assert(db_guard_iter->second.first.contains("")); } @@ -999,7 +1004,7 @@ void DatabaseCatalog::waitTableFinallyDropped(const UUID & uuid) LOG_DEBUG(log, "Waiting for table {} to be finally dropped", toString(uuid)); std::unique_lock lock{tables_marked_dropped_mutex}; - wait_table_finally_dropped.wait(lock, [&]() + wait_table_finally_dropped.wait(lock, [&]() TSA_REQUIRES(tables_marked_dropped_mutex) -> bool { return !tables_marked_dropped_ids.contains(uuid); }); diff --git a/src/Interpreters/DatabaseCatalog.h b/src/Interpreters/DatabaseCatalog.h index d82a0594c9a..4468cc3a5d8 100644 --- a/src/Interpreters/DatabaseCatalog.h +++ b/src/Interpreters/DatabaseCatalog.h @@ -221,7 +221,7 @@ public: DependenciesInfo getLoadingDependenciesInfo(const StorageID & table_id) const; TableNamesSet tryRemoveLoadingDependencies(const StorageID & table_id, bool check_dependencies, bool is_drop_database = false); - TableNamesSet tryRemoveLoadingDependenciesUnlocked(const QualifiedTableName & removing_table, bool check_dependencies, bool is_drop_database = false); + TableNamesSet tryRemoveLoadingDependenciesUnlocked(const QualifiedTableName & removing_table, bool check_dependencies, bool is_drop_database = false) TSA_REQUIRES(databases_mutex); void checkTableCanBeRemovedOrRenamed(const StorageID & table_id) const; void updateLoadingDependencies(const StorageID & table_id, TableNamesSet && new_dependencies); @@ -233,15 +233,15 @@ private: static std::unique_ptr database_catalog; explicit DatabaseCatalog(ContextMutablePtr global_context_); - void assertDatabaseExistsUnlocked(const String & database_name) const; - void assertDatabaseDoesntExistUnlocked(const String & database_name) const; + void assertDatabaseExistsUnlocked(const String & database_name) const TSA_REQUIRES(databases_mutex); + void assertDatabaseDoesntExistUnlocked(const String & database_name) const TSA_REQUIRES(databases_mutex); void shutdownImpl(); struct UUIDToStorageMapPart { - std::unordered_map map; + std::unordered_map map TSA_GUARDED_BY(mutex); mutable std::mutex mutex; }; @@ -273,12 +273,12 @@ private: mutable std::mutex databases_mutex; - ViewDependencies view_dependencies; + ViewDependencies view_dependencies TSA_GUARDED_BY(databases_mutex); - Databases databases; + Databases databases TSA_GUARDED_BY(databases_mutex); UUIDToStorageMap uuid_map; - DependenciesInfos loading_dependencies; + DependenciesInfos loading_dependencies TSA_GUARDED_BY(databases_mutex); Poco::Logger * log; @@ -290,12 +290,12 @@ private: /// In case the element already exists, waits when query will be executed in other thread. See class DDLGuard below. using DatabaseGuard = std::pair; using DDLGuards = std::map; - DDLGuards ddl_guards; + DDLGuards ddl_guards TSA_GUARDED_BY(ddl_guards_mutex); /// If you capture mutex and ddl_guards_mutex, then you need to grab them strictly in this order. mutable std::mutex ddl_guards_mutex; - TablesMarkedAsDropped tables_marked_dropped; - std::unordered_set tables_marked_dropped_ids; + TablesMarkedAsDropped tables_marked_dropped TSA_GUARDED_BY(tables_marked_dropped_mutex); + std::unordered_set tables_marked_dropped_ids TSA_GUARDED_BY(tables_marked_dropped_mutex); mutable std::mutex tables_marked_dropped_mutex; std::unique_ptr drop_task; diff --git a/src/Interpreters/EmbeddedDictionaries.cpp b/src/Interpreters/EmbeddedDictionaries.cpp index b9bc5cd1e5c..0b2efaf3dbe 100644 --- a/src/Interpreters/EmbeddedDictionaries.cpp +++ b/src/Interpreters/EmbeddedDictionaries.cpp @@ -60,7 +60,7 @@ bool EmbeddedDictionaries::reloadDictionary( bool EmbeddedDictionaries::reloadImpl(const bool throw_on_error, const bool force_reload) { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); /** If you can not update the directories, then despite this, do not throw an exception (use the old directories). * If there are no old correct directories, then when using functions that depend on them, diff --git a/src/Interpreters/HashJoin.h b/src/Interpreters/HashJoin.h index b2a7aedaa5a..17d09b25ea1 100644 --- a/src/Interpreters/HashJoin.h +++ b/src/Interpreters/HashJoin.h @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include @@ -339,7 +339,7 @@ public: /// We keep correspondence between used_flags and hash table internal buffer. /// Hash table cannot be modified during HashJoin lifetime and must be protected with lock. - void setLock(RWLockImpl::LockHolder rwlock_holder) + void setLock(TableLockHolder rwlock_holder) { storage_join_lock = rwlock_holder; } @@ -394,7 +394,7 @@ private: /// Should be set via setLock to protect hash table from modification from StorageJoin /// If set HashJoin instance is not available for modification (addJoinedBlock) - RWLockImpl::LockHolder storage_join_lock = nullptr; + TableLockHolder storage_join_lock = nullptr; void dataMapInit(MapsVariant &); diff --git a/src/Interpreters/InterpreterCreateIndexQuery.cpp b/src/Interpreters/InterpreterCreateIndexQuery.cpp new file mode 100644 index 00000000000..29c151d1e4d --- /dev/null +++ b/src/Interpreters/InterpreterCreateIndexQuery.cpp @@ -0,0 +1,77 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +namespace ErrorCodes +{ + extern const int TABLE_IS_READ_ONLY; +} + + +BlockIO InterpreterCreateIndexQuery::execute() +{ + const auto & create_index = query_ptr->as(); + + AccessRightsElements required_access; + required_access.emplace_back(AccessType::ALTER_ADD_INDEX, create_index.getDatabase(), create_index.getTable()); + + if (!create_index.cluster.empty()) + { + DDLQueryOnClusterParams params; + params.access_to_check = std::move(required_access); + return executeDDLQueryOnCluster(query_ptr, getContext(), params); + } + + getContext()->checkAccess(required_access); + auto table_id = getContext()->resolveStorageID(create_index, Context::ResolveOrdinary); + query_ptr->as().setDatabase(table_id.database_name); + + DatabasePtr database = DatabaseCatalog::instance().getDatabase(table_id.database_name); + if (typeid_cast(database.get()) + && !getContext()->getClientInfo().is_replicated_database_internal) + { + auto guard = DatabaseCatalog::instance().getDDLGuard(table_id.database_name, table_id.table_name); + guard->releaseTableLock(); + return typeid_cast(database.get())->tryEnqueueReplicatedDDL(query_ptr, getContext()); + } + + StoragePtr table = DatabaseCatalog::instance().getTable(table_id, getContext()); + if (table->isStaticStorage()) + throw Exception(ErrorCodes::TABLE_IS_READ_ONLY, "Table is read-only"); + + /// Convert ASTCreateIndexQuery to AlterCommand. + AlterCommands alter_commands; + + AlterCommand command; + command.index_decl = create_index.index_decl; + command.type = AlterCommand::ADD_INDEX; + command.index_name = create_index.index_name->as().name(); + command.if_not_exists = create_index.if_not_exists; + + /// Fill name in ASTIndexDeclaration + auto & ast_index_decl = command.index_decl->as(); + ast_index_decl.name = command.index_name; + + alter_commands.emplace_back(std::move(command)); + + auto alter_lock = table->lockForAlter(getContext()->getSettingsRef().lock_acquire_timeout); + StorageInMemoryMetadata metadata = table->getInMemoryMetadata(); + alter_commands.validate(table, getContext()); + alter_commands.prepare(metadata); + table->checkAlterIsPossible(alter_commands, getContext()); + table->alter(alter_commands, getContext(), alter_lock); + + return {}; +} + +} diff --git a/src/Interpreters/InterpreterCreateIndexQuery.h b/src/Interpreters/InterpreterCreateIndexQuery.h new file mode 100644 index 00000000000..63eaaf5f1c2 --- /dev/null +++ b/src/Interpreters/InterpreterCreateIndexQuery.h @@ -0,0 +1,24 @@ +#pragma once + +#include + + +namespace DB +{ + +class Context; + +class InterpreterCreateIndexQuery : public IInterpreter, WithContext +{ +public: + InterpreterCreateIndexQuery(const ASTPtr & query_ptr_, ContextPtr context_) + : WithContext(context_) + , query_ptr(query_ptr_) {} + + BlockIO execute() override; + +private: + ASTPtr query_ptr; +}; + +} diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 75d00fcb8a7..20d88c91709 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -150,10 +150,9 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) /// When attaching old-style database during server startup, we must always use Ordinary engine if (create.attach) throw Exception("Database engine must be specified for ATTACH DATABASE query", ErrorCodes::UNKNOWN_DATABASE_ENGINE); - bool old_style_database = getContext()->getSettingsRef().default_database_engine.value == DefaultDatabaseEngine::Ordinary; auto engine = std::make_shared(); auto storage = std::make_shared(); - engine->name = old_style_database ? "Ordinary" : "Atomic"; + engine->name = "Atomic"; engine->no_empty_args = true; storage->set(storage->engine, engine); create.set(create.storage, storage); @@ -196,8 +195,7 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) if (create_from_user) { - const auto & default_engine = getContext()->getSettingsRef().default_database_engine.value; - if (create.uuid == UUIDHelpers::Nil && default_engine == DefaultDatabaseEngine::Atomic) + if (create.uuid == UUIDHelpers::Nil) create.uuid = UUIDHelpers::generateV4(); /// Will enable Atomic engine for nested database } else if (attach_from_user && create.uuid == UUIDHelpers::Nil) diff --git a/src/Interpreters/InterpreterDropIndexQuery.cpp b/src/Interpreters/InterpreterDropIndexQuery.cpp new file mode 100644 index 00000000000..6cc9334fad2 --- /dev/null +++ b/src/Interpreters/InterpreterDropIndexQuery.cpp @@ -0,0 +1,71 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +namespace ErrorCodes +{ + extern const int TABLE_IS_READ_ONLY; +} + + +BlockIO InterpreterDropIndexQuery::execute() +{ + const auto & drop_index = query_ptr->as(); + + AccessRightsElements required_access; + required_access.emplace_back(AccessType::ALTER_DROP_INDEX, drop_index.getDatabase(), drop_index.getTable()); + + if (!drop_index.cluster.empty()) + { + DDLQueryOnClusterParams params; + params.access_to_check = std::move(required_access); + return executeDDLQueryOnCluster(query_ptr, getContext(), params); + } + + getContext()->checkAccess(required_access); + auto table_id = getContext()->resolveStorageID(drop_index, Context::ResolveOrdinary); + query_ptr->as().setDatabase(table_id.database_name); + + DatabasePtr database = DatabaseCatalog::instance().getDatabase(table_id.database_name); + if (typeid_cast(database.get()) + && !getContext()->getClientInfo().is_replicated_database_internal) + { + auto guard = DatabaseCatalog::instance().getDDLGuard(table_id.database_name, table_id.table_name); + guard->releaseTableLock(); + return typeid_cast(database.get())->tryEnqueueReplicatedDDL(query_ptr, getContext()); + } + + StoragePtr table = DatabaseCatalog::instance().getTable(table_id, getContext()); + if (table->isStaticStorage()) + throw Exception(ErrorCodes::TABLE_IS_READ_ONLY, "Table is read-only"); + + /// Convert ASTDropIndexQuery to AlterCommand. + AlterCommands alter_commands; + + AlterCommand command; + command.ast = drop_index.convertToASTAlterCommand(); + command.type = AlterCommand::DROP_INDEX; + command.index_name = drop_index.index_name->as().name(); + command.if_exists = drop_index.if_exists; + + alter_commands.emplace_back(std::move(command)); + + auto alter_lock = table->lockForAlter(getContext()->getSettingsRef().lock_acquire_timeout); + StorageInMemoryMetadata metadata = table->getInMemoryMetadata(); + alter_commands.validate(table, getContext()); + alter_commands.prepare(metadata); + table->checkAlterIsPossible(alter_commands, getContext()); + table->alter(alter_commands, getContext(), alter_lock); + + return {}; +} + +} diff --git a/src/Interpreters/InterpreterDropIndexQuery.h b/src/Interpreters/InterpreterDropIndexQuery.h new file mode 100644 index 00000000000..c6fb3add72f --- /dev/null +++ b/src/Interpreters/InterpreterDropIndexQuery.h @@ -0,0 +1,24 @@ +#pragma once + +#include + + +namespace DB +{ + +class Context; + +class InterpreterDropIndexQuery : public IInterpreter, WithContext +{ +public: + InterpreterDropIndexQuery(const ASTPtr & query_ptr_, ContextPtr context_) + : WithContext(context_) + , query_ptr(query_ptr_) {} + + BlockIO execute() override; + +private: + ASTPtr query_ptr; +}; + +} diff --git a/src/Interpreters/InterpreterDropQuery.cpp b/src/Interpreters/InterpreterDropQuery.cpp index 41b65e73efa..ac731ec6f4b 100644 --- a/src/Interpreters/InterpreterDropQuery.cpp +++ b/src/Interpreters/InterpreterDropQuery.cpp @@ -62,7 +62,7 @@ BlockIO InterpreterDropQuery::execute() } if (getContext()->getSettingsRef().database_atomic_wait_for_drop_and_detach_synchronously) - drop.no_delay = true; + drop.sync = true; if (drop.table) return executeToTable(drop); @@ -89,7 +89,7 @@ BlockIO InterpreterDropQuery::executeToTable(ASTDropQuery & query) DatabasePtr database; UUID table_to_wait_on = UUIDHelpers::Nil; auto res = executeToTableImpl(getContext(), query, database, table_to_wait_on); - if (query.no_delay) + if (query.sync) waitForTableToBeActuallyDroppedOrDetached(query, database, table_to_wait_on); return res; } @@ -244,7 +244,7 @@ BlockIO InterpreterDropQuery::executeToTableImpl(ContextPtr context_, ASTDropQue DatabaseCatalog::instance().tryRemoveLoadingDependencies(table_id, getContext()->getSettingsRef().check_table_dependencies, is_drop_or_detach_database); - database->dropTable(context_, table_id.table_name, query.no_delay); + database->dropTable(context_, table_id.table_name, query.sync); } db = database; @@ -300,7 +300,7 @@ BlockIO InterpreterDropQuery::executeToDatabase(const ASTDropQuery & query) } catch (...) { - if (query.no_delay) + if (query.sync) { for (const auto & table_uuid : tables_to_wait) waitForTableToBeActuallyDroppedOrDetached(query, database, table_uuid); @@ -308,7 +308,7 @@ BlockIO InterpreterDropQuery::executeToDatabase(const ASTDropQuery & query) throw; } - if (query.no_delay) + if (query.sync) { for (const auto & table_uuid : tables_to_wait) waitForTableToBeActuallyDroppedOrDetached(query, database, table_uuid); @@ -345,7 +345,7 @@ BlockIO InterpreterDropQuery::executeToDatabaseImpl(const ASTDropQuery & query, query_for_table.kind = query.kind; query_for_table.if_exists = true; query_for_table.setDatabase(database_name); - query_for_table.no_delay = query.no_delay; + query_for_table.sync = query.sync; /// Flush should not be done if shouldBeEmptyOnDetach() == false, /// since in this case getTablesIterator() may do some additional work, @@ -368,7 +368,7 @@ BlockIO InterpreterDropQuery::executeToDatabaseImpl(const ASTDropQuery & query, } } - if (!drop && query.no_delay) + if (!drop && query.sync) { /// Avoid "some tables are still in use" when sync mode is enabled for (const auto & table_uuid : uuids_to_wait) @@ -428,7 +428,7 @@ void InterpreterDropQuery::extendQueryLogElemImpl(QueryLogElement & elem, const elem.query_kind = "Drop"; } -void InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind kind, ContextPtr global_context, ContextPtr current_context, const StorageID & target_table_id, bool no_delay) +void InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind kind, ContextPtr global_context, ContextPtr current_context, const StorageID & target_table_id, bool sync) { if (DatabaseCatalog::instance().tryGetTable(target_table_id, current_context)) { @@ -437,7 +437,7 @@ void InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind kind, ContextPtr drop_query->setDatabase(target_table_id.database_name); drop_query->setTable(target_table_id.table_name); drop_query->kind = kind; - drop_query->no_delay = no_delay; + drop_query->sync = sync; drop_query->if_exists = true; ASTPtr ast_drop_query = drop_query; /// FIXME We have to use global context to execute DROP query for inner table diff --git a/src/Interpreters/InterpreterDropQuery.h b/src/Interpreters/InterpreterDropQuery.h index 1a38abcdff9..2b65039954b 100644 --- a/src/Interpreters/InterpreterDropQuery.h +++ b/src/Interpreters/InterpreterDropQuery.h @@ -26,7 +26,7 @@ public: void extendQueryLogElemImpl(QueryLogElement & elem, const ASTPtr &, ContextPtr) const override; - static void executeDropQuery(ASTDropQuery::Kind kind, ContextPtr global_context, ContextPtr current_context, const StorageID & target_table_id, bool no_delay); + static void executeDropQuery(ASTDropQuery::Kind kind, ContextPtr global_context, ContextPtr current_context, const StorageID & target_table_id, bool sync); private: AccessRightsElements getRequiredAccessForDDLOnCluster() const; diff --git a/src/Interpreters/InterpreterFactory.cpp b/src/Interpreters/InterpreterFactory.cpp index c212eb50b97..6b081467ae7 100644 --- a/src/Interpreters/InterpreterFactory.cpp +++ b/src/Interpreters/InterpreterFactory.cpp @@ -3,7 +3,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -42,10 +44,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -298,6 +302,14 @@ std::unique_ptr InterpreterFactory::get(ASTPtr & query, ContextMut { return std::make_unique(query, context); } + else if (query->as()) + { + return std::make_unique(query, context); + } + else if (query->as()) + { + return std::make_unique(query, context); + } else if (query->as()) { return std::make_unique(query, context); diff --git a/src/Interpreters/InterpreterSelectQuery.cpp b/src/Interpreters/InterpreterSelectQuery.cpp index 6dbee1eee70..a2902b235c8 100644 --- a/src/Interpreters/InterpreterSelectQuery.cpp +++ b/src/Interpreters/InterpreterSelectQuery.cpp @@ -1242,8 +1242,12 @@ void InterpreterSelectQuery::executeImpl(QueryPlan & query_plan, std::optional

aggregate_overflow_row, @@ -1688,7 +1697,8 @@ void InterpreterSelectQuery::addEmptySourceToQueryPlan( false, context_->getSettingsRef(), query_info.projection->aggregation_keys, - query_info.projection->aggregate_descriptions); + query_info.projection->aggregate_descriptions, + should_produce_results_in_order_of_bucket_number); } } } @@ -2268,6 +2278,9 @@ void InterpreterSelectQuery::executeAggregation(QueryPlan & query_plan, const Ac bool storage_has_evenly_distributed_read = storage && storage->hasEvenlyDistributedRead(); + const bool should_produce_results_in_order_of_bucket_number + = options.to_stage == QueryProcessingStage::WithMergeableState && settings.distributed_aggregation_memory_efficient; + auto aggregating_step = std::make_unique( query_plan.getCurrentDataStream(), std::move(aggregator_params), @@ -2279,7 +2292,8 @@ void InterpreterSelectQuery::executeAggregation(QueryPlan & query_plan, const Ac temporary_data_merge_threads, storage_has_evenly_distributed_read, std::move(group_by_info), - std::move(group_by_sort_description)); + std::move(group_by_sort_description), + should_produce_results_in_order_of_bucket_number); query_plan.addStep(std::move(aggregating_step)); } @@ -2292,6 +2306,9 @@ void InterpreterSelectQuery::executeMergeAggregated(QueryPlan & query_plan, bool if (query_info.projection && query_info.projection->desc->type == ProjectionDescription::Type::Aggregate) return; + const bool should_produce_results_in_order_of_bucket_number = options.to_stage == QueryProcessingStage::WithMergeableState + && context->getSettingsRef().distributed_aggregation_memory_efficient; + executeMergeAggregatedImpl( query_plan, overflow_row, @@ -2300,7 +2317,8 @@ void InterpreterSelectQuery::executeMergeAggregated(QueryPlan & query_plan, bool has_grouping_sets, context->getSettingsRef(), query_analyzer->aggregationKeys(), - query_analyzer->aggregates()); + query_analyzer->aggregates(), + should_produce_results_in_order_of_bucket_number); } diff --git a/src/Interpreters/MergeJoin.cpp b/src/Interpreters/MergeJoin.cpp index 3dd6b7de574..9ddd4ac0be0 100644 --- a/src/Interpreters/MergeJoin.cpp +++ b/src/Interpreters/MergeJoin.cpp @@ -580,7 +580,7 @@ void MergeJoin::mergeRightBlocks() void MergeJoin::mergeInMemoryRightBlocks() { - std::unique_lock lock(rwlock); + std::lock_guard lock(rwlock); if (right_blocks.empty()) return; @@ -613,7 +613,7 @@ void MergeJoin::mergeInMemoryRightBlocks() void MergeJoin::mergeFlushedRightBlocks() { - std::unique_lock lock(rwlock); + std::lock_guard lock(rwlock); auto callback = [&](const Block & block) { @@ -638,7 +638,7 @@ bool MergeJoin::saveRightBlock(Block && block) { if (is_in_memory) { - std::unique_lock lock(rwlock); + std::lock_guard lock(rwlock); if (!is_in_memory) { diff --git a/src/Interpreters/MergeTreeTransaction.cpp b/src/Interpreters/MergeTreeTransaction.cpp index e6b4818b4d7..432116feaf5 100644 --- a/src/Interpreters/MergeTreeTransaction.cpp +++ b/src/Interpreters/MergeTreeTransaction.cpp @@ -24,16 +24,17 @@ static TableLockHolder getLockForOrdinary(const StoragePtr & storage) return storage->lockForShare(RWLockImpl::NO_QUERY, default_timeout); } -MergeTreeTransaction::MergeTreeTransaction(CSN snapshot_, LocalTID local_tid_, UUID host_id) +MergeTreeTransaction::MergeTreeTransaction(CSN snapshot_, LocalTID local_tid_, UUID host_id, std::list::iterator snapshot_it_) : tid({snapshot_, local_tid_, host_id}) , snapshot(snapshot_) + , snapshot_in_use_it(snapshot_it_) , csn(Tx::UnknownCSN) { } void MergeTreeTransaction::setSnapshot(CSN new_snapshot) { - snapshot = new_snapshot; + snapshot.store(new_snapshot, std::memory_order_relaxed); } MergeTreeTransaction::State MergeTreeTransaction::getState() const @@ -219,19 +220,31 @@ void MergeTreeTransaction::afterCommit(CSN assigned_csn) noexcept /// It's not a problem if server crash before CSN is written, because we already have TID in data part and entry in the log. [[maybe_unused]] CSN prev_value = csn.exchange(assigned_csn); chassert(prev_value == Tx::CommittingCSN); - for (const auto & part : creating_parts) + + DataPartsVector created_parts; + DataPartsVector removed_parts; + RunningMutationsList committed_mutations; + { + /// We don't really need mutex here, because no concurrent modifications of transaction object may happen after commit. + std::lock_guard lock{mutex}; + created_parts = creating_parts; + removed_parts = removing_parts; + committed_mutations = mutations; + } + + for (const auto & part : created_parts) { part->version.creation_csn.store(csn); part->appendCSNToVersionMetadata(VersionMetadata::WhichCSN::CREATION); } - for (const auto & part : removing_parts) + for (const auto & part : removed_parts) { part->version.removal_csn.store(csn); part->appendCSNToVersionMetadata(VersionMetadata::WhichCSN::REMOVAL); } - for (const auto & storage_and_mutation : mutations) + for (const auto & storage_and_mutation : committed_mutations) storage_and_mutation.first->setMutationCSN(storage_and_mutation.second, csn); } @@ -313,7 +326,7 @@ void MergeTreeTransaction::onException() String MergeTreeTransaction::dumpDescription() const { - String res = fmt::format("{} state: {}, snapshot: {}", tid, getState(), snapshot); + String res = fmt::format("{} state: {}, snapshot: {}", tid, getState(), getSnapshot()); if (isReadOnly()) { @@ -335,7 +348,7 @@ String MergeTreeTransaction::dumpDescription() const { String info = fmt::format("{} (created by {}, {})", part->name, part->version.getCreationTID(), part->version.creation_csn); std::get<1>(storage_to_changes[&(part->storage)]).push_back(std::move(info)); - chassert(!part->version.creation_csn || part->version.creation_csn <= snapshot); + chassert(!part->version.creation_csn || part->version.creation_csn <= getSnapshot()); } for (const auto & mutation : mutations) diff --git a/src/Interpreters/MergeTreeTransaction.h b/src/Interpreters/MergeTreeTransaction.h index 8b537a7bcd8..7397ea12c12 100644 --- a/src/Interpreters/MergeTreeTransaction.h +++ b/src/Interpreters/MergeTreeTransaction.h @@ -31,13 +31,13 @@ public: ROLLED_BACK, }; - CSN getSnapshot() const { return snapshot; } + CSN getSnapshot() const { return snapshot.load(std::memory_order_relaxed); } void setSnapshot(CSN new_snapshot); State getState() const; const TransactionID tid; - MergeTreeTransaction(CSN snapshot_, LocalTID local_tid_, UUID host_id); + MergeTreeTransaction(CSN snapshot_, LocalTID local_tid_, UUID host_id, std::list::iterator snapshot_it_); void addNewPart(const StoragePtr & storage, const DataPartPtr & new_part); void removeOldPart(const StoragePtr & storage, const DataPartPtr & part_to_remove, const TransactionInfoContext & context); @@ -71,16 +71,16 @@ private: Stopwatch elapsed; /// Usually it's equal to tid.start_csn, but can be changed by SET SNAPSHOT query (for introspection purposes and time-traveling) - CSN snapshot; - std::list::iterator snapshot_in_use_it; + std::atomic snapshot; + const std::list::iterator snapshot_in_use_it; /// Lists of changes made by transaction - std::unordered_set storages; - std::vector table_read_locks_for_ordinary_db; - DataPartsVector creating_parts; - DataPartsVector removing_parts; + std::unordered_set storages TSA_GUARDED_BY(mutex); + std::vector table_read_locks_for_ordinary_db TSA_GUARDED_BY(mutex); + DataPartsVector creating_parts TSA_GUARDED_BY(mutex); + DataPartsVector removing_parts TSA_GUARDED_BY(mutex); using RunningMutationsList = std::vector>; - RunningMutationsList mutations; + RunningMutationsList mutations TSA_GUARDED_BY(mutex); std::atomic csn; }; diff --git a/src/Interpreters/Set.cpp b/src/Interpreters/Set.cpp index 28bbea54110..ab443f58cf2 100644 --- a/src/Interpreters/Set.cpp +++ b/src/Interpreters/Set.cpp @@ -103,7 +103,7 @@ void NO_INLINE Set::insertFromBlockImplCase( void Set::setHeader(const ColumnsWithTypeAndName & header) { - std::unique_lock lock(rwlock); + std::lock_guard lock(rwlock); if (!data.empty()) return; diff --git a/src/Interpreters/StorageID.h b/src/Interpreters/StorageID.h index 29ba24c2e4c..c60c3aec9c6 100644 --- a/src/Interpreters/StorageID.h +++ b/src/Interpreters/StorageID.h @@ -10,7 +10,7 @@ namespace Poco { namespace Util { -class AbstractConfiguration; +class AbstractConfiguration; // NOLINT(cppcoreguidelines-virtual-class-destructor) } } diff --git a/src/Interpreters/TransactionLog.cpp b/src/Interpreters/TransactionLog.cpp index 4f79aaff2c7..a08f940a748 100644 --- a/src/Interpreters/TransactionLog.cpp +++ b/src/Interpreters/TransactionLog.cpp @@ -43,16 +43,13 @@ catch (...) TransactionLog::TransactionLog() - : log(&Poco::Logger::get("TransactionLog")) + : global_context(Context::getGlobalContextInstance()) + , log(&Poco::Logger::get("TransactionLog")) + , zookeeper_path(global_context->getConfigRef().getString("transaction_log.zookeeper_path", "/clickhouse/txn")) + , zookeeper_path_log(zookeeper_path + "/log") + , fault_probability_before_commit(global_context->getConfigRef().getDouble("transaction_log.fault_probability_before_commit", 0)) + , fault_probability_after_commit(global_context->getConfigRef().getDouble("transaction_log.fault_probability_after_commit", 0)) { - global_context = Context::getGlobalContextInstance(); - global_context->checkTransactionsAreAllowed(); - - zookeeper_path = global_context->getConfigRef().getString("transaction_log.zookeeper_path", "/clickhouse/txn"); - zookeeper_path_log = zookeeper_path + "/log"; - fault_probability_before_commit = global_context->getConfigRef().getDouble("transaction_log.fault_probability_before_commit", 0); - fault_probability_after_commit = global_context->getConfigRef().getDouble("transaction_log.fault_probability_after_commit", 0); - loadLogFromZooKeeper(); updating_thread = ThreadFromGlobalPool(&TransactionLog::runUpdatingThread, this); @@ -128,7 +125,7 @@ void TransactionLog::loadEntries(Strings::const_iterator beg, Strings::const_ite LOG_TRACE(log, "Loading {} entries from {}: {}..{}", entries_count, zookeeper_path_log, *beg, last_entry); futures.reserve(entries_count); for (auto it = beg; it != end; ++it) - futures.emplace_back(zookeeper->asyncGet(fs::path(zookeeper_path_log) / *it)); + futures.emplace_back(TSA_READ_ONE_THREAD(zookeeper)->asyncGet(fs::path(zookeeper_path_log) / *it)); std::vector> loaded; loaded.reserve(entries_count); @@ -213,7 +210,7 @@ void TransactionLog::runUpdatingThread() try { /// Do not wait if we have some transactions to finalize - if (unknown_state_list_loaded.empty()) + if (TSA_READ_ONE_THREAD(unknown_state_list_loaded).empty()) log_updated_event->wait(); if (stop_flag.load()) @@ -230,7 +227,7 @@ void TransactionLog::runUpdatingThread() /// It's possible that we connected to different [Zoo]Keeper instance /// so we may read a bit stale state. - zookeeper->sync(zookeeper_path_log); + TSA_READ_ONE_THREAD(zookeeper)->sync(zookeeper_path_log); } loadNewEntries(); @@ -255,13 +252,13 @@ void TransactionLog::runUpdatingThread() void TransactionLog::loadNewEntries() { - Strings entries_list = zookeeper->getChildren(zookeeper_path_log, nullptr, log_updated_event); + Strings entries_list = TSA_READ_ONE_THREAD(zookeeper)->getChildren(zookeeper_path_log, nullptr, log_updated_event); chassert(!entries_list.empty()); ::sort(entries_list.begin(), entries_list.end()); - auto it = std::upper_bound(entries_list.begin(), entries_list.end(), last_loaded_entry); + auto it = std::upper_bound(entries_list.begin(), entries_list.end(), TSA_READ_ONE_THREAD(last_loaded_entry)); loadEntries(it, entries_list.end()); - chassert(last_loaded_entry == entries_list.back()); - chassert(latest_snapshot == deserializeCSN(last_loaded_entry)); + chassert(TSA_READ_ONE_THREAD(last_loaded_entry) == entries_list.back()); + chassert(latest_snapshot == deserializeCSN(TSA_READ_ONE_THREAD(last_loaded_entry))); latest_snapshot.notify_all(); } @@ -281,7 +278,7 @@ void TransactionLog::removeOldEntries() /// TODO we will need a bit more complex logic for multiple hosts Coordination::Stat stat; - CSN old_tail_ptr = deserializeCSN(zookeeper->get(zookeeper_path + "/tail_ptr", &stat)); + CSN old_tail_ptr = deserializeCSN(TSA_READ_ONE_THREAD(zookeeper)->get(zookeeper_path + "/tail_ptr", &stat)); CSN new_tail_ptr = getOldestSnapshot(); if (new_tail_ptr < old_tail_ptr) throw Exception(ErrorCodes::LOGICAL_ERROR, "Got unexpected tail_ptr {}, oldest snapshot is {}, it's a bug", old_tail_ptr, new_tail_ptr); @@ -290,7 +287,7 @@ void TransactionLog::removeOldEntries() /// (it's not supposed to fail with ZBADVERSION while there is only one host) LOG_TRACE(log, "Updating tail_ptr from {} to {}", old_tail_ptr, new_tail_ptr); - zookeeper->set(zookeeper_path + "/tail_ptr", serializeCSN(new_tail_ptr), stat.version); + TSA_READ_ONE_THREAD(zookeeper)->set(zookeeper_path + "/tail_ptr", serializeCSN(new_tail_ptr), stat.version); tail_ptr.store(new_tail_ptr); /// Now we can find and remove old entries @@ -314,7 +311,7 @@ void TransactionLog::removeOldEntries() continue; LOG_TEST(log, "Removing entry {} -> {}", elem.second.tid, elem.second.csn); - auto code = zookeeper->tryRemove(zookeeper_path_log + "/" + serializeCSN(elem.second.csn)); + auto code = TSA_READ_ONE_THREAD(zookeeper)->tryRemove(zookeeper_path_log + "/" + serializeCSN(elem.second.csn)); if (code == Coordination::Error::ZOK || code == Coordination::Error::ZNONODE) removed_entries.push_back(elem.first); } @@ -376,11 +373,11 @@ MergeTreeTransactionPtr TransactionLog::beginTransaction() std::lock_guard lock{running_list_mutex}; CSN snapshot = latest_snapshot.load(); LocalTID ltid = 1 + local_tid_counter.fetch_add(1); - txn = std::make_shared(snapshot, ltid, ServerUUID::get()); + auto snapshot_lock = snapshots_in_use.insert(snapshots_in_use.end(), snapshot); + txn = std::make_shared(snapshot, ltid, ServerUUID::get(), snapshot_lock); bool inserted = running_list.try_emplace(txn->tid.getHash(), txn).second; if (!inserted) throw Exception(ErrorCodes::LOGICAL_ERROR, "I's a bug: TID {} {} exists", txn->tid.getHash(), txn->tid); - txn->snapshot_in_use_it = snapshots_in_use.insert(snapshots_in_use.end(), snapshot); } LOG_TEST(log, "Beginning transaction {} ({})", txn->tid, txn->tid.getHash()); @@ -595,7 +592,7 @@ TransactionLog::TransactionsList TransactionLog::getTransactionsList() const void TransactionLog::sync() const { - Strings entries_list = zookeeper->getChildren(zookeeper_path_log); + Strings entries_list = getZooKeeper()->getChildren(zookeeper_path_log); chassert(!entries_list.empty()); ::sort(entries_list.begin(), entries_list.end()); CSN newest_csn = deserializeCSN(entries_list.back()); diff --git a/src/Interpreters/TransactionLog.h b/src/Interpreters/TransactionLog.h index 49aa77b9868..71a5d3a6fc6 100644 --- a/src/Interpreters/TransactionLog.h +++ b/src/Interpreters/TransactionLog.h @@ -129,7 +129,7 @@ public: void sync() const; private: - void loadLogFromZooKeeper(); + void loadLogFromZooKeeper() TSA_REQUIRES(mutex); void runUpdatingThread(); void loadEntries(Strings::const_iterator beg, Strings::const_iterator end); @@ -149,8 +149,8 @@ private: CSN getCSNImpl(const TIDHash & tid_hash) const; - ContextPtr global_context; - Poco::Logger * log; + const ContextPtr global_context; + Poco::Logger * const log; /// The newest snapshot available for reading std::atomic latest_snapshot; @@ -167,24 +167,24 @@ private: TransactionID tid; }; using TIDMap = std::unordered_map; - TIDMap tid_to_csn; + TIDMap tid_to_csn TSA_GUARDED_BY(mutex); mutable std::mutex running_list_mutex; /// Transactions that are currently processed - TransactionsList running_list; + TransactionsList running_list TSA_GUARDED_BY(running_list_mutex); /// If we lost connection on attempt to create csn- node then we don't know transaction's state. using UnknownStateList = std::vector>; - UnknownStateList unknown_state_list; - UnknownStateList unknown_state_list_loaded; + UnknownStateList unknown_state_list TSA_GUARDED_BY(running_list_mutex); + UnknownStateList unknown_state_list_loaded TSA_GUARDED_BY(running_list_mutex); /// Ordered list of snapshots that are currently used by some transactions. Needed for background cleanup. - std::list snapshots_in_use; + std::list snapshots_in_use TSA_GUARDED_BY(running_list_mutex); - ZooKeeperPtr zookeeper; - String zookeeper_path; + ZooKeeperPtr zookeeper TSA_GUARDED_BY(mutex); + const String zookeeper_path; - String zookeeper_path_log; + const String zookeeper_path_log; /// Name of the newest entry that was loaded from log in ZK - String last_loaded_entry; + String last_loaded_entry TSA_GUARDED_BY(mutex); /// The oldest CSN such that we store in log entries with TransactionIDs containing this CSN. std::atomic tail_ptr = Tx::UnknownCSN; @@ -193,8 +193,8 @@ private: std::atomic_bool stop_flag = false; ThreadFromGlobalPool updating_thread; - Float64 fault_probability_before_commit = 0; - Float64 fault_probability_after_commit = 0; + const Float64 fault_probability_before_commit = 0; + const Float64 fault_probability_after_commit = 0; }; template diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index de920eaddbf..15d4f7929f8 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -18,7 +19,6 @@ #include #include -#include #include #include @@ -27,6 +27,12 @@ namespace fs = std::filesystem; namespace DB { +namespace ErrorCodes +{ + extern const int NOT_IMPLEMENTED; + extern const int LOGICAL_ERROR; +} + static void executeCreateQuery( const String & query, ContextMutablePtr context, @@ -71,12 +77,6 @@ static void loadDatabase( ReadBufferFromFile in(database_metadata_file, 1024); readStringUntilEOF(database_attach_query, in); } - else if (fs::exists(fs::path(database_path))) - { - /// TODO Remove this code (it's required for compatibility with versions older than 20.7) - /// Database exists, but .sql file is absent. It's old-style Ordinary database (e.g. system or default) - database_attach_query = "ATTACH DATABASE " + backQuoteIfNeed(database) + " ENGINE = Ordinary"; - } else { /// It's first server run and we need create default and system databases. @@ -95,6 +95,15 @@ static void loadDatabase( } } +static void checkUnsupportedVersion(ContextMutablePtr context, const String & database_name) +{ + /// Produce better exception message + String metadata_path = context->getPath() + "metadata/" + database_name; + if (fs::exists(fs::path(metadata_path))) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Data directory for {} database exists, but metadata file does not. " + "Probably you are trying to upgrade from version older than 20.7. " + "If so, you should upgrade through intermediate version.", database_name); +} void loadMetadata(ContextMutablePtr context, const String & default_database_name) { @@ -118,43 +127,33 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam if (it->is_symlink()) continue; - const auto current_file = it->path().filename().string(); - if (!it->is_directory()) - { - /// TODO: DETACH DATABASE PERMANENTLY ? - if (fs::path(current_file).extension() == ".sql") - { - String db_name = fs::path(current_file).stem(); - if (!isSystemOrInformationSchema(db_name)) - databases.emplace(unescapeForFileName(db_name), fs::path(path) / db_name); - } - - /// Temporary fails may be left from previous server runs. - if (fs::path(current_file).extension() == ".tmp") - { - LOG_WARNING(log, "Removing temporary file {}", it->path().string()); - try - { - fs::remove(it->path()); - } - catch (...) - { - /// It does not prevent server to startup. - tryLogCurrentException(log); - } - } - + if (it->is_directory()) continue; + + const auto current_file = it->path().filename().string(); + + /// TODO: DETACH DATABASE PERMANENTLY ? + if (fs::path(current_file).extension() == ".sql") + { + String db_name = fs::path(current_file).stem(); + if (!isSystemOrInformationSchema(db_name)) + databases.emplace(unescapeForFileName(db_name), fs::path(path) / db_name); } - /// For '.svn', '.gitignore' directory and similar. - if (current_file.at(0) == '.') - continue; - - if (isSystemOrInformationSchema(current_file)) - continue; - - databases.emplace(unescapeForFileName(current_file), it->path().string()); + /// Temporary fails may be left from previous server runs. + if (fs::path(current_file).extension() == ".tmp") + { + LOG_WARNING(log, "Removing temporary file {}", it->path().string()); + try + { + fs::remove(it->path()); + } + catch (...) + { + /// It does not prevent server to startup. + tryLogCurrentException(log); + } + } } /// clickhouse-local creates DatabaseMemory as default database by itself @@ -162,7 +161,10 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam bool create_default_db_if_not_exists = !default_database_name.empty(); bool metadata_dir_for_default_db_already_exists = databases.contains(default_database_name); if (create_default_db_if_not_exists && !metadata_dir_for_default_db_already_exists) + { + checkUnsupportedVersion(context, default_database_name); databases.emplace(default_database_name, std::filesystem::path(path) / escapeForFileName(default_database_name)); + } TablesLoader::Databases loaded_databases; for (const auto & [name, db_path] : databases) @@ -192,13 +194,14 @@ static void loadSystemDatabaseImpl(ContextMutablePtr context, const String & dat { String path = context->getPath() + "metadata/" + database_name; String metadata_file = path + ".sql"; - if (fs::exists(fs::path(path)) || fs::exists(fs::path(metadata_file))) + if (fs::exists(fs::path(metadata_file))) { /// 'has_force_restore_data_flag' is true, to not fail on loading query_log table, if it is corrupted. loadDatabase(context, database_name, path, true); } else { + checkUnsupportedVersion(context, database_name); /// Initialize system database manually String database_create_query = "CREATE DATABASE "; database_create_query += database_name; @@ -208,6 +211,122 @@ static void loadSystemDatabaseImpl(ContextMutablePtr context, const String & dat } } +static void convertOrdinaryDatabaseToAtomic(ContextMutablePtr context, const DatabasePtr & database) +{ + /// It's kind of C++ script that creates temporary database with Atomic engine, + /// moves all tables to it, drops old database and then renames new one to old name. + + Poco::Logger * log = &Poco::Logger::get("loadMetadata"); + + String name = database->getDatabaseName(); + + String tmp_name = fmt::format(".tmp_convert.{}.{}", name, thread_local_rng()); + + String name_quoted = backQuoteIfNeed(name); + String tmp_name_quoted = backQuoteIfNeed(tmp_name); + + LOG_INFO(log, "Will convert database {} from Ordinary to Atomic", name_quoted); + + String create_database_query = fmt::format("CREATE DATABASE IF NOT EXISTS {}", tmp_name_quoted); + auto res = executeQuery(create_database_query, context, true); + executeTrivialBlockIO(res, context); + res = {}; + auto tmp_database = DatabaseCatalog::instance().getDatabase(tmp_name); + assert(tmp_database->getEngineName() == "Atomic"); + + size_t num_tables = 0; + for (auto iterator = database->getTablesIterator(context); iterator->isValid(); iterator->next()) + { + ++num_tables; + auto id = iterator->table()->getStorageID(); + id.database_name = tmp_name; + iterator->table()->checkTableCanBeRenamed(id); + } + + LOG_INFO(log, "Will move {} tables to {}", num_tables, tmp_name_quoted); + + for (auto iterator = database->getTablesIterator(context); iterator->isValid(); iterator->next()) + { + auto id = iterator->table()->getStorageID(); + String qualified_quoted_name = id.getFullTableName(); + id.database_name = tmp_name; + String tmp_qualified_quoted_name = id.getFullTableName(); + + String move_table_query = fmt::format("RENAME TABLE {} TO {}", qualified_quoted_name, tmp_qualified_quoted_name); + res = executeQuery(move_table_query, context, true); + executeTrivialBlockIO(res, context); + res = {}; + } + + LOG_INFO(log, "Moved all tables from {} to {}", name_quoted, tmp_name_quoted); + + if (!database->empty()) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Database {} is not empty after moving tables", name_quoted); + + String drop_query = fmt::format("DROP DATABASE {}", name_quoted); + res = executeQuery(drop_query, context, true); + executeTrivialBlockIO(res, context); + res = {}; + + String rename_query = fmt::format("RENAME DATABASE {} TO {}", tmp_name_quoted, name_quoted); + res = executeQuery(rename_query, context, true); + executeTrivialBlockIO(res, context); + + LOG_INFO(log, "Finished database engine conversion of {}", name_quoted); +} + +void maybeConvertOrdinaryDatabaseToAtomic(ContextMutablePtr context, const DatabasePtr & database) +{ + if (database->getEngineName() != "Ordinary") + return; + + if (context->getSettingsRef().allow_deprecated_database_ordinary) + return; + + try + { + /// It's not quite correct to run DDL queries while database is not started up. + startupSystemTables(); + + auto local_context = Context::createCopy(context); + local_context->setSetting("check_table_dependencies", false); + convertOrdinaryDatabaseToAtomic(local_context, database); + + auto new_database = DatabaseCatalog::instance().getDatabase(DatabaseCatalog::SYSTEM_DATABASE); + UUID db_uuid = new_database->getUUID(); + std::vector tables_uuids; + for (auto iterator = new_database->getTablesIterator(context); iterator->isValid(); iterator->next()) + tables_uuids.push_back(iterator->uuid()); + + /// Reload database just in case (and update logger name) + String detach_query = fmt::format("DETACH DATABASE {}", backQuoteIfNeed(DatabaseCatalog::SYSTEM_DATABASE)); + auto res = executeQuery(detach_query, context, true); + executeTrivialBlockIO(res, context); + res = {}; + + /// Unlock UUID mapping, because it will be locked again on database reload. + /// It's safe to do during metadata loading, because cleanup task is not started yet. + DatabaseCatalog::instance().removeUUIDMappingFinally(db_uuid); + for (const auto & uuid : tables_uuids) + DatabaseCatalog::instance().removeUUIDMappingFinally(uuid); + + loadSystemDatabaseImpl(context, DatabaseCatalog::SYSTEM_DATABASE, "Atomic"); + TablesLoader::Databases databases = + { + {DatabaseCatalog::SYSTEM_DATABASE, DatabaseCatalog::instance().getSystemDatabase()}, + }; + TablesLoader loader{context, databases, /* force_restore */ true, /* force_attach */ true}; + loader.loadTables(); + + /// Will startup tables usual way + } + catch (Exception & e) + { + e.addMessage("While trying to convert {} to Atomic", database->getDatabaseName()); + throw; + } +} + void startupSystemTables() { diff --git a/src/Interpreters/loadMetadata.h b/src/Interpreters/loadMetadata.h index e918b5f530c..8dc332defc5 100644 --- a/src/Interpreters/loadMetadata.h +++ b/src/Interpreters/loadMetadata.h @@ -19,4 +19,8 @@ void loadMetadata(ContextMutablePtr context, const String & default_database_nam /// so we startup system tables after all databases are loaded. void startupSystemTables(); +/// Converts database with Ordinary engine to Atomic. Does nothing if database is not Ordinary. +/// Can be called only during server startup when there are no queries from users. +void maybeConvertOrdinaryDatabaseToAtomic(ContextMutablePtr context, const DatabasePtr & database); + } diff --git a/src/Parsers/ASTAlterQuery.cpp b/src/Parsers/ASTAlterQuery.cpp index f53c39b192f..cfcc5decdf5 100644 --- a/src/Parsers/ASTAlterQuery.cpp +++ b/src/Parsers/ASTAlterQuery.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -556,6 +557,7 @@ void ASTAlterQuery::formatQueryImpl(const FormatSettings & settings, FormatState frame.need_parens = false; std::string indent_str = settings.one_line ? "" : std::string(4u * frame.indent, ' '); + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str; switch (alter_object) diff --git a/src/Parsers/ASTCreateIndexQuery.cpp b/src/Parsers/ASTCreateIndexQuery.cpp new file mode 100644 index 00000000000..7a5c80551d6 --- /dev/null +++ b/src/Parsers/ASTCreateIndexQuery.cpp @@ -0,0 +1,61 @@ +#include +#include +#include +#include + + +namespace DB +{ + +/** Get the text that identifies this element. */ +String ASTCreateIndexQuery::getID(char delim) const +{ + return "CreateIndexQuery" + (delim + getDatabase()) + delim + getTable(); +} + +ASTPtr ASTCreateIndexQuery::clone() const +{ + auto res = std::make_shared(*this); + res->children.clear(); + + res->index_name = index_name->clone(); + res->children.push_back(res->index_name); + + res->index_decl = index_decl->clone(); + res->children.push_back(res->index_decl); + return res; +} + +void ASTCreateIndexQuery::formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const +{ + frame.need_parens = false; + + std::string indent_str = settings.one_line ? "" : std::string(4u * frame.indent, ' '); + + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str; + + settings.ostr << "CREATE INDEX " << (if_not_exists ? "IF NOT EXISTS " : ""); + index_name->formatImpl(settings, state, frame); + settings.ostr << " ON "; + + settings.ostr << (settings.hilite ? hilite_none : ""); + + if (table) + { + if (database) + { + settings.ostr << indent_str << backQuoteIfNeed(getDatabase()); + settings.ostr << "."; + } + settings.ostr << indent_str << backQuoteIfNeed(getTable()); + } + + formatOnCluster(settings); + + if (!cluster.empty()) + settings.ostr << " "; + + index_decl->formatImpl(settings, state, frame); +} + +} diff --git a/src/Parsers/ASTCreateIndexQuery.h b/src/Parsers/ASTCreateIndexQuery.h new file mode 100644 index 00000000000..f3c6a7830a4 --- /dev/null +++ b/src/Parsers/ASTCreateIndexQuery.h @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include + + +namespace DB +{ + +/** CREATE INDEX [IF NOT EXISTS] name ON [db].name (expression) TYPE type GRANULARITY value + */ + +class ASTCreateIndexQuery : public ASTQueryWithTableAndOutput, public ASTQueryWithOnCluster +{ +public: + bool if_not_exists{false}; + + ASTPtr index_name; + + /// Stores the IndexDeclaration here. + ASTPtr index_decl; + + String getID(char delim) const override; + + ASTPtr clone() const override; + + ASTPtr getRewrittenASTWithoutOnCluster(const WithoutOnClusterASTRewriteParams & params) const override + { + return removeOnCluster(clone(), params.default_database); + } + + virtual QueryKind getQueryKind() const override { return QueryKind::Create; } + +protected: + void formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; +}; + +} diff --git a/src/Parsers/ASTDropIndexQuery.cpp b/src/Parsers/ASTDropIndexQuery.cpp new file mode 100644 index 00000000000..78152d213b8 --- /dev/null +++ b/src/Parsers/ASTDropIndexQuery.cpp @@ -0,0 +1,63 @@ +#include +#include +#include + + +namespace DB +{ + +/** Get the text that identifies this element. */ +String ASTDropIndexQuery::getID(char delim) const +{ + return "CreateIndexQuery" + (delim + getDatabase()) + delim + getTable(); +} + +ASTPtr ASTDropIndexQuery::clone() const +{ + auto res = std::make_shared(*this); + res->children.clear(); + + res->index_name = index_name->clone(); + res->children.push_back(res->index_name); + + return res; +} + +void ASTDropIndexQuery::formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const +{ + frame.need_parens = false; + + std::string indent_str = settings.one_line ? "" : std::string(4u * frame.indent, ' '); + + settings.ostr << (settings.hilite ? hilite_keyword : "") << indent_str; + + settings.ostr << "DROP INDEX " << (if_exists ? "IF EXISTS " : ""); + index_name->formatImpl(settings, state, frame); + settings.ostr << " ON "; + + settings.ostr << (settings.hilite ? hilite_none : ""); + + if (table) + { + if (database) + { + settings.ostr << indent_str << backQuoteIfNeed(getDatabase()); + settings.ostr << "."; + } + settings.ostr << indent_str << backQuoteIfNeed(getTable()); + } + + formatOnCluster(settings); +} + +ASTPtr ASTDropIndexQuery::convertToASTAlterCommand() const +{ + auto command = std::make_shared(); + command->index = index_name->clone(); + command->if_exists = if_exists; + command->type = ASTAlterCommand::DROP_INDEX; + + return command; +} + +} diff --git a/src/Parsers/ASTDropIndexQuery.h b/src/Parsers/ASTDropIndexQuery.h new file mode 100644 index 00000000000..d7e39f797b5 --- /dev/null +++ b/src/Parsers/ASTDropIndexQuery.h @@ -0,0 +1,42 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + + +namespace DB +{ + +/** DROP INDEX [IF EXISTS] name on [db].name + */ + +class ASTDropIndexQuery : public ASTQueryWithTableAndOutput, public ASTQueryWithOnCluster +{ +public: + bool if_exists{false}; + + ASTPtr index_name; + + String getID(char delim) const override; + + ASTPtr clone() const override; + + ASTPtr getRewrittenASTWithoutOnCluster(const WithoutOnClusterASTRewriteParams & params) const override + { + return removeOnCluster(clone(), params.default_database); + } + + virtual QueryKind getQueryKind() const override { return QueryKind::Drop; } + + /// Convert ASTDropIndexQuery to ASTAlterCommand. + ASTPtr convertToASTAlterCommand() const; + +protected: + void formatQueryImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const override; +}; + +} diff --git a/src/Parsers/ASTDropQuery.cpp b/src/Parsers/ASTDropQuery.cpp index 9e815ee75de..11c1dd4c47a 100644 --- a/src/Parsers/ASTDropQuery.cpp +++ b/src/Parsers/ASTDropQuery.cpp @@ -72,8 +72,8 @@ void ASTDropQuery::formatQueryImpl(const FormatSettings & settings, FormatState if (permanently) settings.ostr << " PERMANENTLY"; - if (no_delay) - settings.ostr << (settings.hilite ? hilite_keyword : "") << " NO DELAY" << (settings.hilite ? hilite_none : ""); + if (sync) + settings.ostr << (settings.hilite ? hilite_keyword : "") << " SYNC" << (settings.hilite ? hilite_none : ""); } } diff --git a/src/Parsers/ASTDropQuery.h b/src/Parsers/ASTDropQuery.h index ef2b609fbac..b4c96353a09 100644 --- a/src/Parsers/ASTDropQuery.h +++ b/src/Parsers/ASTDropQuery.h @@ -31,7 +31,7 @@ public: /// Same as above bool is_view{false}; - bool no_delay{false}; + bool sync{false}; // We detach the object permanently, so it will not be reattached back during server restart. bool permanently{false}; diff --git a/src/Parsers/ASTIndexDeclaration.cpp b/src/Parsers/ASTIndexDeclaration.cpp index d8ebf825674..cc988d1d307 100644 --- a/src/Parsers/ASTIndexDeclaration.cpp +++ b/src/Parsers/ASTIndexDeclaration.cpp @@ -25,9 +25,19 @@ ASTPtr ASTIndexDeclaration::clone() const void ASTIndexDeclaration::formatImpl(const FormatSettings & s, FormatState & state, FormatStateStacked frame) const { - s.ostr << backQuoteIfNeed(name); - s.ostr << " "; - expr->formatImpl(s, state, frame); + if (from_create_index) + { + s.ostr << "("; + expr->formatImpl(s, state, frame); + s.ostr << ")"; + } + else + { + s.ostr << backQuoteIfNeed(name); + s.ostr << " "; + expr->formatImpl(s, state, frame); + } + s.ostr << (s.hilite ? hilite_keyword : "") << " TYPE " << (s.hilite ? hilite_none : ""); type->formatImpl(s, state, frame); s.ostr << (s.hilite ? hilite_keyword : "") << " GRANULARITY " << (s.hilite ? hilite_none : ""); diff --git a/src/Parsers/ASTIndexDeclaration.h b/src/Parsers/ASTIndexDeclaration.h index 8416ec6b0a6..31d5ef0e7f8 100644 --- a/src/Parsers/ASTIndexDeclaration.h +++ b/src/Parsers/ASTIndexDeclaration.h @@ -16,6 +16,7 @@ public: IAST * expr; ASTFunction * type; UInt64 granularity; + bool from_create_index = false; /** Get the text that identifies this element. */ String getID(char) const override { return "Index"; } diff --git a/src/Parsers/ParserAlterQuery.cpp b/src/Parsers/ParserAlterQuery.cpp index 9a3c1dd8bed..bc3af03a3c4 100644 --- a/src/Parsers/ParserAlterQuery.cpp +++ b/src/Parsers/ParserAlterQuery.cpp @@ -840,7 +840,6 @@ bool ParserAlterCommandList::parseImpl(Pos & pos, ASTPtr & node, Expected & expe return true; } - bool ParserAlterQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) { auto query = std::make_shared(); diff --git a/src/Parsers/ParserCreateIndexQuery.cpp b/src/Parsers/ParserCreateIndexQuery.cpp new file mode 100644 index 00000000000..22411c71ee5 --- /dev/null +++ b/src/Parsers/ParserCreateIndexQuery.cpp @@ -0,0 +1,120 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +bool ParserCreateIndexDeclaration::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) +{ + ParserKeyword s_type("TYPE"); + ParserKeyword s_granularity("GRANULARITY"); + + ParserDataType data_type_p; + ParserExpression expression_p; + ParserUnsignedInteger granularity_p; + + ASTPtr expr; + ASTPtr type; + ASTPtr granularity; + + /// Skip name parser for SQL-standard CREATE INDEX + if (!expression_p.parse(pos, expr, expected)) + return false; + + if (!s_type.ignore(pos, expected)) + return false; + + if (!data_type_p.parse(pos, type, expected)) + return false; + + if (!s_granularity.ignore(pos, expected)) + return false; + + if (!granularity_p.parse(pos, granularity, expected)) + return false; + + auto index = std::make_shared(); + index->from_create_index = true; + index->granularity = granularity->as().value.safeGet(); + index->set(index->expr, expr); + index->set(index->type, type); + node = index; + + return true; +} + +bool ParserCreateIndexQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, Expected & expected) +{ + auto query = std::make_shared(); + node = query; + + ParserKeyword s_create("CREATE"); + ParserKeyword s_index("INDEX"); + ParserKeyword s_if_not_exists("IF NOT EXISTS"); + ParserKeyword s_on("ON"); + ParserIdentifier index_name_p; + ParserCreateIndexDeclaration parser_create_idx_decl; + + ASTPtr index_name; + ASTPtr index_decl; + + String cluster_str; + bool if_not_exists = false; + + if (!s_create.ignore(pos, expected)) + return false; + + if (!s_index.ignore(pos, expected)) + return false; + + if (s_if_not_exists.ignore(pos, expected)) + if_not_exists = true; + + if (!index_name_p.parse(pos, index_name, expected)) + return false; + + /// ON [db.] table_name + if (!s_on.ignore(pos, expected)) + return false; + + if (!parseDatabaseAndTableAsAST(pos, expected, query->database, query->table)) + return false; + + /// [ON cluster_name] + if (s_on.ignore(pos, expected)) + { + if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected)) + return false; + } + + if (!parser_create_idx_decl.parse(pos, index_decl, expected)) + return false; + + query->index_name = index_name; + query->children.push_back(index_name); + + query->index_decl = index_decl; + query->children.push_back(index_decl); + + query->if_not_exists = if_not_exists; + query->cluster = cluster_str; + + if (query->database) + query->children.push_back(query->database); + + if (query->table) + query->children.push_back(query->table); + + return true; +} + +} diff --git a/src/Parsers/ParserCreateIndexQuery.h b/src/Parsers/ParserCreateIndexQuery.h new file mode 100644 index 00000000000..3dfdccc301f --- /dev/null +++ b/src/Parsers/ParserCreateIndexQuery.h @@ -0,0 +1,31 @@ +#pragma once + +#include + +namespace DB +{ + +/** Query like this: + * CREATE INDEX [IF NOT EXISTS] name ON [db].name (expression) TYPE type GRANULARITY value + */ + +class ParserCreateIndexQuery : public IParserBase +{ +protected: + const char * getName() const override{ return "CREATE INDEX query"; } + bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; +}; + + +/** Parser for index declaration in create index, where name is ignored. */ +class ParserCreateIndexDeclaration : public IParserBase +{ +public: + ParserCreateIndexDeclaration() {} + +protected: + const char * getName() const override { return "index declaration in create index"; } + bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; +}; + +} diff --git a/src/Parsers/ParserCreateQuery.cpp b/src/Parsers/ParserCreateQuery.cpp index e4a3f87f288..4b6ab67e22f 100644 --- a/src/Parsers/ParserCreateQuery.cpp +++ b/src/Parsers/ParserCreateQuery.cpp @@ -467,6 +467,7 @@ bool ParserCreateTableQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe ParserKeyword s_from("FROM"); ParserKeyword s_on("ON"); ParserToken s_dot(TokenType::Dot); + ParserToken s_comma(TokenType::Comma); ParserToken s_lparen(TokenType::OpeningRoundBracket); ParserToken s_rparen(TokenType::ClosingRoundBracket); ParserStorage storage_p; @@ -574,6 +575,10 @@ bool ParserCreateTableQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe if (!table_properties_p.parse(pos, columns_list, expected)) return false; + /// We allow a trailing comma in the columns list for user convenience. + /// Although it diverges from the SQL standard slightly. + s_comma.ignore(pos, expected); + if (!s_rparen.ignore(pos, expected)) return false; diff --git a/src/Parsers/ParserDropIndexQuery.cpp b/src/Parsers/ParserDropIndexQuery.cpp new file mode 100644 index 00000000000..0844ea16ae0 --- /dev/null +++ b/src/Parsers/ParserDropIndexQuery.cpp @@ -0,0 +1,67 @@ +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +bool ParserDropIndexQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, Expected & expected) +{ + auto query = std::make_shared(); + node = query; + + ParserKeyword s_drop("DROP"); + ParserKeyword s_index("INDEX"); + ParserKeyword s_on("ON"); + ParserKeyword s_if_exists("IF EXISTS"); + ParserIdentifier index_name_p; + + String cluster_str; + bool if_exists = false; + + if (!s_drop.ignore(pos, expected)) + return false; + + if (!s_index.ignore(pos, expected)) + return false; + + if (s_if_exists.ignore(pos, expected)) + if_exists = true; + + if (!index_name_p.parse(pos, query->index_name, expected)) + return false; + + /// ON [db.] table_name + if (!s_on.ignore(pos, expected)) + return false; + + if (!parseDatabaseAndTableAsAST(pos, expected, query->database, query->table)) + return false; + + /// [ON cluster_name] + if (s_on.ignore(pos, expected)) + { + if (!ASTQueryWithOnCluster::parse(pos, cluster_str, expected)) + return false; + + query->cluster = std::move(cluster_str); + } + + if (query->index_name) + query->children.push_back(query->index_name); + + query->if_exists = if_exists; + + if (query->database) + query->children.push_back(query->database); + + if (query->table) + query->children.push_back(query->table); + + return true; +} + +} diff --git a/src/Parsers/ParserDropIndexQuery.h b/src/Parsers/ParserDropIndexQuery.h new file mode 100644 index 00000000000..1b6535c7efb --- /dev/null +++ b/src/Parsers/ParserDropIndexQuery.h @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace DB +{ + +/** Query like this: + * DROP INDEX [IF EXISTS] name ON [db].name + */ + +class ParserDropIndexQuery : public IParserBase +{ +protected: + const char * getName() const override{ return "DROP INDEX query"; } + bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; +}; + +} diff --git a/src/Parsers/ParserDropQuery.cpp b/src/Parsers/ParserDropQuery.cpp index e7263aa47f5..f40a39e6b2f 100644 --- a/src/Parsers/ParserDropQuery.cpp +++ b/src/Parsers/ParserDropQuery.cpp @@ -31,7 +31,7 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, cons bool temporary = false; bool is_dictionary = false; bool is_view = false; - bool no_delay = false; + bool sync = false; bool permanently = false; if (s_database.ignore(pos, expected)) @@ -83,7 +83,7 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, cons /// actually for TRUNCATE NO DELAY / SYNC means nothing if (s_no_delay.ignore(pos, expected) || s_sync.ignore(pos, expected)) - no_delay = true; + sync = true; auto query = std::make_shared(); node = query; @@ -93,7 +93,7 @@ bool parseDropQuery(IParser::Pos & pos, ASTPtr & node, Expected & expected, cons query->temporary = temporary; query->is_dictionary = is_dictionary; query->is_view = is_view; - query->no_delay = no_delay; + query->sync = sync; query->permanently = permanently; query->database = database; query->table = table; diff --git a/src/Parsers/ParserQuery.cpp b/src/Parsers/ParserQuery.cpp index eaea5dd0f5f..a3cafee65d7 100644 --- a/src/Parsers/ParserQuery.cpp +++ b/src/Parsers/ParserQuery.cpp @@ -2,7 +2,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -43,6 +45,8 @@ bool ParserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ParserCreateSettingsProfileQuery create_settings_profile_p; ParserCreateFunctionQuery create_function_p; ParserDropFunctionQuery drop_function_p; + ParserCreateIndexQuery create_index_p; + ParserDropIndexQuery drop_index_p; ParserDropAccessEntityQuery drop_access_entity_p; ParserGrantQuery grant_p; ParserSetRoleQuery set_role_p; @@ -63,6 +67,8 @@ bool ParserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) || create_settings_profile_p.parse(pos, node, expected) || create_function_p.parse(pos, node, expected) || drop_function_p.parse(pos, node, expected) + || create_index_p.parse(pos, node, expected) + || drop_index_p.parse(pos, node, expected) || drop_access_entity_p.parse(pos, node, expected) || grant_p.parse(pos, node, expected) || external_ddl_p.parse(pos, node, expected) diff --git a/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.cpp b/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.cpp index 34b9305c0e1..32ab391cf8c 100644 --- a/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.cpp +++ b/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.cpp @@ -20,7 +20,7 @@ namespace DB } { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (background_exception) std::rethrow_exception(background_exception); @@ -30,7 +30,7 @@ namespace DB void ParallelFormattingOutputFormat::addChunk(Chunk chunk, ProcessingUnitType type, bool can_throw_exception) { { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (background_exception && can_throw_exception) std::rethrow_exception(background_exception); } diff --git a/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.h b/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.h index edde79be896..fb58f5765c1 100644 --- a/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.h +++ b/src/Processors/Formats/Impl/ParallelFormattingOutputFormat.h @@ -236,7 +236,7 @@ private: void onBackgroundException() { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (!background_exception) { background_exception = std::current_exception(); diff --git a/src/Processors/Formats/Impl/ParallelParsingInputFormat.cpp b/src/Processors/Formats/Impl/ParallelParsingInputFormat.cpp index d3e167d35c6..318bcaed466 100644 --- a/src/Processors/Formats/Impl/ParallelParsingInputFormat.cpp +++ b/src/Processors/Formats/Impl/ParallelParsingInputFormat.cpp @@ -121,7 +121,7 @@ void ParallelParsingInputFormat::parserThreadFunction(ThreadGroupStatusPtr threa void ParallelParsingInputFormat::onBackgroundException(size_t offset) { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (!background_exception) { background_exception = std::current_exception(); @@ -233,7 +233,7 @@ Chunk ParallelParsingInputFormat::generate() else { // Pass the unit back to the segmentator. - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); unit.status = READY_TO_INSERT; segmentator_condvar.notify_all(); } diff --git a/src/Processors/QueryPlan/AggregatingStep.cpp b/src/Processors/QueryPlan/AggregatingStep.cpp index 1e62673bc26..0a4b12084eb 100644 --- a/src/Processors/QueryPlan/AggregatingStep.cpp +++ b/src/Processors/QueryPlan/AggregatingStep.cpp @@ -19,13 +19,13 @@ namespace DB { -static ITransformingStep::Traits getTraits() +static ITransformingStep::Traits getTraits(bool should_produce_results_in_order_of_bucket_number) { return ITransformingStep::Traits { { .preserves_distinct_columns = false, /// Actually, we may check that distinct names are in aggregation keys - .returns_single_stream = true, + .returns_single_stream = should_produce_results_in_order_of_bucket_number, /// Actually, may also return single stream if should_produce_results_in_order_of_bucket_number = false .preserves_number_of_streams = false, .preserves_sorting = false, }, @@ -75,9 +75,10 @@ AggregatingStep::AggregatingStep( size_t temporary_data_merge_threads_, bool storage_has_evenly_distributed_read_, InputOrderInfoPtr group_by_info_, - SortDescription group_by_sort_description_) + SortDescription group_by_sort_description_, + bool should_produce_results_in_order_of_bucket_number_) : ITransformingStep( - input_stream_, appendGroupingColumn(params_.getHeader(input_stream_.header, final_), grouping_sets_params_), getTraits(), false) + input_stream_, appendGroupingColumn(params_.getHeader(input_stream_.header, final_), grouping_sets_params_), getTraits(should_produce_results_in_order_of_bucket_number_), false) , params(std::move(params_)) , grouping_sets_params(std::move(grouping_sets_params_)) , final(final_) @@ -88,6 +89,7 @@ AggregatingStep::AggregatingStep( , storage_has_evenly_distributed_read(storage_has_evenly_distributed_read_) , group_by_info(std::move(group_by_info_)) , group_by_sort_description(std::move(group_by_sort_description_)) + , should_produce_results_in_order_of_bucket_number(should_produce_results_in_order_of_bucket_number_) { } @@ -351,7 +353,8 @@ void AggregatingStep::transformPipeline(QueryPipelineBuilder & pipeline, const B return std::make_shared(header, transform_params, many_data, counter++, merge_threads, temporary_data_merge_threads); }); - pipeline.resize(1); + /// We add the explicit resize here, but not in case of aggregating in order, since AIO don't use two-level hash tables and thus returns only buckets with bucket_number = -1. + pipeline.resize(should_produce_results_in_order_of_bucket_number ? 1 : pipeline.getNumStreams(), true /* force */); aggregating = collector.detachProcessors(0); } diff --git a/src/Processors/QueryPlan/AggregatingStep.h b/src/Processors/QueryPlan/AggregatingStep.h index 2879cd1e0e9..0e982d76940 100644 --- a/src/Processors/QueryPlan/AggregatingStep.h +++ b/src/Processors/QueryPlan/AggregatingStep.h @@ -36,7 +36,8 @@ public: size_t temporary_data_merge_threads_, bool storage_has_evenly_distributed_read_, InputOrderInfoPtr group_by_info_, - SortDescription group_by_sort_description_); + SortDescription group_by_sort_description_, + bool should_produce_results_in_order_of_bucket_number_); String getName() const override { return "Aggregating"; } @@ -65,6 +66,10 @@ private: InputOrderInfoPtr group_by_info; SortDescription group_by_sort_description; + /// It determines if we should resize pipeline to 1 at the end. + /// Needed in case of distributed memory efficient aggregation. + const bool should_produce_results_in_order_of_bucket_number; + Processors aggregating_in_order; Processors aggregating_sorted; Processors finalizing; diff --git a/src/Processors/QueryPlan/MergingAggregatedStep.cpp b/src/Processors/QueryPlan/MergingAggregatedStep.cpp index c898b901a6a..d74a6174f00 100644 --- a/src/Processors/QueryPlan/MergingAggregatedStep.cpp +++ b/src/Processors/QueryPlan/MergingAggregatedStep.cpp @@ -7,13 +7,13 @@ namespace DB { -static ITransformingStep::Traits getTraits() +static ITransformingStep::Traits getTraits(bool should_produce_results_in_order_of_bucket_number) { return ITransformingStep::Traits { { .preserves_distinct_columns = false, - .returns_single_stream = true, + .returns_single_stream = should_produce_results_in_order_of_bucket_number, .preserves_number_of_streams = false, .preserves_sorting = false, }, @@ -29,13 +29,16 @@ MergingAggregatedStep::MergingAggregatedStep( bool final_, bool memory_efficient_aggregation_, size_t max_threads_, - size_t memory_efficient_merge_threads_) - : ITransformingStep(input_stream_, params_.getHeader(input_stream_.header, final_), getTraits()) + size_t memory_efficient_merge_threads_, + bool should_produce_results_in_order_of_bucket_number_) + : ITransformingStep( + input_stream_, params_.getHeader(input_stream_.header, final_), getTraits(should_produce_results_in_order_of_bucket_number_)) , params(std::move(params_)) , final(final_) , memory_efficient_aggregation(memory_efficient_aggregation_) , max_threads(max_threads_) , memory_efficient_merge_threads(memory_efficient_merge_threads_) + , should_produce_results_in_order_of_bucket_number(should_produce_results_in_order_of_bucket_number_) { /// Aggregation keys are distinct for (const auto & key : params.keys) @@ -62,6 +65,8 @@ void MergingAggregatedStep::transformPipeline(QueryPipelineBuilder & pipeline, c pipeline.addMergingAggregatedMemoryEfficientTransform(transform_params, num_merge_threads); } + + pipeline.resize(should_produce_results_in_order_of_bucket_number ? 1 : max_threads); } void MergingAggregatedStep::describeActions(FormatSettings & settings) const diff --git a/src/Processors/QueryPlan/MergingAggregatedStep.h b/src/Processors/QueryPlan/MergingAggregatedStep.h index 136422c8c27..419b43615bd 100644 --- a/src/Processors/QueryPlan/MergingAggregatedStep.h +++ b/src/Processors/QueryPlan/MergingAggregatedStep.h @@ -19,7 +19,8 @@ public: bool final_, bool memory_efficient_aggregation_, size_t max_threads_, - size_t memory_efficient_merge_threads_); + size_t memory_efficient_merge_threads_, + bool should_produce_results_in_order_of_bucket_number_); String getName() const override { return "MergingAggregated"; } @@ -36,6 +37,10 @@ private: bool memory_efficient_aggregation; size_t max_threads; size_t memory_efficient_merge_threads; + + /// It determines if we should resize pipeline to 1 at the end. + /// Needed in case of distributed memory efficient aggregation over distributed table. + const bool should_produce_results_in_order_of_bucket_number; }; } diff --git a/src/Server/HTTP/HTTPServerConnection.cpp b/src/Server/HTTP/HTTPServerConnection.cpp index 6b2ef32f6a4..92a994b3a4e 100644 --- a/src/Server/HTTP/HTTPServerConnection.cpp +++ b/src/Server/HTTP/HTTPServerConnection.cpp @@ -26,7 +26,7 @@ void HTTPServerConnection::run() { try { - std::unique_lock lock(mutex); + std::lock_guard lock(mutex); if (!stopped && tcp_server.isOpen() && session.connected()) { HTTPServerResponse response(session); diff --git a/src/Storages/IStorage.h b/src/Storages/IStorage.h index a655da4473b..6dd329db02b 100644 --- a/src/Storages/IStorage.h +++ b/src/Storages/IStorage.h @@ -396,7 +396,7 @@ public: */ virtual void drop() {} - virtual void dropInnerTableIfAny(bool /* no_delay */, ContextPtr /* context */) {} + virtual void dropInnerTableIfAny(bool /* sync */, ContextPtr /* context */) {} /** Clear the table data and leave it empty. * Must be called under exclusive lock (lockExclusively). diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index fdba84048f0..2c4dcfa05ee 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -1324,20 +1324,29 @@ void MergeTreeData::loadDataParts(bool skip_sanity_checks) auto deactivate_part = [&] (DataPartIteratorByStateAndInfo it) { + const DataPartPtr & part = *it; - (*it)->remove_time.store((*it)->modification_time, std::memory_order_relaxed); - auto creation_csn = (*it)->version.creation_csn.load(std::memory_order_relaxed); - if (creation_csn != Tx::RolledBackCSN && creation_csn != Tx::PrehistoricCSN && !(*it)->version.isRemovalTIDLocked()) + part->remove_time.store(part->modification_time, std::memory_order_relaxed); + auto creation_csn = part->version.creation_csn.load(std::memory_order_relaxed); + if (creation_csn != Tx::RolledBackCSN && creation_csn != Tx::PrehistoricCSN && !part->version.isRemovalTIDLocked()) { /// It's possible that covering part was created without transaction, /// but if covered part was created with transaction (i.e. creation_tid is not prehistoric), /// then it must have removal tid in metadata file. throw Exception(ErrorCodes::LOGICAL_ERROR, "Data part {} is Outdated and has creation TID {} and CSN {}, " "but does not have removal tid. It's a bug or a result of manual intervention.", - (*it)->name, (*it)->version.creation_tid, creation_csn); + part->name, part->version.creation_tid, creation_csn); } modifyPartState(it, DataPartState::Outdated); - removePartContributionToDataVolume(*it); + removePartContributionToDataVolume(part); + + /// Explicitly set removal_tid_lock for parts w/o transaction (i.e. w/o txn_version.txt) + /// to avoid keeping part forever (see VersionMetadata::canBeRemoved()) + if (!part->version.isRemovalTIDLocked()) + { + TransactionInfoContext transaction_context{getStorageID(), part->name}; + part->version.lockRemovalTID(Tx::PrehistoricTID, transaction_context); + } }; /// All parts are in "Active" state after loading diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 6590445572d..85231aca253 100644 --- a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -369,6 +369,10 @@ QueryPlanPtr MergeTreeDataSelectExecutor::read( else group_by_info = nullptr; + // We don't have information regarding the `to_stage` of the query processing, only about `from_stage` (which is passed through `processed_stage` argument). + // Thus we cannot assign false here since it may be a query over distributed table. + const bool should_produce_results_in_order_of_bucket_number = true; + auto aggregating_step = std::make_unique( query_plan->getCurrentDataStream(), std::move(params), @@ -380,7 +384,8 @@ QueryPlanPtr MergeTreeDataSelectExecutor::read( temporary_data_merge_threads, /* storage_has_evenly_distributed_read_= */ false, std::move(group_by_info), - std::move(group_by_sort_description)); + std::move(group_by_sort_description), + should_produce_results_in_order_of_bucket_number); query_plan->addStep(std::move(aggregating_step)); }; diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp index 4f500208215..dfb5eb0bd69 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeRestartingThread.cpp @@ -189,7 +189,7 @@ bool ReplicatedMergeTreeRestartingThread::tryStartup() } catch (...) { - std::unique_lock lock(storage.last_queue_update_exception_lock); + std::lock_guard lock(storage.last_queue_update_exception_lock); storage.last_queue_update_exception = getCurrentExceptionMessage(false); throw; } diff --git a/src/Storages/MergeTree/registerStorageMergeTree.cpp b/src/Storages/MergeTree/registerStorageMergeTree.cpp index 73b9b364f7f..e52a0fed674 100644 --- a/src/Storages/MergeTree/registerStorageMergeTree.cpp +++ b/src/Storages/MergeTree/registerStorageMergeTree.cpp @@ -304,12 +304,22 @@ static StoragePtr create(const StorageFactory::Arguments & args) arg_idx, e.message(), getMergeTreeVerboseHelp(is_extended_storage_def)); } } + else if (!args.attach && !args.getLocalContext()->getSettingsRef().allow_deprecated_syntax_for_merge_tree) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, "This syntax for *MergeTree engine is deprecated. " + "Use extended storage definition syntax with ORDER BY/PRIMARY KEY clause." + "See also allow_deprecated_syntax_for_merge_tree setting."); + } /// For Replicated. String zookeeper_path; String replica_name; StorageReplicatedMergeTree::RenamingRestrictions renaming_restrictions = StorageReplicatedMergeTree::RenamingRestrictions::ALLOW_ANY; + bool is_on_cluster = args.getLocalContext()->getClientInfo().query_kind == ClientInfo::QueryKind::SECONDARY_QUERY; + bool is_replicated_database = args.getLocalContext()->getClientInfo().query_kind == ClientInfo::QueryKind::SECONDARY_QUERY && + DatabaseCatalog::instance().getDatabase(args.table_id.database_name)->getEngineName() == "Replicated"; + if (replicated) { bool has_arguments = arg_num + 2 <= arg_cnt; @@ -372,17 +382,11 @@ static StoragePtr create(const StorageFactory::Arguments & args) throw Exception("Expected two string literal arguments: zookeeper_path and replica_name", ErrorCodes::BAD_ARGUMENTS); /// Allow implicit {uuid} macros only for zookeeper_path in ON CLUSTER queries - bool is_on_cluster = args.getLocalContext()->getClientInfo().query_kind == ClientInfo::QueryKind::SECONDARY_QUERY; - bool is_replicated_database = args.getLocalContext()->getClientInfo().query_kind == ClientInfo::QueryKind::SECONDARY_QUERY && - DatabaseCatalog::instance().getDatabase(args.table_id.database_name)->getEngineName() == "Replicated"; bool allow_uuid_macro = is_on_cluster || is_replicated_database || args.query.attach; /// Unfold {database} and {table} macro on table creation, so table can be renamed. if (!args.attach) { - if (is_replicated_database && !is_extended_storage_def) - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Old syntax is not allowed for ReplicatedMergeTree tables in Replicated databases"); - Macros::MacroExpansionInfo info; /// NOTE: it's not recursive info.expand_special_macros_only = true; diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp index 190ffabe2c1..cc80d567d1d 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.cpp @@ -240,7 +240,7 @@ void StorageMaterializedPostgreSQL::shutdown() } -void StorageMaterializedPostgreSQL::dropInnerTableIfAny(bool no_delay, ContextPtr local_context) +void StorageMaterializedPostgreSQL::dropInnerTableIfAny(bool sync, ContextPtr local_context) { /// If it is a table with database engine MaterializedPostgreSQL - return, because delition of /// internal tables is managed there. @@ -252,7 +252,7 @@ void StorageMaterializedPostgreSQL::dropInnerTableIfAny(bool no_delay, ContextPt auto nested_table = tryGetNested() != nullptr; if (nested_table) - InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, getNestedStorageID(), no_delay); + InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, getNestedStorageID(), sync); } diff --git a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h index bb3836a8853..f1eea33d4b0 100644 --- a/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h +++ b/src/Storages/PostgreSQL/StorageMaterializedPostgreSQL.h @@ -84,7 +84,7 @@ public: void shutdown() override; /// Used only for single MaterializedPostgreSQL storage. - void dropInnerTableIfAny(bool no_delay, ContextPtr local_context) override; + void dropInnerTableIfAny(bool sync, ContextPtr local_context) override; NamesAndTypesList getVirtuals() const override; diff --git a/src/Storages/RabbitMQ/RabbitMQSource.cpp b/src/Storages/RabbitMQ/RabbitMQSource.cpp index 71d80f0a632..f6e5bb84037 100644 --- a/src/Storages/RabbitMQ/RabbitMQSource.cpp +++ b/src/Storages/RabbitMQ/RabbitMQSource.cpp @@ -142,7 +142,7 @@ Chunk RabbitMQSource::generateImpl() while (true) { - if (buffer->eof()) + if (buffer->queueEmpty()) break; auto new_rows = executor.execute(); diff --git a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.cpp b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.cpp index e2de5179990..3543085f5a0 100644 --- a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.cpp +++ b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.cpp @@ -47,6 +47,12 @@ ReadBufferFromRabbitMQConsumer::~ReadBufferFromRabbitMQConsumer() } +void ReadBufferFromRabbitMQConsumer::closeChannel() +{ + if (consumer_channel) + consumer_channel->close(); +} + void ReadBufferFromRabbitMQConsumer::subscribe() { for (const auto & queue_name : queues) diff --git a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h index 8a527011a3c..bd55d169744 100644 --- a/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h +++ b/src/Storages/RabbitMQ/ReadBufferFromRabbitMQConsumer.h @@ -3,18 +3,24 @@ #include #include #include -#include -#include #include namespace Poco { - class Logger; +class Logger; +} + +namespace AMQP +{ +class TcpChannel; } namespace DB { +class RabbitMQHandler; +using ChannelPtr = std::unique_ptr; + class ReadBufferFromRabbitMQConsumer : public ReadBuffer { @@ -52,11 +58,7 @@ public: ChannelPtr & getChannel() { return consumer_channel; } void setupChannel(); bool needChannelUpdate(); - void closeChannel() - { - if (consumer_channel) - consumer_channel->close(); - } + void closeChannel(); void updateQueues(std::vector & queues_) { queues = queues_; } size_t queuesCount() { return queues.size(); } diff --git a/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp b/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp index 81cb58e4f5e..1bfaabe142b 100644 --- a/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp +++ b/src/Storages/RocksDB/StorageEmbeddedRocksDB.cpp @@ -336,7 +336,7 @@ StorageEmbeddedRocksDB::StorageEmbeddedRocksDB(const StorageID & table_id_, void StorageEmbeddedRocksDB::truncate(const ASTPtr &, const StorageMetadataPtr & , ContextPtr, TableExclusiveLockHolder &) { - std::unique_lock lock(rocksdb_ptr_mx); + std::lock_guard lock(rocksdb_ptr_mx); rocksdb_ptr->Close(); rocksdb_ptr = nullptr; diff --git a/src/Storages/StorageMaterializedView.cpp b/src/Storages/StorageMaterializedView.cpp index d0685c263f8..2ece0af3359 100644 --- a/src/Storages/StorageMaterializedView.cpp +++ b/src/Storages/StorageMaterializedView.cpp @@ -215,10 +215,10 @@ void StorageMaterializedView::drop() dropInnerTableIfAny(true, getContext()); } -void StorageMaterializedView::dropInnerTableIfAny(bool no_delay, ContextPtr local_context) +void StorageMaterializedView::dropInnerTableIfAny(bool sync, ContextPtr local_context) { if (has_inner_table && tryGetTargetTable()) - InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, target_table_id, no_delay); + InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, target_table_id, sync); } void StorageMaterializedView::truncate(const ASTPtr &, const StorageMetadataPtr &, ContextPtr local_context, TableExclusiveLockHolder &) diff --git a/src/Storages/StorageMaterializedView.h b/src/Storages/StorageMaterializedView.h index 8aec0313ecb..0adf394876c 100644 --- a/src/Storages/StorageMaterializedView.h +++ b/src/Storages/StorageMaterializedView.h @@ -42,7 +42,7 @@ public: SinkToStoragePtr write(const ASTPtr & query, const StorageMetadataPtr & /*metadata_snapshot*/, ContextPtr context) override; void drop() override; - void dropInnerTableIfAny(bool no_delay, ContextPtr local_context) override; + void dropInnerTableIfAny(bool sync, ContextPtr local_context) override; void truncate(const ASTPtr &, const StorageMetadataPtr &, ContextPtr, TableExclusiveLockHolder &) override; diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index 6895142b2a4..fac11db2ab9 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -598,6 +598,9 @@ void StorageReplicatedMergeTree::createNewZooKeeperNodes() auto zookeeper = getZooKeeper(); std::vector futures; + /// We need to confirm /quorum exists here although it's called under createTableIfNotExists because in older CH releases (pre 22.4) + /// it was created here, so if metadata creation is done by an older replica the node might not exists when reaching this call + futures.push_back(zookeeper->asyncTryCreateNoThrow(zookeeper_path + "/quorum", String(), zkutil::CreateMode::Persistent)); futures.push_back(zookeeper->asyncTryCreateNoThrow(zookeeper_path + "/quorum/parallel", String(), zkutil::CreateMode::Persistent)); /// Nodes for remote fs zero-copy replication @@ -2969,7 +2972,7 @@ void StorageReplicatedMergeTree::cloneReplicaIfNeeded(zkutil::ZooKeeperPtr zooke String StorageReplicatedMergeTree::getLastQueueUpdateException() const { - std::unique_lock lock(last_queue_update_exception_lock); + std::lock_guard lock(last_queue_update_exception_lock); return last_queue_update_exception; } @@ -2991,7 +2994,7 @@ void StorageReplicatedMergeTree::queueUpdatingTask() { tryLogCurrentException(log, __PRETTY_FUNCTION__); - std::unique_lock lock(last_queue_update_exception_lock); + std::lock_guard lock(last_queue_update_exception_lock); last_queue_update_exception = getCurrentExceptionMessage(false); if (e.code == Coordination::Error::ZSESSIONEXPIRED) @@ -3006,7 +3009,7 @@ void StorageReplicatedMergeTree::queueUpdatingTask() { tryLogCurrentException(log, __PRETTY_FUNCTION__); - std::unique_lock lock(last_queue_update_exception_lock); + std::lock_guard lock(last_queue_update_exception_lock); last_queue_update_exception = getCurrentExceptionMessage(false); queue_updating_task->scheduleAfter(QUEUE_UPDATE_ERROR_SLEEP_MS); @@ -4340,7 +4343,7 @@ void StorageReplicatedMergeTree::shutdown() /// Ask all parts exchange handlers to finish asap. New ones will fail to start data_parts_exchange_ptr->blocker.cancelForever(); /// Wait for all of them - std::unique_lock lock(data_parts_exchange_ptr->rwlock); + std::lock_guard lock(data_parts_exchange_ptr->rwlock); } } @@ -7399,7 +7402,7 @@ void StorageReplicatedMergeTree::checkBrokenDisks() if (disk_ptr->isBroken()) { { - std::unique_lock lock(last_broken_disks_mutex); + std::lock_guard lock(last_broken_disks_mutex); if (!last_broken_disks.insert(disk_ptr->getName()).second) continue; } @@ -7419,7 +7422,7 @@ void StorageReplicatedMergeTree::checkBrokenDisks() else { { - std::unique_lock lock(last_broken_disks_mutex); + std::lock_guard lock(last_broken_disks_mutex); if (last_broken_disks.erase(disk_ptr->getName()) > 0) LOG_INFO( log, diff --git a/src/Storages/WindowView/StorageWindowView.cpp b/src/Storages/WindowView/StorageWindowView.cpp index cfb19869074..76833b3e19a 100644 --- a/src/Storages/WindowView/StorageWindowView.cpp +++ b/src/Storages/WindowView/StorageWindowView.cpp @@ -1017,7 +1017,7 @@ void StorageWindowView::threadFuncFireProc() if (shutdown_called) return; - std::unique_lock lock(fire_signal_mutex); + std::lock_guard lock(fire_signal_mutex); UInt32 timestamp_now = std::time(nullptr); while (next_fire_signal <= timestamp_now) @@ -1049,7 +1049,7 @@ void StorageWindowView::threadFuncFireProc() void StorageWindowView::threadFuncFireEvent() { - std::unique_lock lock(fire_signal_mutex); + std::lock_guard lock(fire_signal_mutex); LOG_TRACE(log, "Fire events: {}", fire_signal.size()); @@ -1606,7 +1606,7 @@ void StorageWindowView::drop() dropInnerTableIfAny(true, getContext()); } -void StorageWindowView::dropInnerTableIfAny(bool no_delay, ContextPtr local_context) +void StorageWindowView::dropInnerTableIfAny(bool sync, ContextPtr local_context) { if (!std::exchange(has_inner_table, false)) return; @@ -1614,10 +1614,10 @@ void StorageWindowView::dropInnerTableIfAny(bool no_delay, ContextPtr local_cont try { InterpreterDropQuery::executeDropQuery( - ASTDropQuery::Kind::Drop, getContext(), local_context, inner_table_id, no_delay); + ASTDropQuery::Kind::Drop, getContext(), local_context, inner_table_id, sync); if (has_inner_target_table) - InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, target_table_id, no_delay); + InterpreterDropQuery::executeDropQuery(ASTDropQuery::Kind::Drop, getContext(), local_context, target_table_id, sync); } catch (...) { diff --git a/src/Storages/WindowView/StorageWindowView.h b/src/Storages/WindowView/StorageWindowView.h index 86cc80ee8ea..96c034b9590 100644 --- a/src/Storages/WindowView/StorageWindowView.h +++ b/src/Storages/WindowView/StorageWindowView.h @@ -120,7 +120,7 @@ public: void checkTableCanBeDropped() const override; - void dropInnerTableIfAny(bool no_delay, ContextPtr context) override; + void dropInnerTableIfAny(bool sync, ContextPtr context) override; void drop() override; diff --git a/tests/ci/build_report_check.py b/tests/ci/build_report_check.py index 1ba91a38a60..dbf5adfe174 100644 --- a/tests/ci/build_report_check.py +++ b/tests/ci/build_report_check.py @@ -151,6 +151,8 @@ def main(): needs_data = json.load(file_handler) required_builds = len(needs_data) + logging.info("The next builds are required: %s", ", ".join(needs_data)) + gh = Github(get_best_robot_token()) pr_info = PRInfo() rerun_helper = RerunHelper(gh, pr_info, build_check_name) @@ -159,7 +161,6 @@ def main(): sys.exit(0) builds_for_check = CI_CONFIG["builds_report_config"][build_check_name] - logging.info("My reports list %s", builds_for_check) required_builds = required_builds or len(builds_for_check) # Collect reports from json artifacts diff --git a/tests/ci/integration_test_check.py b/tests/ci/integration_test_check.py index cf0dfe51e9b..9fda2d09ae6 100644 --- a/tests/ci/integration_test_check.py +++ b/tests/ci/integration_test_check.py @@ -225,7 +225,7 @@ if __name__ == "__main__": output_path_log = os.path.join(result_path, "main_script_log.txt") runner_path = os.path.join(repo_path, "tests/integration", "ci-runner.py") - run_command = f"sudo -E {runner_path} | tee {output_path_log}" + run_command = f"sudo -E {runner_path}" logging.info("Going to run command: `%s`", run_command) logging.info( "ENV parameters for runner:\n%s", diff --git a/tests/clickhouse-test b/tests/clickhouse-test index 75159053f26..8744e8bf95b 100755 --- a/tests/clickhouse-test +++ b/tests/clickhouse-test @@ -5,6 +5,7 @@ # pylint: disable=too-many-lines import enum +from queue import Full import shutil import sys import os @@ -1489,9 +1490,9 @@ def collect_build_flags(args): result.append(BuildFlags.RELEASE) value = clickhouse_execute( - args, "SELECT value FROM system.settings WHERE name = 'default_database_engine'" + args, "SELECT value FROM system.settings WHERE name = 'allow_deprecated_database_ordinary'" ) - if value == b"Ordinary": + if value == b"1": result.append(BuildFlags.ORDINARY_DATABASE) value = int( @@ -1581,15 +1582,18 @@ def do_run_tests(jobs, test_suite: TestSuite, parallel): for _ in range(jobs): parallel_tests_array.append((None, batch_size, test_suite)) - with closing(multiprocessing.Pool(processes=jobs)) as pool: - pool.map_async(run_tests_array, parallel_tests_array) + try: + with closing(multiprocessing.Pool(processes=jobs)) as pool: + pool.map_async(run_tests_array, parallel_tests_array) - for suit in test_suite.parallel_tests: - queue.put(suit, timeout=args.timeout * 1.1) + for suit in test_suite.parallel_tests: + queue.put(suit, timeout=args.timeout * 1.1) - for _ in range(jobs): - queue.put(None, timeout=args.timeout * 1.1) + for _ in range(jobs): + queue.put(None, timeout=args.timeout * 1.1) + queue.close() + except Full: queue.close() pool.join() diff --git a/tests/config/users.d/database_ordinary.xml b/tests/config/users.d/database_ordinary.xml index b3b81ee25ff..8ffd2f27a62 100644 --- a/tests/config/users.d/database_ordinary.xml +++ b/tests/config/users.d/database_ordinary.xml @@ -1,7 +1,7 @@ - Ordinary + 1 diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index e917e5a1885..0d32547358c 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -3390,14 +3390,6 @@ class ClickHouseInstance: ], user="root", ) - self.exec_in_container( - [ - "bash", - "-c", - "cp /usr/share/clickhouse-odbc-bridge_fresh /usr/bin/clickhouse-odbc-bridge && chmod 777 /usr/bin/clickhouse", - ], - user="root", - ) self.exec_in_container( ["bash", "-c", "{} --daemon".format(self.clickhouse_start_command)], user=str(os.getuid()), @@ -3411,7 +3403,11 @@ class ClickHouseInstance: self.wait_start(time_left) def restart_with_latest_version( - self, stop_start_wait_sec=300, callback_onstop=None, signal=15 + self, + stop_start_wait_sec=300, + callback_onstop=None, + signal=15, + fix_metadata=False, ): begin_time = time.time() if not self.stay_alive: @@ -3458,14 +3454,23 @@ class ClickHouseInstance: "echo 'restart_with_latest_version: From version' && /usr/share/clickhouse_original server --version && echo 'To version' /usr/share/clickhouse_fresh server --version", ] ) - self.exec_in_container( - [ - "bash", - "-c", - "cp /usr/share/clickhouse-odbc-bridge_fresh /usr/bin/clickhouse-odbc-bridge && chmod 777 /usr/bin/clickhouse", - ], - user="root", - ) + if fix_metadata: + # Versions older than 20.7 might not create .sql file for system and default database + # Create it manually if upgrading from older version + self.exec_in_container( + [ + "bash", + "-c", + "echo 'ATTACH DATABASE system ENGINE=Ordinary' > /var/lib/clickhouse/metadata/system.sql", + ], + ) + self.exec_in_container( + [ + "bash", + "-c", + "echo 'ATTACH DATABASE system ENGINE=Ordinary' > /var/lib/clickhouse/metadata/default.sql", + ], + ) self.exec_in_container( ["bash", "-c", "{} --daemon".format(self.clickhouse_start_command)], user=str(os.getuid()), diff --git a/tests/integration/runner b/tests/integration/runner index 7a02ec309a0..d82e73068af 100755 --- a/tests/integration/runner +++ b/tests/integration/runner @@ -25,7 +25,7 @@ VOLUME_NAME = "clickhouse_integration_tests" CONTAINER_NAME = f"{VOLUME_NAME}_{random_str()}" CONFIG_DIR_IN_REPO = "programs/server" -INTERGATION_DIR_IN_REPO = "tests/integration" +INTEGRATION_DIR_IN_REPO = "tests/integration" SRC_DIR_IN_REPO = "src" DIND_INTEGRATION_TESTS_IMAGE_NAME = "clickhouse/integration-tests-runner" @@ -84,7 +84,7 @@ def check_args_and_update_paths(args): ) else: args.cases_dir = os.path.abspath( - os.path.join(CLICKHOUSE_ROOT, INTERGATION_DIR_IN_REPO) + os.path.join(CLICKHOUSE_ROOT, INTEGRATION_DIR_IN_REPO) ) logging.info("Cases dir is not set. Will use %s" % (args.cases_dir)) @@ -392,15 +392,11 @@ if __name__ == "__main__": command=args.command, ) - try: - print("Trying to kill container", CONTAINER_NAME, "if it's already running") - subprocess.check_call( - f'docker kill $(docker ps -a -q --filter name={CONTAINER_NAME} --format="{{{{.ID}}}}")', - shell=True, - ) - print("Container killed") - except: - print("Nothing to kill") + containers = subprocess.check_output(f"docker ps -a -q --filter name={CONTAINER_NAME} --format={{{{.ID}}}}", shell=True, universal_newlines=True).splitlines() + if containers: + print(f"Trying to kill containers name={CONTAINER_NAME} ids={containers}") + subprocess.check_call(f"docker kill {' '.join(containers)}", shell=True) + print(f"Containers {containers} killed") print(("Running pytest container as: '" + cmd + "'.")) subprocess.check_call(cmd, shell=True) diff --git a/tests/integration/test_atomic_drop_table/test.py b/tests/integration/test_atomic_drop_table/test.py index 1fe88dde099..6ffa60de7b5 100644 --- a/tests/integration/test_atomic_drop_table/test.py +++ b/tests/integration/test_atomic_drop_table/test.py @@ -20,7 +20,8 @@ def start_cluster(): try: cluster.start() node1.query( - "CREATE DATABASE zktest ENGINE=Ordinary;" + "CREATE DATABASE zktest ENGINE=Ordinary;", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different behaviour with Atomic node1.query( """ diff --git a/tests/integration/test_attach_partition_with_large_destination/test.py b/tests/integration/test_attach_partition_with_large_destination/test.py index 0a4ab9fada1..a82e63bb7ba 100644 --- a/tests/integration/test_attach_partition_with_large_destination/test.py +++ b/tests/integration/test_attach_partition_with_large_destination/test.py @@ -34,7 +34,10 @@ def create_force_drop_flag(node): @pytest.mark.parametrize("engine", ["Ordinary", "Atomic"]) def test_attach_partition_with_large_destination(started_cluster, engine): # Initialize - node.query("CREATE DATABASE db ENGINE={}".format(engine)) + node.query( + "CREATE DATABASE db ENGINE={}".format(engine), + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query( "CREATE TABLE db.destination (n UInt64) ENGINE=ReplicatedMergeTree('/test/destination', 'r1') ORDER BY n PARTITION BY n % 2" ) diff --git a/tests/integration/test_backup_restore/test.py b/tests/integration/test_backup_restore/test.py index 905abef05b0..193e638186c 100644 --- a/tests/integration/test_backup_restore/test.py +++ b/tests/integration/test_backup_restore/test.py @@ -15,7 +15,8 @@ def started_cluster(): try: cluster.start() instance.query( - "CREATE DATABASE test ENGINE = Ordinary" + "CREATE DATABASE test ENGINE = Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different path in shadow/ with Atomic instance.query("DROP TABLE IF EXISTS test.tbl") instance.query( diff --git a/tests/integration/test_backup_with_other_granularity/test.py b/tests/integration/test_backup_with_other_granularity/test.py index 9cb998fb505..d30c45c3691 100644 --- a/tests/integration/test_backup_with_other_granularity/test.py +++ b/tests/integration/test_backup_with_other_granularity/test.py @@ -54,7 +54,7 @@ def test_backup_from_old_version(started_cluster): node1.query("ALTER TABLE source_table FREEZE PARTITION tuple();") - node1.restart_with_latest_version() + node1.restart_with_latest_version(fix_metadata=True) node1.query( "CREATE TABLE dest_table (A Int64, B String, Y String) ENGINE = ReplicatedMergeTree('/test/dest_table1', '1') ORDER BY tuple()" @@ -107,7 +107,7 @@ def test_backup_from_old_version_setting(started_cluster): node2.query("ALTER TABLE source_table FREEZE PARTITION tuple();") - node2.restart_with_latest_version() + node2.restart_with_latest_version(fix_metadata=True) node2.query( "CREATE TABLE dest_table (A Int64, B String, Y String) ENGINE = ReplicatedMergeTree('/test/dest_table2', '1') ORDER BY tuple() SETTINGS enable_mixed_granularity_parts = 1" @@ -163,7 +163,7 @@ def test_backup_from_old_version_config(started_cluster): "1", ) - node3.restart_with_latest_version(callback_onstop=callback) + node3.restart_with_latest_version(callback_onstop=callback, fix_metadata=True) node3.query( "CREATE TABLE dest_table (A Int64, B String, Y String) ENGINE = ReplicatedMergeTree('/test/dest_table3', '1') ORDER BY tuple() SETTINGS enable_mixed_granularity_parts = 1" @@ -202,7 +202,8 @@ def test_backup_from_old_version_config(started_cluster): def test_backup_and_alter(started_cluster): node4.query( - "CREATE DATABASE test ENGINE=Ordinary" + "CREATE DATABASE test ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different path in shadow/ with Atomic node4.query( diff --git a/tests/integration/test_backward_compatibility/test_aggregate_function_state_avg.py b/tests/integration/test_backward_compatibility/test_aggregate_function_state_avg.py index b3ad9011239..13dd28ee8af 100644 --- a/tests/integration/test_backward_compatibility/test_aggregate_function_state_avg.py +++ b/tests/integration/test_backward_compatibility/test_aggregate_function_state_avg.py @@ -71,7 +71,7 @@ def test_backward_compatability(start_cluster): assert node1.query("SELECT avgMerge(x) FROM state") == "2.5\n" - node1.restart_with_latest_version() + node1.restart_with_latest_version(fix_metadata=True) assert node1.query("SELECT avgMerge(x) FROM state") == "2.5\n" diff --git a/tests/integration/test_backward_compatibility/test_convert_ordinary.py b/tests/integration/test_backward_compatibility/test_convert_ordinary.py new file mode 100644 index 00000000000..59ceca23a51 --- /dev/null +++ b/tests/integration/test_backward_compatibility/test_convert_ordinary.py @@ -0,0 +1,77 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__, name="convert_ordinary") +node = cluster.add_instance( + "node", + image="yandex/clickhouse-server", + tag="19.17.8.54", + stay_alive=True, + with_installed_binary=True, +) + + +@pytest.fixture(scope="module") +def start_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +def q(query): + return node.query(query, settings={"log_queries": 1}) + + +def test_convert_system_db_to_atomic(start_cluster): + q( + "CREATE TABLE t(date Date, id UInt32) ENGINE = MergeTree PARTITION BY toYYYYMM(date) ORDER BY id" + ) + q("INSERT INTO t VALUES (today(), 1)") + q("INSERT INTO t SELECT number % 1000, number FROM system.numbers LIMIT 1000000") + + assert "1000001\n" == q("SELECT count() FROM t") + assert "499999500001\n" == q("SELECT sum(id) FROM t") + assert "1970-01-01\t1000\t499500000\n1970-01-02\t1000\t499501000\n" == q( + "SELECT date, count(), sum(id) FROM t GROUP BY date ORDER BY date LIMIT 2" + ) + q("SYSTEM FLUSH LOGS") + + assert "query_log" in q("SHOW TABLES FROM system") + assert "part_log" in q("SHOW TABLES FROM system") + q("SYSTEM FLUSH LOGS") + assert "1\n" == q("SELECT count() != 0 FROM system.query_log") + assert "1\n" == q("SELECT count() != 0 FROM system.part_log") + + node.restart_with_latest_version(fix_metadata=True) + + assert "Ordinary" in node.query("SHOW CREATE DATABASE default") + assert "Atomic" in node.query("SHOW CREATE DATABASE system") + assert "query_log" in node.query("SHOW TABLES FROM system") + assert "part_log" in node.query("SHOW TABLES FROM system") + node.query("SYSTEM FLUSH LOGS") + + assert "query_log_0" in node.query("SHOW TABLES FROM system") + assert "part_log_0" in node.query("SHOW TABLES FROM system") + assert "1\n" == node.query("SELECT count() != 0 FROM system.query_log_0") + assert "1\n" == node.query("SELECT count() != 0 FROM system.part_log_0") + assert "1970-01-01\t1000\t499500000\n1970-01-02\t1000\t499501000\n" == node.query( + "SELECT date, count(), sum(id) FROM t GROUP BY date ORDER BY date LIMIT 2" + ) + assert "INFORMATION_SCHEMA\ndefault\ninformation_schema\nsystem\n" == node.query( + "SELECT name FROM system.databases ORDER BY name" + ) + + errors_count = node.count_in_log("") + assert "0\n" == errors_count or ( + "1\n" == errors_count + and "1\n" == node.count_in_log("Can't receive Netlink response") + ) + assert "0\n" == node.count_in_log(" Database") + errors_count = node.count_in_log("always include the lines below") + assert "0\n" == errors_count or ( + "1\n" == errors_count + and "1\n" == node.count_in_log("Can't receive Netlink response") + ) diff --git a/tests/integration/test_broken_detached_part_clean_up/configs/store_cleanup.xml b/tests/integration/test_broken_detached_part_clean_up/configs/store_cleanup.xml index 3b0260dd07a..5fbe87cce00 100644 --- a/tests/integration/test_broken_detached_part_clean_up/configs/store_cleanup.xml +++ b/tests/integration/test_broken_detached_part_clean_up/configs/store_cleanup.xml @@ -1,6 +1,6 @@ 0 - 15 + 60 1 5 + 1 diff --git a/tests/integration/test_cluster_copier/configs_three_nodes/users.xml b/tests/integration/test_cluster_copier/configs_three_nodes/users.xml index ce3538a31b8..f017daff974 100644 --- a/tests/integration/test_cluster_copier/configs_three_nodes/users.xml +++ b/tests/integration/test_cluster_copier/configs_three_nodes/users.xml @@ -3,6 +3,7 @@ 1 + 1 diff --git a/tests/integration/test_cluster_copier/configs_two_nodes/users.xml b/tests/integration/test_cluster_copier/configs_two_nodes/users.xml index ce3538a31b8..f017daff974 100644 --- a/tests/integration/test_cluster_copier/configs_two_nodes/users.xml +++ b/tests/integration/test_cluster_copier/configs_two_nodes/users.xml @@ -3,6 +3,7 @@ 1 + 1 diff --git a/tests/integration/test_config_substitutions/configs/config_allow_databases.xml b/tests/integration/test_config_substitutions/configs/config_allow_databases.xml index 98008306787..be727360dcf 100644 --- a/tests/integration/test_config_substitutions/configs/config_allow_databases.xml +++ b/tests/integration/test_config_substitutions/configs/config_allow_databases.xml @@ -1,4 +1,9 @@ + + + 1 + + diff --git a/tests/integration/test_cross_replication/test.py b/tests/integration/test_cross_replication/test.py index 143b8823bf2..2a73acadafd 100644 --- a/tests/integration/test_cross_replication/test.py +++ b/tests/integration/test_cross_replication/test.py @@ -37,7 +37,7 @@ def started_cluster(): CREATE DATABASE shard_{shard}; CREATE TABLE shard_{shard}.replicated(date Date, id UInt32, shard_id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/replicated', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/replicated', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( shard=shard, replica=node.name ) diff --git a/tests/integration/test_default_compression_codec/test.py b/tests/integration/test_default_compression_codec/test.py index 4af276b9728..5d033ac8f7e 100644 --- a/tests/integration/test_default_compression_codec/test.py +++ b/tests/integration/test_default_compression_codec/test.py @@ -421,7 +421,7 @@ def test_default_codec_version_update(start_cluster): ) old_version = node3.query("SELECT version()") - node3.restart_with_latest_version() + node3.restart_with_latest_version(fix_metadata=True) new_version = node3.query("SELECT version()") logging.debug(f"Updated from {old_version} to {new_version}") assert ( diff --git a/tests/integration/test_delayed_replica_failover/test.py b/tests/integration/test_delayed_replica_failover/test.py index 387d6a12f48..a480ee3f278 100644 --- a/tests/integration/test_delayed_replica_failover/test.py +++ b/tests/integration/test_delayed_replica_failover/test.py @@ -32,7 +32,7 @@ def started_cluster(): node.query( """ CREATE TABLE replicated (d Date, x UInt32) ENGINE = - ReplicatedMergeTree('/clickhouse/tables/{shard}/replicated', '{instance}', d, d, 8192)""".format( + ReplicatedMergeTree('/clickhouse/tables/{shard}/replicated', '{instance}') PARTITION BY toYYYYMM(d) ORDER BY d""".format( shard=shard, instance=node.name ) ) diff --git a/tests/integration/test_dictionaries_dependency/test.py b/tests/integration/test_dictionaries_dependency/test.py index f57d4e42813..2042db69fa2 100644 --- a/tests/integration/test_dictionaries_dependency/test.py +++ b/tests/integration/test_dictionaries_dependency/test.py @@ -16,7 +16,10 @@ def start_cluster(): for node in nodes: node.query("CREATE DATABASE IF NOT EXISTS test") # Different internal dictionary name with Atomic - node.query("CREATE DATABASE IF NOT EXISTS test_ordinary ENGINE=Ordinary") + node.query( + "CREATE DATABASE IF NOT EXISTS test_ordinary ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query("CREATE DATABASE IF NOT EXISTS atest") node.query("CREATE DATABASE IF NOT EXISTS ztest") node.query("CREATE TABLE test.source(x UInt64, y UInt64) ENGINE=Log") diff --git a/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml b/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml index 26db7f54514..ef8abbd9174 100644 --- a/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml +++ b/tests/integration/test_distributed_ddl/configs/users.d/query_log.xml @@ -3,6 +3,7 @@ 1 + 1 diff --git a/tests/integration/test_distributed_ddl/configs_secure/users.d/query_log.xml b/tests/integration/test_distributed_ddl/configs_secure/users.d/query_log.xml index 26db7f54514..ef8abbd9174 100644 --- a/tests/integration/test_distributed_ddl/configs_secure/users.d/query_log.xml +++ b/tests/integration/test_distributed_ddl/configs_secure/users.d/query_log.xml @@ -3,6 +3,7 @@ 1 + 1 diff --git a/tests/integration/test_distributed_ddl/test.py b/tests/integration/test_distributed_ddl/test.py index 2789541b519..85d0a5f0999 100755 --- a/tests/integration/test_distributed_ddl/test.py +++ b/tests/integration/test_distributed_ddl/test.py @@ -552,7 +552,9 @@ def test_replicated_without_arguments(test_cluster): ) test_cluster.ddl_check_query( - instance, "CREATE DATABASE test_ordinary ON CLUSTER cluster ENGINE=Ordinary" + instance, + "CREATE DATABASE test_ordinary ON CLUSTER cluster ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) assert ( "are supported only for ON CLUSTER queries with Atomic database engine" diff --git a/tests/integration/test_distributed_storage_configuration/test.py b/tests/integration/test_distributed_storage_configuration/test.py index fa4e01bb7b3..950ce1034fe 100644 --- a/tests/integration/test_distributed_storage_configuration/test.py +++ b/tests/integration/test_distributed_storage_configuration/test.py @@ -20,7 +20,8 @@ def start_cluster(): try: cluster.start() node.query( - "CREATE DATABASE IF NOT EXISTS test ENGINE=Ordinary" + "CREATE DATABASE IF NOT EXISTS test ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different paths with Atomic yield cluster finally: diff --git a/tests/integration/test_extreme_deduplication/test.py b/tests/integration/test_extreme_deduplication/test.py index 2c8772aad4e..71f783d37c9 100644 --- a/tests/integration/test_extreme_deduplication/test.py +++ b/tests/integration/test_extreme_deduplication/test.py @@ -40,7 +40,7 @@ def test_deduplication_window_in_seconds(started_cluster): node1.query( """ CREATE TABLE simple ON CLUSTER test_cluster (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}', date, id, 8192)""" + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id""" ) node.query("INSERT INTO simple VALUES (0, 0)") @@ -77,7 +77,7 @@ def test_deduplication_works_in_case_of_intensive_inserts(started_cluster): node1.query( """ CREATE TABLE simple ON CLUSTER test_cluster (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}', date, id, 8192)""" + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id""" ) node1.query("INSERT INTO simple VALUES (0, 0)") diff --git a/tests/integration/test_filesystem_layout/test.py b/tests/integration/test_filesystem_layout/test.py index 34e377e0ae4..898bbc40eb9 100644 --- a/tests/integration/test_filesystem_layout/test.py +++ b/tests/integration/test_filesystem_layout/test.py @@ -16,7 +16,10 @@ def started_cluster(): def test_file_path_escaping(started_cluster): - node.query("CREATE DATABASE IF NOT EXISTS test ENGINE = Ordinary") + node.query( + "CREATE DATABASE IF NOT EXISTS test ENGINE = Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query( """ CREATE TABLE test.`T.a_b,l-e!` (`~Id` UInt32) diff --git a/tests/integration/test_force_drop_table/test.py b/tests/integration/test_force_drop_table/test.py index c1eec1cd277..ae6c7a1a3bf 100644 --- a/tests/integration/test_force_drop_table/test.py +++ b/tests/integration/test_force_drop_table/test.py @@ -33,7 +33,10 @@ def create_force_drop_flag(node): @pytest.mark.parametrize("engine", ["Ordinary", "Atomic"]) def test_drop_materialized_view(started_cluster, engine): - node.query("CREATE DATABASE d ENGINE={}".format(engine)) + node.query( + "CREATE DATABASE d ENGINE={}".format(engine), + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query( "CREATE TABLE d.rmt (n UInt64) ENGINE=ReplicatedMergeTree('/test/rmt', 'r1') ORDER BY n PARTITION BY n % 2" ) diff --git a/tests/integration/test_https_replication/test.py b/tests/integration/test_https_replication/test.py index 4cf9f19b870..301487aa6cf 100644 --- a/tests/integration/test_https_replication/test.py +++ b/tests/integration/test_https_replication/test.py @@ -19,7 +19,7 @@ def _fill_nodes(nodes, shard): CREATE DATABASE test; CREATE TABLE test_table(date Date, id UInt32, dummy UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( shard=shard, replica=node.name ) diff --git a/tests/integration/test_insert_into_distributed/test.py b/tests/integration/test_insert_into_distributed/test.py index b8d94d2a043..a52809f817c 100644 --- a/tests/integration/test_insert_into_distributed/test.py +++ b/tests/integration/test_insert_into_distributed/test.py @@ -52,7 +52,7 @@ CREATE TABLE distributed (x UInt32) ENGINE = Distributed('test_cluster', 'defaul ) remote.query( - "CREATE TABLE local2 (d Date, x UInt32, s String) ENGINE = MergeTree(d, x, 8192)" + "CREATE TABLE local2 (d Date, x UInt32, s String) ENGINE = MergeTree PARTITION BY toYYYYMM(d) ORDER BY x" ) instance_test_inserts_batching.query( """ @@ -61,7 +61,7 @@ CREATE TABLE distributed (d Date, x UInt32) ENGINE = Distributed('test_cluster', ) instance_test_inserts_local_cluster.query( - "CREATE TABLE local (d Date, x UInt32) ENGINE = MergeTree(d, x, 8192)" + "CREATE TABLE local (d Date, x UInt32) ENGINE = MergeTree PARTITION BY toYYYYMM(d) ORDER BY x" ) instance_test_inserts_local_cluster.query( """ @@ -71,12 +71,12 @@ CREATE TABLE distributed_on_local (d Date, x UInt32) ENGINE = Distributed('test_ node1.query( """ -CREATE TABLE replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/replicated', 'node1', date, id, 8192) +CREATE TABLE replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/replicated', 'node1') PARTITION BY toYYYYMM(date) ORDER BY id """ ) node2.query( """ -CREATE TABLE replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/replicated', 'node2', date, id, 8192) +CREATE TABLE replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/replicated', 'node2') PARTITION BY toYYYYMM(date) ORDER BY id """ ) @@ -94,12 +94,12 @@ CREATE TABLE distributed (date Date, id UInt32) ENGINE = Distributed('shard_with shard1.query( """ -CREATE TABLE low_cardinality (d Date, x UInt32, s LowCardinality(String)) ENGINE = MergeTree(d, x, 8192)""" +CREATE TABLE low_cardinality (d Date, x UInt32, s LowCardinality(String)) ENGINE = MergeTree PARTITION BY toYYYYMM(d) ORDER BY x""" ) shard2.query( """ -CREATE TABLE low_cardinality (d Date, x UInt32, s LowCardinality(String)) ENGINE = MergeTree(d, x, 8192)""" +CREATE TABLE low_cardinality (d Date, x UInt32, s LowCardinality(String)) ENGINE = MergeTree PARTITION BY toYYYYMM(d) ORDER BY x""" ) shard1.query( @@ -143,7 +143,7 @@ CREATE TABLE distributed_one_replica_no_internal_replication (date Date, id UInt node2.query( """ -CREATE TABLE single_replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/single_replicated', 'node2', date, id, 8192) +CREATE TABLE single_replicated(date Date, id UInt32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/single_replicated', 'node2') PARTITION BY toYYYYMM(date) ORDER BY id """ ) @@ -228,11 +228,11 @@ def test_inserts_batching(started_cluster): # 4. Full batch of inserts after ALTER (that have different block structure). # 5. What was left to insert with the column structure before ALTER. expected = """\ -20000101_20000101_1_1_0\t[1] -20000101_20000101_2_2_0\t[2,3,4] -20000101_20000101_3_3_0\t[5,6,7] -20000101_20000101_4_4_0\t[10,11,12] -20000101_20000101_5_5_0\t[8,9] +200001_1_1_0\t[1] +200001_2_2_0\t[2,3,4] +200001_3_3_0\t[5,6,7] +200001_4_4_0\t[10,11,12] +200001_5_5_0\t[8,9] """ assert TSV(result) == TSV(expected) diff --git a/tests/integration/test_insert_into_distributed_sync_async/test.py b/tests/integration/test_insert_into_distributed_sync_async/test.py index e0c454feee6..12423cc4747 100755 --- a/tests/integration/test_insert_into_distributed_sync_async/test.py +++ b/tests/integration/test_insert_into_distributed_sync_async/test.py @@ -23,7 +23,7 @@ def started_cluster(): for node in (node1, node2): node.query( """ -CREATE TABLE local_table(date Date, val UInt64) ENGINE = MergeTree(date, (date, val), 8192); +CREATE TABLE local_table(date Date, val UInt64) ENGINE = MergeTree() PARTITION BY toYYYYMM(date) ORDER BY (date, val); """ ) diff --git a/tests/integration/test_insert_into_distributed_through_materialized_view/configs/enable_distributed_inserts_batching.xml b/tests/integration/test_insert_into_distributed_through_materialized_view/configs/enable_distributed_inserts_batching.xml index de0c930b8ab..295d1c8d3cc 100644 --- a/tests/integration/test_insert_into_distributed_through_materialized_view/configs/enable_distributed_inserts_batching.xml +++ b/tests/integration/test_insert_into_distributed_through_materialized_view/configs/enable_distributed_inserts_batching.xml @@ -3,6 +3,7 @@ 1 3 + 1 diff --git a/tests/integration/test_insert_into_distributed_through_materialized_view/test.py b/tests/integration/test_insert_into_distributed_through_materialized_view/test.py index 7c2ce9f05f2..a5f92002450 100644 --- a/tests/integration/test_insert_into_distributed_through_materialized_view/test.py +++ b/tests/integration/test_insert_into_distributed_through_materialized_view/test.py @@ -44,7 +44,8 @@ CREATE TABLE distributed (x UInt32) ENGINE = Distributed('test_cluster', 'defaul ) remote.query( - "CREATE TABLE local2 (d Date, x UInt32, s String) ENGINE = MergeTree(d, x, 8192)" + "CREATE TABLE local2 (d Date, x UInt32, s String) ENGINE = MergeTree(d, x, 8192)", + settings={"allow_deprecated_syntax_for_merge_tree": 1}, ) instance_test_inserts_batching.query( """ @@ -65,7 +66,8 @@ CREATE TABLE distributed (d Date, x UInt32) ENGINE = Distributed('test_cluster', "CREATE MATERIALIZED VIEW local_view to distributed_on_local AS SELECT d,x FROM local_source" ) instance_test_inserts_local_cluster.query( - "CREATE TABLE local (d Date, x UInt32) ENGINE = MergeTree(d, x, 8192)" + "CREATE TABLE local (d Date, x UInt32) ENGINE = MergeTree(d, x, 8192)", + settings={"allow_deprecated_syntax_for_merge_tree": 1}, ) instance_test_inserts_local_cluster.query( """ diff --git a/tests/integration/test_keeper_multinode_blocade_leader/test.py b/tests/integration/test_keeper_multinode_blocade_leader/test.py index c2d4039e122..d6d01a5d0a6 100644 --- a/tests/integration/test_keeper_multinode_blocade_leader/test.py +++ b/tests/integration/test_keeper_multinode_blocade_leader/test.py @@ -95,7 +95,10 @@ def test_blocade_leader(started_cluster): wait_nodes() try: for i, node in enumerate([node1, node2, node3]): - node.query("CREATE DATABASE IF NOT EXISTS ordinary ENGINE=Ordinary") + node.query( + "CREATE DATABASE IF NOT EXISTS ordinary ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query( "CREATE TABLE IF NOT EXISTS ordinary.t1 (value UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/t1', '{}') ORDER BY tuple()".format( i + 1 @@ -296,7 +299,10 @@ def test_blocade_leader_twice(started_cluster): wait_nodes() try: for i, node in enumerate([node1, node2, node3]): - node.query("CREATE DATABASE IF NOT EXISTS ordinary ENGINE=Ordinary") + node.query( + "CREATE DATABASE IF NOT EXISTS ordinary ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query( "CREATE TABLE IF NOT EXISTS ordinary.t2 (value UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/t2', '{}') ORDER BY tuple()".format( i + 1 diff --git a/tests/integration/test_materialized_mysql_database/configs/users.xml b/tests/integration/test_materialized_mysql_database/configs/users.xml index 4b7f5a1b109..0e116f115fe 100644 --- a/tests/integration/test_materialized_mysql_database/configs/users.xml +++ b/tests/integration/test_materialized_mysql_database/configs/users.xml @@ -3,7 +3,6 @@ 1 - Atomic 1 0 diff --git a/tests/integration/test_materialized_mysql_database/configs/users_disable_bytes_settings.xml b/tests/integration/test_materialized_mysql_database/configs/users_disable_bytes_settings.xml index f590ebff6b4..a00b6ca6b9a 100644 --- a/tests/integration/test_materialized_mysql_database/configs/users_disable_bytes_settings.xml +++ b/tests/integration/test_materialized_mysql_database/configs/users_disable_bytes_settings.xml @@ -3,7 +3,6 @@ 1 - Atomic 1 0 diff --git a/tests/integration/test_materialized_mysql_database/configs/users_disable_rows_settings.xml b/tests/integration/test_materialized_mysql_database/configs/users_disable_rows_settings.xml index e0fa0a9097b..3a7cc2537e5 100644 --- a/tests/integration/test_materialized_mysql_database/configs/users_disable_rows_settings.xml +++ b/tests/integration/test_materialized_mysql_database/configs/users_disable_rows_settings.xml @@ -3,7 +3,6 @@ 1 - Atomic 0 1 diff --git a/tests/integration/test_merge_tree_empty_parts/test.py b/tests/integration/test_merge_tree_empty_parts/test.py index 7ca275e96de..57bf49e6803 100644 --- a/tests/integration/test_merge_tree_empty_parts/test.py +++ b/tests/integration/test_merge_tree_empty_parts/test.py @@ -25,7 +25,7 @@ def started_cluster(): def test_empty_parts_alter_delete(started_cluster): node1.query( "CREATE TABLE empty_parts_delete (d Date, key UInt64, value String) \ - ENGINE = ReplicatedMergeTree('/clickhouse/tables/empty_parts_delete', 'r1', d, key, 8192)" + ENGINE = ReplicatedMergeTree('/clickhouse/tables/empty_parts_delete', 'r1') PARTITION BY toYYYYMM(d) ORDER BY key" ) node1.query("INSERT INTO empty_parts_delete VALUES (toDate('2020-10-10'), 1, 'a')") @@ -44,7 +44,7 @@ def test_empty_parts_alter_delete(started_cluster): def test_empty_parts_summing(started_cluster): node1.query( "CREATE TABLE empty_parts_summing (d Date, key UInt64, value Int64) \ - ENGINE = ReplicatedSummingMergeTree('/clickhouse/tables/empty_parts_summing', 'r1', d, key, 8192)" + ENGINE = ReplicatedSummingMergeTree('/clickhouse/tables/empty_parts_summing', 'r1') PARTITION BY toYYYYMM(d) ORDER BY key" ) node1.query("INSERT INTO empty_parts_summing VALUES (toDate('2020-10-10'), 1, 1)") diff --git a/tests/integration/test_merge_tree_s3_restore/test.py b/tests/integration/test_merge_tree_s3_restore/test.py index e6ca4a78c25..f4acc4ac91e 100644 --- a/tests/integration/test_merge_tree_s3_restore/test.py +++ b/tests/integration/test_merge_tree_s3_restore/test.py @@ -93,7 +93,8 @@ def create_table( node.query( "CREATE DATABASE IF NOT EXISTS s3 ENGINE = {engine}".format( engine="Atomic" if db_atomic else "Ordinary" - ) + ), + settings={"allow_deprecated_database_ordinary": 1}, ) create_table_statement = """ diff --git a/tests/integration/test_mutations_with_merge_tree/test.py b/tests/integration/test_mutations_with_merge_tree/test.py index d1843017b9f..7831cde7dea 100644 --- a/tests/integration/test_mutations_with_merge_tree/test.py +++ b/tests/integration/test_mutations_with_merge_tree/test.py @@ -17,7 +17,7 @@ def started_cluster(): try: cluster.start() instance_test_mutations.query( - """CREATE TABLE test_mutations_with_ast_elements(date Date, a UInt64, b String) ENGINE = MergeTree(date, (a, date), 8192)""" + """CREATE TABLE test_mutations_with_ast_elements(date Date, a UInt64, b String) ENGINE = MergeTree PARTITION BY toYYYYMM(date) ORDER BY (a, date)""" ) instance_test_mutations.query( """INSERT INTO test_mutations_with_ast_elements SELECT '2019-07-29' AS date, 1, toString(number) FROM numbers(1) SETTINGS force_index_by_date = 0, force_primary_key = 0""" diff --git a/tests/integration/test_partition/test.py b/tests/integration/test_partition/test.py index b396b58df10..f3df66631a5 100644 --- a/tests/integration/test_partition/test.py +++ b/tests/integration/test_partition/test.py @@ -14,7 +14,8 @@ def started_cluster(): try: cluster.start() q( - "CREATE DATABASE test ENGINE = Ordinary" + "CREATE DATABASE test ENGINE = Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different path in shadow/ with Atomic yield cluster diff --git a/tests/integration/test_parts_removal/__init__.py b/tests/integration/test_parts_removal/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_parts_removal/test.py b/tests/integration/test_parts_removal/test.py new file mode 100644 index 00000000000..4772178d63b --- /dev/null +++ b/tests/integration/test_parts_removal/test.py @@ -0,0 +1,71 @@ +# pylint: disable=unused-argument +# pylint: disable=redefined-outer-name +# pylint: disable=line-too-long + +import pytest + +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) + +node = cluster.add_instance("node", stay_alive=True) + + +def query_split(node, query): + return list( + map(lambda x: x.strip().split("\t"), node.query(query).strip().split("\n")) + ) + + +@pytest.fixture(scope="module") +def start_cluster(): + try: + cluster.start() + yield cluster + finally: + cluster.shutdown() + + +def test_parts_removal_on_abnormal_exit(start_cluster): + node.query( + """ + create table test_parts_removal (key Int) engine=MergeTree order by key; + insert into test_parts_removal values (1); -- all_1_1_0 + insert into test_parts_removal values (2); -- all_1_2_0 + optimize table test_parts_removal; -- all_2_2_0 + """ + ) + + parts = query_split( + node, "select name, _state from system.parts where table = 'test_parts_removal'" + ) + assert parts == [ + ["all_1_1_0", "Outdated"], + ["all_1_2_1", "Active"], + ["all_2_2_0", "Outdated"], + ] + + node.restart_clickhouse(kill=True) + + parts = query_split( + node, "select name, _state from system.parts where table = 'test_parts_removal'" + ) + assert parts == [ + ["all_1_1_0", "Outdated"], + ["all_1_2_1", "Active"], + ["all_2_2_0", "Outdated"], + ] + + node.query( + """ + detach table test_parts_removal; + attach table test_parts_removal; + """ + ) + + parts = query_split( + node, "select name, _state from system.parts where table = 'test_parts_removal'" + ) + assert parts == [ + ["all_1_2_1", "Active"], + ] diff --git a/tests/integration/test_polymorphic_parts/configs/users.d/not_optimize_count.xml b/tests/integration/test_polymorphic_parts/configs/users.d/not_optimize_count.xml index 7f8036c4f87..c654cf0791a 100644 --- a/tests/integration/test_polymorphic_parts/configs/users.d/not_optimize_count.xml +++ b/tests/integration/test_polymorphic_parts/configs/users.d/not_optimize_count.xml @@ -2,6 +2,7 @@ 0 + 1 diff --git a/tests/integration/test_polymorphic_parts/test.py b/tests/integration/test_polymorphic_parts/test.py index edd65ec002c..32b5e531fa8 100644 --- a/tests/integration/test_polymorphic_parts/test.py +++ b/tests/integration/test_polymorphic_parts/test.py @@ -66,7 +66,8 @@ def create_tables_old_format(name, nodes, shard): ENGINE = ReplicatedMergeTree('/clickhouse/tables/test/{shard}/{name}', '{repl}', date, id, 64) """.format( name=name, shard=shard, repl=i - ) + ), + settings={"allow_deprecated_syntax_for_merge_tree": 1}, ) @@ -495,7 +496,7 @@ def test_polymorphic_parts_diff_versions_2(start_cluster_diff_versions): with pytest.raises(Exception): node_old.query("SYSTEM SYNC REPLICA polymorphic_table_2", timeout=3) - node_old.restart_with_latest_version() + node_old.restart_with_latest_version(fix_metadata=True) node_old.query("SYSTEM SYNC REPLICA polymorphic_table_2", timeout=20) @@ -720,7 +721,8 @@ def test_in_memory_alters(start_cluster): def test_polymorphic_parts_index(start_cluster): node1.query( - "CREATE DATABASE test_index ENGINE=Ordinary" + "CREATE DATABASE test_index ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different paths with Atomic node1.query( """ diff --git a/tests/integration/test_random_inserts/test.py b/tests/integration/test_random_inserts/test.py index 4d6aaa9276d..9ac0c5b024c 100644 --- a/tests/integration/test_random_inserts/test.py +++ b/tests/integration/test_random_inserts/test.py @@ -44,7 +44,7 @@ def test_random_inserts(started_cluster): node1.query( """ CREATE TABLE simple ON CLUSTER test_cluster (date Date, i UInt32, s String) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}', date, i, 8192)""" + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{shard}/simple', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY i""" ) with PartitionManager() as pm_random_drops: diff --git a/tests/integration/test_replace_partition/test.py b/tests/integration/test_replace_partition/test.py index 7ce79d9aca8..579b22286b9 100644 --- a/tests/integration/test_replace_partition/test.py +++ b/tests/integration/test_replace_partition/test.py @@ -20,13 +20,13 @@ def _fill_nodes(nodes, shard): CREATE DATABASE test; CREATE TABLE real_table(date Date, id UInt32, dummy UInt32) - ENGINE = MergeTree(date, id, 8192); + ENGINE = MergeTree PARTITION BY toYYYYMM(date) ORDER BY id; CREATE TABLE other_table(date Date, id UInt32, dummy UInt32) - ENGINE = MergeTree(date, id, 8192); + ENGINE = MergeTree PARTITION BY toYYYYMM(date) ORDER BY id; CREATE TABLE test_table(date Date, id UInt32, dummy UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( shard=shard, replica=node.name ) diff --git a/tests/integration/test_replicated_database/test.py b/tests/integration/test_replicated_database/test.py index 92c109974e1..11ca0d2f962 100644 --- a/tests/integration/test_replicated_database/test.py +++ b/tests/integration/test_replicated_database/test.py @@ -96,13 +96,16 @@ def test_create_replicated_table(started_cluster): "Explicit zookeeper_path and replica_name are specified" in main_node.query_and_get_error( "CREATE TABLE testdb.replicated_table (d Date, k UInt64, i32 Int32) " - "ENGINE=ReplicatedMergeTree('/test/tmp', 'r', d, k, 8192);" + "ENGINE=ReplicatedMergeTree('/test/tmp', 'r') ORDER BY k PARTITION BY toYYYYMM(d);" ) ) - assert "Old syntax is not allowed" in main_node.query_and_get_error( - "CREATE TABLE testdb.replicated_table (d Date, k UInt64, i32 Int32) " - "ENGINE=ReplicatedMergeTree('/test/tmp/{shard}', '{replica}', d, k, 8192);" + assert ( + "This syntax for *MergeTree engine is deprecated" + in main_node.query_and_get_error( + "CREATE TABLE testdb.replicated_table (d Date, k UInt64, i32 Int32) " + "ENGINE=ReplicatedMergeTree('/test/tmp/{shard}', '{replica}', d, k, 8192);" + ) ) main_node.query( @@ -393,7 +396,7 @@ def test_alters_from_different_replicas(started_cluster): main_node.query( "CREATE TABLE testdb.concurrent_test " "(CounterID UInt32, StartDate Date, UserID UInt32, VisitID UInt32, NestedColumn Nested(A UInt8, S String), ToDrop UInt32) " - "ENGINE = MergeTree(StartDate, intHash32(UserID), (CounterID, StartDate, intHash32(UserID), VisitID), 8192);" + "ENGINE = MergeTree PARTITION BY toYYYYMM(StartDate) ORDER BY (CounterID, StartDate, intHash32(UserID), VisitID);" ) main_node.query( @@ -443,7 +446,7 @@ def test_alters_from_different_replicas(started_cluster): " `Added0` UInt32,\\n `Added1` UInt32,\\n `Added2` UInt32,\\n `AddedNested1.A` Array(UInt32),\\n" " `AddedNested1.B` Array(UInt64),\\n `AddedNested1.C` Array(String),\\n `AddedNested2.A` Array(UInt32),\\n" " `AddedNested2.B` Array(UInt64)\\n)\\n" - "ENGINE = MergeTree(StartDate, intHash32(UserID), (CounterID, StartDate, intHash32(UserID), VisitID), 8192)" + "ENGINE = MergeTree\\nPARTITION BY toYYYYMM(StartDate)\\nORDER BY (CounterID, StartDate, intHash32(UserID), VisitID)\\nSETTINGS index_granularity = 8192" ) assert_create_query([main_node, competing_node], "testdb.concurrent_test", expected) diff --git a/tests/integration/test_replicated_merge_tree_compatibility/test.py b/tests/integration/test_replicated_merge_tree_compatibility/test.py index 00367daad33..eb2b14ffb1a 100644 --- a/tests/integration/test_replicated_merge_tree_compatibility/test.py +++ b/tests/integration/test_replicated_merge_tree_compatibility/test.py @@ -52,7 +52,10 @@ def test_replicated_merge_tree_defaults_compatibility(started_cluster): """ for node in (node1, node2): - node.query("CREATE DATABASE test ENGINE = Ordinary") + node.query( + "CREATE DATABASE test ENGINE = Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) node.query(create_query.format(replica=node.name)) node1.query("DETACH TABLE test.table") diff --git a/tests/integration/test_replicated_merge_tree_s3_restore/test.py b/tests/integration/test_replicated_merge_tree_s3_restore/test.py index 904bcfa4280..d743dedbdde 100644 --- a/tests/integration/test_replicated_merge_tree_s3_restore/test.py +++ b/tests/integration/test_replicated_merge_tree_s3_restore/test.py @@ -76,7 +76,8 @@ def create_table(node, table_name, schema, attach=False, db_atomic=False, uuid=" "CREATE DATABASE IF NOT EXISTS s3 {on_cluster} ENGINE = {engine}".format( engine="Atomic" if db_atomic else "Ordinary", on_cluster="ON CLUSTER '{cluster}'", - ) + ), + settings={"allow_deprecated_database_ordinary": 1}, ) create_table_statement = """ diff --git a/tests/integration/test_replication_credentials/test.py b/tests/integration/test_replication_credentials/test.py index e5313cb6bd4..79588fcd38b 100644 --- a/tests/integration/test_replication_credentials/test.py +++ b/tests/integration/test_replication_credentials/test.py @@ -10,7 +10,7 @@ def _fill_nodes(nodes, shard): """ CREATE DATABASE test; CREATE TABLE test_table(date Date, id UInt32, dummy UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( shard=shard, replica=node.name ) diff --git a/tests/integration/test_send_request_to_leader_replica/test.py b/tests/integration/test_send_request_to_leader_replica/test.py index 60df18bf7d3..b56e1315672 100644 --- a/tests/integration/test_send_request_to_leader_replica/test.py +++ b/tests/integration/test_send_request_to_leader_replica/test.py @@ -40,7 +40,7 @@ def started_cluster(): node.query( """ CREATE TABLE sometable(date Date, id UInt32, value Int32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/sometable', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/sometable', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( replica=node.name ), @@ -51,7 +51,7 @@ def started_cluster(): node.query( """ CREATE TABLE someothertable(date Date, id UInt32, value Int32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/someothertable', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/someothertable', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( replica=node.name ), diff --git a/tests/integration/test_server_initialization/clickhouse_path/metadata/default.sql b/tests/integration/test_server_initialization/clickhouse_path/metadata/default.sql new file mode 100644 index 00000000000..9fae1d57726 --- /dev/null +++ b/tests/integration/test_server_initialization/clickhouse_path/metadata/default.sql @@ -0,0 +1 @@ +ATTACH DATABASE default ENGINE=Ordinary \ No newline at end of file diff --git a/tests/integration/test_storage_rabbitmq/test.py b/tests/integration/test_storage_rabbitmq/test.py index c1bd136126f..64f104dd4ff 100644 --- a/tests/integration/test_storage_rabbitmq/test.py +++ b/tests/integration/test_storage_rabbitmq/test.py @@ -2745,7 +2745,40 @@ def test_rabbitmq_predefined_configuration(rabbitmq_cluster): break -if __name__ == "__main__": - cluster.start() - input("Cluster created, press any key to destroy...") - cluster.shutdown() +def test_rabbitmq_msgpack(rabbitmq_cluster): + + instance.query( + """ + drop table if exists rabbit_in; + drop table if exists rabbit_out; + create table + rabbit_in (val String) + engine=RabbitMQ + settings rabbitmq_host_port = 'rabbitmq1:5672', + rabbitmq_exchange_name = 'xhep', + rabbitmq_format = 'MsgPack', + rabbitmq_num_consumers = 1; + create table + rabbit_out (val String) + engine=RabbitMQ + settings rabbitmq_host_port = 'rabbitmq1:5672', + rabbitmq_exchange_name = 'xhep', + rabbitmq_format = 'MsgPack', + rabbitmq_num_consumers = 1; + set stream_like_engine_allow_direct_select=1; + insert into rabbit_out select 'kek'; + """ + ) + + result = "" + try_no = 0 + while True: + result = instance.query("select * from rabbit_in;") + if result.strip() == "kek": + break + else: + try_no = try_no + 1 + if try_no == 20: + break + time.sleep(1) + assert result.strip() == "kek" diff --git a/tests/integration/test_system_merges/test.py b/tests/integration/test_system_merges/test.py index 775706f4df6..b908ba0eb33 100644 --- a/tests/integration/test_system_merges/test.py +++ b/tests/integration/test_system_merges/test.py @@ -26,9 +26,13 @@ def started_cluster(): try: cluster.start() node1.query( - "CREATE DATABASE test ENGINE=Ordinary" + "CREATE DATABASE test ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, ) # Different paths with Atomic - node2.query("CREATE DATABASE test ENGINE=Ordinary") + node2.query( + "CREATE DATABASE test ENGINE=Ordinary", + settings={"allow_deprecated_database_ordinary": 1}, + ) yield cluster finally: diff --git a/tests/integration/test_transactions/test.py b/tests/integration/test_transactions/test.py index 8983e70b4cb..2dfdc889856 100644 --- a/tests/integration/test_transactions/test.py +++ b/tests/integration/test_transactions/test.py @@ -108,13 +108,13 @@ def test_rollback_unfinished_on_restart(start_cluster): assert ( res == "0_2_2_0\t1\ttid0\tcsn1_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" - "0_2_4_1\t0\ttid4\tcsn18446744073709551615_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" + "0_2_4_1\t0\ttid4\tcsn18446744073709551615_\ttid0\tcsn0_\n" "0_4_4_0\t1\ttid2\tcsn_2\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" - "0_8_8_0\t0\ttid5\tcsn18446744073709551615_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" + "0_8_8_0\t0\ttid5\tcsn18446744073709551615_\ttid0\tcsn0_\n" "1_1_1_0\t0\ttid0\tcsn1_\ttid1\tcsn_1\n" "1_3_3_0\t1\ttid2\tcsn_2\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" - "1_3_3_0_7\t0\ttid3\tcsn18446744073709551615_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" + "1_3_3_0_7\t0\ttid3\tcsn18446744073709551615_\ttid0\tcsn0_\n" "1_5_5_0\t1\ttid6\tcsn_6\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" - "1_6_6_0\t0\ttid3\tcsn18446744073709551615_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" - "1_6_6_0_7\t0\ttid3\tcsn18446744073709551615_\t(0,0,'00000000-0000-0000-0000-000000000000')\tcsn0_\n" + "1_6_6_0\t0\ttid3\tcsn18446744073709551615_\ttid0\tcsn0_\n" + "1_6_6_0_7\t0\ttid3\tcsn18446744073709551615_\ttid0\tcsn0_\n" ) diff --git a/tests/integration/test_union_header/test.py b/tests/integration/test_union_header/test.py index f883057c1d8..2e7f6cb399a 100644 --- a/tests/integration/test_union_header/test.py +++ b/tests/integration/test_union_header/test.py @@ -27,7 +27,7 @@ def started_cluster(): log_type UInt32, account_id String ) - ENGINE = MergeTree(event_date, (event_time, account_id), 8192); + ENGINE = MergeTree PARTITION BY toYYYYMM(event_date) ORDER BY (event_time, account_id); """ ) diff --git a/tests/integration/test_version_update_after_mutation/test.py b/tests/integration/test_version_update_after_mutation/test.py index 2971cbc9792..a21186bba8d 100644 --- a/tests/integration/test_version_update_after_mutation/test.py +++ b/tests/integration/test_version_update_after_mutation/test.py @@ -58,8 +58,8 @@ def test_mutate_and_upgrade(start_cluster): node2.query("DETACH TABLE mt") # stop being leader node1.query("DETACH TABLE mt") # stop being leader - node1.restart_with_latest_version(signal=9) - node2.restart_with_latest_version(signal=9) + node1.restart_with_latest_version(signal=9, fix_metadata=True) + node2.restart_with_latest_version(signal=9, fix_metadata=True) # After hard restart table can be in readonly mode exec_query_with_retry( @@ -111,7 +111,7 @@ def test_upgrade_while_mutation(start_cluster): node3.query("ALTER TABLE mt1 DELETE WHERE id % 2 == 0") node3.query("DETACH TABLE mt1") # stop being leader - node3.restart_with_latest_version(signal=9) + node3.restart_with_latest_version(signal=9, fix_metadata=True) # checks for readonly exec_query_with_retry(node3, "OPTIMIZE TABLE mt1", sleep_time=5, retry_count=60) diff --git a/tests/integration/test_zookeeper_config/test.py b/tests/integration/test_zookeeper_config/test.py index d3d90ca0d4f..65f82c2286b 100644 --- a/tests/integration/test_zookeeper_config/test.py +++ b/tests/integration/test_zookeeper_config/test.py @@ -48,7 +48,7 @@ def test_chroot_with_same_root(started_cluster): node.query( """ CREATE TABLE simple (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( replica=node.name ) @@ -68,7 +68,7 @@ def test_chroot_with_different_root(started_cluster): node.query( """ CREATE TABLE simple_different (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple_different', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple_different', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( replica=node.name ) diff --git a/tests/integration/test_zookeeper_config/test_password.py b/tests/integration/test_zookeeper_config/test_password.py index 580b426db6f..71f059b3277 100644 --- a/tests/integration/test_zookeeper_config/test_password.py +++ b/tests/integration/test_zookeeper_config/test_password.py @@ -35,7 +35,7 @@ def test_identity(started_cluster): node1.query( """ CREATE TABLE simple (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '{replica}', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id; """.format( replica=node1.name ) @@ -45,6 +45,6 @@ def test_identity(started_cluster): node2.query( """ CREATE TABLE simple (date Date, id UInt32) - ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '1', date, id, 8192); + ENGINE = ReplicatedMergeTree('/clickhouse/tables/0/simple', '1') PARTITION BY toYYYYMM(date) ORDER BY id; """ ) diff --git a/tests/performance/merge_table_streams.xml b/tests/performance/merge_table_streams.xml index efeb4547f37..1e053c98738 100644 --- a/tests/performance/merge_table_streams.xml +++ b/tests/performance/merge_table_streams.xml @@ -1,6 +1,7 @@ 5 + 1