From ddb02282f195401c68c9e81f339b3e2893bbd330 Mon Sep 17 00:00:00 2001 From: taiyang-li Date: Sun, 24 Jan 2021 11:52:17 +0800 Subject: [PATCH 01/78] enable local disk config reload --- src/Disks/DiskLocal.cpp | 114 +++++++++++++++++++++++-------------- src/Disks/DiskLocal.h | 9 ++- src/Disks/DiskSelector.cpp | 5 +- src/Disks/DiskSelector.h | 2 +- 4 files changed, 83 insertions(+), 47 deletions(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index ed6b82fa73d..ea902bae6dc 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -34,6 +34,60 @@ std::mutex DiskLocal::reservation_mutex; using DiskLocalPtr = std::shared_ptr; +static void loadDiskLocalConfig(const String & name, + const Poco::Util::AbstractConfiguration & config, + const String & config_prefix, + const Context & context, + String & path, + UInt64 & keep_free_space_bytes) +{ + path = config.getString(config_prefix + ".path", ""); + if (name == "default") + { + if (!path.empty()) + throw Exception( + "\"default\" disk path should be provided in not in ", + ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + path = context.getPath(); + } + else + { + if (path.empty()) + throw Exception("Disk path can not be empty. Disk " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + if (path.back() != '/') + throw Exception("Disk path must end with /. Disk " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + } + + if (Poco::File disk{path}; !disk.canRead() || !disk.canWrite()) + { + throw Exception("There is no RW access to disk " + name + " (" + path + ")", ErrorCodes::PATH_ACCESS_DENIED); + } + + bool has_space_ratio = config.has(config_prefix + ".keep_free_space_ratio"); + + if (config.has(config_prefix + ".keep_free_space_bytes") && has_space_ratio) + throw Exception( + "Only one of 'keep_free_space_bytes' and 'keep_free_space_ratio' can be specified", + ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG); + + keep_free_space_bytes = config.getUInt64(config_prefix + ".keep_free_space_bytes", 0); + + if (has_space_ratio) + { + auto ratio = config.getDouble(config_prefix + ".keep_free_space_ratio"); + if (ratio < 0 || ratio > 1) + throw Exception("'keep_free_space_ratio' have to be between 0 and 1", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG); + String tmp_path = path; + if (tmp_path.empty()) + tmp_path = context.getPath(); + + // Create tmp disk for getting total disk space. + keep_free_space_bytes = static_cast(DiskLocal("tmp", tmp_path, 0).getTotalSpace() * ratio); + } +} + + + class DiskLocalReservation : public IReservation { public: @@ -337,6 +391,19 @@ void DiskLocal::sync(int fd) const throw Exception("Cannot fsync", ErrorCodes::CANNOT_FSYNC); } +void DiskLocal::updateFromConfig(const Poco::Util::AbstractConfiguration & config, + const String & config_prefix, + const Context & context) +{ + String new_disk_path; + UInt64 new_keep_free_space_bytes; + loadDiskLocalConfig(name, config, config_prefix, context, new_disk_path, new_keep_free_space_bytes); + if (disk_path != new_disk_path) + throw Exception("Disk path can't update from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + keep_free_space_bytes = new_keep_free_space_bytes; +} + + DiskPtr DiskLocalReservation::getDisk(size_t i) const { if (i != 0) @@ -388,50 +455,9 @@ void registerDiskLocal(DiskFactory & factory) const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context) -> DiskPtr { - String path = config.getString(config_prefix + ".path", ""); - if (name == "default") - { - if (!path.empty()) - throw Exception( - "\"default\" disk path should be provided in not it ", - ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - path = context.getPath(); - } - else - { - if (path.empty()) - throw Exception("Disk path can not be empty. Disk " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - if (path.back() != '/') - throw Exception("Disk path must end with /. Disk " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - } - - if (Poco::File disk{path}; !disk.canRead() || !disk.canWrite()) - { - throw Exception("There is no RW access to disk " + name + " (" + path + ")", ErrorCodes::PATH_ACCESS_DENIED); - } - - bool has_space_ratio = config.has(config_prefix + ".keep_free_space_ratio"); - - if (config.has(config_prefix + ".keep_free_space_bytes") && has_space_ratio) - throw Exception( - "Only one of 'keep_free_space_bytes' and 'keep_free_space_ratio' can be specified", - ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG); - - UInt64 keep_free_space_bytes = config.getUInt64(config_prefix + ".keep_free_space_bytes", 0); - - if (has_space_ratio) - { - auto ratio = config.getDouble(config_prefix + ".keep_free_space_ratio"); - if (ratio < 0 || ratio > 1) - throw Exception("'keep_free_space_ratio' have to be between 0 and 1", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG); - String tmp_path = path; - if (tmp_path.empty()) - tmp_path = context.getPath(); - - // Create tmp disk for getting total disk space. - keep_free_space_bytes = static_cast(DiskLocal("tmp", tmp_path, 0).getTotalSpace() * ratio); - } - + String path; + UInt64 keep_free_space_bytes; + loadDiskLocalConfig(name, config, config_prefix, context, path, keep_free_space_bytes); return std::make_shared(name, path, keep_free_space_bytes); }; factory.registerDiskType("local", creator); diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index 09f08a01d27..ee87abe2359 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -7,6 +7,7 @@ #include #include +#include namespace DB { @@ -17,6 +18,7 @@ namespace ErrorCodes class DiskLocalReservation; +class Context; class DiskLocal : public IDisk { public: @@ -106,13 +108,17 @@ public: const String getType() const override { return "local"; } + void updateFromConfig(const Poco::Util::AbstractConfiguration & config, + const String & config_prefix, + const Context & context); + private: bool tryReserve(UInt64 bytes); private: const String name; const String disk_path; - const UInt64 keep_free_space_bytes; + UInt64 keep_free_space_bytes; UInt64 reserved_bytes = 0; UInt64 reservation_count = 0; @@ -120,4 +126,5 @@ private: static std::mutex reservation_mutex; }; + } diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index 0fb728a4f02..8d17515262d 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -66,9 +66,9 @@ DiskSelectorPtr DiskSelector::updateFromConfig( if (!std::all_of(disk_name.begin(), disk_name.end(), isWordCharASCII)) throw Exception("Disk name can contain only alphanumeric and '_' (" + disk_name + ")", ErrorCodes::EXCESSIVE_ELEMENT_IN_CONFIG); + auto disk_config_prefix = config_prefix + "." + disk_name; if (result->getDisksMap().count(disk_name) == 0) { - auto disk_config_prefix = config_prefix + "." + disk_name; result->addToDiskMap(disk_name, factory.create(disk_name, config, disk_config_prefix, context)); } else @@ -77,6 +77,9 @@ DiskSelectorPtr DiskSelector::updateFromConfig( /// TODO: Ideally ClickHouse shall complain if disk has changed, but /// implementing that may appear as not trivial task. + auto disk = std::static_pointer_cast(result->getDisksMap().find(disk_name)->second); + if (disk) + disk->updateFromConfig(config, disk_config_prefix, context); } } diff --git a/src/Disks/DiskSelector.h b/src/Disks/DiskSelector.h index 5d023fe1fbc..a6027616511 100644 --- a/src/Disks/DiskSelector.h +++ b/src/Disks/DiskSelector.h @@ -34,7 +34,7 @@ public: /// Get all disks with names const DisksMap & getDisksMap() const { return disks; } - void addToDiskMap(String name, DiskPtr disk) + void addToDiskMap(const String & name, DiskPtr disk) { disks.emplace(name, disk); } From 901de8968f9df6fd9e5dc3e763b1719b141fb555 Mon Sep 17 00:00:00 2001 From: taiyang-li Date: Tue, 26 Jan 2021 15:07:17 +0800 Subject: [PATCH 02/78] update disk local if config changed --- src/Disks/DiskLocal.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index ea902bae6dc..e0599740651 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -400,7 +400,10 @@ void DiskLocal::updateFromConfig(const Poco::Util::AbstractConfiguration & confi loadDiskLocalConfig(name, config, config_prefix, context, new_disk_path, new_keep_free_space_bytes); if (disk_path != new_disk_path) throw Exception("Disk path can't update from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - keep_free_space_bytes = new_keep_free_space_bytes; + + // Config of DiskLocal has been updated + if (keep_free_space_bytes != new_keep_free_space_bytes) + keep_free_space_bytes = new_keep_free_space_bytes; } From 44200abfb4b8869032d4a542ba555cf0f1badd99 Mon Sep 17 00:00:00 2001 From: taiyang-li Date: Fri, 29 Jan 2021 19:13:33 +0800 Subject: [PATCH 03/78] add interface updateFromConfigIfChanged --- src/Disks/DiskLocal.cpp | 6 +++--- src/Disks/DiskLocal.h | 4 ++-- src/Disks/DiskSelector.cpp | 6 +++--- src/Disks/IDisk.h | 7 +++++++ 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index e0599740651..86bf358b11f 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -391,17 +391,17 @@ void DiskLocal::sync(int fd) const throw Exception("Cannot fsync", ErrorCodes::CANNOT_FSYNC); } -void DiskLocal::updateFromConfig(const Poco::Util::AbstractConfiguration & config, +void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context) { String new_disk_path; UInt64 new_keep_free_space_bytes; loadDiskLocalConfig(name, config, config_prefix, context, new_disk_path, new_keep_free_space_bytes); + if (disk_path != new_disk_path) throw Exception("Disk path can't update from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - - // Config of DiskLocal has been updated + if (keep_free_space_bytes != new_keep_free_space_bytes) keep_free_space_bytes = new_keep_free_space_bytes; } diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index ee87abe2359..d9f3434cf5e 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -108,9 +108,9 @@ public: const String getType() const override { return "local"; } - void updateFromConfig(const Poco::Util::AbstractConfiguration & config, + void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, - const Context & context); + const Context & context) override; private: bool tryReserve(UInt64 bytes); diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index 8d17515262d..f8dc5cd22ff 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -77,9 +77,9 @@ DiskSelectorPtr DiskSelector::updateFromConfig( /// TODO: Ideally ClickHouse shall complain if disk has changed, but /// implementing that may appear as not trivial task. - auto disk = std::static_pointer_cast(result->getDisksMap().find(disk_name)->second); - if (disk) - disk->updateFromConfig(config, disk_config_prefix, context); + + /// TODO: Implete updateFromConfigIfChanged for DiskS3 + result->getDisksMap().find(disk_name)->second->updateFromConfigIfChanged(config, disk_config_prefix, context); } } diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h index b68bca07de1..ede057d8ef1 100644 --- a/src/Disks/IDisk.h +++ b/src/Disks/IDisk.h @@ -12,6 +12,7 @@ #include #include #include +#include namespace CurrentMetrics @@ -30,6 +31,7 @@ using Reservations = std::vector; class ReadBufferFromFileBase; class WriteBufferFromFileBase; +class Context; /** * Mode of opening a file for write. @@ -195,6 +197,11 @@ public: /// Returns executor to perform asynchronous operations. virtual Executor & getExecutor() { return *executor; } + /// Reload config if config changed + virtual void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & /* config */, + const String & /* config_prefix */, + const Context & /* context */) { } + private: std::unique_ptr executor; }; From 3c61cecab5093812641843ab2298325db49799d0 Mon Sep 17 00:00:00 2001 From: taiyang-li Date: Wed, 10 Feb 2021 12:08:08 +0800 Subject: [PATCH 04/78] add atomic for keep_free_space_bytes --- src/Disks/DiskLocal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index d9f3434cf5e..59fb6196ab3 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -118,7 +118,7 @@ private: private: const String name; const String disk_path; - UInt64 keep_free_space_bytes; + std::atomic keep_free_space_bytes; UInt64 reserved_bytes = 0; UInt64 reservation_count = 0; From 89d8c1264c95857d883c6e508ef03ebc32733fe5 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Fri, 19 Feb 2021 18:11:21 +0800 Subject: [PATCH 05/78] add test of upload disk --- src/Disks/DiskSelector.cpp | 5 +- .../test.py | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index f8dc5cd22ff..dc84a0a8af0 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -75,10 +75,7 @@ DiskSelectorPtr DiskSelector::updateFromConfig( { old_disks_minus_new_disks.erase(disk_name); - /// TODO: Ideally ClickHouse shall complain if disk has changed, but - /// implementing that may appear as not trivial task. - - /// TODO: Implete updateFromConfigIfChanged for DiskS3 + // TODO: Implement updateFromConfigIfChanged for DiskS3 (DiskLocal already done) result->getDisksMap().find(disk_name)->second->updateFromConfigIfChanged(config, disk_config_prefix, context); } } diff --git a/tests/integration/test_reloading_storage_configuration/test.py b/tests/integration/test_reloading_storage_configuration/test.py index edcba4e8a60..485069e1808 100644 --- a/tests/integration/test_reloading_storage_configuration/test.py +++ b/tests/integration/test_reloading_storage_configuration/test.py @@ -7,8 +7,10 @@ import xml.etree.ElementTree as ET import helpers.client import helpers.cluster +from helpers.test_tools import TSV import pytest + cluster = helpers.cluster.ClickHouseCluster(__file__) node1 = cluster.add_instance('node1', @@ -76,6 +78,37 @@ def add_disk(node, name, path, separate_file=False): else: tree.write(os.path.join(node.config_d_dir, "storage_configuration.xml")) +def update_disk(node, name, path, keep_free_space_bytes, separate_file=False): + separate_configuration_path = os.path.join(node.config_d_dir, + "separate_configuration.xml") + + try: + if separate_file: + tree = ET.parse(separate_configuration_path) + else: + tree = ET.parse( + os.path.join(node.config_d_dir, "storage_configuration.xml")) + except: + tree = ET.ElementTree( + ET.fromstring('')) + + root = tree.getroot() + disk = root.find("storage_configuration").find("disks").find(name) + assert disk is not None + + new_path = disk.find("path") + assert new_path is not None + new_path.text = path + + new_keep_free_space_bytes = disk.find("keep_free_space_bytes") + assert new_keep_free_space_bytes is not None + new_keep_free_space_bytes.text = keep_free_space_bytes + + if separate_file: + tree.write(separate_configuration_path) + else: + tree.write(os.path.join(node.config_d_dir, "storage_configuration.xml")) + def add_policy(node, name, volumes): tree = ET.parse(os.path.join(node.config_d_dir, "storage_configuration.xml")) @@ -123,6 +156,38 @@ def test_add_disk(started_cluster): except: """""" +def test_update_disk(started_cluster): + try: + name = "test_update_disk" + engine = "MergeTree()" + + start_over() + node1.restart_clickhouse(kill=True) + time.sleep(2) + + node1.query(""" + CREATE TABLE {name} ( + d UInt64 + ) ENGINE = {engine} + ORDER BY d + SETTINGS storage_policy='jbods_with_external' + """.format(name=name, engine=engine)) + + assert node1.query( "SELECT path, keep_free_space FROM system.disks where name = '%s'" % (name)) == TSV([ + ["/jbod2/", "10485760"]]) + + update_disk(node1, "jbod2", "/jbod2/", "20971520") + node1.query("SYSTEM RELOAD CONFIG") + + assert node1.query( + "SELECT path, keep_free_space FROM system.disks where name = '%s'" % (name)) == TSV([ + ["/jbod2/", "20971520"]]) + + finally: + try: + node1.query("DROP TABLE IF EXISTS {}".format(name)) + except: + """""" def test_add_disk_to_separate_config(started_cluster): try: From e76b224f722517cb2833ff98c84223188af0e437 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Fri, 19 Feb 2021 20:09:26 +0800 Subject: [PATCH 06/78] fix code style --- src/Disks/DiskLocal.cpp | 2 +- src/Disks/DiskLocal.h | 2 +- src/Disks/IDisk.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index 86bf358b11f..03163aa1770 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -391,7 +391,7 @@ void DiskLocal::sync(int fd) const throw Exception("Cannot fsync", ErrorCodes::CANNOT_FSYNC); } -void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, +void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context) { diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index 59fb6196ab3..8d32a8e2176 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -108,7 +108,7 @@ public: const String getType() const override { return "local"; } - void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, + void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, const String & config_prefix, const Context & context) override; diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h index ede057d8ef1..0ce3bddbf34 100644 --- a/src/Disks/IDisk.h +++ b/src/Disks/IDisk.h @@ -198,7 +198,7 @@ public: virtual Executor & getExecutor() { return *executor; } /// Reload config if config changed - virtual void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & /* config */, + virtual void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & /* config */, const String & /* config_prefix */, const Context & /* context */) { } From d370381980e2d5a80f9c1515096d58153b2a507d Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Sat, 20 Feb 2021 09:47:53 +0800 Subject: [PATCH 07/78] fix code style --- src/Disks/DiskLocal.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index 03163aa1770..bd5e20ab2b9 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -401,7 +401,6 @@ void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguratio if (disk_path != new_disk_path) throw Exception("Disk path can't update from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); - if (keep_free_space_bytes != new_keep_free_space_bytes) keep_free_space_bytes = new_keep_free_space_bytes; } From 1980e75a962dab55bd54468f2c454ef946596911 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Sat, 20 Feb 2021 11:01:11 +0800 Subject: [PATCH 08/78] fix code style --- src/Disks/DiskLocal.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index 2a31bc3f00a..77d2b70b36a 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -83,8 +83,6 @@ static void loadDiskLocalConfig(const String & name, } } - - class DiskLocalReservation : public IReservation { public: @@ -380,7 +378,6 @@ void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguratio keep_free_space_bytes = new_keep_free_space_bytes; } - DiskPtr DiskLocalReservation::getDisk(size_t i) const { if (i != 0) @@ -398,7 +395,6 @@ void DiskLocalReservation::update(UInt64 new_size) disk->reserved_bytes += size; } - DiskLocalReservation::~DiskLocalReservation() { try From 50d013231ca494056e8e7e8c26ff1fa3dc917549 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Sat, 20 Feb 2021 11:33:49 +0800 Subject: [PATCH 09/78] fix test_reloading_storage_configuration --- .../test_reloading_storage_configuration/test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration/test_reloading_storage_configuration/test.py b/tests/integration/test_reloading_storage_configuration/test.py index 485069e1808..7fa37d0909c 100644 --- a/tests/integration/test_reloading_storage_configuration/test.py +++ b/tests/integration/test_reloading_storage_configuration/test.py @@ -173,16 +173,14 @@ def test_update_disk(started_cluster): SETTINGS storage_policy='jbods_with_external' """.format(name=name, engine=engine)) - assert node1.query( "SELECT path, keep_free_space FROM system.disks where name = '%s'" % (name)) == TSV([ + assert node1.query("SELECT path, keep_free_space FROM system.disks where name = 'jbod2'") == TSV([ ["/jbod2/", "10485760"]]) update_disk(node1, "jbod2", "/jbod2/", "20971520") node1.query("SYSTEM RELOAD CONFIG") - assert node1.query( - "SELECT path, keep_free_space FROM system.disks where name = '%s'" % (name)) == TSV([ + assert node1.query("SELECT path, keep_free_space FROM system.disks where name = 'jbod2'") == TSV([ ["/jbod2/", "20971520"]]) - finally: try: node1.query("DROP TABLE IF EXISTS {}".format(name)) From e4027755ad001dc899d683832ca00560edc10118 Mon Sep 17 00:00:00 2001 From: Mikhail Date: Mon, 26 Jul 2021 07:09:54 +0300 Subject: [PATCH 10/78] Update order-by.md Some minor corrections. --- .../statements/select/order-by.md | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index a8fec5cfa26..ac90977b162 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -274,7 +274,7 @@ This modifier also can be combined with [LIMIT … WITH TIES modifier](../../../ `WITH FILL` modifier can be set after `ORDER BY expr` with optional `FROM expr`, `TO expr` and `STEP expr` parameters. All missed values of `expr` column will be filled sequentially and other columns will be filled as defaults. -Use following syntax for filling multiple columns add `WITH FILL` modifier with optional parameters after each field name in `ORDER BY` section. +To fill multiple columns, add `WITH FILL` modifier with optional parameters after each field name in `ORDER BY` section. ``` sql ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] @@ -286,16 +286,16 @@ When `TO const_expr` not defined sequence of filling use maximum `expr` field va When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. When `STEP const_numeric_expr` omitted then sequence of filling use `1.0` for numeric type, `1 day` for Date type and `1 second` for DateTime type. -For example, the following query +Example query without `WITH FILL`: ``` sql SELECT n, source FROM ( SELECT toFloat32(number % 10) AS n, 'original' AS source FROM numbers(10) WHERE number % 3 = 1 -) ORDER BY n +) ORDER BY n; ``` -returns +Result: ``` text ┌─n─┬─source───┐ @@ -305,16 +305,16 @@ returns └───┴──────────┘ ``` -but after apply `WITH FILL` modifier +Same query after applying `WITH FILL` modifier: ``` sql SELECT n, source FROM ( SELECT toFloat32(number % 10) AS n, 'original' AS source FROM numbers(10) WHERE number % 3 = 1 -) ORDER BY n WITH FILL FROM 0 TO 5.51 STEP 0.5 +) ORDER BY n WITH FILL FROM 0 TO 5.51 STEP 0.5; ``` -returns +Result: ``` text ┌───n─┬─source───┐ @@ -334,7 +334,7 @@ returns └─────┴──────────┘ ``` -For the case when we have multiple fields `ORDER BY field2 WITH FILL, field1 WITH FILL` order of filling will follow the order of fields in `ORDER BY` clause. +For the case with multiple fields `ORDER BY field2 WITH FILL, field1 WITH FILL` order of filling will follow the order of fields in `ORDER BY` clause. Example: @@ -350,7 +350,7 @@ ORDER BY d1 WITH FILL STEP 5; ``` -returns +Result: ``` text ┌───d1───────┬───d2───────┬─source───┐ @@ -366,7 +366,7 @@ returns Field `d1` does not fill and use default value cause we do not have repeated values for `d2` value, and sequence for `d1` can’t be properly calculated. -The following query with a changed field in `ORDER BY` +The following query with a changed field in `ORDER BY`: ``` sql SELECT @@ -380,7 +380,7 @@ ORDER BY d2 WITH FILL; ``` -returns +Result: ``` text ┌───d1───────┬───d2───────┬─source───┐ @@ -400,4 +400,4 @@ returns └────────────┴────────────┴──────────┘ ``` -[Original article](https://clickhouse.tech/docs/en/sql-reference/statements/select/order-by/) +[Original article](https://clickhouse.tech/docs/en/sql-reference/statements/select/order-by/) \ No newline at end of file From c0352044cd8e6a8d53785bb895ae24c52a68672a Mon Sep 17 00:00:00 2001 From: Mikhail Date: Mon, 26 Jul 2021 07:17:29 +0300 Subject: [PATCH 11/78] Update order-by.md Requested edit. --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index ac90977b162..3d71e2799ea 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -280,7 +280,7 @@ To fill multiple columns, add `WITH FILL` modifier with optional parameters afte ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] ``` -`WITH FILL` can be applied only for fields with Numeric (all kind of float, decimal, int) or Date/DateTime types. +`WITH FILL` can be applied only for fields with Numeric (all kind of float, decimal, int) or Date/DateTime types. When applied for `String` fields, missed values will be filled with empty strings. When `FROM const_expr` not defined sequence of filling use minimal `expr` field value from `ORDER BY`. When `TO const_expr` not defined sequence of filling use maximum `expr` field value from `ORDER BY`. When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. From d541cd5c00595db58233f4132ae6febb8688303b Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Wed, 28 Jul 2021 04:18:16 +0300 Subject: [PATCH 12/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: olgarev <56617294+olgarev@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index 3d71e2799ea..d73986a7aeb 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -280,7 +280,7 @@ To fill multiple columns, add `WITH FILL` modifier with optional parameters afte ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] ``` -`WITH FILL` can be applied only for fields with Numeric (all kind of float, decimal, int) or Date/DateTime types. When applied for `String` fields, missed values will be filled with empty strings. +`WITH FILL` can be applied for fields with Numeric (all kind of float, decimal, int) or Date/DateTime types. When applied for `String` fields, missed values are filled with empty strings. When `FROM const_expr` not defined sequence of filling use minimal `expr` field value from `ORDER BY`. When `TO const_expr` not defined sequence of filling use maximum `expr` field value from `ORDER BY`. When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. @@ -400,4 +400,4 @@ Result: └────────────┴────────────┴──────────┘ ``` -[Original article](https://clickhouse.tech/docs/en/sql-reference/statements/select/order-by/) \ No newline at end of file +[Original article](https://clickhouse.tech/docs/en/sql-reference/statements/select/order-by/) From 08a7140882c3967bb6f01cd6490102f0c3037c4b Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Wed, 28 Jul 2021 04:18:33 +0300 Subject: [PATCH 13/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: olgarev <56617294+olgarev@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index d73986a7aeb..09f061b6ca4 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -286,7 +286,7 @@ When `TO const_expr` not defined sequence of filling use maximum `expr` field va When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. When `STEP const_numeric_expr` omitted then sequence of filling use `1.0` for numeric type, `1 day` for Date type and `1 second` for DateTime type. -Example query without `WITH FILL`: +Example of query without `WITH FILL`: ``` sql SELECT n, source FROM ( From fce9351256adeb9374a46b8699003a1a45a34a76 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 23 Mar 2021 15:01:13 +0000 Subject: [PATCH 14/78] MongoDB SSL Connection --- src/Storages/StorageMongoDB.cpp | 19 +++++++++-- src/Storages/StorageMongoDB.h | 3 ++ src/Storages/StorageMongoDBSocketFactory.cpp | 36 ++++++++++++++++++++ src/Storages/StorageMongoDBSocketFactory.h | 19 +++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 src/Storages/StorageMongoDBSocketFactory.cpp create mode 100644 src/Storages/StorageMongoDBSocketFactory.h diff --git a/src/Storages/StorageMongoDB.cpp b/src/Storages/StorageMongoDB.cpp index e27d16ecc68..5e758225b44 100644 --- a/src/Storages/StorageMongoDB.cpp +++ b/src/Storages/StorageMongoDB.cpp @@ -1,4 +1,5 @@ #include "StorageMongoDB.h" +#include "StorageMongoDBSocketFactory.h" #include #include @@ -33,6 +34,7 @@ StorageMongoDB::StorageMongoDB( const std::string & collection_name_, const std::string & username_, const std::string & password_, + const std::string & options_, const ColumnsDescription & columns_, const ConstraintsDescription & constraints_, const String & comment) @@ -43,6 +45,8 @@ StorageMongoDB::StorageMongoDB( , collection_name(collection_name_) , username(username_) , password(password_) + , options(options_) + , uri("mongodb://" + username_ + ":" + password_ + "@" + host_ + ":" + std::to_string(port_) + "/" + database_name_ + "?" + options_) { StorageInMemoryMetadata storage_metadata; storage_metadata.setColumns(columns_); @@ -56,7 +60,10 @@ void StorageMongoDB::connectIfNotConnected() { std::lock_guard lock{connection_mutex}; if (!connection) - connection = std::make_shared(host, port); + { + StorageMongoDBSocketFactory factory; + connection = std::make_shared(uri, factory); + } if (!authenticated) { @@ -102,9 +109,9 @@ void registerStorageMongoDB(StorageFactory & factory) { ASTs & engine_args = args.engine_args; - if (engine_args.size() != 5) + if (engine_args.size() < 5 || engine_args.size() > 6) throw Exception( - "Storage MongoDB requires 5 parameters: MongoDB('host:port', database, collection, 'user', 'password').", + "Storage MongoDB requires from 5 to 6 parameters: MongoDB('host:port', database, collection, 'user', 'password' [, 'options']).", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); for (auto & engine_arg : engine_args) @@ -118,6 +125,11 @@ void registerStorageMongoDB(StorageFactory & factory) const String & username = engine_args[3]->as().value.safeGet(); const String & password = engine_args[4]->as().value.safeGet(); + String options = ""; + + if (engine_args.size() >= 6) + options = engine_args[5]->as().value.safeGet(); + return StorageMongoDB::create( args.table_id, parsed_host_port.first, @@ -126,6 +138,7 @@ void registerStorageMongoDB(StorageFactory & factory) collection, username, password, + options, args.columns, args.constraints, args.comment); diff --git a/src/Storages/StorageMongoDB.h b/src/Storages/StorageMongoDB.h index 2553acdd40c..3014b88a9ca 100644 --- a/src/Storages/StorageMongoDB.h +++ b/src/Storages/StorageMongoDB.h @@ -26,6 +26,7 @@ public: const std::string & collection_name_, const std::string & username_, const std::string & password_, + const std::string & options_, const ColumnsDescription & columns_, const ConstraintsDescription & constraints_, const String & comment); @@ -50,6 +51,8 @@ private: const std::string collection_name; const std::string username; const std::string password; + const std::string options; + const std::string uri; std::shared_ptr connection; bool authenticated = false; diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp new file mode 100644 index 00000000000..ed2548c7e3b --- /dev/null +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -0,0 +1,36 @@ +#include "StorageMongoDBSocketFactory.h" + +#include +#include +#include + + +#pragma clang diagnostic ignored "-Wunused-parameter" +#pragma GCC diagnostic ignored "-Wunused-parameter" + + +namespace DB +{ + +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) +{ + return secure ? createSecureSocket(host, port) : createPlainSocket(host, port); +} + +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std::string & host, int port) +{ + Poco::Net::SocketAddress address(host, port); + Poco::Net::StreamSocket socket(address); + + return socket; +} + +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port) +{ + Poco::Net::SocketAddress address(host, port); + Poco::Net::SecureStreamSocket socket(address, host); + + return socket; +} + +} diff --git a/src/Storages/StorageMongoDBSocketFactory.h b/src/Storages/StorageMongoDBSocketFactory.h new file mode 100644 index 00000000000..91b05698401 --- /dev/null +++ b/src/Storages/StorageMongoDBSocketFactory.h @@ -0,0 +1,19 @@ +#pragma once + +#include + + +namespace DB +{ + +class StorageMongoDBSocketFactory : public Poco::MongoDB::Connection::SocketFactory +{ +public: + virtual Poco::Net::StreamSocket createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) override; + +private: + Poco::Net::StreamSocket createPlainSocket(const std::string & host, int port); + Poco::Net::StreamSocket createSecureSocket(const std::string & host, int port); +}; + +} From 7e785d6b123ac12f0fcce84c737a7946ffa746b1 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Tue, 23 Mar 2021 15:18:12 +0000 Subject: [PATCH 15/78] Docs --- .../table-engines/integrations/mongodb.md | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/docs/en/engines/table-engines/integrations/mongodb.md b/docs/en/engines/table-engines/integrations/mongodb.md index a378ab03f55..9454b783b22 100644 --- a/docs/en/engines/table-engines/integrations/mongodb.md +++ b/docs/en/engines/table-engines/integrations/mongodb.md @@ -15,7 +15,7 @@ CREATE TABLE [IF NOT EXISTS] [db.]table_name name1 [type1], name2 [type2], ... -) ENGINE = MongoDB(host:port, database, collection, user, password); +) ENGINE = MongoDB(host:port, database, collection, user, password [, options]); ``` **Engine Parameters** @@ -30,18 +30,30 @@ CREATE TABLE [IF NOT EXISTS] [db.]table_name - `password` — User password. +- `options` — MongoDB connection string options (optional parameter). + ## Usage Example {#usage-example} -Table in ClickHouse which allows to read data from MongoDB collection: +Create a table in ClickHouse which allows to read data from MongoDB collection: ``` text CREATE TABLE mongo_table ( - key UInt64, + key UInt64, data String ) ENGINE = MongoDB('mongo1:27017', 'test', 'simple_table', 'testuser', 'clickhouse'); ``` +To read from an SSL secured MongoDB server: + +``` text +CREATE TABLE mongo_table_ssl +( + key UInt64, + data String +) ENGINE = MongoDB('mongo2:27017', 'test', 'simple_table', 'testuser', 'clickhouse', 'ssl=true'); +``` + Query: ``` sql @@ -54,4 +66,6 @@ SELECT COUNT() FROM mongo_table; └─────────┘ ``` + + [Original article](https://clickhouse.tech/docs/en/engines/table-engines/integrations/mongodb/) From fea1a5d32157cc312b9d7dfd6ed093fed76e7b43 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Tue, 23 Mar 2021 15:27:06 +0000 Subject: [PATCH 16/78] Refactor --- docs/en/engines/table-engines/integrations/mongodb.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/en/engines/table-engines/integrations/mongodb.md b/docs/en/engines/table-engines/integrations/mongodb.md index 9454b783b22..9839893d4e8 100644 --- a/docs/en/engines/table-engines/integrations/mongodb.md +++ b/docs/en/engines/table-engines/integrations/mongodb.md @@ -66,6 +66,4 @@ SELECT COUNT() FROM mongo_table; └─────────┘ ``` - - [Original article](https://clickhouse.tech/docs/en/engines/table-engines/integrations/mongodb/) From 8acc290bc5abff9aba959d0cdda5a50b85a3eaea Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Tue, 27 Jul 2021 15:54:13 +0000 Subject: [PATCH 17/78] Add integration tests --- .../compose/docker_compose_mongo_secure.yml | 13 ++++++ tests/integration/helpers/cluster.py | 34 ++++++++++---- tests/integration/helpers/mongo_cert.pem | 44 +++++++++++++++++++ tests/integration/helpers/mongo_secure.conf | 5 +++ .../configs_secure/config.d/ssl_conf.xml | 8 ++++ .../integration/test_storage_mongodb/test.py | 41 ++++++++++++++--- 6 files changed, 131 insertions(+), 14 deletions(-) create mode 100644 docker/test/integration/runner/compose/docker_compose_mongo_secure.yml create mode 100644 tests/integration/helpers/mongo_cert.pem create mode 100644 tests/integration/helpers/mongo_secure.conf create mode 100644 tests/integration/test_storage_mongodb/configs_secure/config.d/ssl_conf.xml diff --git a/docker/test/integration/runner/compose/docker_compose_mongo_secure.yml b/docker/test/integration/runner/compose/docker_compose_mongo_secure.yml new file mode 100644 index 00000000000..5d283cfc343 --- /dev/null +++ b/docker/test/integration/runner/compose/docker_compose_mongo_secure.yml @@ -0,0 +1,13 @@ +version: '2.3' +services: + mongo1: + image: mongo:3.6 + restart: always + environment: + MONGO_INITDB_ROOT_USERNAME: root + MONGO_INITDB_ROOT_PASSWORD: clickhouse + volumes: + - ${MONGO_CONFIG_PATH}:/mongo/ + ports: + - ${MONGO_EXTERNAL_PORT}:${MONGO_INTERNAL_PORT} + command: --config /mongo/mongo_secure.conf --profile=2 --verbose diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index 720d5e2068b..7f89bda5111 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -110,6 +110,7 @@ def subprocess_check_call(args, detach=False, nothrow=False): #logging.info('run:' + ' '.join(args)) return run_and_check(args, detach=detach, nothrow=nothrow) + def get_odbc_bridge_path(): path = os.environ.get('CLICKHOUSE_TESTS_ODBC_BRIDGE_BIN_PATH') if path is None: @@ -261,6 +262,7 @@ class ClickHouseCluster: self.with_hdfs = False self.with_kerberized_hdfs = False self.with_mongo = False + self.with_mongo_secure = False self.with_net_trics = False self.with_redis = False self.with_cassandra = False @@ -548,7 +550,6 @@ class ClickHouseCluster: return self.base_mysql_client_cmd - def setup_mysql_cmd(self, instance, env_variables, docker_compose_yml_dir): self.with_mysql = True env_variables['MYSQL_HOST'] = self.mysql_host @@ -680,6 +681,17 @@ class ClickHouseCluster: '--file', p.join(docker_compose_yml_dir, 'docker_compose_rabbitmq.yml')] return self.base_rabbitmq_cmd + def setup_mongo_secure_cmd(self, instance, env_variables, docker_compose_yml_dir): + self.with_mongo = self.with_mongo_secure = True + env_variables['MONGO_HOST'] = self.mongo_host + env_variables['MONGO_EXTERNAL_PORT'] = str(self.mongo_port) + env_variables['MONGO_INTERNAL_PORT'] = "27017" + env_variables['MONGO_CONFIG_PATH'] = HELPERS_DIR + self.base_cmd.extend(['--file', p.join(docker_compose_yml_dir, 'docker_compose_mongo_secure.yml')]) + self.base_mongo_cmd = ['docker-compose', '--env-file', instance.env_file, '--project-name', self.project_name, + '--file', p.join(docker_compose_yml_dir, 'docker_compose_mongo_secure.yml')] + return self.base_mongo_cmd + def setup_mongo_cmd(self, instance, env_variables, docker_compose_yml_dir): self.with_mongo = True env_variables['MONGO_HOST'] = self.mongo_host @@ -723,7 +735,8 @@ class ClickHouseCluster: macros=None, with_zookeeper=False, with_zookeeper_secure=False, with_mysql_client=False, with_mysql=False, with_mysql8=False, with_mysql_cluster=False, with_kafka=False, with_kerberized_kafka=False, with_rabbitmq=False, clickhouse_path_dir=None, - with_odbc_drivers=False, with_postgres=False, with_postgres_cluster=False, with_hdfs=False, with_kerberized_hdfs=False, with_mongo=False, + with_odbc_drivers=False, with_postgres=False, with_postgres_cluster=False, with_hdfs=False, + with_kerberized_hdfs=False, with_mongo=False, with_mongo_secure=False, with_redis=False, with_minio=False, with_cassandra=False, with_jdbc_bridge=False, hostname=None, env_variables=None, image="yandex/clickhouse-integration-test", tag=None, stay_alive=False, ipv4_address=None, ipv6_address=None, with_installed_binary=False, tmpfs=None, @@ -776,7 +789,7 @@ class ClickHouseCluster: with_kerberized_kafka=with_kerberized_kafka, with_rabbitmq=with_rabbitmq, with_kerberized_hdfs=with_kerberized_hdfs, - with_mongo=with_mongo, + with_mongo=with_mongo or with_mongo_secure, with_redis=with_redis, with_minio=with_minio, with_cassandra=with_cassandra, @@ -861,8 +874,11 @@ class ClickHouseCluster: if with_kerberized_hdfs and not self.with_kerberized_hdfs: cmds.append(self.setup_kerberized_hdfs_cmd(instance, env_variables, docker_compose_yml_dir)) - if with_mongo and not self.with_mongo: - cmds.append(self.setup_mongo_cmd(instance, env_variables, docker_compose_yml_dir)) + if (with_mongo or with_mongo_secure) and not (self.with_mongo or self.with_mongo_secure): + if with_mongo_secure: + cmds.append(self.setup_mongo_secure_cmd(instance, env_variables, docker_compose_yml_dir)) + else + cmds.append(self.setup_mongo_cmd(instance, env_variables, docker_compose_yml_dir)) if self.with_net_trics: for cmd in cmds: @@ -1234,7 +1250,6 @@ class ClickHouseCluster: logging.debug("Waiting for Kafka to start up") time.sleep(1) - def wait_hdfs_to_start(self, timeout=300, check_marker=False): start = time.time() while time.time() - start < timeout: @@ -1251,9 +1266,11 @@ class ClickHouseCluster: raise Exception("Can't wait HDFS to start") - def wait_mongo_to_start(self, timeout=180): + def wait_mongo_to_start(self, timeout=30, secure=False): connection_str = 'mongodb://{user}:{password}@{host}:{port}'.format( host='localhost', port=self.mongo_port, user='root', password='clickhouse') + if secure: + connection_str += '/?tls=true&tlsAllowInvalidCertificates=true' connection = pymongo.MongoClient(connection_str) start = time.time() while time.time() - start < timeout: @@ -1320,7 +1337,6 @@ class ClickHouseCluster: raise Exception("Can't wait Schema Registry to start") - def wait_cassandra_to_start(self, timeout=180): self.cassandra_ip = self.get_instance_ip(self.cassandra_host) cass_client = cassandra.cluster.Cluster([self.cassandra_ip], port=self.cassandra_port, load_balancing_policy=RoundRobinPolicy()) @@ -1505,7 +1521,7 @@ class ClickHouseCluster: if self.with_mongo and self.base_mongo_cmd: logging.debug('Setup Mongo') run_and_check(self.base_mongo_cmd + common_opts) - self.wait_mongo_to_start(30) + self.wait_mongo_to_start(30, secure=self.with_mongo_secure) if self.with_redis and self.base_redis_cmd: logging.debug('Setup Redis') diff --git a/tests/integration/helpers/mongo_cert.pem b/tests/integration/helpers/mongo_cert.pem new file mode 100644 index 00000000000..9e18b1d4469 --- /dev/null +++ b/tests/integration/helpers/mongo_cert.pem @@ -0,0 +1,44 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAtz2fpa8hyUff8u8jYlh20HbkOO8hQi64Ke2Prack2Br0lhOr +1MI6I8nVk5iDrt+7ix2Cnt+2aZKb6HJv0CG1V25yWg+jgsXeIT1KHTJf8rTmYxhb +t+ye+S1Z0h/Rt+xqSd9XXfzOLPGHYfyx6ZQ4AumO/HoEFD4IH/qiREjwtOfRXuhz +CohqtUTyYR7pJmZqBSuGac461WVRisnjfKRxeVa3itc84/RgktgYej2x4PQBFk13 +xAXKrWmHkwdgWklTuuK8Gtoqz65Y4/J9CSl+Bd08QDdRnaVvq1u1eNTZg1BVyeRv +jFYBMSathKASrng5nK66Fdilw6tO/9khaP0SDQIDAQABAoIBAAm/5qGrKtIJ1/mW +Dbzq1g+Lc+MvngZmc/gPIsjrjsNM09y0WT0txGgpEgsTX1ZLoy/otw16+7qsSU1Z +4WcilAJ95umx0VJg8suz9iCNkJtaUrPNFPw5Q9AgQJo0hTUTCCi8EGr4y4OKqlhl +WJYEA+LryGbYmyT0k/wXmtClTOFjKS09mK4deQ1DqbBxayR9MUZgRJzEODA8eGXs +Rc6fJUenMVNMzIVLgpossRtKImoZNcf5UtCKL3HECunndQeMu4zuqLMU+EzL1F/o +iHKF7v3CVmsK0OxNJfOfT0abN3XaJttFwTJyghQjgP8OX1IKjlj3vo9xwEDfVUEf +GVIER0UCgYEA2j+kjaT3Dw2lNlBEsA8uvVlLJHBc9JBtMGduGYsIEgsL/iStqXv4 +xoA9N3CwkN/rrQpDfi/16DMXAAYdjGulPwiMNFBY22TCzhtPC2mAnfaSForxwZCs +lwc3KkIloo3N5XvN78AuZf8ewiS+bOEj+HHHqqSb1+u/csuaXO9neesCgYEA1u/I +Mlt/pxJkH+c3yOskwCh/CNhq9szi8G9LXROIQ58BT2ydJSEPpt7AhUTtQGimQQTW +KLiffJSkjuVaFckR1GjCoAmFGYw9wUb+TmFNScz5pJ2dXse8aBysAMIQfEIcRAEa +gKnkLBH6nw3+/Hm3xwoBc35t8Pa2ek7LsWDfbecCgYBhilQW4gVw+t49uf4Y2ZBA +G+pTbMx+mRXTrkYssFB5D+raOLZMqxVyUdoKLxkahpkkCxRDD1hN4JeE8Ta/jVSb +KUzQDKDJ3OybhOT86rgK4SpFXO/TXL9l+FmVT17WmZ3N1Fkjr7aM60pp5lYc/zo+ +TUu5XjwwcjJsMcbZhj2u5QKBgQCDNuUn4PYAP9kCJPzIWs0XxmEvPDeorZIJqFgA +3XC9n2+EVlFlHlbYz3oGofqY7Io6fUJkn7k1q+T+G4QwcozA+KeAXe90lkoJGVcc +8IfnewwYc+RjvVoG0SIsYE0CHrX0yhus2oqiYON4gGnfJkuMZk5WfKOPjH4AEuSF +SBd+lwKBgQCHG/DA6u2mYmezPF9hebWFoyAVSr2PDXDhu8cNNHCpx9GewJXhuK/P +tW8mazHzUuJKBvmaUXDIXFh4K6FNhjH16p5jR1w3hsPE7NEZhjfVRaUYPmBqaOYR +jp8H+Sh5g4Rwbtfp6Qhu6UAKi/y6Vozs5GkJtSiNrjNDVrD+sGGrXA== +-----END RSA PRIVATE KEY----- +-----BEGIN CERTIFICATE----- +MIICqDCCAZACFBdaMnuT0pWhmrh05UT3HXJ+kI0yMA0GCSqGSIb3DQEBCwUAMA0x +CzAJBgNVBAMMAmNhMB4XDTIxMDQwNjE3MDQxNVoXDTIyMDQwNjE3MDQxNVowFDES +MBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC +AQEAtz2fpa8hyUff8u8jYlh20HbkOO8hQi64Ke2Prack2Br0lhOr1MI6I8nVk5iD +rt+7ix2Cnt+2aZKb6HJv0CG1V25yWg+jgsXeIT1KHTJf8rTmYxhbt+ye+S1Z0h/R +t+xqSd9XXfzOLPGHYfyx6ZQ4AumO/HoEFD4IH/qiREjwtOfRXuhzCohqtUTyYR7p +JmZqBSuGac461WVRisnjfKRxeVa3itc84/RgktgYej2x4PQBFk13xAXKrWmHkwdg +WklTuuK8Gtoqz65Y4/J9CSl+Bd08QDdRnaVvq1u1eNTZg1BVyeRvjFYBMSathKAS +rng5nK66Fdilw6tO/9khaP0SDQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAct2If +isMLHIqyL9GjY4b0xcxF4svFU/DUwNanStmoFMW1ifPf1cCqeMzyQOxBCDdMs0RT +hBbDYHW0BMXDqYIr3Ktbu38/3iVyr3pb56YOCKy8yHXpmKEaUBhCknSLcQyvNfeS +tM+DWsKFTZfyR5px+WwXbGKVMYwLaTON+/wcv1MeKMig3CxluaCpEJVYYwAiUc4K +sgvQNAunwGmPLPoXtUnpR2ZWiQA5R6yjS1oIe+8vpryFP6kjhWs0HR0jZEtLulV5 +WXUuxkqTXiBIvYpsmusoR44e9rptwLbV1wL/LUScRt9ttqFM3N5/Pof+2UwkSjGB +GAyPmw0Pkqtt+lva +-----END CERTIFICATE----- diff --git a/tests/integration/helpers/mongo_secure.conf b/tests/integration/helpers/mongo_secure.conf new file mode 100644 index 00000000000..1128b16b546 --- /dev/null +++ b/tests/integration/helpers/mongo_secure.conf @@ -0,0 +1,5 @@ +net: + ssl: + mode: requireSSL + PEMKeyFile: /mongo/mongo_cert.pem + allowConnectionsWithoutCertificates: true diff --git a/tests/integration/test_storage_mongodb/configs_secure/config.d/ssl_conf.xml b/tests/integration/test_storage_mongodb/configs_secure/config.d/ssl_conf.xml new file mode 100644 index 00000000000..e14aac81e17 --- /dev/null +++ b/tests/integration/test_storage_mongodb/configs_secure/config.d/ssl_conf.xml @@ -0,0 +1,8 @@ + + + + + none + + + diff --git a/tests/integration/test_storage_mongodb/test.py b/tests/integration/test_storage_mongodb/test.py index 75af909faec..55fa6b7dbc3 100644 --- a/tests/integration/test_storage_mongodb/test.py +++ b/tests/integration/test_storage_mongodb/test.py @@ -5,24 +5,29 @@ from helpers.client import QueryRuntimeException from helpers.cluster import ClickHouseCluster -cluster = ClickHouseCluster(__file__) -node = cluster.add_instance('node', with_mongo=True) - @pytest.fixture(scope="module") -def started_cluster(): +def started_cluster(request): try: + cluster = ClickHouseCluster(__file__) + node = cluster.add_instance('node', + main_configs=["configs_secure/config.d/ssl_conf.xml"], + with_mongo=True, + with_mongo_secure=request.param) cluster.start() yield cluster finally: cluster.shutdown() -def get_mongo_connection(started_cluster): +def get_mongo_connection(started_cluster, secure=False): connection_str = 'mongodb://root:clickhouse@localhost:{}'.format(started_cluster.mongo_port) + if secure: + connection_str += '/?tls=true&tlsAllowInvalidCertificates=true' return pymongo.MongoClient(connection_str) +@pytest.mark.parametrize('started_cluster', [False], indirect=['started_cluster']) def test_simple_select(started_cluster): mongo_connection = get_mongo_connection(started_cluster) db = mongo_connection['test'] @@ -33,6 +38,7 @@ def test_simple_select(started_cluster): data.append({'key': i, 'data': hex(i * i)}) simple_mongo_table.insert_many(data) + node = started_cluster.instance['node'] node.query( "CREATE TABLE simple_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'simple_table', 'root', 'clickhouse')") @@ -42,6 +48,7 @@ def test_simple_select(started_cluster): assert node.query("SELECT data from simple_mongo_table where key = 42") == hex(42 * 42) + '\n' +@pytest.mark.parametrize('started_cluster', [False], indirect=['started_cluster']) def test_complex_data_type(started_cluster): mongo_connection = get_mongo_connection(started_cluster) db = mongo_connection['test'] @@ -52,6 +59,7 @@ def test_complex_data_type(started_cluster): data.append({'key': i, 'data': hex(i * i), 'dict': {'a': i, 'b': str(i)}}) incomplete_mongo_table.insert_many(data) + node = started_cluster.instance['node'] node.query( "CREATE TABLE incomplete_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'complex_table', 'root', 'clickhouse')") @@ -61,6 +69,7 @@ def test_complex_data_type(started_cluster): assert node.query("SELECT data from incomplete_mongo_table where key = 42") == hex(42 * 42) + '\n' +@pytest.mark.parametrize('started_cluster', [False], indirect=['started_cluster']) def test_incorrect_data_type(started_cluster): mongo_connection = get_mongo_connection(started_cluster) db = mongo_connection['test'] @@ -71,6 +80,7 @@ def test_incorrect_data_type(started_cluster): data.append({'key': i, 'data': hex(i * i), 'aaaa': 'Hello'}) strange_mongo_table.insert_many(data) + node = started_cluster.instance['node'] node.query( "CREATE TABLE strange_mongo_table(key String, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'strange_table', 'root', 'clickhouse')") @@ -85,3 +95,24 @@ def test_incorrect_data_type(started_cluster): with pytest.raises(QueryRuntimeException): node.query("SELECT bbbb FROM strange_mongo_table2") + + +@pytest.mark.parametrize('started_cluster', [True], indirect=['started_cluster']) +def test_secure_connection(started_cluster): + mongo_connection = get_mongo_connection(secure=True) + db = mongo_connection['test'] + db.add_user('root', 'clickhouse') + simple_mongo_table = db['simple_table'] + data = [] + for i in range(0, 100): + data.append({'key': i, 'data': hex(i * i)}) + simple_mongo_table.insert_many(data) + + node = started_cluster.instance['node'] + node.query( + "CREATE TABLE simple_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'simple_table', 'root', 'clickhouse', 'ssl=true')") + + assert node.query("SELECT COUNT() FROM simple_mongo_table") == '100\n' + assert node.query("SELECT sum(key) FROM simple_mongo_table") == str(sum(range(0, 100))) + '\n' + + assert node.query("SELECT data from simple_mongo_table where key = 42") == hex(42 * 42) + '\n' From 58ea75915d7434672abbff20f492d6fe3d8c2e18 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Mon, 24 May 2021 12:11:03 +0000 Subject: [PATCH 18/78] Fix --- src/Storages/StorageMongoDBSocketFactory.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index ed2548c7e3b..a6ff53a23d7 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -4,6 +4,9 @@ #include #include +#if USE_SSL +# include +#endif #pragma clang diagnostic ignored "-Wunused-parameter" #pragma GCC diagnostic ignored "-Wunused-parameter" @@ -14,7 +17,11 @@ namespace DB Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) { +#if USE_SSL return secure ? createSecureSocket(host, port) : createPlainSocket(host, port); +#else + return createPlainSocket(host, port); +#endif } Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std::string & host, int port) @@ -25,6 +32,7 @@ Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std return socket; } +#if USE_SSL Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port) { Poco::Net::SocketAddress address(host, port); @@ -32,5 +40,6 @@ Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const st return socket; } +#endif } From 0d029c1cbc8fe71d364c6e798c1966d0d3a18a53 Mon Sep 17 00:00:00 2001 From: Ivan <5627721+abyss7@users.noreply.github.com> Date: Fri, 28 May 2021 15:02:10 +0300 Subject: [PATCH 19/78] Update StorageMongoDBSocketFactory.cpp --- src/Storages/StorageMongoDBSocketFactory.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index a6ff53a23d7..3352238a5c8 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -2,7 +2,6 @@ #include #include -#include #if USE_SSL # include From 14171435606d041e18d9c6f4b17a24d69e8d7504 Mon Sep 17 00:00:00 2001 From: Ivan <5627721+abyss7@users.noreply.github.com> Date: Fri, 28 May 2021 18:25:24 +0300 Subject: [PATCH 20/78] Update StorageMongoDBSocketFactory.cpp --- src/Storages/StorageMongoDBSocketFactory.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index 3352238a5c8..cb6ffd052c7 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -1,5 +1,9 @@ #include "StorageMongoDBSocketFactory.h" +#if !defined(ARCADIA_BUILD) +# include +#endif + #include #include From 6b9e0f5a6f0359a5eac66e099a915c6bd26e65cd Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Tue, 27 Jul 2021 17:43:41 +0000 Subject: [PATCH 21/78] Fix --- tests/integration/helpers/cluster.py | 2 +- tests/integration/test_storage_mongodb/test.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index 7f89bda5111..6fe01b5df03 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -877,7 +877,7 @@ class ClickHouseCluster: if (with_mongo or with_mongo_secure) and not (self.with_mongo or self.with_mongo_secure): if with_mongo_secure: cmds.append(self.setup_mongo_secure_cmd(instance, env_variables, docker_compose_yml_dir)) - else + else: cmds.append(self.setup_mongo_cmd(instance, env_variables, docker_compose_yml_dir)) if self.with_net_trics: diff --git a/tests/integration/test_storage_mongodb/test.py b/tests/integration/test_storage_mongodb/test.py index 55fa6b7dbc3..415f1c1cb33 100644 --- a/tests/integration/test_storage_mongodb/test.py +++ b/tests/integration/test_storage_mongodb/test.py @@ -38,7 +38,7 @@ def test_simple_select(started_cluster): data.append({'key': i, 'data': hex(i * i)}) simple_mongo_table.insert_many(data) - node = started_cluster.instance['node'] + node = started_cluster.instances['node'] node.query( "CREATE TABLE simple_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'simple_table', 'root', 'clickhouse')") @@ -59,7 +59,7 @@ def test_complex_data_type(started_cluster): data.append({'key': i, 'data': hex(i * i), 'dict': {'a': i, 'b': str(i)}}) incomplete_mongo_table.insert_many(data) - node = started_cluster.instance['node'] + node = started_cluster.instances['node'] node.query( "CREATE TABLE incomplete_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'complex_table', 'root', 'clickhouse')") @@ -80,7 +80,7 @@ def test_incorrect_data_type(started_cluster): data.append({'key': i, 'data': hex(i * i), 'aaaa': 'Hello'}) strange_mongo_table.insert_many(data) - node = started_cluster.instance['node'] + node = started_cluster.instances['node'] node.query( "CREATE TABLE strange_mongo_table(key String, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'strange_table', 'root', 'clickhouse')") @@ -99,7 +99,7 @@ def test_incorrect_data_type(started_cluster): @pytest.mark.parametrize('started_cluster', [True], indirect=['started_cluster']) def test_secure_connection(started_cluster): - mongo_connection = get_mongo_connection(secure=True) + mongo_connection = get_mongo_connection(started_cluster, secure=True) db = mongo_connection['test'] db.add_user('root', 'clickhouse') simple_mongo_table = db['simple_table'] @@ -108,7 +108,7 @@ def test_secure_connection(started_cluster): data.append({'key': i, 'data': hex(i * i)}) simple_mongo_table.insert_many(data) - node = started_cluster.instance['node'] + node = started_cluster.instances['node'] node.query( "CREATE TABLE simple_mongo_table(key UInt64, data String) ENGINE = MongoDB('mongo1:27017', 'test', 'simple_table', 'root', 'clickhouse', 'ssl=true')") From a154ccef7bfa8caff93bf20545ff30c5f508cb2b Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Wed, 28 Jul 2021 08:54:36 +0000 Subject: [PATCH 22/78] Fix --- src/Storages/StorageMongoDBSocketFactory.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index cb6ffd052c7..ac323a50252 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -11,8 +11,11 @@ # include #endif -#pragma clang diagnostic ignored "-Wunused-parameter" -#pragma GCC diagnostic ignored "-Wunused-parameter" +#ifdef __clang__ +# pragma clang diagnostic ignored "-Wunused-parameter" +#else +# pragma GCC diagnostic ignored "-Wunused-parameter" +#endif namespace DB From d8fb1cb0e0e86a47abe2db29db7309835448a2f7 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Wed, 28 Jul 2021 14:08:57 +0000 Subject: [PATCH 23/78] Ref. --- src/Storages/StorageMongoDBSocketFactory.cpp | 22 +++++++++----------- src/Storages/StorageMongoDBSocketFactory.h | 4 ++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index ac323a50252..12e554f26ba 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -11,12 +11,6 @@ # include #endif -#ifdef __clang__ -# pragma clang diagnostic ignored "-Wunused-parameter" -#else -# pragma GCC diagnostic ignored "-Wunused-parameter" -#endif - namespace DB { @@ -24,25 +18,29 @@ namespace DB Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) { #if USE_SSL - return secure ? createSecureSocket(host, port) : createPlainSocket(host, port); + return secure ? createSecureSocket(host, port, connectTimeout) : createPlainSocket(host, port, connectTimeout); #else - return createPlainSocket(host, port); + return createPlainSocket(host, port, connectTimeout); #endif } -Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std::string & host, int port) +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std::string & host, int port, Poco::Timespan connectTimeout) { Poco::Net::SocketAddress address(host, port); - Poco::Net::StreamSocket socket(address); + Poco::Net::StreamSocket socket; + + socket.connect(address, connectTimeout); return socket; } #if USE_SSL -Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port) +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout) { Poco::Net::SocketAddress address(host, port); - Poco::Net::SecureStreamSocket socket(address, host); + Poco::Net::SecureStreamSocket socket; + + socket.connect(address, connectTimeout); return socket; } diff --git a/src/Storages/StorageMongoDBSocketFactory.h b/src/Storages/StorageMongoDBSocketFactory.h index 91b05698401..af091bf4f92 100644 --- a/src/Storages/StorageMongoDBSocketFactory.h +++ b/src/Storages/StorageMongoDBSocketFactory.h @@ -12,8 +12,8 @@ public: virtual Poco::Net::StreamSocket createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) override; private: - Poco::Net::StreamSocket createPlainSocket(const std::string & host, int port); - Poco::Net::StreamSocket createSecureSocket(const std::string & host, int port); + Poco::Net::StreamSocket createPlainSocket(const std::string & host, int port, Poco::Timespan connectTimeout); + Poco::Net::StreamSocket createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout); }; } From 3a393042a2405543826041693fca4114bf80e573 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Wed, 28 Jul 2021 15:28:30 +0000 Subject: [PATCH 24/78] Fix --- src/Storages/StorageMongoDB.cpp | 2 +- src/Storages/StorageMongoDBSocketFactory.cpp | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Storages/StorageMongoDB.cpp b/src/Storages/StorageMongoDB.cpp index 5e758225b44..fd9c02700e9 100644 --- a/src/Storages/StorageMongoDB.cpp +++ b/src/Storages/StorageMongoDB.cpp @@ -46,7 +46,7 @@ StorageMongoDB::StorageMongoDB( , username(username_) , password(password_) , options(options_) - , uri("mongodb://" + username_ + ":" + password_ + "@" + host_ + ":" + std::to_string(port_) + "/" + database_name_ + "?" + options_) + , uri("mongodb://" + host_ + ":" + std::to_string(port_) + "/" + database_name_ + "?" + options_) { StorageInMemoryMetadata storage_metadata; storage_metadata.setColumns(columns_); diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index 12e554f26ba..955507fc2bc 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -17,11 +17,7 @@ namespace DB Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) { -#if USE_SSL return secure ? createSecureSocket(host, port, connectTimeout) : createPlainSocket(host, port, connectTimeout); -#else - return createPlainSocket(host, port, connectTimeout); -#endif } Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std::string & host, int port, Poco::Timespan connectTimeout) @@ -34,16 +30,19 @@ Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std return socket; } -#if USE_SSL + Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout) { +#if USE_SSL Poco::Net::SocketAddress address(host, port); Poco::Net::SecureStreamSocket socket; socket.connect(address, connectTimeout); return socket; -} +#else + return createPlainSocket(host, port, connectTimeout); #endif +} } From a665cd330873ece732a1ebca7592dd6cfb32c2cd Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Thu, 29 Jul 2021 08:38:39 +0000 Subject: [PATCH 25/78] Fixes --- src/Storages/StorageMongoDB.cpp | 2 +- src/Storages/StorageMongoDBSocketFactory.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Storages/StorageMongoDB.cpp b/src/Storages/StorageMongoDB.cpp index fd9c02700e9..1fd58a293dc 100644 --- a/src/Storages/StorageMongoDB.cpp +++ b/src/Storages/StorageMongoDB.cpp @@ -125,7 +125,7 @@ void registerStorageMongoDB(StorageFactory & factory) const String & username = engine_args[3]->as().value.safeGet(); const String & password = engine_args[4]->as().value.safeGet(); - String options = ""; + String options; if (engine_args.size() >= 6) options = engine_args[5]->as().value.safeGet(); diff --git a/src/Storages/StorageMongoDBSocketFactory.h b/src/Storages/StorageMongoDBSocketFactory.h index af091bf4f92..5fc423c63cb 100644 --- a/src/Storages/StorageMongoDBSocketFactory.h +++ b/src/Storages/StorageMongoDBSocketFactory.h @@ -12,8 +12,8 @@ public: virtual Poco::Net::StreamSocket createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) override; private: - Poco::Net::StreamSocket createPlainSocket(const std::string & host, int port, Poco::Timespan connectTimeout); - Poco::Net::StreamSocket createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout); + static Poco::Net::StreamSocket createPlainSocket(const std::string & host, int port, Poco::Timespan connectTimeout); + static Poco::Net::StreamSocket createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout); }; } From 17570868bdfd79f68045b62c0ce90aea6b2cbf5b Mon Sep 17 00:00:00 2001 From: Mikhail Date: Thu, 29 Jul 2021 19:17:48 +0300 Subject: [PATCH 26/78] some corrections and translation --- .../statements/select/order-by.md | 2 +- .../statements/select/order-by.md | 35 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index 09f061b6ca4..19cdee0dace 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -364,7 +364,7 @@ Result: └────────────┴────────────┴──────────┘ ``` -Field `d1` does not fill and use default value cause we do not have repeated values for `d2` value, and sequence for `d1` can’t be properly calculated. +Field `d1` does not fill in and use the default value cause we do not have repeated values for `d2` value, and sequence for `d1` can’t be properly calculated. The following query with a changed field in `ORDER BY`: diff --git a/docs/ru/sql-reference/statements/select/order-by.md b/docs/ru/sql-reference/statements/select/order-by.md index cb49d167b13..530837d8346 100644 --- a/docs/ru/sql-reference/statements/select/order-by.md +++ b/docs/ru/sql-reference/statements/select/order-by.md @@ -271,8 +271,8 @@ SELECT * FROM collate_test ORDER BY s ASC COLLATE 'en'; Этот модификатор также может быть скобинирован с модификатором [LIMIT ... WITH TIES](../../../sql-reference/statements/select/limit.md#limit-with-ties) -`WITH FILL` модификатор может быть установлен после `ORDER BY expr` с опциональными параметрами `FROM expr`, `TO expr` и `STEP expr`. -Все пропущенные значнеия для колонки `expr` будут заполненые значениями соответсвующими предполагаемой последовательности значений колонки, другие колонки будут заполнены значенями по умолчанию. +Модификатор `WITH FILL` может быть установлен после `ORDER BY expr` с опциональными параметрами `FROM expr`, `TO expr` и `STEP expr`. +Все пропущенные значения для колонки `expr` будут заполнены значениями, соответствующими предполагаемой последовательности значений колонки, другие колонки будут заполнены значенями по умолчанию. Используйте следующую конструкцию для заполнения нескольких колонок с модификатором `WITH FILL` с необязательными параметрами после каждого имени поля в секции `ORDER BY`. @@ -280,22 +280,22 @@ SELECT * FROM collate_test ORDER BY s ASC COLLATE 'en'; ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] ``` -`WITH FILL` может быть применене только к полям с числовыми (все разновидности float, int, decimal) или временными (все разновидности Date, DateTime) типами. +`WITH FILL` может быть применен к полям с числовыми (все разновидности float, int, decimal) или временными (все разновидности Date, DateTime) типами. В случае применения к полям типа `String`, недостающие значения заполняются пустой строкой. Когда не определен `FROM const_expr`, последовательность заполнения использует минимальное значение поля `expr` из `ORDER BY`. Когда не определен `TO const_expr`, последовательность заполнения использует максимальное значение поля `expr` из `ORDER BY`. -Когда `STEP const_numeric_expr` определен, тогда `const_numeric_expr` интерпретируется `как есть` для числовых типов, как `дни` для типа Date и как `секунды` для типа DateTime. +Когда `STEP const_numeric_expr` определен, `const_numeric_expr` интерпретируется `как есть` для числовых типов, как `дни` для типа Date и как `секунды` для типа DateTime. Когда `STEP const_numeric_expr` не указан, тогда используется `1.0` для числовых типов, `1 день` для типа Date и `1 секунда` для типа DateTime. - -Для примера, следующий запрос +Пример запроса без использования `WITH FILL`: ```sql SELECT n, source FROM ( SELECT toFloat32(number % 10) AS n, 'original' AS source FROM numbers(10) WHERE number % 3 = 1 -) ORDER BY n +) ORDER BY n; ``` -возвращает +Результат: + ```text ┌─n─┬─source───┐ │ 1 │ original │ @@ -304,7 +304,8 @@ SELECT n, source FROM ( └───┴──────────┘ ``` -но после применения модификатора `WITH FILL` +Тот же запрос после применения модификатора `WITH FILL`: + ```sql SELECT n, source FROM ( SELECT toFloat32(number % 10) AS n, 'original' AS source @@ -312,7 +313,8 @@ SELECT n, source FROM ( ) ORDER BY n WITH FILL FROM 0 TO 5.51 STEP 0.5 ``` -возвращает +Результат: + ```text ┌───n─┬─source───┐ │ 0 │ │ @@ -331,9 +333,10 @@ SELECT n, source FROM ( └─────┴──────────┘ ``` -Для случая когда у нас есть несколько полей `ORDER BY field2 WITH FILL, field1 WITH FILL` порядок заполнения будет следовать порядку полей в секции `ORDER BY`. +Для случая с несколькими полями `ORDER BY field2 WITH FILL, field1 WITH FILL` порядок заполнения будет следовать порядку полей в секции `ORDER BY`. Пример: + ```sql SELECT toDate((number * 10) * 86400) AS d1, @@ -346,7 +349,8 @@ ORDER BY d1 WITH FILL STEP 5; ``` -возвращает +Результат: + ```text ┌───d1───────┬───d2───────┬─source───┐ │ 1970-01-11 │ 1970-01-02 │ original │ @@ -359,9 +363,9 @@ ORDER BY └────────────┴────────────┴──────────┘ ``` -Поле `d1` не заполняет и используется значение по умолчанию поскольку у нас нет повторяющихся значения для `d2` поэтому мы не можем правильно рассчитать последователность заполнения для`d1`. +Поле `d1` не заполняется и использует значение по умолчанию. Поскольку у нас нет повторяющихся значений для `d2`, мы не можем правильно рассчитать последователность заполнения для `d1`. -Cледующий запрос (с измененым порядком в ORDER BY) +Cледующий запрос (с измененым порядком в ORDER BY): ```sql SELECT toDate((number * 10) * 86400) AS d1, @@ -374,7 +378,8 @@ ORDER BY d2 WITH FILL; ``` -возвращает +Результат: + ```text ┌───d1───────┬───d2───────┬─source───┐ │ 1970-01-11 │ 1970-01-02 │ original │ From e4f3518a211fb640b215ded5b572ff9d6ffafaa6 Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Thu, 29 Jul 2021 19:58:24 +0300 Subject: [PATCH 27/78] Update docs/ru/sql-reference/statements/select/order-by.md Co-authored-by: olgarev <56617294+olgarev@users.noreply.github.com> --- docs/ru/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ru/sql-reference/statements/select/order-by.md b/docs/ru/sql-reference/statements/select/order-by.md index 530837d8346..f8eb5450365 100644 --- a/docs/ru/sql-reference/statements/select/order-by.md +++ b/docs/ru/sql-reference/statements/select/order-by.md @@ -272,7 +272,7 @@ SELECT * FROM collate_test ORDER BY s ASC COLLATE 'en'; Этот модификатор также может быть скобинирован с модификатором [LIMIT ... WITH TIES](../../../sql-reference/statements/select/limit.md#limit-with-ties) Модификатор `WITH FILL` может быть установлен после `ORDER BY expr` с опциональными параметрами `FROM expr`, `TO expr` и `STEP expr`. -Все пропущенные значения для колонки `expr` будут заполнены значениями, соответствующими предполагаемой последовательности значений колонки, другие колонки будут заполнены значенями по умолчанию. +Все пропущенные значения для колонки `expr` будут заполнены значениями, соответствующими предполагаемой последовательности значений колонки, другие колонки будут заполнены значениями по умолчанию. Используйте следующую конструкцию для заполнения нескольких колонок с модификатором `WITH FILL` с необязательными параметрами после каждого имени поля в секции `ORDER BY`. From adca166266bb0e9269efd9ff02ce5e1b9aa6b0ae Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Thu, 29 Jul 2021 19:58:47 +0300 Subject: [PATCH 28/78] Update docs/ru/sql-reference/statements/select/order-by.md Co-authored-by: olgarev <56617294+olgarev@users.noreply.github.com> --- docs/ru/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ru/sql-reference/statements/select/order-by.md b/docs/ru/sql-reference/statements/select/order-by.md index f8eb5450365..ba54df5e858 100644 --- a/docs/ru/sql-reference/statements/select/order-by.md +++ b/docs/ru/sql-reference/statements/select/order-by.md @@ -280,7 +280,7 @@ SELECT * FROM collate_test ORDER BY s ASC COLLATE 'en'; ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] ``` -`WITH FILL` может быть применен к полям с числовыми (все разновидности float, int, decimal) или временными (все разновидности Date, DateTime) типами. В случае применения к полям типа `String`, недостающие значения заполняются пустой строкой. +`WITH FILL` может быть применен к полям с числовыми (все разновидности float, int, decimal) или временными (все разновидности Date, DateTime) типами. В случае применения к полям типа `String` недостающие значения заполняются пустой строкой. Когда не определен `FROM const_expr`, последовательность заполнения использует минимальное значение поля `expr` из `ORDER BY`. Когда не определен `TO const_expr`, последовательность заполнения использует максимальное значение поля `expr` из `ORDER BY`. Когда `STEP const_numeric_expr` определен, `const_numeric_expr` интерпретируется `как есть` для числовых типов, как `дни` для типа Date и как `секунды` для типа DateTime. From bfab2630d1cd27533500b5091c7d34dc586cca2a Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Thu, 29 Jul 2021 19:58:58 +0300 Subject: [PATCH 29/78] Update docs/ru/sql-reference/statements/select/order-by.md Co-authored-by: olgarev <56617294+olgarev@users.noreply.github.com> --- docs/ru/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ru/sql-reference/statements/select/order-by.md b/docs/ru/sql-reference/statements/select/order-by.md index ba54df5e858..a7ba44ec6e1 100644 --- a/docs/ru/sql-reference/statements/select/order-by.md +++ b/docs/ru/sql-reference/statements/select/order-by.md @@ -333,7 +333,7 @@ SELECT n, source FROM ( └─────┴──────────┘ ``` -Для случая с несколькими полями `ORDER BY field2 WITH FILL, field1 WITH FILL` порядок заполнения будет следовать порядку полей в секции `ORDER BY`. +Для случая с несколькими полями `ORDER BY field2 WITH FILL, field1 WITH FILL` порядок заполнения будет соответствовать порядку полей в секции `ORDER BY`. Пример: From 9bbc39f6d1f86af8d703015a2b8441616bbe7c8d Mon Sep 17 00:00:00 2001 From: Filatenkov Artur <58165623+FArthur-cmd@users.noreply.github.com> Date: Thu, 29 Jul 2021 21:02:49 +0300 Subject: [PATCH 30/78] Update order-by.md --- docs/ru/sql-reference/statements/select/order-by.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/docs/ru/sql-reference/statements/select/order-by.md b/docs/ru/sql-reference/statements/select/order-by.md index 1fbb0c64eb4..d7d2e9c7574 100644 --- a/docs/ru/sql-reference/statements/select/order-by.md +++ b/docs/ru/sql-reference/statements/select/order-by.md @@ -274,7 +274,6 @@ SELECT * FROM collate_test ORDER BY s ASC COLLATE 'en'; Модификатор `WITH FILL` может быть установлен после `ORDER BY expr` с опциональными параметрами `FROM expr`, `TO expr` и `STEP expr`. Все пропущенные значения для колонки `expr` будут заполнены значениями, соответствующими предполагаемой последовательности значений колонки, другие колонки будут заполнены значениями по умолчанию. - Используйте следующую конструкцию для заполнения нескольких колонок с модификатором `WITH FILL` с необязательными параметрами после каждого имени поля в секции `ORDER BY`. ```sql @@ -284,7 +283,7 @@ ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_ `WITH FILL` может быть применен к полям с числовыми (все разновидности float, int, decimal) или временными (все разновидности Date, DateTime) типами. В случае применения к полям типа `String` недостающие значения заполняются пустой строкой. Когда не определен `FROM const_expr`, последовательность заполнения использует минимальное значение поля `expr` из `ORDER BY`. Когда не определен `TO const_expr`, последовательность заполнения использует максимальное значение поля `expr` из `ORDER BY`. -Когда `STEP const_numeric_expr` определен, `const_numeric_expr` интерпретируется `как есть` для числовых типов, как `дни` для типа Date и как `секунды` для типа DateTime. +Когда `STEP const_numeric_expr` определен, `const_numeric_expr` интерпретируется "как есть" для числовых типов, как "дни" для типа `Date` и как "секунды" для типа `DateTime`. Когда `STEP const_numeric_expr` не указан, тогда используется `1.0` для числовых типов, `1 день` для типа Date и `1 секунда` для типа DateTime. @@ -297,7 +296,6 @@ SELECT n, source FROM ( ``` Результат: - ```text ┌─n─┬─source───┐ │ 1 │ original │ @@ -307,7 +305,6 @@ SELECT n, source FROM ( ``` Тот же запрос после применения модификатора `WITH FILL`: - ```sql SELECT n, source FROM ( SELECT toFloat32(number % 10) AS n, 'original' AS source @@ -338,12 +335,10 @@ SELECT n, source FROM ( Для случая с несколькими полями `ORDER BY field2 WITH FILL, field1 WITH FILL` порядок заполнения будет соответствовать порядку полей в секции `ORDER BY`. Пример: - ```sql SELECT toDate((number * 10) * 86400) AS d1, toDate(number * 86400) AS d2, - 'original' AS source FROM numbers(10) WHERE (number % 3) = 1 @@ -353,7 +348,6 @@ ORDER BY ``` Результат: - ```text ┌───d1───────┬───d2───────┬─source───┐ │ 1970-01-11 │ 1970-01-02 │ original │ @@ -368,7 +362,6 @@ ORDER BY Поле `d1` не заполняется и использует значение по умолчанию. Поскольку у нас нет повторяющихся значений для `d2`, мы не можем правильно рассчитать последователность заполнения для `d1`. - Cледующий запрос (с измененым порядком в ORDER BY): ```sql SELECT @@ -383,7 +376,6 @@ ORDER BY ``` Результат: - ```text ┌───d1───────┬───d2───────┬─source───┐ │ 1970-01-11 │ 1970-01-02 │ original │ From 66631ca6801528f947a809a832fb9706a3767d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=87=E5=BA=B7?= Date: Sun, 1 Aug 2021 07:51:24 +0800 Subject: [PATCH 31/78] add column default_database --- docs/en/operations/settings/settings-users.md | 2 +- docs/en/sql-reference/statements/create/user.md | 1 + src/Storages/System/StorageSystemUsers.cpp | 10 ++++++++-- ...2001_add_default_database_to_system_users.reference | 1 + .../02001_add_default_database_to_system_users.sql | 3 +++ 5 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 tests/queries/0_stateless/02001_add_default_database_to_system_users.reference create mode 100644 tests/queries/0_stateless/02001_add_default_database_to_system_users.sql diff --git a/docs/en/operations/settings/settings-users.md b/docs/en/operations/settings/settings-users.md index ee834dca98a..2c8315ad069 100644 --- a/docs/en/operations/settings/settings-users.md +++ b/docs/en/operations/settings/settings-users.md @@ -28,7 +28,7 @@ Structure of the `users` section: profile_name default - + default diff --git a/docs/en/sql-reference/statements/create/user.md b/docs/en/sql-reference/statements/create/user.md index ea275b9a2f8..dfa065f5d0a 100644 --- a/docs/en/sql-reference/statements/create/user.md +++ b/docs/en/sql-reference/statements/create/user.md @@ -15,6 +15,7 @@ CREATE USER [IF NOT EXISTS | OR REPLACE] name1 [ON CLUSTER cluster_name1] [NOT IDENTIFIED | IDENTIFIED {[WITH {no_password | plaintext_password | sha256_password | sha256_hash | double_sha1_password | double_sha1_hash}] BY {'password' | 'hash'}} | {WITH ldap SERVER 'server_name'} | {WITH kerberos [REALM 'realm']}] [HOST {LOCAL | NAME 'name' | REGEXP 'name_regexp' | IP 'address' | LIKE 'pattern'} [,...] | ANY | NONE] [DEFAULT ROLE role [,...]] + [DEFAULT DATABASE database | NONE] [GRANTEES {user | role | ANY | NONE} [,...] [EXCEPT {user | role} [,...]]] [SETTINGS variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY | WRITABLE] | PROFILE 'profile_name'] [,...] ``` diff --git a/src/Storages/System/StorageSystemUsers.cpp b/src/Storages/System/StorageSystemUsers.cpp index 90b0a914d58..a48e12a1476 100644 --- a/src/Storages/System/StorageSystemUsers.cpp +++ b/src/Storages/System/StorageSystemUsers.cpp @@ -50,6 +50,7 @@ NamesAndTypesList StorageSystemUsers::getNamesAndTypes() {"grantees_any", std::make_shared()}, {"grantees_list", std::make_shared(std::make_shared())}, {"grantees_except", std::make_shared(std::make_shared())}, + {"default_database", std::make_shared()}, }; return names_and_types; } @@ -85,6 +86,7 @@ void StorageSystemUsers::fillData(MutableColumns & res_columns, ContextPtr conte auto & column_grantees_list_offsets = assert_cast(*res_columns[column_index++]).getOffsets(); auto & column_grantees_except = assert_cast(assert_cast(*res_columns[column_index]).getData()); auto & column_grantees_except_offsets = assert_cast(*res_columns[column_index++]).getOffsets(); + auto & column_default_database = assert_cast(*res_columns[column_index++]); auto add_row = [&](const String & name, const UUID & id, @@ -92,7 +94,8 @@ void StorageSystemUsers::fillData(MutableColumns & res_columns, ContextPtr conte const Authentication & authentication, const AllowedClientHosts & allowed_hosts, const RolesOrUsersSet & default_roles, - const RolesOrUsersSet & grantees) + const RolesOrUsersSet & grantees, + const String default_database) { column_name.insertData(name.data(), name.length()); column_id.push_back(id.toUnderType()); @@ -180,6 +183,8 @@ void StorageSystemUsers::fillData(MutableColumns & res_columns, ContextPtr conte for (const auto & except_name : grantees_ast->except_names) column_grantees_except.insertData(except_name.data(), except_name.length()); column_grantees_except_offsets.push_back(column_grantees_except.size()); + + column_default_database.insertData(default_database.data(),default_database.length()); }; for (const auto & id : ids) @@ -192,7 +197,8 @@ void StorageSystemUsers::fillData(MutableColumns & res_columns, ContextPtr conte if (!storage) continue; - add_row(user->getName(), id, storage->getStorageName(), user->authentication, user->allowed_client_hosts, user->default_roles, user->grantees); + add_row(user->getName(), id, storage->getStorageName(), user->authentication, user->allowed_client_hosts, + user->default_roles, user->grantees, user->default_database); } } diff --git a/tests/queries/0_stateless/02001_add_default_database_to_system_users.reference b/tests/queries/0_stateless/02001_add_default_database_to_system_users.reference new file mode 100644 index 00000000000..bec3a35ee8b --- /dev/null +++ b/tests/queries/0_stateless/02001_add_default_database_to_system_users.reference @@ -0,0 +1 @@ +system diff --git a/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql b/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql new file mode 100644 index 00000000000..2952f3fd23e --- /dev/null +++ b/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql @@ -0,0 +1,3 @@ +create user if not exists u_02001 default database system; +select default_database from system.users where name = 'u_02001'; +drop user if exists u_02001; \ No newline at end of file From 80f33244d4011ed9b6588de91cee202d063b8bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=87=E5=BA=B7?= Date: Sun, 1 Aug 2021 08:12:37 +0800 Subject: [PATCH 32/78] add column default_database --- .../0_stateless/02001_add_default_database_to_system_users.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql b/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql index 2952f3fd23e..b006f9acb22 100644 --- a/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql +++ b/tests/queries/0_stateless/02001_add_default_database_to_system_users.sql @@ -1,3 +1,3 @@ create user if not exists u_02001 default database system; select default_database from system.users where name = 'u_02001'; -drop user if exists u_02001; \ No newline at end of file +drop user if exists u_02001; From 130253e3b9b0b28bfdbc5d2c076b2c381f1d3c7b Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 30 Jul 2021 14:20:57 +0000 Subject: [PATCH 33/78] Fix bridge-server interaction in case of metadata inconsistency --- programs/library-bridge/HandlerFactory.cpp | 4 +- programs/library-bridge/Handlers.cpp | 146 +++++++++++++----- programs/library-bridge/Handlers.h | 15 +- .../SharedLibraryHandlerFactory.cpp | 20 +-- .../SharedLibraryHandlerFactory.h | 4 +- src/Bridge/IBridgeHelper.cpp | 15 -- src/Bridge/IBridgeHelper.h | 8 +- src/Bridge/LibraryBridgeHelper.cpp | 112 ++++++++++++-- src/Bridge/LibraryBridgeHelper.h | 25 ++- src/Bridge/XDBCBridgeHelper.h | 37 +++-- src/Dictionaries/LibraryDictionarySource.cpp | 27 +++- tests/integration/test_library_bridge/test.py | 65 +++++++- 12 files changed, 358 insertions(+), 120 deletions(-) diff --git a/programs/library-bridge/HandlerFactory.cpp b/programs/library-bridge/HandlerFactory.cpp index 9f53a24156f..3e31f298fca 100644 --- a/programs/library-bridge/HandlerFactory.cpp +++ b/programs/library-bridge/HandlerFactory.cpp @@ -12,8 +12,8 @@ namespace DB Poco::URI uri{request.getURI()}; LOG_DEBUG(log, "Request URI: {}", uri.toString()); - if (uri == "/ping" && request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET) - return std::make_unique(keep_alive_timeout); + if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET) + return std::make_unique(keep_alive_timeout, getContext()); if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST) return std::make_unique(keep_alive_timeout, getContext()); diff --git a/programs/library-bridge/Handlers.cpp b/programs/library-bridge/Handlers.cpp index 89219d1b910..7e79830a7b2 100644 --- a/programs/library-bridge/Handlers.cpp +++ b/programs/library-bridge/Handlers.cpp @@ -19,6 +19,16 @@ namespace DB { namespace { + void processError(HTTPServerResponse & response, const std::string & message) + { + response.setStatusAndReason(HTTPResponse::HTTP_INTERNAL_SERVER_ERROR); + + if (!response.sent()) + *response.send() << message << std::endl; + + LOG_WARNING(&Poco::Logger::get("LibraryBridge"), message); + } + std::shared_ptr parseColumns(std::string && column_string) { auto sample_block = std::make_shared(); @@ -30,13 +40,13 @@ namespace return sample_block; } - std::vector parseIdsFromBinary(const std::string & ids_string) - { - ReadBufferFromString buf(ids_string); - std::vector ids; - readVectorBinary(ids, buf); - return ids; - } + // std::vector parseIdsFromBinary(const std::string & ids_string) + // { + // ReadBufferFromString buf(ids_string); + // std::vector ids; + // readVectorBinary(ids, buf); + // return ids; + // } std::vector parseNamesFromBinary(const std::string & names_string) { @@ -67,13 +77,36 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe std::string method = params.get("method"); std::string dictionary_id = params.get("dictionary_id"); - LOG_TRACE(log, "Library method: '{}', dictionary id: {}", method, dictionary_id); + LOG_TRACE(log, "Library method: '{}', dictionary id: {}", method, dictionary_id); WriteBufferFromHTTPServerResponse out(response, request.getMethod() == Poco::Net::HTTPRequest::HTTP_HEAD, keep_alive_timeout); try { - if (method == "libNew") + bool lib_new = (method == "libNew"); + if (method == "libClone") + { + if (!params.has("from_dictionary_id")) + { + processError(response, "No 'from_dictionary_id' in request URL"); + return; + } + + std::string from_dictionary_id = params.get("from_dictionary_id"); + bool cloned = false; + cloned = SharedLibraryHandlerFactory::instance().clone(from_dictionary_id, dictionary_id); + + if (cloned) + { + writeStringBinary("1", out); + } + else + { + LOG_TRACE(log, "Cannot clone from dictionary with id: {}, will call libNew instead"); + lib_new = true; + } + } + if (lib_new) { auto & read_buf = request.getStream(); params.read(read_buf); @@ -92,6 +125,8 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe std::string library_path = params.get("library_path"); const auto & settings_string = params.get("library_settings"); + + LOG_DEBUG(log, "Parsing library settings from binary string"); std::vector library_settings = parseNamesFromBinary(settings_string); /// Needed for library dictionary @@ -102,6 +137,8 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe } const auto & attributes_string = params.get("attributes_names"); + + LOG_DEBUG(log, "Parsing attributes names from binary string"); std::vector attributes_names = parseNamesFromBinary(attributes_string); /// Needed to parse block from binary string format @@ -140,54 +177,71 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe SharedLibraryHandlerFactory::instance().create(dictionary_id, library_path, library_settings, sample_block_with_nulls, attributes_names); writeStringBinary("1", out); } - else if (method == "libClone") - { - if (!params.has("from_dictionary_id")) - { - processError(response, "No 'from_dictionary_id' in request URL"); - return; - } - - std::string from_dictionary_id = params.get("from_dictionary_id"); - LOG_TRACE(log, "Calling libClone from {} to {}", from_dictionary_id, dictionary_id); - SharedLibraryHandlerFactory::instance().clone(from_dictionary_id, dictionary_id); - writeStringBinary("1", out); - } else if (method == "libDelete") { - SharedLibraryHandlerFactory::instance().remove(dictionary_id); + auto deleted = SharedLibraryHandlerFactory::instance().remove(dictionary_id); + + /// Do not throw, a warning is ok. + if (!deleted) + LOG_WARNING(log, "Cannot delete library for with dictionary id: {}, because such id was not found.", dictionary_id); + writeStringBinary("1", out); } else if (method == "isModified") { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + if (!library_handler) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + bool res = library_handler->isModified(); writeStringBinary(std::to_string(res), out); } else if (method == "supportsSelectiveLoad") { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + if (!library_handler) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + bool res = library_handler->supportsSelectiveLoad(); writeStringBinary(std::to_string(res), out); } else if (method == "loadAll") { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + if (!library_handler) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + const auto & sample_block = library_handler->getSampleBlock(); auto input = library_handler->loadAll(); + LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); BlockOutputStreamPtr output = FormatFactory::instance().getOutputStream(FORMAT, out, sample_block, getContext()); copyData(*input, *output); } else if (method == "loadIds") { + LOG_DEBUG(log, "Getting diciontary ids for dictionary with id: {}", dictionary_id); String ids_string; readString(ids_string, request.getStream()); - std::vector ids = parseIdsFromBinary(ids_string); + + Strings ids_strs; + splitInto<'-'>(ids_strs, ids_string); + std::vector ids; + for (const auto & id : ids_strs) + ids.push_back(parse(id)); + if (ids.empty()) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Received no ids"); + + // std::vector ids = parseIdsFromBinary(ids_string); auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + if (!library_handler) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + const auto & sample_block = library_handler->getSampleBlock(); auto input = library_handler->loadIds(ids); + + LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); BlockOutputStreamPtr output = FormatFactory::instance().getOutputStream(FORMAT, out, sample_block, getContext()); copyData(*input, *output); } @@ -219,8 +273,13 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe auto block = reader->read(); auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + if (!library_handler) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + const auto & sample_block = library_handler->getSampleBlock(); auto input = library_handler->loadKeys(block.getColumns()); + + LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); BlockOutputStreamPtr output = FormatFactory::instance().getOutputStream(FORMAT, out, sample_block, getContext()); copyData(*input, *output); } @@ -228,8 +287,9 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe catch (...) { auto message = getCurrentExceptionMessage(true); - response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_INTERNAL_SERVER_ERROR, message); // can't call process_error, because of too soon response sending + LOG_ERROR(log, "Failed to process request for dictionary_id: {}. Error: {}", dictionary_id, message); + response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_INTERNAL_SERVER_ERROR, message); // can't call process_error, because of too soon response sending try { writeStringBinary(message, out); @@ -239,8 +299,6 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe { tryLogCurrentException(log); } - - tryLogCurrentException(log); } try @@ -254,24 +312,30 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe } -void LibraryRequestHandler::processError(HTTPServerResponse & response, const std::string & message) -{ - response.setStatusAndReason(HTTPResponse::HTTP_INTERNAL_SERVER_ERROR); - - if (!response.sent()) - *response.send() << message << std::endl; - - LOG_WARNING(log, message); -} - - -void PingHandler::handleRequest(HTTPServerRequest & /* request */, HTTPServerResponse & response) +void PingHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { try { + LOG_TRACE(log, "Request URI: {}", request.getURI()); + HTMLForm params(getContext()->getSettingsRef(), request); + + if (!params.has("dictionary_id")) + { + processError(response, "No 'dictionary_id in request URL"); + return; + } + + std::string dictionary_id = params.get("dictionary_id"); + auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); + String res; + if (library_handler) + res = "dictionary=1"; + else + res = "dictionary=0"; + setResponseDefaultHeaders(response, keep_alive_timeout); - const char * data = "Ok.\n"; - response.sendBuffer(data, strlen(data)); + LOG_TRACE(log, "Senging ping response: {} (dictionary id: {})", res, dictionary_id); + response.sendBuffer(res.data(), res.size()); } catch (...) { diff --git a/programs/library-bridge/Handlers.h b/programs/library-bridge/Handlers.h index dac61d3a735..a8e4ba0dd2a 100644 --- a/programs/library-bridge/Handlers.h +++ b/programs/library-bridge/Handlers.h @@ -22,8 +22,7 @@ class LibraryRequestHandler : public HTTPRequestHandler, WithContext public: LibraryRequestHandler( - size_t keep_alive_timeout_, - ContextPtr context_) + size_t keep_alive_timeout_, ContextPtr context_) : WithContext(context_) , log(&Poco::Logger::get("LibraryRequestHandler")) , keep_alive_timeout(keep_alive_timeout_) @@ -35,18 +34,18 @@ public: private: static constexpr inline auto FORMAT = "RowBinary"; - void processError(HTTPServerResponse & response, const std::string & message); - Poco::Logger * log; size_t keep_alive_timeout; }; -class PingHandler : public HTTPRequestHandler +class PingHandler : public HTTPRequestHandler, WithContext { public: - explicit PingHandler(size_t keep_alive_timeout_) - : keep_alive_timeout(keep_alive_timeout_) + explicit PingHandler(size_t keep_alive_timeout_, ContextPtr context_) + : WithContext(context_) + , keep_alive_timeout(keep_alive_timeout_) + , log(&Poco::Logger::get("LibraryRequestHandler")) { } @@ -54,6 +53,8 @@ public: private: const size_t keep_alive_timeout; + Poco::Logger * log; + }; } diff --git a/programs/library-bridge/SharedLibraryHandlerFactory.cpp b/programs/library-bridge/SharedLibraryHandlerFactory.cpp index 270e07a1046..900981191c8 100644 --- a/programs/library-bridge/SharedLibraryHandlerFactory.cpp +++ b/programs/library-bridge/SharedLibraryHandlerFactory.cpp @@ -18,7 +18,7 @@ SharedLibraryHandlerPtr SharedLibraryHandlerFactory::get(const std::string & dic if (library_handler != library_handlers.end()) return library_handler->second; - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Not found dictionary with id: {}", dictionary_id); + return nullptr; } @@ -30,32 +30,32 @@ void SharedLibraryHandlerFactory::create( const std::vector & attributes_names) { std::lock_guard lock(mutex); - library_handlers[dictionary_id] = std::make_shared(library_path, library_settings, sample_block, attributes_names); + if (!library_handlers.count(dictionary_id)) + library_handlers.emplace(std::make_pair(dictionary_id, std::make_shared(library_path, library_settings, sample_block, attributes_names))); + else + LOG_WARNING(&Poco::Logger::get("SharedLibraryHandlerFactory"), "Library handler with dictionary id {} already exists", dictionary_id); } -void SharedLibraryHandlerFactory::clone(const std::string & from_dictionary_id, const std::string & to_dictionary_id) +bool SharedLibraryHandlerFactory::clone(const std::string & from_dictionary_id, const std::string & to_dictionary_id) { std::lock_guard lock(mutex); auto from_library_handler = library_handlers.find(from_dictionary_id); - /// This is not supposed to happen as libClone is called from copy constructor of LibraryDictionarySource - /// object, and shared library handler of from_dictionary is removed only in its destructor. - /// And if for from_dictionary there was no shared library handler, it would have received and exception in - /// its constructor, so no libClone would be made from it. if (from_library_handler == library_handlers.end()) - throw Exception(ErrorCodes::LOGICAL_ERROR, "No shared library handler found"); + return false; /// libClone method will be called in copy constructor library_handlers[to_dictionary_id] = std::make_shared(*from_library_handler->second); + return true; } -void SharedLibraryHandlerFactory::remove(const std::string & dictionary_id) +bool SharedLibraryHandlerFactory::remove(const std::string & dictionary_id) { std::lock_guard lock(mutex); /// libDelete is called in destructor. - library_handlers.erase(dictionary_id); + return library_handlers.erase(dictionary_id); } diff --git a/programs/library-bridge/SharedLibraryHandlerFactory.h b/programs/library-bridge/SharedLibraryHandlerFactory.h index 473d90618a2..115cc78ae52 100644 --- a/programs/library-bridge/SharedLibraryHandlerFactory.h +++ b/programs/library-bridge/SharedLibraryHandlerFactory.h @@ -24,9 +24,9 @@ public: const Block & sample_block, const std::vector & attributes_names); - void clone(const std::string & from_dictionary_id, const std::string & to_dictionary_id); + bool clone(const std::string & from_dictionary_id, const std::string & to_dictionary_id); - void remove(const std::string & dictionary_id); + bool remove(const std::string & dictionary_id); private: /// map: dict_id -> sharedLibraryHandler diff --git a/src/Bridge/IBridgeHelper.cpp b/src/Bridge/IBridgeHelper.cpp index b6f3446d0a6..a90688cc8b5 100644 --- a/src/Bridge/IBridgeHelper.cpp +++ b/src/Bridge/IBridgeHelper.cpp @@ -33,21 +33,6 @@ Poco::URI IBridgeHelper::getPingURI() const } -bool IBridgeHelper::checkBridgeIsRunning() const -{ - try - { - ReadWriteBufferFromHTTP buf( - getPingURI(), Poco::Net::HTTPRequest::HTTP_GET, {}, ConnectionTimeouts::getHTTPTimeouts(getContext())); - return checkString(PING_OK_ANSWER, buf); - } - catch (...) - { - return false; - } -} - - void IBridgeHelper::startBridgeSync() const { if (!checkBridgeIsRunning()) diff --git a/src/Bridge/IBridgeHelper.h b/src/Bridge/IBridgeHelper.h index caaf031b7d8..6a9c19da68f 100644 --- a/src/Bridge/IBridgeHelper.h +++ b/src/Bridge/IBridgeHelper.h @@ -28,16 +28,18 @@ public: static const inline std::string MAIN_METHOD = Poco::Net::HTTPRequest::HTTP_POST; explicit IBridgeHelper(ContextPtr context_) : WithContext(context_) {} - virtual ~IBridgeHelper() = default; - void startBridgeSync() const; + virtual ~IBridgeHelper() = default; Poco::URI getMainURI() const; Poco::URI getPingURI() const; + void startBridgeSync() const; protected: + virtual bool checkBridgeIsRunning() const = 0; + /// clickhouse-odbc-bridge, clickhouse-library-bridge virtual String serviceAlias() const = 0; @@ -61,8 +63,6 @@ protected: private: - bool checkBridgeIsRunning() const; - std::unique_ptr startBridgeCommand() const; }; diff --git a/src/Bridge/LibraryBridgeHelper.cpp b/src/Bridge/LibraryBridgeHelper.cpp index b13be0aba29..6f976200153 100644 --- a/src/Bridge/LibraryBridgeHelper.cpp +++ b/src/Bridge/LibraryBridgeHelper.cpp @@ -23,12 +23,14 @@ namespace DB LibraryBridgeHelper::LibraryBridgeHelper( ContextPtr context_, const Block & sample_block_, - const Field & dictionary_id_) + const Field & dictionary_id_, + const LibraryInitData & library_data_) : IBridgeHelper(context_->getGlobalContext()) , log(&Poco::Logger::get("LibraryBridgeHelper")) , sample_block(sample_block_) , config(context_->getConfigRef()) , http_timeout(context_->getGlobalContext()->getSettingsRef().http_receive_timeout.value) + , library_data(library_data_) , dictionary_id(dictionary_id_) { bridge_port = config.getUInt("library_bridge.port", DEFAULT_PORT); @@ -61,26 +63,85 @@ void LibraryBridgeHelper::startBridge(std::unique_ptr cmd) const } -bool LibraryBridgeHelper::initLibrary(const std::string & library_path, const std::string library_settings, const std::string attributes_names) +bool LibraryBridgeHelper::checkBridgeIsRunning() const { - startBridgeSync(); - auto uri = createRequestURI(LIB_NEW_METHOD); + String result; + try + { + ReadWriteBufferFromHTTP buf(createRequestURI(PING), Poco::Net::HTTPRequest::HTTP_GET, {}, ConnectionTimeouts::getHTTPTimeouts(getContext())); + readString(result, buf); + } + catch (...) + { + return false; + } + /* + * When pinging bridge we also pass current dicionary_id. The bridge will check if there is such + * dictionary. It is possible that such dictionary_id is not present only in two cases: + * 1. It is dictionary source creation and initialization of library handler on bridge side did not happen yet. + * 2. Bridge crashed or restarted for some reason while server did not. + **/ + static constexpr auto dictionary_check = "dictionary="; + if (result.size() != (std::strlen(dictionary_check) + 1)) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {}. Check bridge and server have the same version.", + result, std::strlen(dictionary_check)); + + UInt8 dictionary_id_exists; + auto parsed = tryParse(dictionary_id_exists, result.substr(std::strlen(dictionary_check))); + if (!parsed || (dictionary_id_exists != 0 && dictionary_id_exists != 1)) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {} ({}). Check bridge and server have the same version.", + result, parsed ? toString(dictionary_id_exists) : "failed to parse"); + + if (dictionary_id_exists && !library_initialized) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Library was not initialized, but bridge responded to already have dictionary id: {}", dictionary_id); + + if (!dictionary_id_exists && library_initialized) + { + LOG_WARNING(log, "Library bridge does not have library handler with dictionaty id: {}. It will be reinitialized.", dictionary_id); + try + { + if (!initLibrary(false)) + throw Exception(ErrorCodes::LOGICAL_ERROR, + "Failed to reinitialize library handler on bridge side for dictionary with id: {}", dictionary_id); + } + catch (...) + { + tryLogCurrentException(log); + return false; + } + } + + return true; +} + + +ReadWriteBufferFromHTTP::OutStreamCallback LibraryBridgeHelper::getInitLibraryCallback() const +{ /// Sample block must contain null values WriteBufferFromOwnString out; auto output_stream = getContext()->getOutputStream(LibraryBridgeHelper::DEFAULT_FORMAT, out, sample_block); formatBlock(output_stream, sample_block); auto block_string = out.str(); - auto out_stream_callback = [library_path, library_settings, attributes_names, block_string, this](std::ostream & os) + return [block_string, this](std::ostream & os) { - os << "library_path=" << escapeForFileName(library_path) << "&"; - os << "library_settings=" << escapeForFileName(library_settings) << "&"; - os << "attributes_names=" << escapeForFileName(attributes_names) << "&"; + os << "library_path=" << escapeForFileName(library_data.library_path) << "&"; + os << "library_settings=" << escapeForFileName(library_data.library_settings) << "&"; + os << "attributes_names=" << escapeForFileName(library_data.dict_attributes) << "&"; os << "sample_block=" << escapeForFileName(sample_block.getNamesAndTypesList().toString()) << "&"; os << "null_values=" << escapeForFileName(block_string); }; - return executeRequest(uri, out_stream_callback); +} + + +bool LibraryBridgeHelper::initLibrary(bool check_bridge) const +{ + /// Do not check if we call initLibrary from checkBridgeSync. + if (check_bridge) + startBridgeSync(); + auto uri = createRequestURI(LIB_NEW_METHOD); + return executeRequest(uri, getInitLibraryCallback()); } @@ -89,15 +150,20 @@ bool LibraryBridgeHelper::cloneLibrary(const Field & other_dictionary_id) startBridgeSync(); auto uri = createRequestURI(LIB_CLONE_METHOD); uri.addQueryParameter("from_dictionary_id", toString(other_dictionary_id)); - return executeRequest(uri); + return executeRequest(uri, getInitLibraryCallback()); } bool LibraryBridgeHelper::removeLibrary() { - startBridgeSync(); - auto uri = createRequestURI(LIB_DELETE_METHOD); - return executeRequest(uri); + /// Do not force bridge restart if it is not running in case of removeLibrary + /// because in this case after restart it will not have this dictionaty id in memory anyway. + if (checkBridgeIsRunning()) + { + auto uri = createRequestURI(LIB_DELETE_METHOD); + return executeRequest(uri); + } + return true; } @@ -125,11 +191,25 @@ BlockInputStreamPtr LibraryBridgeHelper::loadAll() } -BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::string ids_string) +BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::string, const std::vector ids) { startBridgeSync(); auto uri = createRequestURI(LOAD_IDS_METHOD); - return loadBase(uri, [ids_string](std::ostream & os) { os << ids_string; }); + + uri.addQueryParameter("ids_num", toString(ids.size())); + String ids_str; + for (const auto & id : ids) + { + if (!ids_str.empty()) + ids_str += '-'; + ids_str += toString(id); + } + + uri.addQueryParameter("ids", ids_str); + std::cerr << "\n\nLibraryBridgeHelper: " << toString(dictionary_id) << " , ids_num: " << ids.size() << " , ids: " << ids_str << std::endl << std::endl; + LOG_ERROR(log, "dictionary_id: {}, ids_num: {}, ids: {}", dictionary_id, ids.size(), ids_str); + + return loadBase(uri, [ids_str](std::ostream & os) { os << ids_str; }); } @@ -149,7 +229,7 @@ BlockInputStreamPtr LibraryBridgeHelper::loadKeys(const Block & requested_block) } -bool LibraryBridgeHelper::executeRequest(const Poco::URI & uri, ReadWriteBufferFromHTTP::OutStreamCallback out_stream_callback) +bool LibraryBridgeHelper::executeRequest(const Poco::URI & uri, ReadWriteBufferFromHTTP::OutStreamCallback out_stream_callback) const { ReadWriteBufferFromHTTP buf( uri, diff --git a/src/Bridge/LibraryBridgeHelper.h b/src/Bridge/LibraryBridgeHelper.h index 12fe0c33363..815fce72b33 100644 --- a/src/Bridge/LibraryBridgeHelper.h +++ b/src/Bridge/LibraryBridgeHelper.h @@ -15,11 +15,18 @@ class LibraryBridgeHelper : public IBridgeHelper { public: + struct LibraryInitData + { + String library_path; + String library_settings; + String dict_attributes; + }; + static constexpr inline size_t DEFAULT_PORT = 9012; - LibraryBridgeHelper(ContextPtr context_, const Block & sample_block, const Field & dictionary_id_); + LibraryBridgeHelper(ContextPtr context_, const Block & sample_block, const Field & dictionary_id_, const LibraryInitData & library_data_); - bool initLibrary(const std::string & library_path, std::string library_settings, std::string attributes_names); + bool initLibrary(bool check_bridge = true) const; bool cloneLibrary(const Field & other_dictionary_id); @@ -31,16 +38,21 @@ public: BlockInputStreamPtr loadAll(); - BlockInputStreamPtr loadIds(std::string ids_string); + BlockInputStreamPtr loadIds(std::string ids_string, const std::vector ids); BlockInputStreamPtr loadKeys(const Block & requested_block); BlockInputStreamPtr loadBase(const Poco::URI & uri, ReadWriteBufferFromHTTP::OutStreamCallback out_stream_callback = {}); - bool executeRequest(const Poco::URI & uri, ReadWriteBufferFromHTTP::OutStreamCallback out_stream_callback = {}); + bool executeRequest(const Poco::URI & uri, ReadWriteBufferFromHTTP::OutStreamCallback out_stream_callback = {}) const; + LibraryInitData getLibraryData() const { return library_data; } + + void setInitialized() { library_initialized = true; } protected: + bool checkBridgeIsRunning() const override; + void startBridge(std::unique_ptr cmd) const override; String serviceAlias() const override { return "clickhouse-library-bridge"; } @@ -61,6 +73,8 @@ protected: Poco::URI createBaseURI() const override; + ReadWriteBufferFromHTTP::OutStreamCallback getInitLibraryCallback() const; + private: static constexpr inline auto LIB_NEW_METHOD = "libNew"; static constexpr inline auto LIB_CLONE_METHOD = "libClone"; @@ -69,6 +83,7 @@ private: static constexpr inline auto LOAD_IDS_METHOD = "loadIds"; static constexpr inline auto LOAD_KEYS_METHOD = "loadKeys"; static constexpr inline auto IS_MODIFIED_METHOD = "isModified"; + static constexpr inline auto PING = "ping"; static constexpr inline auto SUPPORTS_SELECTIVE_LOAD_METHOD = "supportsSelectiveLoad"; Poco::URI createRequestURI(const String & method) const; @@ -78,9 +93,11 @@ private: const Poco::Util::AbstractConfiguration & config; const Poco::Timespan http_timeout; + LibraryInitData library_data; Field dictionary_id; std::string bridge_host; size_t bridge_port; + bool library_initialized = false; }; } diff --git a/src/Bridge/XDBCBridgeHelper.h b/src/Bridge/XDBCBridgeHelper.h index 8be4c194962..13f0f528c8f 100644 --- a/src/Bridge/XDBCBridgeHelper.h +++ b/src/Bridge/XDBCBridgeHelper.h @@ -60,20 +60,33 @@ public: static constexpr inline auto SCHEMA_ALLOWED_HANDLER = "/schema_allowed"; XDBCBridgeHelper( - ContextPtr context_, - Poco::Timespan http_timeout_, - const std::string & connection_string_) - : IXDBCBridgeHelper(context_->getGlobalContext()) - , log(&Poco::Logger::get(BridgeHelperMixin::getName() + "BridgeHelper")) - , connection_string(connection_string_) - , http_timeout(http_timeout_) - , config(context_->getGlobalContext()->getConfigRef()) -{ - bridge_host = config.getString(BridgeHelperMixin::configPrefix() + ".host", DEFAULT_HOST); - bridge_port = config.getUInt(BridgeHelperMixin::configPrefix() + ".port", DEFAULT_PORT); -} + ContextPtr context_, + Poco::Timespan http_timeout_, + const std::string & connection_string_) + : IXDBCBridgeHelper(context_->getGlobalContext()) + , log(&Poco::Logger::get(BridgeHelperMixin::getName() + "BridgeHelper")) + , connection_string(connection_string_) + , http_timeout(http_timeout_) + , config(context_->getGlobalContext()->getConfigRef()) + { + bridge_host = config.getString(BridgeHelperMixin::configPrefix() + ".host", DEFAULT_HOST); + bridge_port = config.getUInt(BridgeHelperMixin::configPrefix() + ".port", DEFAULT_PORT); + } protected: + bool checkBridgeIsRunning() const override + { + try + { + ReadWriteBufferFromHTTP buf(getPingURI(), Poco::Net::HTTPRequest::HTTP_GET, {}, ConnectionTimeouts::getHTTPTimeouts(getContext())); + return checkString(PING_OK_ANSWER, buf); + } + catch (...) + { + return false; + } + } + auto getConnectionString() const { return connection_string; } String getName() const override { return BridgeHelperMixin::getName(); } diff --git a/src/Dictionaries/LibraryDictionarySource.cpp b/src/Dictionaries/LibraryDictionarySource.cpp index 0b8b52a2d67..d5a63735643 100644 --- a/src/Dictionaries/LibraryDictionarySource.cpp +++ b/src/Dictionaries/LibraryDictionarySource.cpp @@ -48,17 +48,34 @@ LibraryDictionarySource::LibraryDictionarySource( throw Exception(ErrorCodes::FILE_DOESNT_EXIST, "LibraryDictionarySource: Can't load library {}: file doesn't exist", path); description.init(sample_block); - bridge_helper = std::make_shared(context, description.sample_block, dictionary_id); - auto res = bridge_helper->initLibrary(path, getLibrarySettingsString(config, config_prefix + ".settings"), getDictAttributesString()); + + LibraryBridgeHelper::LibraryInitData library_data + { + .library_path = path, + .library_settings = getLibrarySettingsString(config, config_prefix + ".settings"), + .dict_attributes = getDictAttributesString() + }; + bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, library_data); + auto res = bridge_helper->initLibrary(); if (!res) throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to create shared library from path: {}", path); + else + bridge_helper->setInitialized(); } LibraryDictionarySource::~LibraryDictionarySource() { - bridge_helper->removeLibrary(); + try + { + bridge_helper->removeLibrary(); + } + catch (...) + { + tryLogCurrentException("LibraryDictionarySource"); + } + } @@ -72,7 +89,7 @@ LibraryDictionarySource::LibraryDictionarySource(const LibraryDictionarySource & , context(other.context) , description{other.description} { - bridge_helper = std::make_shared(context, description.sample_block, dictionary_id); + bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, other.bridge_helper->getLibraryData()); bridge_helper->cloneLibrary(other.dictionary_id); } @@ -99,7 +116,7 @@ BlockInputStreamPtr LibraryDictionarySource::loadAll() BlockInputStreamPtr LibraryDictionarySource::loadIds(const std::vector & ids) { LOG_TRACE(log, "loadIds {} size = {}", toString(), ids.size()); - return bridge_helper->loadIds(getDictIdsString(ids)); + return bridge_helper->loadIds(getDictIdsString(ids), ids); } diff --git a/tests/integration/test_library_bridge/test.py b/tests/integration/test_library_bridge/test.py index ba44918bd60..dc15f6a8430 100644 --- a/tests/integration/test_library_bridge/test.py +++ b/tests/integration/test_library_bridge/test.py @@ -8,8 +8,23 @@ from helpers.cluster import ClickHouseCluster, run_and_check cluster = ClickHouseCluster(__file__) instance = cluster.add_instance('instance', - dictionaries=['configs/dictionaries/dict1.xml'], - main_configs=['configs/config.d/config.xml']) + dictionaries=['configs/dictionaries/dict1.xml'], main_configs=['configs/config.d/config.xml'], stay_alive=True) + + +def create_dict_simple(): + instance.query('DROP DICTIONARY IF EXISTS lib_dict_c') + instance.query(''' + CREATE DICTIONARY lib_dict_c (key UInt64, value1 UInt64, value2 UInt64, value3 UInt64) + PRIMARY KEY key SOURCE(library(PATH '/etc/clickhouse-server/config.d/dictionaries_lib/dict_lib.so')) + LAYOUT(CACHE( + SIZE_IN_CELLS 10000000 + BLOCK_SIZE 4096 + FILE_SIZE 16777216 + READ_BUFFER_SIZE 1048576 + MAX_STORED_KEYS 1048576)) + LIFETIME(2) ; + ''') + @pytest.fixture(scope="module") def ch_cluster(): @@ -160,6 +175,52 @@ def test_null_values(ch_cluster): assert(result == expected) +def test_recover_after_bridge_crash(ch_cluster): + if instance.is_built_with_memory_sanitizer(): + pytest.skip("Memory Sanitizer cannot work with third-party shared libraries") + + create_dict_simple() + + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(0));''') + assert(result.strip() == '100') + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + instance.exec_in_container(['bash', '-c', 'kill -9 `pidof clickhouse-library-bridge`'], user='root') + instance.query('SYSTEM RELOAD DICTIONARY lib_dict_c') + + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(0));''') + assert(result.strip() == '100') + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + instance.exec_in_container(['bash', '-c', 'kill -9 `pidof clickhouse-library-bridge`'], user='root') + instance.query('DROP DICTIONARY lib_dict_c') + + +def test_server_restart_bridge_might_be_stil_alive(ch_cluster): + if instance.is_built_with_memory_sanitizer(): + pytest.skip("Memory Sanitizer cannot work with third-party shared libraries") + + create_dict_simple() + + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + instance.restart_clickhouse() + + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + instance.exec_in_container(['bash', '-c', 'kill -9 `pidof clickhouse-library-bridge`'], user='root') + instance.restart_clickhouse() + + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + instance.query('DROP DICTIONARY lib_dict_c') + + if __name__ == '__main__': cluster.start() input("Cluster created, press any key to destroy...") From 9c6a8b00593e8a9ad431e682b547c326704fc749 Mon Sep 17 00:00:00 2001 From: kssenii Date: Sun, 1 Aug 2021 08:51:40 +0000 Subject: [PATCH 34/78] Restore previous ids passing --- programs/library-bridge/Handlers.cpp | 28 +++++------- src/Bridge/LibraryBridgeHelper.cpp | 17 +------- tests/integration/test_library_bridge/test.py | 43 +++++++++++++++++++ 3 files changed, 55 insertions(+), 33 deletions(-) diff --git a/programs/library-bridge/Handlers.cpp b/programs/library-bridge/Handlers.cpp index 7e79830a7b2..0b49a801ca3 100644 --- a/programs/library-bridge/Handlers.cpp +++ b/programs/library-bridge/Handlers.cpp @@ -40,13 +40,12 @@ namespace return sample_block; } - // std::vector parseIdsFromBinary(const std::string & ids_string) - // { - // ReadBufferFromString buf(ids_string); - // std::vector ids; - // readVectorBinary(ids, buf); - // return ids; - // } + std::vector parseIdsFromBinary(ReadBuffer & buf) + { + std::vector ids; + readVectorBinary(ids, buf); + return ids; + } std::vector parseNamesFromBinary(const std::string & names_string) { @@ -212,6 +211,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); + LOG_DEBUG(log, "Calling loadAll() for dictionary id: {}", dictionary_id); auto input = library_handler->loadAll(); LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); @@ -222,23 +222,14 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe { LOG_DEBUG(log, "Getting diciontary ids for dictionary with id: {}", dictionary_id); String ids_string; - readString(ids_string, request.getStream()); - - Strings ids_strs; - splitInto<'-'>(ids_strs, ids_string); - std::vector ids; - for (const auto & id : ids_strs) - ids.push_back(parse(id)); - if (ids.empty()) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Received no ids"); - - // std::vector ids = parseIdsFromBinary(ids_string); + std::vector ids = parseIdsFromBinary(request.getStream()); auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); + LOG_DEBUG(log, "Calling loadIds() for dictionary id: {}", dictionary_id); auto input = library_handler->loadIds(ids); LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); @@ -277,6 +268,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); + LOG_DEBUG(log, "Calling loadKeys() for dictionary id: {}", dictionary_id); auto input = library_handler->loadKeys(block.getColumns()); LOG_DEBUG(log, "Started sending result data for dictionary id: {}", dictionary_id); diff --git a/src/Bridge/LibraryBridgeHelper.cpp b/src/Bridge/LibraryBridgeHelper.cpp index 6f976200153..9cdc2c7864a 100644 --- a/src/Bridge/LibraryBridgeHelper.cpp +++ b/src/Bridge/LibraryBridgeHelper.cpp @@ -191,24 +191,11 @@ BlockInputStreamPtr LibraryBridgeHelper::loadAll() } -BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::string, const std::vector ids) +BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::string ids_str, const std::vector ids) { startBridgeSync(); auto uri = createRequestURI(LOAD_IDS_METHOD); - - uri.addQueryParameter("ids_num", toString(ids.size())); - String ids_str; - for (const auto & id : ids) - { - if (!ids_str.empty()) - ids_str += '-'; - ids_str += toString(id); - } - - uri.addQueryParameter("ids", ids_str); - std::cerr << "\n\nLibraryBridgeHelper: " << toString(dictionary_id) << " , ids_num: " << ids.size() << " , ids: " << ids_str << std::endl << std::endl; - LOG_ERROR(log, "dictionary_id: {}, ids_num: {}, ids: {}", dictionary_id, ids.size(), ids_str); - + uri.addQueryParameter("ids_num", toString(ids.size())); /// Not used parameter, but helpful return loadBase(uri, [ids_str](std::ostream & os) { os << ids_str; }); } diff --git a/tests/integration/test_library_bridge/test.py b/tests/integration/test_library_bridge/test.py index dc15f6a8430..0b01627a9c8 100644 --- a/tests/integration/test_library_bridge/test.py +++ b/tests/integration/test_library_bridge/test.py @@ -113,6 +113,10 @@ def test_load_ids(ch_cluster): result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(0));''') assert(result.strip() == '100') + + # Just check bridge is ok with a large vector of random ids + instance.query('''select number, dictGet(lib_dict_c, 'value1', toUInt64(rand())) from numbers(5000);''') + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') assert(result.strip() == '101') instance.query('DROP DICTIONARY lib_dict_c') @@ -221,6 +225,45 @@ def test_server_restart_bridge_might_be_stil_alive(ch_cluster): instance.query('DROP DICTIONARY lib_dict_c') +def test_bridge_dies_with_parent(ch_cluster): + if instance.is_built_with_memory_sanitizer(): + pytest.skip("Memory Sanitizer cannot work with third-party shared libraries") + if instance.is_built_with_address_sanitizer(): + pytest.skip("Leak sanitizer falsely reports about a leak of 16 bytes in clickhouse-odbc-bridge") + + create_dict_simple() + result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') + assert(result.strip() == '101') + + clickhouse_pid = instance.get_process_pid("clickhouse server") + bridge_pid = instance.get_process_pid("library-bridge") + assert clickhouse_pid is not None + assert bridge_pid is not None + + while clickhouse_pid is not None: + try: + instance.exec_in_container(["kill", str(clickhouse_pid)], privileged=True, user='root') + except: + pass + clickhouse_pid = instance.get_process_pid("clickhouse server") + time.sleep(1) + + for i in range(30): + time.sleep(1) + bridge_pid = instance.get_process_pid("library-bridge") + if bridge_pid is None: + break + + if bridge_pid: + out = instance.exec_in_container(["gdb", "-p", str(bridge_pid), "--ex", "thread apply all bt", "--ex", "q"], + privileged=True, user='root') + logging.debug(f"Bridge is running, gdb output:\n{out}") + + assert clickhouse_pid is None + assert bridge_pid is None + instance.start_clickhouse(20) + + if __name__ == '__main__': cluster.start() input("Cluster created, press any key to destroy...") From 2bdb97d5e01f901ca585114c786d6c445d1a9df8 Mon Sep 17 00:00:00 2001 From: kssenii Date: Sun, 1 Aug 2021 09:56:48 +0000 Subject: [PATCH 35/78] Some small changes --- src/Bridge/LibraryBridgeHelper.cpp | 35 +++++++++++++++---- src/Bridge/LibraryBridgeHelper.h | 4 ++- src/Dictionaries/LibraryDictionarySource.cpp | 27 +++++++------- src/Dictionaries/LibraryDictionarySource.h | 2 -- tests/integration/test_library_bridge/test.py | 2 +- 5 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/Bridge/LibraryBridgeHelper.cpp b/src/Bridge/LibraryBridgeHelper.cpp index 9cdc2c7864a..b39e3f01903 100644 --- a/src/Bridge/LibraryBridgeHelper.cpp +++ b/src/Bridge/LibraryBridgeHelper.cpp @@ -1,6 +1,5 @@ #include "LibraryBridgeHelper.h" -#include #include #include #include @@ -8,6 +7,8 @@ #include #include #include +#include +#include #include #include #include @@ -20,6 +21,12 @@ namespace DB { +namespace ErrorCodes +{ + extern const int EXTERNAL_LIBRARY_ERROR; + extern const int LOGICAL_ERROR; +} + LibraryBridgeHelper::LibraryBridgeHelper( ContextPtr context_, const Block & sample_block_, @@ -93,23 +100,29 @@ bool LibraryBridgeHelper::checkBridgeIsRunning() const throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {} ({}). Check bridge and server have the same version.", result, parsed ? toString(dictionary_id_exists) : "failed to parse"); + LOG_TRACE(log, "dictionary_id: {}, dictionary_id_exists on bridge side: {}, library confirmed to be initialized on server side: {}", + toString(dictionary_id), toString(dictionary_id_exists), library_initialized); + if (dictionary_id_exists && !library_initialized) throw Exception(ErrorCodes::LOGICAL_ERROR, "Library was not initialized, but bridge responded to already have dictionary id: {}", dictionary_id); if (!dictionary_id_exists && library_initialized) { LOG_WARNING(log, "Library bridge does not have library handler with dictionaty id: {}. It will be reinitialized.", dictionary_id); + bool reinitialized = false; try { - if (!initLibrary(false)) - throw Exception(ErrorCodes::LOGICAL_ERROR, - "Failed to reinitialize library handler on bridge side for dictionary with id: {}", dictionary_id); + reinitialized = initLibrary(false); } catch (...) { tryLogCurrentException(log); return false; } + + if (!reinitialized) + throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, + "Failed to reinitialize library handler on bridge side for dictionary with id: {}", dictionary_id); } return true; @@ -191,12 +204,13 @@ BlockInputStreamPtr LibraryBridgeHelper::loadAll() } -BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::string ids_str, const std::vector ids) +BlockInputStreamPtr LibraryBridgeHelper::loadIds(const std::vector & ids) { startBridgeSync(); auto uri = createRequestURI(LOAD_IDS_METHOD); uri.addQueryParameter("ids_num", toString(ids.size())); /// Not used parameter, but helpful - return loadBase(uri, [ids_str](std::ostream & os) { os << ids_str; }); + auto ids_string = getDictIdsString(ids); + return loadBase(uri, [ids_string](std::ostream & os) { os << ids_string; }); } @@ -246,4 +260,13 @@ BlockInputStreamPtr LibraryBridgeHelper::loadBase(const Poco::URI & uri, ReadWri return std::make_shared>(input_stream, std::move(read_buf_ptr)); } + +String LibraryBridgeHelper::getDictIdsString(const std::vector & ids) +{ + WriteBufferFromOwnString out; + writeVectorBinary(ids, out); + return out.str(); +} + + } diff --git a/src/Bridge/LibraryBridgeHelper.h b/src/Bridge/LibraryBridgeHelper.h index 815fce72b33..e4fe8e06965 100644 --- a/src/Bridge/LibraryBridgeHelper.h +++ b/src/Bridge/LibraryBridgeHelper.h @@ -38,7 +38,7 @@ public: BlockInputStreamPtr loadAll(); - BlockInputStreamPtr loadIds(std::string ids_string, const std::vector ids); + BlockInputStreamPtr loadIds(const std::vector & ids); BlockInputStreamPtr loadKeys(const Block & requested_block); @@ -88,6 +88,8 @@ private: Poco::URI createRequestURI(const String & method) const; + static String getDictIdsString(const std::vector & ids); + Poco::Logger * log; const Block sample_block; const Poco::Util::AbstractConfiguration & config; diff --git a/src/Dictionaries/LibraryDictionarySource.cpp b/src/Dictionaries/LibraryDictionarySource.cpp index d5a63735643..273f9d3c3b4 100644 --- a/src/Dictionaries/LibraryDictionarySource.cpp +++ b/src/Dictionaries/LibraryDictionarySource.cpp @@ -55,13 +55,14 @@ LibraryDictionarySource::LibraryDictionarySource( .library_settings = getLibrarySettingsString(config, config_prefix + ".settings"), .dict_attributes = getDictAttributesString() }; - bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, library_data); - auto res = bridge_helper->initLibrary(); - if (!res) - throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to create shared library from path: {}", path); - else + bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, library_data); + + bool initialized = bridge_helper->initLibrary(); + if (initialized) bridge_helper->setInitialized(); + else + throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to create shared library from path: {}", path); } @@ -90,7 +91,11 @@ LibraryDictionarySource::LibraryDictionarySource(const LibraryDictionarySource & , description{other.description} { bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, other.bridge_helper->getLibraryData()); - bridge_helper->cloneLibrary(other.dictionary_id); + bool cloned = bridge_helper->cloneLibrary(other.dictionary_id); + if (cloned) + bridge_helper->setInitialized(); + else + throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to clone library"); } @@ -116,7 +121,7 @@ BlockInputStreamPtr LibraryDictionarySource::loadAll() BlockInputStreamPtr LibraryDictionarySource::loadIds(const std::vector & ids) { LOG_TRACE(log, "loadIds {} size = {}", toString(), ids.size()); - return bridge_helper->loadIds(getDictIdsString(ids), ids); + return bridge_helper->loadIds(ids); } @@ -164,14 +169,6 @@ String LibraryDictionarySource::getLibrarySettingsString(const Poco::Util::Abstr } -String LibraryDictionarySource::getDictIdsString(const std::vector & ids) -{ - WriteBufferFromOwnString out; - writeVectorBinary(ids, out); - return out.str(); -} - - String LibraryDictionarySource::getDictAttributesString() { std::vector attributes_names(dict_struct.attributes.size()); diff --git a/src/Dictionaries/LibraryDictionarySource.h b/src/Dictionaries/LibraryDictionarySource.h index 88e133666e6..18a2d1b97b2 100644 --- a/src/Dictionaries/LibraryDictionarySource.h +++ b/src/Dictionaries/LibraryDictionarySource.h @@ -70,8 +70,6 @@ public: std::string toString() const override; private: - static String getDictIdsString(const std::vector & ids); - String getDictAttributesString(); static String getLibrarySettingsString(const Poco::Util::AbstractConfiguration & config, const std::string & config_root); diff --git a/tests/integration/test_library_bridge/test.py b/tests/integration/test_library_bridge/test.py index 0b01627a9c8..4bef197f8bd 100644 --- a/tests/integration/test_library_bridge/test.py +++ b/tests/integration/test_library_bridge/test.py @@ -115,7 +115,7 @@ def test_load_ids(ch_cluster): assert(result.strip() == '100') # Just check bridge is ok with a large vector of random ids - instance.query('''select number, dictGet(lib_dict_c, 'value1', toUInt64(rand())) from numbers(5000);''') + instance.query('''select number, dictGet(lib_dict_c, 'value1', toUInt64(rand())) from numbers(1000);''') result = instance.query('''select dictGet(lib_dict_c, 'value1', toUInt64(1));''') assert(result.strip() == '101') From 10d860acc37b21a1095d555cab27fb33217671e2 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Sun, 1 Aug 2021 14:26:11 +0000 Subject: [PATCH 36/78] Empty commit From 15f4ff9d6edac5e305241f7aabc067ef7ddb81b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=87=E5=BA=B7?= Date: Mon, 2 Aug 2021 08:29:28 +0800 Subject: [PATCH 37/78] skip parallel test --- tests/queries/skip_list.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/queries/skip_list.json b/tests/queries/skip_list.json index 3eede6783f8..46bc1d8ae01 100644 --- a/tests/queries/skip_list.json +++ b/tests/queries/skip_list.json @@ -525,6 +525,7 @@ "01902_table_function_merge_db_repr", "01946_test_zstd_decompression_with_escape_sequence_at_the_end_of_buffer", "01946_test_wrong_host_name_access", - "01213_alter_rename_with_default_zookeeper" /// Warning: Removing leftovers from table. + "01213_alter_rename_with_default_zookeeper", /// Warning: Removing leftovers from table. + "02001_add_default_database_to_system_users" ///create user ] } From 583c2d67cbdbcfd34aa4594281dfd4ee00220dd4 Mon Sep 17 00:00:00 2001 From: Kseniia Sumarokova <54203879+kssenii@users.noreply.github.com> Date: Mon, 2 Aug 2021 09:32:23 +0300 Subject: [PATCH 38/78] Update DiskSelector.cpp --- src/Disks/DiskSelector.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index 1bf71fb2efd..1e081a59701 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -75,7 +75,6 @@ DiskSelectorPtr DiskSelector::updateFromConfig( old_disks_minus_new_disks.erase(disk_name); - // TODO: Implement updateFromConfigIfChanged for DiskS3 (DiskLocal already done) result->getDisksMap().find(disk_name)->second->updateFromConfigIfChanged(config, disk_config_prefix, context); } } From 9007ddfe227133263ec1666ff3b87ff8fb59a2c9 Mon Sep 17 00:00:00 2001 From: Kseniia Sumarokova <54203879+kssenii@users.noreply.github.com> Date: Mon, 2 Aug 2021 09:34:39 +0300 Subject: [PATCH 39/78] Update DiskLocal.h --- src/Disks/DiskLocal.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index d682952bd4f..714d4683590 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -17,7 +17,6 @@ namespace ErrorCodes class DiskLocalReservation; -class Context; class DiskLocal : public IDisk { public: From 322870acd29561e1415897a3ae5a43de0172543d Mon Sep 17 00:00:00 2001 From: Kseniia Sumarokova <54203879+kssenii@users.noreply.github.com> Date: Mon, 2 Aug 2021 09:36:44 +0300 Subject: [PATCH 40/78] Update IDisk.h --- src/Disks/IDisk.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h index 229e03e3bbf..0f902dd8338 100644 --- a/src/Disks/IDisk.h +++ b/src/Disks/IDisk.h @@ -35,7 +35,6 @@ using Reservations = std::vector; class ReadBufferFromFileBase; class WriteBufferFromFileBase; -class Context; class MMappedFileCache; /** @@ -232,7 +231,7 @@ public: /// Required for remote disk to ensure that replica has access to data written by other node virtual bool checkUniqueId(const String & id) const { return exists(id); } - /// Reload config if config changed + /// Reload config if it was changed virtual void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & /* config */, const String & /* config_prefix */, ContextPtr /* context */) { } From b25dfdd6af0b2ea5749938731b2e3ee2d4ca52cd Mon Sep 17 00:00:00 2001 From: kssenii Date: Mon, 2 Aug 2021 08:15:21 +0000 Subject: [PATCH 41/78] Use method which was added for s3 instead --- src/Disks/DiskLocal.cpp | 8 +++++--- src/Disks/DiskLocal.h | 4 +--- src/Disks/DiskSelector.cpp | 2 -- src/Disks/IDisk.h | 5 ----- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index edebb6ff71e..b4d7f74f10f 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -367,15 +367,17 @@ SyncGuardPtr DiskLocal::getDirectorySyncGuard(const String & path) const return std::make_unique(fs::path(disk_path) / path); } -void DiskLocal::updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, - const String & config_prefix, ContextPtr context) + +void DiskLocal::applyNewSettings(const Poco::Util::AbstractConfiguration & config, ContextPtr context, const String & config_prefix, const DisksMap &) { String new_disk_path; UInt64 new_keep_free_space_bytes; + loadDiskLocalConfig(name, config, config_prefix, context, new_disk_path, new_keep_free_space_bytes); if (disk_path != new_disk_path) - throw Exception("Disk path can't update from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + throw Exception("Disk path can't be updated from config " + name, ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG); + if (keep_free_space_bytes != new_keep_free_space_bytes) keep_free_space_bytes = new_keep_free_space_bytes; } diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index 714d4683590..0cf0cf39bbc 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -105,9 +105,7 @@ public: SyncGuardPtr getDirectorySyncGuard(const String & path) const override; - void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & config, - const String & config_prefix, - ContextPtr context) override; + void applyNewSettings(const Poco::Util::AbstractConfiguration & config, ContextPtr context, const String & config_prefix, const DisksMap &) override; private: bool tryReserve(UInt64 bytes); diff --git a/src/Disks/DiskSelector.cpp b/src/Disks/DiskSelector.cpp index 1e081a59701..bc7810479c5 100644 --- a/src/Disks/DiskSelector.cpp +++ b/src/Disks/DiskSelector.cpp @@ -74,8 +74,6 @@ DiskSelectorPtr DiskSelector::updateFromConfig( disk->applyNewSettings(config, context, disk_config_prefix, result->getDisksMap()); old_disks_minus_new_disks.erase(disk_name); - - result->getDisksMap().find(disk_name)->second->updateFromConfigIfChanged(config, disk_config_prefix, context); } } diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h index 0f902dd8338..61c805961ae 100644 --- a/src/Disks/IDisk.h +++ b/src/Disks/IDisk.h @@ -231,11 +231,6 @@ public: /// Required for remote disk to ensure that replica has access to data written by other node virtual bool checkUniqueId(const String & id) const { return exists(id); } - /// Reload config if it was changed - virtual void updateFromConfigIfChanged(const Poco::Util::AbstractConfiguration & /* config */, - const String & /* config_prefix */, - ContextPtr /* context */) { } - /// Invoked on partitions freeze query. virtual void onFreeze(const String &) { } From 7364f35da6d4cd87b1c9ef5757c386b2c99358ac Mon Sep 17 00:00:00 2001 From: vdimir Date: Mon, 2 Aug 2021 13:49:56 +0300 Subject: [PATCH 42/78] Add alwaysReturnsEmptySet for Merge Join --- src/Interpreters/MergeJoin.h | 2 ++ .../01015_empty_in_inner_right_join.reference | 17 +++++++++++++++++ ...l => 01015_empty_in_inner_right_join.sql.j2} | 6 ++++++ 3 files changed, 25 insertions(+) rename tests/queries/0_stateless/{01015_empty_in_inner_right_join.sql => 01015_empty_in_inner_right_join.sql.j2} (96%) diff --git a/src/Interpreters/MergeJoin.h b/src/Interpreters/MergeJoin.h index 11e5dc86dc2..844c730de4f 100644 --- a/src/Interpreters/MergeJoin.h +++ b/src/Interpreters/MergeJoin.h @@ -32,6 +32,8 @@ public: size_t getTotalRowCount() const override { return right_blocks.row_count; } size_t getTotalByteCount() const override { return right_blocks.bytes; } + /// Has to be called only after setTotals()/mergeRightBlocks() + bool alwaysReturnsEmptySet() const override { return (is_right || is_inner) && min_max_right_blocks.empty(); } BlockInputStreamPtr createStreamWithNonJoinedRows(const Block & result_sample_block, UInt64 max_block_size) const override; diff --git a/tests/queries/0_stateless/01015_empty_in_inner_right_join.reference b/tests/queries/0_stateless/01015_empty_in_inner_right_join.reference index f41483b25da..1aa2f963bd7 100644 --- a/tests/queries/0_stateless/01015_empty_in_inner_right_join.reference +++ b/tests/queries/0_stateless/01015_empty_in_inner_right_join.reference @@ -15,3 +15,20 @@ multiple sets INNER JOIN empty set AND IN non-empty set 0 multiple sets INNER JOIN non-empty set AND IN non-empty set 1 IN empty set equals 0 10 IN empty set sum if 10 +IN empty set 0 +IN non-empty set 1 +NOT IN empty set 10 +INNER JOIN empty set 0 +INNER JOIN non-empty set 1 +RIGHT JOIN empty set 0 +RIGHT JOIN non-empty set 1 +LEFT JOIN empty set 10 +LEFT JOIN non-empty set 10 +multiple sets IN empty set OR IN non-empty set 1 +multiple sets IN empty set OR NOT IN non-empty set 9 +multiple sets NOT IN empty set AND IN non-empty set 1 +multiple sets INNER JOIN empty set AND IN empty set 0 +multiple sets INNER JOIN empty set AND IN non-empty set 0 +multiple sets INNER JOIN non-empty set AND IN non-empty set 1 +IN empty set equals 0 10 +IN empty set sum if 10 diff --git a/tests/queries/0_stateless/01015_empty_in_inner_right_join.sql b/tests/queries/0_stateless/01015_empty_in_inner_right_join.sql.j2 similarity index 96% rename from tests/queries/0_stateless/01015_empty_in_inner_right_join.sql rename to tests/queries/0_stateless/01015_empty_in_inner_right_join.sql.j2 index e0d06a7e3b6..6c13654598e 100644 --- a/tests/queries/0_stateless/01015_empty_in_inner_right_join.sql +++ b/tests/queries/0_stateless/01015_empty_in_inner_right_join.sql.j2 @@ -1,5 +1,9 @@ SET joined_subquery_requires_alias = 0; +{% for join_algorithm in ['partial_merge', 'hash'] -%} + +SET join_algorithm = '{{ join_algorithm }}'; + SELECT 'IN empty set',count() FROM system.numbers WHERE number IN (SELECT toUInt64(1) WHERE 0); SELECT 'IN non-empty set',count() FROM (SELECT number FROM system.numbers LIMIT 10) t1 WHERE t1.number IN (SELECT toUInt64(1) WHERE 1); SELECT 'NOT IN empty set',count() FROM (SELECT number FROM system.numbers LIMIT 10) WHERE number NOT IN (SELECT toUInt64(1) WHERE 0); @@ -22,3 +26,5 @@ SELECT 'multiple sets INNER JOIN non-empty set AND IN non-empty set',count() FRO SELECT 'IN empty set equals 0', count() FROM numbers(10) WHERE (number IN (SELECT toUInt64(1) WHERE 0)) = 0; SELECT 'IN empty set sum if', sum(if(number IN (SELECT toUInt64(1) WHERE 0), 2, 1)) FROM numbers(10); + +{% endfor -%} From 593b7e9bbb0f0492e0e50b228ca39eff355b53fc Mon Sep 17 00:00:00 2001 From: vdimir Date: Thu, 29 Jul 2021 14:50:48 +0300 Subject: [PATCH 43/78] Do not throw exception if unknwon column met while join rewriting --- src/Interpreters/JoinToSubqueryTransformVisitor.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Interpreters/JoinToSubqueryTransformVisitor.cpp b/src/Interpreters/JoinToSubqueryTransformVisitor.cpp index 8595d3a4179..eabdeaefc04 100644 --- a/src/Interpreters/JoinToSubqueryTransformVisitor.cpp +++ b/src/Interpreters/JoinToSubqueryTransformVisitor.cpp @@ -551,8 +551,6 @@ std::vector normalizeColumnNamesExtractNeeded( else needed_columns[*table_pos].no_clashes.emplace(ident->shortName()); } - else if (!got_alias) - throw Exception("Unknown column name '" + ident->name() + "'", ErrorCodes::UNKNOWN_IDENTIFIER); } return needed_columns; From 5d2aec78c3b97794473f20e735ce2c6a6bcab434 Mon Sep 17 00:00:00 2001 From: vdimir Date: Mon, 2 Aug 2021 15:08:14 +0300 Subject: [PATCH 44/78] Add testcase for multiple joins and unknwon identifier in where section --- .../0_stateless/00820_multiple_joins.reference | 1 + tests/queries/0_stateless/00820_multiple_joins.sql | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/tests/queries/0_stateless/00820_multiple_joins.reference b/tests/queries/0_stateless/00820_multiple_joins.reference index 6d317230813..f9d9a664784 100644 --- a/tests/queries/0_stateless/00820_multiple_joins.reference +++ b/tests/queries/0_stateless/00820_multiple_joins.reference @@ -48,3 +48,4 @@ 6 6 60 60 66 66 120 12 12 120 120 132 132 240 18 18 180 180 198 198 360 +1 diff --git a/tests/queries/0_stateless/00820_multiple_joins.sql b/tests/queries/0_stateless/00820_multiple_joins.sql index 890d003a124..df82a199337 100644 --- a/tests/queries/0_stateless/00820_multiple_joins.sql +++ b/tests/queries/0_stateless/00820_multiple_joins.sql @@ -2,6 +2,7 @@ DROP TABLE IF EXISTS table1; DROP TABLE IF EXISTS table2; DROP TABLE IF EXISTS table3; DROP TABLE IF EXISTS table5; +DROP TABLE IF EXISTS table_set; CREATE TABLE table1 (a UInt32) ENGINE = Memory; CREATE TABLE table2 (a UInt32, b UInt32) ENGINE = Memory; @@ -76,6 +77,18 @@ join table3 as t3 on t2_b = t3_b; --join table2 as t2 on t1_t2_x = t2.a --join table3 as t3 on t1_t3_x = t2_t3_x; + +CREATE TABLE table_set ( x UInt32 ) ENGINE = Set; +INSERT INTO table_set VALUES (0), (1), (2); + +select count() +from table1 as t1 +join table2 as t2 on t1.a = t2.a +join table3 as t3 on t2.b = t3.b +join table5 as t5 on t3.c = t5.c +WHERE t1.a in table_set; + +DROP TABLE table_set; DROP TABLE table1; DROP TABLE table2; DROP TABLE table3; From 8577938efef1186290080d256e98eff0664c01df Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Mon, 2 Aug 2021 12:34:50 +0000 Subject: [PATCH 45/78] Throw exception in case SSL is not enabled --- src/Storages/StorageMongoDBSocketFactory.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index 955507fc2bc..d80088865b9 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -1,5 +1,7 @@ #include "StorageMongoDBSocketFactory.h" +#include + #if !defined(ARCADIA_BUILD) # include #endif @@ -15,6 +17,11 @@ namespace DB { +namespace ErrorCodes +{ + extern const int FEATURE_IS_NOT_ENABLED_AT_BUILD_TIME; +} + Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSocket(const std::string & host, int port, Poco::Timespan connectTimeout, bool secure) { return secure ? createSecureSocket(host, port, connectTimeout) : createPlainSocket(host, port, connectTimeout); @@ -41,7 +48,7 @@ Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const st return socket; #else - return createPlainSocket(host, port, connectTimeout); + throw Exception("SSL is not enabled at build time.", ErrorCodes::FEATURE_IS_NOT_ENABLED_AT_BUILD_TIME); #endif } From 91928fdf5a79e7eb4f05a477c1382a77f5ab1c77 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Mon, 2 Aug 2021 13:11:49 +0000 Subject: [PATCH 46/78] Fix --- src/Storages/StorageMongoDBSocketFactory.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index d80088865b9..5f8fcab369f 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -3,6 +3,7 @@ #include #if !defined(ARCADIA_BUILD) +# include "config_core.h" # include #endif @@ -13,6 +14,14 @@ # include #endif +#ifdef __clang__ +# pragma clang diagnostic ignored "-Wunused-parameter" +#endif + +#if USE_EMBEDDED_COMPILER +# pragma GCC diagnostic ignored "-Wunused-parameter" +#endif + namespace DB { From 294695bb7dc3fde2f8343d85e5ea7c0b960da545 Mon Sep 17 00:00:00 2001 From: kssenii Date: Mon, 2 Aug 2021 07:18:51 +0000 Subject: [PATCH 47/78] Review fixes --- programs/library-bridge/HandlerFactory.cpp | 2 +- programs/library-bridge/Handlers.cpp | 24 ++++++++------ programs/library-bridge/Handlers.h | 4 +-- .../SharedLibraryHandlerFactory.cpp | 6 ---- src/Bridge/IBridgeHelper.cpp | 8 ++--- src/Bridge/IBridgeHelper.h | 7 +++-- src/Bridge/LibraryBridgeHelper.cpp | 31 ++++++++++--------- src/Bridge/LibraryBridgeHelper.h | 6 ++-- src/Bridge/XDBCBridgeHelper.h | 2 +- src/Dictionaries/LibraryDictionarySource.cpp | 13 +++----- src/Dictionaries/LibraryDictionarySource.h | 2 +- .../configs/config.d/config.xml | 4 +++ 12 files changed, 56 insertions(+), 53 deletions(-) diff --git a/programs/library-bridge/HandlerFactory.cpp b/programs/library-bridge/HandlerFactory.cpp index 3e31f298fca..43087082c46 100644 --- a/programs/library-bridge/HandlerFactory.cpp +++ b/programs/library-bridge/HandlerFactory.cpp @@ -13,7 +13,7 @@ namespace DB LOG_DEBUG(log, "Request URI: {}", uri.toString()); if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_GET) - return std::make_unique(keep_alive_timeout, getContext()); + return std::make_unique(keep_alive_timeout, getContext()); if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST) return std::make_unique(keep_alive_timeout, getContext()); diff --git a/programs/library-bridge/Handlers.cpp b/programs/library-bridge/Handlers.cpp index 0b49a801ca3..2b6d0057bb2 100644 --- a/programs/library-bridge/Handlers.cpp +++ b/programs/library-bridge/Handlers.cpp @@ -17,6 +17,12 @@ namespace DB { + +namespace ErrorCodes +{ + extern const int BAD_REQUEST_PARAMETER; +} + namespace { void processError(HTTPServerResponse & response, const std::string & message) @@ -190,7 +196,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + throw Exception(ErrorCodes::BAD_REQUEST_PARAMETER, "Not found dictionary with id: {}", dictionary_id); bool res = library_handler->isModified(); writeStringBinary(std::to_string(res), out); @@ -199,7 +205,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + throw Exception(ErrorCodes::BAD_REQUEST_PARAMETER, "Not found dictionary with id: {}", dictionary_id); bool res = library_handler->supportsSelectiveLoad(); writeStringBinary(std::to_string(res), out); @@ -208,7 +214,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe { auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + throw Exception(ErrorCodes::BAD_REQUEST_PARAMETER, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); LOG_DEBUG(log, "Calling loadAll() for dictionary id: {}", dictionary_id); @@ -226,7 +232,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + throw Exception(ErrorCodes::BAD_REQUEST_PARAMETER, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); LOG_DEBUG(log, "Calling loadIds() for dictionary id: {}", dictionary_id); @@ -265,7 +271,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); if (!library_handler) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Not found dictionary with id: {}", dictionary_id); + throw Exception(ErrorCodes::BAD_REQUEST_PARAMETER, "Not found dictionary with id: {}", dictionary_id); const auto & sample_block = library_handler->getSampleBlock(); LOG_DEBUG(log, "Calling loadKeys() for dictionary id: {}", dictionary_id); @@ -304,7 +310,7 @@ void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServe } -void PingHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) +void LibraryExistsHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { try { @@ -313,7 +319,7 @@ void PingHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse if (!params.has("dictionary_id")) { - processError(response, "No 'dictionary_id in request URL"); + processError(response, "No 'dictionary_id' in request URL"); return; } @@ -321,9 +327,9 @@ void PingHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse auto library_handler = SharedLibraryHandlerFactory::instance().get(dictionary_id); String res; if (library_handler) - res = "dictionary=1"; + res = "1"; else - res = "dictionary=0"; + res = "0"; setResponseDefaultHeaders(response, keep_alive_timeout); LOG_TRACE(log, "Senging ping response: {} (dictionary id: {})", res, dictionary_id); diff --git a/programs/library-bridge/Handlers.h b/programs/library-bridge/Handlers.h index a8e4ba0dd2a..58af24b06d1 100644 --- a/programs/library-bridge/Handlers.h +++ b/programs/library-bridge/Handlers.h @@ -39,10 +39,10 @@ private: }; -class PingHandler : public HTTPRequestHandler, WithContext +class LibraryExistsHandler : public HTTPRequestHandler, WithContext { public: - explicit PingHandler(size_t keep_alive_timeout_, ContextPtr context_) + explicit LibraryExistsHandler(size_t keep_alive_timeout_, ContextPtr context_) : WithContext(context_) , keep_alive_timeout(keep_alive_timeout_) , log(&Poco::Logger::get("LibraryRequestHandler")) diff --git a/programs/library-bridge/SharedLibraryHandlerFactory.cpp b/programs/library-bridge/SharedLibraryHandlerFactory.cpp index 900981191c8..a9358ca552a 100644 --- a/programs/library-bridge/SharedLibraryHandlerFactory.cpp +++ b/programs/library-bridge/SharedLibraryHandlerFactory.cpp @@ -4,12 +4,6 @@ namespace DB { -namespace ErrorCodes -{ - extern const int BAD_ARGUMENTS; - extern const int LOGICAL_ERROR; -} - SharedLibraryHandlerPtr SharedLibraryHandlerFactory::get(const std::string & dictionary_id) { std::lock_guard lock(mutex); diff --git a/src/Bridge/IBridgeHelper.cpp b/src/Bridge/IBridgeHelper.cpp index a90688cc8b5..5c884a2ca3d 100644 --- a/src/Bridge/IBridgeHelper.cpp +++ b/src/Bridge/IBridgeHelper.cpp @@ -33,9 +33,9 @@ Poco::URI IBridgeHelper::getPingURI() const } -void IBridgeHelper::startBridgeSync() const +void IBridgeHelper::startBridgeSync() { - if (!checkBridgeIsRunning()) + if (!bridgeHandShake()) { LOG_TRACE(getLog(), "{} is not running, will try to start it", serviceAlias()); startBridge(startBridgeCommand()); @@ -49,7 +49,7 @@ void IBridgeHelper::startBridgeSync() const ++counter; LOG_TRACE(getLog(), "Checking {} is running, try {}", serviceAlias(), counter); - if (checkBridgeIsRunning()) + if (bridgeHandShake()) { started = true; break; @@ -66,7 +66,7 @@ void IBridgeHelper::startBridgeSync() const } -std::unique_ptr IBridgeHelper::startBridgeCommand() const +std::unique_ptr IBridgeHelper::startBridgeCommand() { if (startBridgeManually()) throw Exception(serviceAlias() + " is not running. Please, start it manually", ErrorCodes::EXTERNAL_SERVER_IS_NOT_RESPONDING); diff --git a/src/Bridge/IBridgeHelper.h b/src/Bridge/IBridgeHelper.h index 6a9c19da68f..0537658663d 100644 --- a/src/Bridge/IBridgeHelper.h +++ b/src/Bridge/IBridgeHelper.h @@ -35,10 +35,11 @@ public: Poco::URI getPingURI() const; - void startBridgeSync() const; + void startBridgeSync(); protected: - virtual bool checkBridgeIsRunning() const = 0; + /// Check bridge is running. Can also check something else in the mean time. + virtual bool bridgeHandShake() = 0; /// clickhouse-odbc-bridge, clickhouse-library-bridge virtual String serviceAlias() const = 0; @@ -63,7 +64,7 @@ protected: private: - std::unique_ptr startBridgeCommand() const; + std::unique_ptr startBridgeCommand(); }; } diff --git a/src/Bridge/LibraryBridgeHelper.cpp b/src/Bridge/LibraryBridgeHelper.cpp index b39e3f01903..9341ddda922 100644 --- a/src/Bridge/LibraryBridgeHelper.cpp +++ b/src/Bridge/LibraryBridgeHelper.cpp @@ -70,7 +70,7 @@ void LibraryBridgeHelper::startBridge(std::unique_ptr cmd) const } -bool LibraryBridgeHelper::checkBridgeIsRunning() const +bool LibraryBridgeHelper::bridgeHandShake() { String result; try @@ -89,13 +89,11 @@ bool LibraryBridgeHelper::checkBridgeIsRunning() const * 1. It is dictionary source creation and initialization of library handler on bridge side did not happen yet. * 2. Bridge crashed or restarted for some reason while server did not. **/ - static constexpr auto dictionary_check = "dictionary="; - if (result.size() != (std::strlen(dictionary_check) + 1)) - throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {}. Check bridge and server have the same version.", - result, std::strlen(dictionary_check)); + if (result.size() != 1) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {}. Check bridge and server have the same version.", result); UInt8 dictionary_id_exists; - auto parsed = tryParse(dictionary_id_exists, result.substr(std::strlen(dictionary_check))); + auto parsed = tryParse(dictionary_id_exists, result); if (!parsed || (dictionary_id_exists != 0 && dictionary_id_exists != 1)) throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected message from library bridge: {} ({}). Check bridge and server have the same version.", result, parsed ? toString(dictionary_id_exists) : "failed to parse"); @@ -106,13 +104,16 @@ bool LibraryBridgeHelper::checkBridgeIsRunning() const if (dictionary_id_exists && !library_initialized) throw Exception(ErrorCodes::LOGICAL_ERROR, "Library was not initialized, but bridge responded to already have dictionary id: {}", dictionary_id); + /// Here we want to say bridge to recreate a new library handler for current dictionary, + /// because it responded to have lost it, but we know that it has already been created. (It is a direct result of bridge crash). if (!dictionary_id_exists && library_initialized) { LOG_WARNING(log, "Library bridge does not have library handler with dictionaty id: {}. It will be reinitialized.", dictionary_id); bool reinitialized = false; try { - reinitialized = initLibrary(false); + auto uri = createRequestURI(LIB_NEW_METHOD); + reinitialized = executeRequest(uri, getInitLibraryCallback()); } catch (...) { @@ -148,13 +149,12 @@ ReadWriteBufferFromHTTP::OutStreamCallback LibraryBridgeHelper::getInitLibraryCa } -bool LibraryBridgeHelper::initLibrary(bool check_bridge) const +bool LibraryBridgeHelper::initLibrary() { - /// Do not check if we call initLibrary from checkBridgeSync. - if (check_bridge) - startBridgeSync(); + startBridgeSync(); auto uri = createRequestURI(LIB_NEW_METHOD); - return executeRequest(uri, getInitLibraryCallback()); + library_initialized = executeRequest(uri, getInitLibraryCallback()); + return library_initialized; } @@ -163,7 +163,10 @@ bool LibraryBridgeHelper::cloneLibrary(const Field & other_dictionary_id) startBridgeSync(); auto uri = createRequestURI(LIB_CLONE_METHOD); uri.addQueryParameter("from_dictionary_id", toString(other_dictionary_id)); - return executeRequest(uri, getInitLibraryCallback()); + /// We also pass initialization settings in order to create a library handler + /// in case from_dictionary_id does not exist in bridge side (possible in case of bridge crash). + library_initialized = executeRequest(uri, getInitLibraryCallback()); + return library_initialized; } @@ -171,7 +174,7 @@ bool LibraryBridgeHelper::removeLibrary() { /// Do not force bridge restart if it is not running in case of removeLibrary /// because in this case after restart it will not have this dictionaty id in memory anyway. - if (checkBridgeIsRunning()) + if (bridgeHandShake()) { auto uri = createRequestURI(LIB_DELETE_METHOD); return executeRequest(uri); diff --git a/src/Bridge/LibraryBridgeHelper.h b/src/Bridge/LibraryBridgeHelper.h index e4fe8e06965..6615162efe7 100644 --- a/src/Bridge/LibraryBridgeHelper.h +++ b/src/Bridge/LibraryBridgeHelper.h @@ -26,7 +26,7 @@ public: LibraryBridgeHelper(ContextPtr context_, const Block & sample_block, const Field & dictionary_id_, const LibraryInitData & library_data_); - bool initLibrary(bool check_bridge = true) const; + bool initLibrary(); bool cloneLibrary(const Field & other_dictionary_id); @@ -48,10 +48,8 @@ public: LibraryInitData getLibraryData() const { return library_data; } - void setInitialized() { library_initialized = true; } - protected: - bool checkBridgeIsRunning() const override; + bool bridgeHandShake() override; void startBridge(std::unique_ptr cmd) const override; diff --git a/src/Bridge/XDBCBridgeHelper.h b/src/Bridge/XDBCBridgeHelper.h index 13f0f528c8f..b3613c381d0 100644 --- a/src/Bridge/XDBCBridgeHelper.h +++ b/src/Bridge/XDBCBridgeHelper.h @@ -74,7 +74,7 @@ public: } protected: - bool checkBridgeIsRunning() const override + bool bridgeHandShake() override { try { diff --git a/src/Dictionaries/LibraryDictionarySource.cpp b/src/Dictionaries/LibraryDictionarySource.cpp index 273f9d3c3b4..4747d3f1216 100644 --- a/src/Dictionaries/LibraryDictionarySource.cpp +++ b/src/Dictionaries/LibraryDictionarySource.cpp @@ -41,6 +41,9 @@ LibraryDictionarySource::LibraryDictionarySource( , sample_block{sample_block_} , context(Context::createCopy(context_)) { + if (fs::path(path).is_relative()) + path = fs::canonical(path); + if (created_from_ddl && !pathStartsWith(path, context->getDictionariesLibPath())) throw Exception(ErrorCodes::PATH_ACCESS_DENIED, "File path {} is not inside {}", path, context->getDictionariesLibPath()); @@ -58,10 +61,7 @@ LibraryDictionarySource::LibraryDictionarySource( bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, library_data); - bool initialized = bridge_helper->initLibrary(); - if (initialized) - bridge_helper->setInitialized(); - else + if (!bridge_helper->initLibrary()) throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to create shared library from path: {}", path); } @@ -91,10 +91,7 @@ LibraryDictionarySource::LibraryDictionarySource(const LibraryDictionarySource & , description{other.description} { bridge_helper = std::make_shared(context, description.sample_block, dictionary_id, other.bridge_helper->getLibraryData()); - bool cloned = bridge_helper->cloneLibrary(other.dictionary_id); - if (cloned) - bridge_helper->setInitialized(); - else + if (!bridge_helper->cloneLibrary(other.dictionary_id)) throw Exception(ErrorCodes::EXTERNAL_LIBRARY_ERROR, "Failed to clone library"); } diff --git a/src/Dictionaries/LibraryDictionarySource.h b/src/Dictionaries/LibraryDictionarySource.h index 18a2d1b97b2..b5cf4f68b05 100644 --- a/src/Dictionaries/LibraryDictionarySource.h +++ b/src/Dictionaries/LibraryDictionarySource.h @@ -80,7 +80,7 @@ private: const DictionaryStructure dict_struct; const std::string config_prefix; - const std::string path; + std::string path; const Field dictionary_id; Block sample_block; diff --git a/tests/integration/test_library_bridge/configs/config.d/config.xml b/tests/integration/test_library_bridge/configs/config.d/config.xml index 9bea75fbb6f..7811c1e26b7 100644 --- a/tests/integration/test_library_bridge/configs/config.d/config.xml +++ b/tests/integration/test_library_bridge/configs/config.d/config.xml @@ -8,5 +8,9 @@ 10 /var/log/clickhouse-server/stderr.log /var/log/clickhouse-server/stdout.log + + /var/log/clickhouse-server/clickhouse-library-bridge.log + /var/log/clickhouse-server/clickhouse-library-bridge.err.log + trace From 7a2e7117656afc68ab6c6ae8ef41b0a46644d94a Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Mon, 2 Aug 2021 17:48:53 +0300 Subject: [PATCH 48/78] Try update AMQP-CPP --- contrib/AMQP-CPP | 2 +- contrib/amqpcpp-cmake/CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/AMQP-CPP b/contrib/AMQP-CPP index 03781aaff0f..dda64f5debc 160000 --- a/contrib/AMQP-CPP +++ b/contrib/AMQP-CPP @@ -1 +1 @@ -Subproject commit 03781aaff0f10ef41f902b8cf865fe0067180c10 +Subproject commit dda64f5debceb15ad7e521af3e7d78e5d929b3d6 diff --git a/contrib/amqpcpp-cmake/CMakeLists.txt b/contrib/amqpcpp-cmake/CMakeLists.txt index 4e8342af125..05b2c3849a2 100644 --- a/contrib/amqpcpp-cmake/CMakeLists.txt +++ b/contrib/amqpcpp-cmake/CMakeLists.txt @@ -10,7 +10,7 @@ set (SRCS "${LIBRARY_DIR}/src/deferredconsumer.cpp" "${LIBRARY_DIR}/src/deferredextreceiver.cpp" "${LIBRARY_DIR}/src/deferredget.cpp" - "${LIBRARY_DIR}/src/deferredpublisher.cpp" + "${LIBRARY_DIR}/src/deferredrecall.cpp" "${LIBRARY_DIR}/src/deferredreceiver.cpp" "${LIBRARY_DIR}/src/field.cpp" "${LIBRARY_DIR}/src/flags.cpp" From 71e1c82f8772e6d1878739f7ad8c356ad909f2b8 Mon Sep 17 00:00:00 2001 From: OmarBazaraa Date: Mon, 2 Aug 2021 15:16:39 +0000 Subject: [PATCH 49/78] Refactor --- src/Storages/StorageMongoDBSocketFactory.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/Storages/StorageMongoDBSocketFactory.cpp b/src/Storages/StorageMongoDBSocketFactory.cpp index 5f8fcab369f..9a2b120e9ed 100644 --- a/src/Storages/StorageMongoDBSocketFactory.cpp +++ b/src/Storages/StorageMongoDBSocketFactory.cpp @@ -3,7 +3,6 @@ #include #if !defined(ARCADIA_BUILD) -# include "config_core.h" # include #endif @@ -14,14 +13,6 @@ # include #endif -#ifdef __clang__ -# pragma clang diagnostic ignored "-Wunused-parameter" -#endif - -#if USE_EMBEDDED_COMPILER -# pragma GCC diagnostic ignored "-Wunused-parameter" -#endif - namespace DB { @@ -47,7 +38,7 @@ Poco::Net::StreamSocket StorageMongoDBSocketFactory::createPlainSocket(const std } -Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host, int port, Poco::Timespan connectTimeout) +Poco::Net::StreamSocket StorageMongoDBSocketFactory::createSecureSocket(const std::string & host [[maybe_unused]], int port [[maybe_unused]], Poco::Timespan connectTimeout [[maybe_unused]]) { #if USE_SSL Poco::Net::SocketAddress address(host, port); From c645dc9d7d7314429b1f895600a3a0da619af719 Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Mon, 2 Aug 2021 19:23:13 +0300 Subject: [PATCH 50/78] fix --- contrib/amqpcpp-cmake/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/amqpcpp-cmake/CMakeLists.txt b/contrib/amqpcpp-cmake/CMakeLists.txt index 05b2c3849a2..5637db4cf41 100644 --- a/contrib/amqpcpp-cmake/CMakeLists.txt +++ b/contrib/amqpcpp-cmake/CMakeLists.txt @@ -15,6 +15,7 @@ set (SRCS "${LIBRARY_DIR}/src/field.cpp" "${LIBRARY_DIR}/src/flags.cpp" "${LIBRARY_DIR}/src/linux_tcp/openssl.cpp" + "${LIBRARY_DIR}/src/linux_tcp/sslerrorprinter.cpp" "${LIBRARY_DIR}/src/linux_tcp/tcpconnection.cpp" "${LIBRARY_DIR}/src/inbuffer.cpp" "${LIBRARY_DIR}/src/receivedframe.cpp" From 31d027b8d0790a04938f2b6a6de1fbb47b8b6331 Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Mon, 2 Aug 2021 19:33:15 +0300 Subject: [PATCH 51/78] Try update arrow --- contrib/arrow | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/arrow b/contrib/arrow index debf751a129..078e21bad34 160000 --- a/contrib/arrow +++ b/contrib/arrow @@ -1 +1 @@ -Subproject commit debf751a129bdda9ff4d1e895e08957ff77000a1 +Subproject commit 078e21bad344747b7656ef2d7a4f7410a0a303eb From 36c6d7e1c03c9700fa8f879e530a19957cb2272f Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Mon, 2 Aug 2021 20:13:35 +0300 Subject: [PATCH 52/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: Alexey Boykov <33257111+mathalex@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index 66dac8620fe..4c1c7e20b73 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -280,7 +280,7 @@ To fill multiple columns, add `WITH FILL` modifier with optional parameters afte ORDER BY expr [WITH FILL] [FROM const_expr] [TO const_expr] [STEP const_numeric_expr], ... exprN [WITH FILL] [FROM expr] [TO expr] [STEP numeric_expr] ``` -`WITH FILL` can be applied for fields with Numeric (all kind of float, decimal, int) or Date/DateTime types. When applied for `String` fields, missed values are filled with empty strings. +`WITH FILL` can be applied for fields with Numeric (all kinds of float, decimal, int) or Date/DateTime types. When applied for `String` fields, missed values are filled with empty strings. When `FROM const_expr` not defined sequence of filling use minimal `expr` field value from `ORDER BY`. When `TO const_expr` not defined sequence of filling use maximum `expr` field value from `ORDER BY`. When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. From 1a4d9fec9bb529fa1581c2e6a7167817f21d4aff Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Mon, 2 Aug 2021 20:13:48 +0300 Subject: [PATCH 53/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: Alexey Boykov <33257111+mathalex@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index 4c1c7e20b73..e6d4b8dec55 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -286,7 +286,7 @@ When `TO const_expr` not defined sequence of filling use maximum `expr` field va When `STEP const_numeric_expr` defined then `const_numeric_expr` interprets `as is` for numeric types as `days` for Date type and as `seconds` for DateTime type. When `STEP const_numeric_expr` omitted then sequence of filling use `1.0` for numeric type, `1 day` for Date type and `1 second` for DateTime type. -Example of query without `WITH FILL`: +Example of a query without `WITH FILL`: ``` sql SELECT n, source FROM ( From c5df30594df27d6f4de572cf04b27cf2e258e1aa Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Mon, 2 Aug 2021 20:13:58 +0300 Subject: [PATCH 54/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: Alexey Boykov <33257111+mathalex@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index e6d4b8dec55..bc22356cb5d 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -334,7 +334,7 @@ Result: └─────┴──────────┘ ``` -For the case with multiple fields `ORDER BY field2 WITH FILL, field1 WITH FILL` order of filling will follow the order of fields in `ORDER BY` clause. +For the case with multiple fields `ORDER BY field2 WITH FILL, field1 WITH FILL` order of filling will follow the order of fields in the `ORDER BY` clause. Example: From e03539fdd4cccc55719151a6d4e68f33b240e11d Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Mon, 2 Aug 2021 20:14:09 +0300 Subject: [PATCH 55/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: Alexey Boykov <33257111+mathalex@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index bc22356cb5d..23a108bcf81 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -364,7 +364,7 @@ Result: └────────────┴────────────┴──────────┘ ``` -Field `d1` does not fill in and use the default value cause we do not have repeated values for `d2` value, and sequence for `d1` can’t be properly calculated. +Field `d1` does not fill in and use the default value cause we do not have repeated values for `d2` value, and the sequence for `d1` can’t be properly calculated. The following query with a changed field in `ORDER BY`: From d10835f22360a6c835546bc74a5236b0dc4da331 Mon Sep 17 00:00:00 2001 From: michon470 <71978106+michon470@users.noreply.github.com> Date: Mon, 2 Aug 2021 20:14:22 +0300 Subject: [PATCH 56/78] Update docs/en/sql-reference/statements/select/order-by.md Co-authored-by: Alexey Boykov <33257111+mathalex@users.noreply.github.com> --- docs/en/sql-reference/statements/select/order-by.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/sql-reference/statements/select/order-by.md b/docs/en/sql-reference/statements/select/order-by.md index 23a108bcf81..156f68935b5 100644 --- a/docs/en/sql-reference/statements/select/order-by.md +++ b/docs/en/sql-reference/statements/select/order-by.md @@ -366,7 +366,7 @@ Result: Field `d1` does not fill in and use the default value cause we do not have repeated values for `d2` value, and the sequence for `d1` can’t be properly calculated. -The following query with a changed field in `ORDER BY`: +The following query with the changed field in `ORDER BY`: ``` sql SELECT From 40a3c8281a9c8b8a7e1d18fb2fc1a55dd4144502 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Sat, 31 Jul 2021 16:58:54 +0300 Subject: [PATCH 57/78] Fix synchronization in GRPCServer --- src/Server/GRPCServer.cpp | 70 +++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/src/Server/GRPCServer.cpp b/src/Server/GRPCServer.cpp index ac59f82d419..b90b0c33f17 100644 --- a/src/Server/GRPCServer.cpp +++ b/src/Server/GRPCServer.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -480,6 +481,40 @@ namespace }; + /// A boolean state protected by mutex able to wait until other thread sets it to a specific value. + class BoolState + { + public: + explicit BoolState(bool initial_value) : value(initial_value) {} + + bool get() const + { + std::lock_guard lock{mutex}; + return value; + } + + void set(bool new_value) + { + std::lock_guard lock{mutex}; + if (value == new_value) + return; + value = new_value; + changed.notify_all(); + } + + void wait(bool wanted_value) const + { + std::unique_lock lock{mutex}; + changed.wait(lock, [this, wanted_value]() { return value == wanted_value; }); + } + + private: + bool value; + mutable std::mutex mutex; + mutable std::condition_variable changed; + }; + + /// Handles a connection after a responder is started (i.e. after getting a new call). class Call { @@ -564,18 +599,15 @@ namespace UInt64 waited_for_client_writing = 0; /// The following fields are accessed both from call_thread and queue_thread. - std::atomic reading_query_info = false; + BoolState reading_query_info{false}; std::atomic failed_to_read_query_info = false; GRPCQueryInfo next_query_info_while_reading; std::atomic want_to_cancel = false; std::atomic check_query_info_contains_cancel_only = false; - std::atomic sending_result = false; + BoolState sending_result{false}; std::atomic failed_to_send_result = false; ThreadFromGlobalPool call_thread; - std::condition_variable read_finished; - std::condition_variable write_finished; - std::mutex dummy_mutex; /// Doesn't protect anything. }; Call::Call(CallType call_type_, std::unique_ptr responder_, IServer & iserver_, Poco::Logger * log_) @@ -610,6 +642,7 @@ namespace { try { + setThreadName("GRPCServerCall"); receiveQuery(); executeQuery(); processInput(); @@ -1230,8 +1263,7 @@ namespace { auto start_reading = [&] { - assert(!reading_query_info); - reading_query_info = true; + reading_query_info.set(true); responder->read(next_query_info_while_reading, [this](bool ok) { /// Called on queue_thread. @@ -1256,18 +1288,16 @@ namespace /// on queue_thread. failed_to_read_query_info = true; } - reading_query_info = false; - read_finished.notify_one(); + reading_query_info.set(false); }); }; auto finish_reading = [&] { - if (reading_query_info) + if (reading_query_info.get()) { Stopwatch client_writing_watch; - std::unique_lock lock{dummy_mutex}; - read_finished.wait(lock, [this] { return !reading_query_info; }); + reading_query_info.wait(false); waited_for_client_writing += client_writing_watch.elapsedNanoseconds(); } throwIfFailedToReadQueryInfo(); @@ -1430,11 +1460,10 @@ namespace /// Wait for previous write to finish. /// (gRPC doesn't allow to start sending another result while the previous is still being sending.) - if (sending_result) + if (sending_result.get()) { Stopwatch client_reading_watch; - std::unique_lock lock{dummy_mutex}; - write_finished.wait(lock, [this] { return !sending_result; }); + sending_result.wait(false); waited_for_client_reading += client_reading_watch.elapsedNanoseconds(); } throwIfFailedToSendResult(); @@ -1445,14 +1474,13 @@ namespace if (write_buffer) write_buffer->finalize(); - sending_result = true; + sending_result.set(true); auto callback = [this](bool ok) { /// Called on queue_thread. if (!ok) failed_to_send_result = true; - sending_result = false; - write_finished.notify_one(); + sending_result.set(false); }; Stopwatch client_reading_final_watch; @@ -1472,8 +1500,7 @@ namespace if (send_final_message) { /// Wait until the result is actually sent. - std::unique_lock lock{dummy_mutex}; - write_finished.wait(lock, [this] { return !sending_result; }); + sending_result.wait(false); waited_for_client_reading += client_reading_final_watch.elapsedNanoseconds(); throwIfFailedToSendResult(); LOG_TRACE(log, "Final result has been sent to the client"); @@ -1584,7 +1611,7 @@ private: { /// Called on call_thread. That's why we can't destroy the `call` right now /// (thread can't join to itself). Thus here we only move the `call` from - /// `current_call` to `finished_calls` and run() will actually destroy the `call`. + /// `current_calls` to `finished_calls` and run() will actually destroy the `call`. std::lock_guard lock{mutex}; auto it = current_calls.find(call); finished_calls.push_back(std::move(it->second)); @@ -1593,6 +1620,7 @@ private: void run() { + setThreadName("GRPCServerQueue"); while (true) { { From b541b5eca35b12b7419ffd27c6494cd4d473961d Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Mon, 2 Aug 2021 21:51:33 +0800 Subject: [PATCH 58/78] Normalize hostname in CI --- tests/clickhouse-test | 4 ++++ tests/queries/0_stateless/02001_hostname_test.reference | 2 ++ tests/queries/0_stateless/02001_hostname_test.sql | 2 ++ 3 files changed, 8 insertions(+) create mode 100644 tests/queries/0_stateless/02001_hostname_test.reference create mode 100644 tests/queries/0_stateless/02001_hostname_test.sql diff --git a/tests/clickhouse-test b/tests/clickhouse-test index 362d1d225d9..d83b3f08c42 100755 --- a/tests/clickhouse-test +++ b/tests/clickhouse-test @@ -27,6 +27,7 @@ except ImportError: import random import string import multiprocessing +import socket from contextlib import closing USE_JINJA = True @@ -267,6 +268,9 @@ def run_single_test(args, ext, server_logs_level, client_options, case_file, std os.system("LC_ALL=C sed -i -e 's|/auto_{{shard}}||g' {file}".format(file=stdout_file)) os.system("LC_ALL=C sed -i -e 's|auto_{{replica}}||g' {file}".format(file=stdout_file)) + # Normalize hostname in stdout file. + os.system("LC_ALL=C sed -i -e 's/{hostname}/localhost/g' {file}".format(hostname=socket.gethostname(), file=stdout_file)) + stdout = open(stdout_file, 'rb').read() if os.path.exists(stdout_file) else b'' stdout = str(stdout, errors='replace', encoding='utf-8') stderr = open(stderr_file, 'rb').read() if os.path.exists(stderr_file) else b'' diff --git a/tests/queries/0_stateless/02001_hostname_test.reference b/tests/queries/0_stateless/02001_hostname_test.reference new file mode 100644 index 00000000000..da8a2d07eab --- /dev/null +++ b/tests/queries/0_stateless/02001_hostname_test.reference @@ -0,0 +1,2 @@ +localhost +localhost 2 diff --git a/tests/queries/0_stateless/02001_hostname_test.sql b/tests/queries/0_stateless/02001_hostname_test.sql new file mode 100644 index 00000000000..a8c7a8dab0c --- /dev/null +++ b/tests/queries/0_stateless/02001_hostname_test.sql @@ -0,0 +1,2 @@ +select hostname(); +select hostName() h, count() from cluster(test_cluster_two_shards, system.one) group by h; From d6844facaf63618894ef0429047976c6d1bc6ff1 Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Tue, 3 Aug 2021 11:01:01 +0300 Subject: [PATCH 59/78] one more fix --- contrib/AMQP-CPP | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/AMQP-CPP b/contrib/AMQP-CPP index dda64f5debc..8d83306aa0e 160000 --- a/contrib/AMQP-CPP +++ b/contrib/AMQP-CPP @@ -1 +1 @@ -Subproject commit dda64f5debceb15ad7e521af3e7d78e5d929b3d6 +Subproject commit 8d83306aa0ef33704ee951f789163ca90ab74402 From 7ad4a420035ac171f919b1107118d5cf59c502bc Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Tue, 3 Aug 2021 11:27:23 +0300 Subject: [PATCH 60/78] upadte CMakeLists --- contrib/arrow-cmake/CMakeLists.txt | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/contrib/arrow-cmake/CMakeLists.txt b/contrib/arrow-cmake/CMakeLists.txt index 2237be9913a..2c72055a3e7 100644 --- a/contrib/arrow-cmake/CMakeLists.txt +++ b/contrib/arrow-cmake/CMakeLists.txt @@ -194,9 +194,18 @@ set(ARROW_SRCS "${LIBRARY_DIR}/compute/cast.cc" "${LIBRARY_DIR}/compute/exec.cc" "${LIBRARY_DIR}/compute/function.cc" + "${LIBRARY_DIR}/compute/function_internal.cc" "${LIBRARY_DIR}/compute/kernel.cc" "${LIBRARY_DIR}/compute/registry.cc" + "${LIBRARY_DIR}/compute/exec/exec_plan.cc" + "${LIBRARY_DIR}/compute/exec/expression.cc" + "${LIBRARY_DIR}/compute/exec/key_compare.cc" + "${LIBRARY_DIR}/compute/exec/key_encode.cc" + "${LIBRARY_DIR}/compute/exec/key_hash.cc" + "${LIBRARY_DIR}/compute/exec/key_map.cc" + "${LIBRARY_DIR}/compute/exec/util.cc" + "${LIBRARY_DIR}/compute/kernels/aggregate_basic.cc" "${LIBRARY_DIR}/compute/kernels/aggregate_mode.cc" "${LIBRARY_DIR}/compute/kernels/aggregate_quantile.cc" @@ -207,6 +216,7 @@ set(ARROW_SRCS "${LIBRARY_DIR}/compute/kernels/scalar_arithmetic.cc" "${LIBRARY_DIR}/compute/kernels/scalar_boolean.cc" "${LIBRARY_DIR}/compute/kernels/scalar_cast_boolean.cc" + "${LIBRARY_DIR}/compute/kernels/scalar_cast_dictionary.cc" "${LIBRARY_DIR}/compute/kernels/scalar_cast_internal.cc" "${LIBRARY_DIR}/compute/kernels/scalar_cast_nested.cc" "${LIBRARY_DIR}/compute/kernels/scalar_cast_numeric.cc" @@ -214,15 +224,18 @@ set(ARROW_SRCS "${LIBRARY_DIR}/compute/kernels/scalar_cast_temporal.cc" "${LIBRARY_DIR}/compute/kernels/scalar_compare.cc" "${LIBRARY_DIR}/compute/kernels/scalar_fill_null.cc" + "${LIBRARY_DIR}/compute/kernels/scalar_if_else.cc" "${LIBRARY_DIR}/compute/kernels/scalar_nested.cc" "${LIBRARY_DIR}/compute/kernels/scalar_set_lookup.cc" "${LIBRARY_DIR}/compute/kernels/scalar_string.cc" + "${LIBRARY_DIR}/compute/kernels/scalar_temporal.cc" "${LIBRARY_DIR}/compute/kernels/scalar_validity.cc" + "${LIBRARY_DIR}/compute/kernels/util_internal.cc" "${LIBRARY_DIR}/compute/kernels/vector_hash.cc" "${LIBRARY_DIR}/compute/kernels/vector_nested.cc" + "${LIBRARY_DIR}/compute/kernels/vector_replace.cc" "${LIBRARY_DIR}/compute/kernels/vector_selection.cc" "${LIBRARY_DIR}/compute/kernels/vector_sort.cc" - "${LIBRARY_DIR}/compute/kernels/util_internal.cc" "${LIBRARY_DIR}/csv/chunker.cc" "${LIBRARY_DIR}/csv/column_builder.cc" @@ -231,6 +244,7 @@ set(ARROW_SRCS "${LIBRARY_DIR}/csv/options.cc" "${LIBRARY_DIR}/csv/parser.cc" "${LIBRARY_DIR}/csv/reader.cc" + "${LIBRARY_DIR}/csv/writer.cc" "${LIBRARY_DIR}/ipc/dictionary.cc" "${LIBRARY_DIR}/ipc/feather.cc" @@ -247,6 +261,7 @@ set(ARROW_SRCS "${LIBRARY_DIR}/io/interfaces.cc" "${LIBRARY_DIR}/io/memory.cc" "${LIBRARY_DIR}/io/slow.cc" + "${LIBRARY_DIR}/io/stdio.cc" "${LIBRARY_DIR}/io/transform.cc" "${LIBRARY_DIR}/tensor/coo_converter.cc" @@ -257,9 +272,9 @@ set(ARROW_SRCS "${LIBRARY_DIR}/util/bit_block_counter.cc" "${LIBRARY_DIR}/util/bit_run_reader.cc" "${LIBRARY_DIR}/util/bit_util.cc" - "${LIBRARY_DIR}/util/bitmap.cc" "${LIBRARY_DIR}/util/bitmap_builders.cc" "${LIBRARY_DIR}/util/bitmap_ops.cc" + "${LIBRARY_DIR}/util/bitmap.cc" "${LIBRARY_DIR}/util/bpacking.cc" "${LIBRARY_DIR}/util/cancel.cc" "${LIBRARY_DIR}/util/compression.cc" From 4f7e007d355bc76625af927f39e82e37082e4679 Mon Sep 17 00:00:00 2001 From: Amos Bird Date: Tue, 3 Aug 2021 14:23:46 +0800 Subject: [PATCH 61/78] Specialize date time comparision. --- src/Functions/FunctionsComparison.h | 37 +++++++++++++++++------ tests/performance/datetime_comparison.xml | 1 + 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Functions/FunctionsComparison.h b/src/Functions/FunctionsComparison.h index 389b150e381..b9c7c211b74 100644 --- a/src/Functions/FunctionsComparison.h +++ b/src/Functions/FunctionsComparison.h @@ -1218,17 +1218,36 @@ public: { return res; } - else if ((isColumnedAsDecimal(left_type) || isColumnedAsDecimal(right_type)) - // Comparing Date and DateTime64 requires implicit conversion, - // otherwise Date is treated as number. - && !(date_and_datetime && (isDate(left_type) || isDate(right_type)))) + else if ((isColumnedAsDecimal(left_type) || isColumnedAsDecimal(right_type))) { - // compare - if (!allowDecimalComparison(left_type, right_type) && !date_and_datetime) - throw Exception("No operation " + getName() + " between " + left_type->getName() + " and " + right_type->getName(), - ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); + // Comparing Date and DateTime64 requires implicit conversion, + if (date_and_datetime && (isDate(left_type) || isDate(right_type))) + { + DataTypePtr common_type = getLeastSupertype({left_type, right_type}); + ColumnPtr c0_converted = castColumn(col_with_type_and_name_left, common_type); + ColumnPtr c1_converted = castColumn(col_with_type_and_name_right, common_type); + return executeDecimal({c0_converted, common_type, "left"}, {c1_converted, common_type, "right"}); + } + else + { + // compare + if (!allowDecimalComparison(left_type, right_type) && !date_and_datetime) + throw Exception( + "No operation " + getName() + " between " + left_type->getName() + " and " + right_type->getName(), + ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); + return executeDecimal(col_with_type_and_name_left, col_with_type_and_name_right); + } - return executeDecimal(col_with_type_and_name_left, col_with_type_and_name_right); + } + else if (date_and_datetime) + { + DataTypePtr common_type = getLeastSupertype({left_type, right_type}); + ColumnPtr c0_converted = castColumn(col_with_type_and_name_left, common_type); + ColumnPtr c1_converted = castColumn(col_with_type_and_name_right, common_type); + if (!((res = executeNumLeftType(c0_converted.get(), c1_converted.get())) + || (res = executeNumLeftType(c0_converted.get(), c1_converted.get())))) + throw Exception("Date related common types can only be UInt32 or UInt64", ErrorCodes::LOGICAL_ERROR); + return res; } else if (left_type->equals(*right_type)) { diff --git a/tests/performance/datetime_comparison.xml b/tests/performance/datetime_comparison.xml index 2d47ded0b1a..8d7b0c8c4de 100644 --- a/tests/performance/datetime_comparison.xml +++ b/tests/performance/datetime_comparison.xml @@ -2,4 +2,5 @@ SELECT count() FROM numbers(1000000000) WHERE materialize(now()) > toString(toDateTime('2020-09-30 00:00:00')) SELECT count() FROM numbers(1000000000) WHERE materialize(now()) > toUInt32(toDateTime('2020-09-30 00:00:00')) SELECT count() FROM numbers(1000000000) WHERE materialize(now()) > toDateTime('2020-09-30 00:00:00') + SELECT count() FROM numbers(1000000000) WHERE materialize(now()) > toDate('2020-09-30 00:00:00') From cc0c3a90336f7527209e2be8d00089db8b9c2697 Mon Sep 17 00:00:00 2001 From: vdimir Date: Tue, 3 Aug 2021 12:34:08 +0300 Subject: [PATCH 62/78] Make IJoin::alwaysReturnsEmptySet pure virtual --- src/Interpreters/IJoin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Interpreters/IJoin.h b/src/Interpreters/IJoin.h index c2cf007d823..8fa85de4951 100644 --- a/src/Interpreters/IJoin.h +++ b/src/Interpreters/IJoin.h @@ -37,7 +37,7 @@ public: virtual size_t getTotalRowCount() const = 0; virtual size_t getTotalByteCount() const = 0; - virtual bool alwaysReturnsEmptySet() const { return false; } + virtual bool alwaysReturnsEmptySet() const = 0; /// StorageJoin/Dictionary is already filled. No need to call addJoinedBlock. /// Different query plan is used for such joins. From f4473c5de49b2f3b2f9b1c4741b2f672dbb4beb1 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Tue, 3 Aug 2021 14:18:47 +0300 Subject: [PATCH 63/78] GlobalSubqueriesVisitor external storage check fix --- src/Interpreters/GlobalSubqueriesVisitor.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/GlobalSubqueriesVisitor.h b/src/Interpreters/GlobalSubqueriesVisitor.h index 71ff7fd2e0e..fbf9b22a734 100644 --- a/src/Interpreters/GlobalSubqueriesVisitor.h +++ b/src/Interpreters/GlobalSubqueriesVisitor.h @@ -63,7 +63,7 @@ public: return; bool is_table = false; - ASTPtr subquery_or_table_name = ast; /// ASTTableIdentifier | ASTSubquery | ASTTableExpression + ASTPtr subquery_or_table_name; /// ASTTableIdentifier | ASTSubquery | ASTTableExpression if (const auto * ast_table_expr = ast->as()) { @@ -76,7 +76,10 @@ public: } } else if (ast->as()) + { + subquery_or_table_name = ast; is_table = true; + } if (!subquery_or_table_name) throw Exception("Global subquery requires subquery or table name", ErrorCodes::WRONG_GLOBAL_SUBQUERY); From 0dc7da92b4401524c7ad69495b32bcf17c160414 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Tue, 3 Aug 2021 14:24:21 +0300 Subject: [PATCH 64/78] Added test --- .../02002_global_subqueries_subquery_or_table_name.reference | 0 .../02002_global_subqueries_subquery_or_table_name.sql | 5 +++++ 2 files changed, 5 insertions(+) create mode 100644 tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.reference create mode 100644 tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.sql diff --git a/tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.reference b/tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.sql b/tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.sql new file mode 100644 index 00000000000..1b24617569c --- /dev/null +++ b/tests/queries/0_stateless/02002_global_subqueries_subquery_or_table_name.sql @@ -0,0 +1,5 @@ +SELECT + cityHash64(number GLOBAL IN (NULL, -2147483648, -9223372036854775808), nan, 1024, NULL, NULL, 1.000100016593933, NULL), + (NULL, cityHash64(inf, -2147483648, NULL, NULL, 10.000100135803223), cityHash64(1.1754943508222875e-38, NULL, NULL, NULL), 2147483647) +FROM cluster(test_cluster_two_shards_localhost, numbers((NULL, cityHash64(0., 65536, NULL, NULL, 10000000000., NULL), 0) GLOBAL IN (some_identifier), 65536)) +WHERE number GLOBAL IN [1025] --{serverError 284} From 06903485d793d46b146626f026d70d1e059289a9 Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Tue, 3 Aug 2021 14:38:29 +0300 Subject: [PATCH 65/78] Fix build on freebsd --- contrib/AMQP-CPP | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/AMQP-CPP b/contrib/AMQP-CPP index 8d83306aa0e..e367d72cd44 160000 --- a/contrib/AMQP-CPP +++ b/contrib/AMQP-CPP @@ -1 +1 @@ -Subproject commit 8d83306aa0ef33704ee951f789163ca90ab74402 +Subproject commit e367d72cd44578c1f03cef96fba21d12cac56b14 From f9d1598847a7a3567a3ec916b2bac7da0bb54e49 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Tue, 3 Aug 2021 14:42:14 +0300 Subject: [PATCH 66/78] Update 10_question.md --- .github/ISSUE_TEMPLATE/10_question.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/10_question.md b/.github/ISSUE_TEMPLATE/10_question.md index 6e23fbdc605..a112b9599d5 100644 --- a/.github/ISSUE_TEMPLATE/10_question.md +++ b/.github/ISSUE_TEMPLATE/10_question.md @@ -7,6 +7,6 @@ assignees: '' --- -Make sure to check documentation https://clickhouse.yandex/docs/en/ first. If the question is concise and probably has a short answer, asking it in Telegram chat https://telegram.me/clickhouse_en is probably the fastest way to find the answer. For more complicated questions, consider asking them on StackOverflow with "clickhouse" tag https://stackoverflow.com/questions/tagged/clickhouse +> Make sure to check documentation https://clickhouse.yandex/docs/en/ first. If the question is concise and probably has a short answer, asking it in Telegram chat https://telegram.me/clickhouse_en is probably the fastest way to find the answer. For more complicated questions, consider asking them on StackOverflow with "clickhouse" tag https://stackoverflow.com/questions/tagged/clickhouse -If you still prefer GitHub issues, remove all this text and ask your question here. +> If you still prefer GitHub issues, remove all this text and ask your question here. From 73dd3bd6cc966bc56ca2f13b034fade9ea8616d5 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Tue, 3 Aug 2021 14:43:00 +0300 Subject: [PATCH 67/78] Update 40_bug-report.md --- .github/ISSUE_TEMPLATE/40_bug-report.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/40_bug-report.md b/.github/ISSUE_TEMPLATE/40_bug-report.md index 5c8611d47e6..d62ec578f8d 100644 --- a/.github/ISSUE_TEMPLATE/40_bug-report.md +++ b/.github/ISSUE_TEMPLATE/40_bug-report.md @@ -7,11 +7,11 @@ assignees: '' --- -You have to provide the following information whenever possible. +> You have to provide the following information whenever possible. **Describe the bug** -A clear and concise description of what works not as it is supposed to. +> A clear and concise description of what works not as it is supposed to. **Does it reproduce on recent release?** @@ -19,7 +19,7 @@ A clear and concise description of what works not as it is supposed to. **Enable crash reporting** -If possible, change "enabled" to true in "send_crash_reports" section in `config.xml`: +> If possible, change "enabled" to true in "send_crash_reports" section in `config.xml`: ``` @@ -39,12 +39,12 @@ If possible, change "enabled" to true in "send_crash_reports" section in `config **Expected behavior** -A clear and concise description of what you expected to happen. +> A clear and concise description of what you expected to happen. **Error message and/or stacktrace** -If applicable, add screenshots to help explain your problem. +> If applicable, add screenshots to help explain your problem. **Additional context** -Add any other context about the problem here. +> Add any other context about the problem here. From cd566f0a44f16eaa7fe21d04f684ef639a918f1c Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Tue, 3 Aug 2021 14:43:26 +0300 Subject: [PATCH 68/78] Update 20_feature-request.md --- .github/ISSUE_TEMPLATE/20_feature-request.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/20_feature-request.md b/.github/ISSUE_TEMPLATE/20_feature-request.md index 99a762a2019..f59dbc2c40f 100644 --- a/.github/ISSUE_TEMPLATE/20_feature-request.md +++ b/.github/ISSUE_TEMPLATE/20_feature-request.md @@ -7,16 +7,20 @@ assignees: '' --- -(you don't have to strictly follow this form) +> (you don't have to strictly follow this form) **Use case** -A clear and concise description of what is the intended usage scenario is. + +> A clear and concise description of what is the intended usage scenario is. **Describe the solution you'd like** -A clear and concise description of what you want to happen. + +> A clear and concise description of what you want to happen. **Describe alternatives you've considered** -A clear and concise description of any alternative solutions or features you've considered. + +> A clear and concise description of any alternative solutions or features you've considered. **Additional context** -Add any other context or screenshots about the feature request here. + +> Add any other context or screenshots about the feature request here. From 3184902c6804ecc6de61238f58a3e1d8d2a69b67 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Tue, 3 Aug 2021 14:43:56 +0300 Subject: [PATCH 69/78] Update 50_build-issue.md --- .github/ISSUE_TEMPLATE/50_build-issue.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/50_build-issue.md b/.github/ISSUE_TEMPLATE/50_build-issue.md index 73c97cc3cfb..a358575cd7c 100644 --- a/.github/ISSUE_TEMPLATE/50_build-issue.md +++ b/.github/ISSUE_TEMPLATE/50_build-issue.md @@ -7,10 +7,11 @@ assignees: '' --- -Make sure that `git diff` result is empty and you've just pulled fresh master. Try cleaning up cmake cache. Just in case, official build instructions are published here: https://clickhouse.yandex/docs/en/development/build/ +> Make sure that `git diff` result is empty and you've just pulled fresh master. Try cleaning up cmake cache. Just in case, official build instructions are published here: https://clickhouse.yandex/docs/en/development/build/ **Operating system** -OS kind or distribution, specific version/release, non-standard kernel if any. If you are trying to build inside virtual machine, please mention it too. + +> OS kind or distribution, specific version/release, non-standard kernel if any. If you are trying to build inside virtual machine, please mention it too. **Cmake version** From cb9627255ae7ef4a786ba65394c94dbca4009233 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Tue, 3 Aug 2021 15:16:55 +0300 Subject: [PATCH 70/78] Fixed tests --- src/Interpreters/GlobalSubqueriesVisitor.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Interpreters/GlobalSubqueriesVisitor.h b/src/Interpreters/GlobalSubqueriesVisitor.h index fbf9b22a734..6a87527dc9c 100644 --- a/src/Interpreters/GlobalSubqueriesVisitor.h +++ b/src/Interpreters/GlobalSubqueriesVisitor.h @@ -80,6 +80,10 @@ public: subquery_or_table_name = ast; is_table = true; } + else if (ast->as()) + { + subquery_or_table_name = ast; + } if (!subquery_or_table_name) throw Exception("Global subquery requires subquery or table name", ErrorCodes::WRONG_GLOBAL_SUBQUERY); From 019a2215509633cd3830c51edbf1384deb726235 Mon Sep 17 00:00:00 2001 From: Yatsishin Ilya <2159081+qoega@users.noreply.github.com> Date: Tue, 3 Aug 2021 17:29:59 +0300 Subject: [PATCH 71/78] fix unbundled build --- contrib/AMQP-CPP | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/AMQP-CPP b/contrib/AMQP-CPP index e367d72cd44..1a6c51f4ac5 160000 --- a/contrib/AMQP-CPP +++ b/contrib/AMQP-CPP @@ -1 +1 @@ -Subproject commit e367d72cd44578c1f03cef96fba21d12cac56b14 +Subproject commit 1a6c51f4ac51ac56610fa95081bd2f349911375a From 1df8a5f57a0f48a1098b704a114f7a4acc3bd550 Mon Sep 17 00:00:00 2001 From: Malte Date: Tue, 3 Aug 2021 14:40:28 -0400 Subject: [PATCH 72/78] Update gui.md We had some issues figuring out how to use the session settings and hope this helps other folks as well --- docs/en/interfaces/third-party/gui.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/en/interfaces/third-party/gui.md b/docs/en/interfaces/third-party/gui.md index fffe0c87a53..2d7f3a24011 100644 --- a/docs/en/interfaces/third-party/gui.md +++ b/docs/en/interfaces/third-party/gui.md @@ -84,6 +84,8 @@ Features: - Table data preview. - Full-text search. +By default, DBeaver does not connect using a session (the CLI for example does). If you require session support (for example to set settings for your session), edit the driver connection properties and set session_id to a random string (it uses the http connection under the hood). Then you can use any setting from the query window + ### clickhouse-cli {#clickhouse-cli} [clickhouse-cli](https://github.com/hatarist/clickhouse-cli) is an alternative command-line client for ClickHouse, written in Python 3. From cc2c1b80df83a6bf187c7a934fd78c021aecfb97 Mon Sep 17 00:00:00 2001 From: kssenii Date: Tue, 3 Aug 2021 19:47:59 +0000 Subject: [PATCH 73/78] Fix tests --- src/Bridge/LibraryBridgeHelper.cpp | 7 ++++--- src/Bridge/LibraryBridgeHelper.h | 1 + tests/integration/test_library_bridge/test.py | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Bridge/LibraryBridgeHelper.cpp b/src/Bridge/LibraryBridgeHelper.cpp index 9341ddda922..95a96074be0 100644 --- a/src/Bridge/LibraryBridgeHelper.cpp +++ b/src/Bridge/LibraryBridgeHelper.cpp @@ -39,6 +39,7 @@ LibraryBridgeHelper::LibraryBridgeHelper( , http_timeout(context_->getGlobalContext()->getSettingsRef().http_receive_timeout.value) , library_data(library_data_) , dictionary_id(dictionary_id_) + , http_timeouts(ConnectionTimeouts::getHTTPTimeouts(context_)) { bridge_port = config.getUInt("library_bridge.port", DEFAULT_PORT); bridge_host = config.getString("library_bridge.host", DEFAULT_HOST); @@ -75,7 +76,7 @@ bool LibraryBridgeHelper::bridgeHandShake() String result; try { - ReadWriteBufferFromHTTP buf(createRequestURI(PING), Poco::Net::HTTPRequest::HTTP_GET, {}, ConnectionTimeouts::getHTTPTimeouts(getContext())); + ReadWriteBufferFromHTTP buf(createRequestURI(PING), Poco::Net::HTTPRequest::HTTP_GET, {}, http_timeouts); readString(result, buf); } catch (...) @@ -239,7 +240,7 @@ bool LibraryBridgeHelper::executeRequest(const Poco::URI & uri, ReadWriteBufferF uri, Poco::Net::HTTPRequest::HTTP_POST, std::move(out_stream_callback), - ConnectionTimeouts::getHTTPTimeouts(getContext())); + http_timeouts); bool res; readBoolText(res, buf); @@ -253,7 +254,7 @@ BlockInputStreamPtr LibraryBridgeHelper::loadBase(const Poco::URI & uri, ReadWri uri, Poco::Net::HTTPRequest::HTTP_POST, std::move(out_stream_callback), - ConnectionTimeouts::getHTTPTimeouts(getContext()), + http_timeouts, 0, Poco::Net::HTTPBasicCredentials{}, DBMS_DEFAULT_BUFFER_SIZE, diff --git a/src/Bridge/LibraryBridgeHelper.h b/src/Bridge/LibraryBridgeHelper.h index 6615162efe7..fec644240da 100644 --- a/src/Bridge/LibraryBridgeHelper.h +++ b/src/Bridge/LibraryBridgeHelper.h @@ -98,6 +98,7 @@ private: std::string bridge_host; size_t bridge_port; bool library_initialized = false; + ConnectionTimeouts http_timeouts; }; } diff --git a/tests/integration/test_library_bridge/test.py b/tests/integration/test_library_bridge/test.py index 4bef197f8bd..607afb6db5f 100644 --- a/tests/integration/test_library_bridge/test.py +++ b/tests/integration/test_library_bridge/test.py @@ -2,6 +2,7 @@ import os import os.path as p import pytest import time +import logging from helpers.cluster import ClickHouseCluster, run_and_check From 6a5925b6ce1d0f8edf4e544a0560aa346171b489 Mon Sep 17 00:00:00 2001 From: zhangxiao871 Date: Wed, 4 Aug 2021 11:07:39 +0800 Subject: [PATCH 74/78] Fix CURR_DATABASE empty for 01034_move_partition_from_table_zookeeper.sh --- .../0_stateless/01034_move_partition_from_table_zookeeper.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/queries/0_stateless/01034_move_partition_from_table_zookeeper.sh b/tests/queries/0_stateless/01034_move_partition_from_table_zookeeper.sh index ae3dd7851c8..af4969d77c8 100755 --- a/tests/queries/0_stateless/01034_move_partition_from_table_zookeeper.sh +++ b/tests/queries/0_stateless/01034_move_partition_from_table_zookeeper.sh @@ -75,8 +75,8 @@ $CLICKHOUSE_CLIENT --query="DROP TABLE dst;" $CLICKHOUSE_CLIENT --query="SELECT 'MOVE incompatible schema different order by';" -$CLICKHOUSE_CLIENT --query="CREATE TABLE src (p UInt64, k String, d UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/$CURR_DATABASE/src3', '1') PARTITION BY p ORDER BY (p, k, d);" -$CLICKHOUSE_CLIENT --query="CREATE TABLE dst (p UInt64, k String, d UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/$CURR_DATABASE/dst3', '1') PARTITION BY p ORDER BY (d, k, p);" +$CLICKHOUSE_CLIENT --query="CREATE TABLE src (p UInt64, k String, d UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/src3', '1') PARTITION BY p ORDER BY (p, k, d);" +$CLICKHOUSE_CLIENT --query="CREATE TABLE dst (p UInt64, k String, d UInt64) ENGINE = ReplicatedMergeTree('/clickhouse/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/dst3', '1') PARTITION BY p ORDER BY (d, k, p);" $CLICKHOUSE_CLIENT --query="INSERT INTO src VALUES (0, '0', 1);" From c2410920d3386c216c2e3034155694bc03dfa3e6 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Wed, 4 Aug 2021 09:14:20 +0300 Subject: [PATCH 75/78] Safer `ReadBufferFromS3` for merges and backports. --- src/IO/ReadBufferFromS3.h | 2 +- src/Storages/StorageS3.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/IO/ReadBufferFromS3.h b/src/IO/ReadBufferFromS3.h index 46fb79a4a14..e24d571b557 100644 --- a/src/IO/ReadBufferFromS3.h +++ b/src/IO/ReadBufferFromS3.h @@ -43,7 +43,7 @@ public: const String & bucket_, const String & key_, UInt64 max_single_read_retries_, - size_t buffer_size_ = DBMS_DEFAULT_BUFFER_SIZE); + size_t buffer_size_); bool nextImpl() override; diff --git a/src/Storages/StorageS3.cpp b/src/Storages/StorageS3.cpp index 1008bae346d..fc3ce3a10ed 100644 --- a/src/Storages/StorageS3.cpp +++ b/src/Storages/StorageS3.cpp @@ -208,7 +208,8 @@ bool StorageS3Source::initialize() file_path = fs::path(bucket) / current_key; read_buf = wrapReadBufferWithCompressionMethod( - std::make_unique(client, bucket, current_key, max_single_read_retries), chooseCompressionMethod(current_key, compression_hint)); + std::make_unique(client, bucket, current_key, max_single_read_retries, DBMS_DEFAULT_BUFFER_SIZE), + chooseCompressionMethod(current_key, compression_hint)); auto input_format = FormatFactory::instance().getInput(format, *read_buf, sample_block, getContext(), max_block_size); pipeline = std::make_unique(); pipeline->init(Pipe(input_format)); From cc32a61488f35933e1870651200df1c9b50f38aa Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Wed, 4 Aug 2021 10:40:02 +0300 Subject: [PATCH 76/78] Fix. --- src/Disks/S3/DiskS3.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Disks/S3/DiskS3.cpp b/src/Disks/S3/DiskS3.cpp index 1f1c73c32c3..6dd29165566 100644 --- a/src/Disks/S3/DiskS3.cpp +++ b/src/Disks/S3/DiskS3.cpp @@ -363,7 +363,8 @@ int DiskS3::readSchemaVersion(const String & source_bucket, const String & sourc settings->client, source_bucket, source_path + SCHEMA_VERSION_OBJECT, - settings->s3_max_single_read_retries); + settings->s3_max_single_read_retries, + DBMS_DEFAULT_BUFFER_SIZE); readIntText(version, buffer); From 77ade599d368dbc0d9c22790167d4fe2c9e3c508 Mon Sep 17 00:00:00 2001 From: Dmitriy Lushnikov <40070832+lushndm@users.noreply.github.com> Date: Wed, 4 Aug 2021 12:52:30 +0300 Subject: [PATCH 77/78] Update index.md --- docs/ru/engines/table-engines/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ru/engines/table-engines/index.md b/docs/ru/engines/table-engines/index.md index b17b2124250..1636efc5112 100644 --- a/docs/ru/engines/table-engines/index.md +++ b/docs/ru/engines/table-engines/index.md @@ -87,7 +87,7 @@ toc_title: "Введение" Виртуальный столбец — это неотъемлемый атрибут движка таблиц, определенный в исходном коде движка. -Виртуальные столбцы не надо указывать в запросе `CREATE TABLE` и их не отображаются в результатах запросов `SHOW CREATE TABLE` и `DESCRIBE TABLE`. Также виртуальные столбцы доступны только для чтения, поэтому вы не можете вставлять в них данные. +Виртуальные столбцы не надо указывать в запросе `CREATE TABLE` и они не отображаются в результатах запросов `SHOW CREATE TABLE` и `DESCRIBE TABLE`. Также виртуальные столбцы доступны только для чтения, поэтому вы не можете вставлять в них данные. Чтобы получить данные из виртуального столбца, необходимо указать его название в запросе `SELECT`. `SELECT *` не отображает данные из виртуальных столбцов. From 659b874934e896d2eaed2aa3f309c552583ed980 Mon Sep 17 00:00:00 2001 From: Dmitriy Lushnikov <40070832+lushndm@users.noreply.github.com> Date: Wed, 4 Aug 2021 13:02:11 +0300 Subject: [PATCH 78/78] Update gui.md --- docs/ru/interfaces/third-party/gui.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ru/interfaces/third-party/gui.md b/docs/ru/interfaces/third-party/gui.md index dc96c32e996..9cb28a2c9a2 100644 --- a/docs/ru/interfaces/third-party/gui.md +++ b/docs/ru/interfaces/third-party/gui.md @@ -111,7 +111,7 @@ toc_title: "Визуальные интерфейсы от сторонних р ### DataGrip {#datagrip} -[DataGrip](https://www.jetbrains.com/datagrip/) — это IDE для баз данных о JetBrains с выделенной поддержкой ClickHouse. Он также встроен в другие инструменты на основе IntelliJ: PyCharm, IntelliJ IDEA, GoLand, PhpStorm и другие. +[DataGrip](https://www.jetbrains.com/datagrip/) — это IDE для баз данных от JetBrains с выделенной поддержкой ClickHouse. Он также встроен в другие инструменты на основе IntelliJ: PyCharm, IntelliJ IDEA, GoLand, PhpStorm и другие. Основные возможности: