From bc107c70fa863237685c5c353937e9d4af9dd674 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Wed, 1 Mar 2023 18:50:51 +0100 Subject: [PATCH 01/68] merge and mutation make thread group for setting memory trackers right --- src/Common/MemoryTracker.cpp | 9 +++ src/Common/MemoryTracker.h | 2 + src/Common/ThreadStatus.h | 8 +-- src/Storages/MergeTree/MergeList.cpp | 72 +++++++------------ src/Storages/MergeTree/MergeList.h | 27 +++---- .../MergeTree/MergePlainMergeTreeTask.cpp | 4 +- src/Storages/MergeTree/MergeTreeData.cpp | 2 +- .../MergeTree/MutatePlainMergeTreeTask.cpp | 4 +- .../ReplicatedMergeMutateTaskBase.cpp | 4 +- 9 files changed, 56 insertions(+), 76 deletions(-) diff --git a/src/Common/MemoryTracker.cpp b/src/Common/MemoryTracker.cpp index e2129e1013e..4cfb7f764e5 100644 --- a/src/Common/MemoryTracker.cpp +++ b/src/Common/MemoryTracker.cpp @@ -28,6 +28,7 @@ #include #include #include +#include namespace @@ -118,6 +119,14 @@ MemoryTracker::~MemoryTracker() } } +String MemoryTracker::getDebugLog() const +{ + return fmt::format("MemoryTracker(addr {} level {} peak {} ammount {})", + size_t(this), + magic_enum::enum_name(level), + ReadableSize(getPeak()), + ReadableSize(get())); +} void MemoryTracker::logPeakMemoryUsage() { diff --git a/src/Common/MemoryTracker.h b/src/Common/MemoryTracker.h index 66b56730b75..66037345eb0 100644 --- a/src/Common/MemoryTracker.h +++ b/src/Common/MemoryTracker.h @@ -215,6 +215,8 @@ public: /// Prints info about peak memory consumption into log. void logPeakMemoryUsage(); + + String getDebugLog() const; }; extern MemoryTracker total_memory_tracker; diff --git a/src/Common/ThreadStatus.h b/src/Common/ThreadStatus.h index 77c924f9650..4f7e9ca6830 100644 --- a/src/Common/ThreadStatus.h +++ b/src/Common/ThreadStatus.h @@ -40,7 +40,7 @@ class TaskStatsInfoGetter; class InternalTextLogsQueue; struct ViewRuntimeData; class QueryViewsLog; -class MemoryTrackerThreadSwitcher; +class ThreadGroupSwitcher; using InternalTextLogsQueuePtr = std::shared_ptr; using InternalTextLogsQueueWeakPtr = std::weak_ptr; @@ -176,12 +176,6 @@ private: bool performance_counters_finalized = false; String query_id_from_query_context; - /// Requires access to query_id. - friend class MemoryTrackerThreadSwitcher; - void setQueryId(const String & query_id_) - { - query_id_from_query_context = query_id_; - } struct TimePoint { diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index fa1887a02e6..4705733bb80 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -11,38 +11,24 @@ namespace DB { -MemoryTrackerThreadSwitcher::MemoryTrackerThreadSwitcher(MergeListEntry & merge_list_entry_) +ThreadGroupSwitcher::ThreadGroupSwitcher(MergeListEntry & merge_list_entry_) : merge_list_entry(merge_list_entry_) { - // Each merge is executed into separate background processing pool thread - background_thread_memory_tracker = CurrentThread::getMemoryTracker(); - background_thread_memory_tracker_prev_parent = background_thread_memory_tracker->getParent(); - background_thread_memory_tracker->setParent(&merge_list_entry->memory_tracker); + prev_thread_group = CurrentThread::getGroup(); + if (!prev_thread_group) + return; - prev_untracked_memory_limit = current_thread->untracked_memory_limit; - current_thread->untracked_memory_limit = merge_list_entry->max_untracked_memory; - - /// Avoid accounting memory from another mutation/merge - /// (NOTE: consider moving such code to ThreadFromGlobalPool and related places) - prev_untracked_memory = current_thread->untracked_memory; - current_thread->untracked_memory = merge_list_entry->untracked_memory; - - prev_query_id = std::string(current_thread->getQueryId()); - current_thread->setQueryId(merge_list_entry->query_id); + CurrentThread::detachGroupIfNotDetached(); + CurrentThread::attachToGroup(merge_list_entry_->thread_group); } - -MemoryTrackerThreadSwitcher::~MemoryTrackerThreadSwitcher() +ThreadGroupSwitcher::~ThreadGroupSwitcher() { - // Unplug memory_tracker from current background processing pool thread - background_thread_memory_tracker->setParent(background_thread_memory_tracker_prev_parent); + if (!prev_thread_group) + return; - current_thread->untracked_memory_limit = prev_untracked_memory_limit; - - merge_list_entry->untracked_memory = current_thread->untracked_memory; - current_thread->untracked_memory = prev_untracked_memory; - - current_thread->setQueryId(prev_query_id); + CurrentThread::detachGroup(); + CurrentThread::attachTo(prev_thread_group); } MergeListElement::MergeListElement( @@ -55,7 +41,6 @@ MergeListElement::MergeListElement( , result_part_path{future_part->path} , result_part_info{future_part->part_info} , num_parts{future_part->parts.size()} - , max_untracked_memory(settings.max_untracked_memory) , query_id(table_id.getShortName() + "::" + result_part_name) , thread_id{getThreadId()} , merge_type{future_part->merge_type} @@ -78,6 +63,12 @@ MergeListElement::MergeListElement( is_mutation = (result_part_info.getDataVersion() != source_data_version); } + thread_group = std::make_shared(); + + thread_group->master_thread_id = CurrentThread::get().thread_id; + + auto & memory_tracker = thread_group->memory_tracker; + memory_tracker.setDescription(description.c_str()); /// MemoryTracker settings should be set here, because /// later (see MemoryTrackerThreadSwitcher) @@ -97,15 +88,16 @@ MergeListElement::MergeListElement( /// /// NOTE: Remember, that Thread level MemoryTracker does not have any settings, /// so it's parent is required. - MemoryTracker * query_memory_tracker = CurrentThread::getMemoryTracker(); - MemoryTracker * parent_query_memory_tracker; - if (query_memory_tracker->level == VariableContext::Thread && - (parent_query_memory_tracker = query_memory_tracker->getParent()) && - parent_query_memory_tracker != &total_memory_tracker) - { - memory_tracker.setOrRaiseHardLimit(parent_query_memory_tracker->getHardLimit()); - } + MemoryTracker * cur_memory_tracker = CurrentThread::getMemoryTracker(); + if (cur_memory_tracker->level == VariableContext::Thread) + { + MemoryTracker * query_memory_tracker = cur_memory_tracker->getParent(); + if (query_memory_tracker != &total_memory_tracker) + { + memory_tracker.setOrRaiseHardLimit(query_memory_tracker->getHardLimit()); + } + } } MergeInfo MergeListElement::getInfo() const @@ -128,7 +120,7 @@ MergeInfo MergeListElement::getInfo() const res.rows_read = rows_read.load(std::memory_order_relaxed); res.rows_written = rows_written.load(std::memory_order_relaxed); res.columns_written = columns_written.load(std::memory_order_relaxed); - res.memory_usage = memory_tracker.get(); + res.memory_usage = getMemoryTracker().get(); res.thread_id = thread_id; res.merge_type = toString(merge_type); res.merge_algorithm = toString(merge_algorithm.load(std::memory_order_relaxed)); @@ -142,14 +134,4 @@ MergeInfo MergeListElement::getInfo() const return res; } -MergeListElement::~MergeListElement() -{ - if (untracked_memory != 0) - { - CurrentThread::getMemoryTracker()->adjustWithUntrackedMemory(untracked_memory); - untracked_memory = 0; - } -} - - } diff --git a/src/Storages/MergeTree/MergeList.h b/src/Storages/MergeTree/MergeList.h index 17a56272a57..b557f745f0d 100644 --- a/src/Storages/MergeTree/MergeList.h +++ b/src/Storages/MergeTree/MergeList.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -63,23 +64,19 @@ struct Settings; /** * Since merge is executed with multiple threads, this class - * switches the parent MemoryTracker to account all the memory used. + * switches the parent MemoryTracker as part of the thread group to account all the memory used. */ -class MemoryTrackerThreadSwitcher : boost::noncopyable +class ThreadGroupSwitcher : boost::noncopyable { public: - explicit MemoryTrackerThreadSwitcher(MergeListEntry & merge_list_entry_); - ~MemoryTrackerThreadSwitcher(); + explicit ThreadGroupSwitcher(MergeListEntry & merge_list_entry_); + ~ThreadGroupSwitcher(); private: MergeListEntry & merge_list_entry; - MemoryTracker * background_thread_memory_tracker; - MemoryTracker * background_thread_memory_tracker_prev_parent = nullptr; - Int64 prev_untracked_memory_limit; - Int64 prev_untracked_memory; - String prev_query_id; + ThreadGroupStatusPtr prev_thread_group; }; -using MemoryTrackerThreadSwitcherPtr = std::unique_ptr; +using ThreadGroupSwitcherPtr = std::unique_ptr; struct MergeListElement : boost::noncopyable { @@ -113,10 +110,6 @@ struct MergeListElement : boost::noncopyable /// Updated only for Vertical algorithm std::atomic columns_written{}; - /// Used to adjust ThreadStatus::untracked_memory_limit - UInt64 max_untracked_memory; - /// Used to avoid losing any allocation context - UInt64 untracked_memory = 0; /// Used for identifying mutations/merges in trace_log std::string query_id; @@ -128,7 +121,7 @@ struct MergeListElement : boost::noncopyable /// Description used for logging /// Needs to outlive memory_tracker since it's used in its destructor const String description{"Mutate/Merge"}; - MemoryTracker memory_tracker{VariableContext::Process}; + ThreadGroupStatusPtr thread_group; MergeListElement( const StorageID & table_id_, @@ -137,9 +130,9 @@ struct MergeListElement : boost::noncopyable MergeInfo getInfo() const; - MergeListElement * ptr() { return this; } + const MemoryTracker & getMemoryTracker() const { return thread_group->memory_tracker; } - ~MergeListElement(); + MergeListElement * ptr() { return this; } MergeListElement & ref() { return *this; } }; diff --git a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp index 1ccdefd2b6a..c2b3f9dfc8d 100644 --- a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp @@ -32,9 +32,9 @@ bool MergePlainMergeTreeTask::executeStep() ProfileEventsScope profile_events_scope(&profile_counters); /// Make out memory tracker a parent of current thread memory tracker - MemoryTrackerThreadSwitcherPtr switcher; + ThreadGroupSwitcherPtr switcher; if (merge_list_entry) - switcher = std::make_unique(*merge_list_entry); + switcher = std::make_unique(*merge_list_entry); switch (state) { diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 84c301e5986..2a80dc7fb5e 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -7415,7 +7415,7 @@ try part_log_elem.rows = (*merge_entry)->rows_written; part_log_elem.bytes_uncompressed = (*merge_entry)->bytes_written_uncompressed; - part_log_elem.peak_memory_usage = (*merge_entry)->memory_tracker.getPeak(); + part_log_elem.peak_memory_usage = (*merge_entry)->getMemoryTracker().getPeak(); } if (profile_counters) diff --git a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp index 9bd0f148d6c..a05a12eabe4 100644 --- a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp @@ -68,9 +68,9 @@ bool MutatePlainMergeTreeTask::executeStep() ProfileEventsScope profile_events_scope(&profile_counters); /// Make out memory tracker a parent of current thread memory tracker - MemoryTrackerThreadSwitcherPtr switcher; + ThreadGroupSwitcherPtr switcher; if (merge_list_entry) - switcher = std::make_unique(*merge_list_entry); + switcher = std::make_unique(*merge_list_entry); switch (state) { diff --git a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp index 9ce7eb42666..0af96cec323 100644 --- a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp @@ -128,9 +128,9 @@ bool ReplicatedMergeMutateTaskBase::executeStep() bool ReplicatedMergeMutateTaskBase::executeImpl() { - MemoryTrackerThreadSwitcherPtr switcher; + ThreadGroupSwitcherPtr switcher; if (merge_mutate_entry) - switcher = std::make_unique(*merge_mutate_entry); + switcher = std::make_unique(*merge_mutate_entry); auto remove_processed_entry = [&] () -> bool { From da4f2bd9232e5fa19a9d6b5e8119b5a9627af301 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Fri, 3 Mar 2023 15:21:53 +0100 Subject: [PATCH 02/68] do not attach empty thread group --- src/Common/MemoryTracker.cpp | 2 +- src/Storages/MergeTree/MergeList.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common/MemoryTracker.cpp b/src/Common/MemoryTracker.cpp index 4cfb7f764e5..16c0d1e9eb1 100644 --- a/src/Common/MemoryTracker.cpp +++ b/src/Common/MemoryTracker.cpp @@ -121,7 +121,7 @@ MemoryTracker::~MemoryTracker() String MemoryTracker::getDebugLog() const { - return fmt::format("MemoryTracker(addr {} level {} peak {} ammount {})", + return fmt::format("MemoryTracker(addr {} level {} peak {} amount {})", size_t(this), magic_enum::enum_name(level), ReadableSize(getPeak()), diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index 4705733bb80..a9c55495b4b 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -28,7 +28,7 @@ ThreadGroupSwitcher::~ThreadGroupSwitcher() return; CurrentThread::detachGroup(); - CurrentThread::attachTo(prev_thread_group); + CurrentThread::attachToGroup(prev_thread_group); } MergeListElement::MergeListElement( From 6a6d45e6e76669677378c94dc3997a29dccec912 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Fri, 3 Mar 2023 17:21:45 +0100 Subject: [PATCH 03/68] set up performance_counters for thread group --- src/Storages/MergeTree/MergeList.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index a9c55495b4b..65725c1a632 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -65,6 +65,11 @@ MergeListElement::MergeListElement( thread_group = std::make_shared(); + auto p_counters = CurrentThread::get().current_performance_counters; + while (p_counters && p_counters->level != VariableContext::Process) + p_counters = p_counters->getParent(); + thread_group->performance_counters.setParent(p_counters); + thread_group->master_thread_id = CurrentThread::get().thread_id; auto & memory_tracker = thread_group->memory_tracker; From 0fcf7c0363ba5279878fe1c9d3f536a2b28cfe72 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Fri, 3 Mar 2023 23:09:36 +0100 Subject: [PATCH 04/68] std::optional instead shared_ptr --- src/Storages/MergeTree/MergeList.cpp | 31 +++++++++++++++++-- src/Storages/MergeTree/MergeList.h | 13 +++++--- .../MergeTree/MergePlainMergeTreeTask.cpp | 4 +-- .../MergeTree/MutatePlainMergeTreeTask.cpp | 4 +-- .../ReplicatedMergeMutateTaskBase.cpp | 4 +-- 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index 65725c1a632..f53c6ad81ee 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -11,14 +11,14 @@ namespace DB { -ThreadGroupSwitcher::ThreadGroupSwitcher(MergeListEntry & merge_list_entry_) +ThreadGroupSwitcher::ThreadGroupSwitcher(MergeListEntry * merge_list_entry_) : merge_list_entry(merge_list_entry_) { prev_thread_group = CurrentThread::getGroup(); if (!prev_thread_group) return; - CurrentThread::detachGroupIfNotDetached(); + CurrentThread::detachGroup(); CurrentThread::attachToGroup(merge_list_entry_->thread_group); } @@ -27,10 +27,37 @@ ThreadGroupSwitcher::~ThreadGroupSwitcher() if (!prev_thread_group) return; + if (!merge_list_entry) + return; + CurrentThread::detachGroup(); CurrentThread::attachToGroup(prev_thread_group); } +ThreadGroupSwitcher::ThreadGroupSwitcher(ThreadGroupSwitcher && other) +{ + this->swap(other); +} + +ThreadGroupSwitcher& ThreadGroupSwitcher::operator=(ThreadGroupSwitcher && other) +{ + if (this != &other) + { + auto tmp = ThreadGroupSwitcher(); + tmp.swap(other); + this->swap(tmp); + } + return *this; +} + +void ThreadGroupSwitcher::swap(ThreadGroupSwitcher & other) +{ + std::swap(merge_list_entry, other.merge_list_entry); + std::swap(prev_thread_group, other.prev_thread_group); + std::swap(prev_query_id, other.prev_query_id); +} + + MergeListElement::MergeListElement( const StorageID & table_id_, FutureMergedMutatedPartPtr future_part, diff --git a/src/Storages/MergeTree/MergeList.h b/src/Storages/MergeTree/MergeList.h index b557f745f0d..200a574698c 100644 --- a/src/Storages/MergeTree/MergeList.h +++ b/src/Storages/MergeTree/MergeList.h @@ -66,18 +66,21 @@ struct Settings; * Since merge is executed with multiple threads, this class * switches the parent MemoryTracker as part of the thread group to account all the memory used. */ -class ThreadGroupSwitcher : boost::noncopyable +class ThreadGroupSwitcher : private boost::noncopyable { public: - explicit ThreadGroupSwitcher(MergeListEntry & merge_list_entry_); + explicit ThreadGroupSwitcher(MergeListEntry * merge_list_entry_); + ThreadGroupSwitcher(ThreadGroupSwitcher && other); + ThreadGroupSwitcher& operator=(ThreadGroupSwitcher && other); ~ThreadGroupSwitcher(); private: - MergeListEntry & merge_list_entry; + ThreadGroupSwitcher() = default; + void swap(ThreadGroupSwitcher & other); + + MergeListEntry * merge_list_entry = nullptr; ThreadGroupStatusPtr prev_thread_group; }; -using ThreadGroupSwitcherPtr = std::unique_ptr; - struct MergeListElement : boost::noncopyable { const StorageID table_id; diff --git a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp index c2b3f9dfc8d..052c6467b37 100644 --- a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp @@ -32,9 +32,9 @@ bool MergePlainMergeTreeTask::executeStep() ProfileEventsScope profile_events_scope(&profile_counters); /// Make out memory tracker a parent of current thread memory tracker - ThreadGroupSwitcherPtr switcher; + std::optional switcher; if (merge_list_entry) - switcher = std::make_unique(*merge_list_entry); + switcher = ThreadGroupSwitcher(merge_list_entry.get()); switch (state) { diff --git a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp index a05a12eabe4..673bfaa0d47 100644 --- a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp @@ -68,9 +68,9 @@ bool MutatePlainMergeTreeTask::executeStep() ProfileEventsScope profile_events_scope(&profile_counters); /// Make out memory tracker a parent of current thread memory tracker - ThreadGroupSwitcherPtr switcher; + std::optional switcher; if (merge_list_entry) - switcher = std::make_unique(*merge_list_entry); + switcher = ThreadGroupSwitcher(merge_list_entry.get()); switch (state) { diff --git a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp index 0af96cec323..fbc6522170e 100644 --- a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp @@ -128,9 +128,9 @@ bool ReplicatedMergeMutateTaskBase::executeStep() bool ReplicatedMergeMutateTaskBase::executeImpl() { - ThreadGroupSwitcherPtr switcher; + std::optional switcher; if (merge_mutate_entry) - switcher = std::make_unique(*merge_mutate_entry); + switcher = ThreadGroupSwitcher(merge_mutate_entry.get()); auto remove_processed_entry = [&] () -> bool { From da3e744405bc9707ac8b453f0637303a191d4847 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Mon, 6 Mar 2023 13:53:19 +0100 Subject: [PATCH 05/68] set context from the master thread --- src/Storages/MergeTree/MergeList.cpp | 11 +++++++---- src/Storages/MergeTree/MergeList.h | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index f53c6ad81ee..3a2e5a4ff35 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -34,12 +34,12 @@ ThreadGroupSwitcher::~ThreadGroupSwitcher() CurrentThread::attachToGroup(prev_thread_group); } -ThreadGroupSwitcher::ThreadGroupSwitcher(ThreadGroupSwitcher && other) +ThreadGroupSwitcher::ThreadGroupSwitcher(ThreadGroupSwitcher && other) noexcept { this->swap(other); } -ThreadGroupSwitcher& ThreadGroupSwitcher::operator=(ThreadGroupSwitcher && other) +ThreadGroupSwitcher& ThreadGroupSwitcher::operator=(ThreadGroupSwitcher && other) noexcept { if (this != &other) { @@ -50,7 +50,7 @@ ThreadGroupSwitcher& ThreadGroupSwitcher::operator=(ThreadGroupSwitcher && other return *this; } -void ThreadGroupSwitcher::swap(ThreadGroupSwitcher & other) +void ThreadGroupSwitcher::swap(ThreadGroupSwitcher & other) noexcept { std::swap(merge_list_entry, other.merge_list_entry); std::swap(prev_thread_group, other.prev_thread_group); @@ -92,7 +92,10 @@ MergeListElement::MergeListElement( thread_group = std::make_shared(); - auto p_counters = CurrentThread::get().current_performance_counters; + thread_group->query_context = CurrentThread::get().getQueryContext(); + thread_group->global_context = CurrentThread::get().getGlobalContext(); + + auto * p_counters = CurrentThread::get().current_performance_counters; while (p_counters && p_counters->level != VariableContext::Process) p_counters = p_counters->getParent(); thread_group->performance_counters.setParent(p_counters); diff --git a/src/Storages/MergeTree/MergeList.h b/src/Storages/MergeTree/MergeList.h index 200a574698c..2a166470203 100644 --- a/src/Storages/MergeTree/MergeList.h +++ b/src/Storages/MergeTree/MergeList.h @@ -70,12 +70,12 @@ class ThreadGroupSwitcher : private boost::noncopyable { public: explicit ThreadGroupSwitcher(MergeListEntry * merge_list_entry_); - ThreadGroupSwitcher(ThreadGroupSwitcher && other); - ThreadGroupSwitcher& operator=(ThreadGroupSwitcher && other); + ThreadGroupSwitcher(ThreadGroupSwitcher && other) noexcept; + ThreadGroupSwitcher& operator=(ThreadGroupSwitcher && other) noexcept; ~ThreadGroupSwitcher(); private: - ThreadGroupSwitcher() = default; - void swap(ThreadGroupSwitcher & other); + ThreadGroupSwitcher() noexcept = default; + void swap(ThreadGroupSwitcher & other) noexcept; MergeListEntry * merge_list_entry = nullptr; ThreadGroupStatusPtr prev_thread_group; From aeb8766ad59ec2b813a811142cb6e9cd0aa57572 Mon Sep 17 00:00:00 2001 From: Sema Checherinda Date: Sat, 18 Mar 2023 21:14:32 +0100 Subject: [PATCH 06/68] adjust after rebase --- src/Storages/MergeTree/MergeList.cpp | 45 +++---------------- src/Storages/MergeTree/MergeList.h | 22 ++++++--- .../MergeTree/MergePlainMergeTreeTask.cpp | 2 +- .../MergeTree/MutatePlainMergeTreeTask.cpp | 2 +- .../ReplicatedMergeMutateTaskBase.cpp | 2 +- 5 files changed, 25 insertions(+), 48 deletions(-) diff --git a/src/Storages/MergeTree/MergeList.cpp b/src/Storages/MergeTree/MergeList.cpp index 3a2e5a4ff35..c13c5e6105e 100644 --- a/src/Storages/MergeTree/MergeList.cpp +++ b/src/Storages/MergeTree/MergeList.cpp @@ -11,15 +11,16 @@ namespace DB { -ThreadGroupSwitcher::ThreadGroupSwitcher(MergeListEntry * merge_list_entry_) - : merge_list_entry(merge_list_entry_) +ThreadGroupSwitcher::ThreadGroupSwitcher(ThreadGroupStatusPtr thread_group) { + chassert(thread_group); + prev_thread_group = CurrentThread::getGroup(); if (!prev_thread_group) return; - CurrentThread::detachGroup(); - CurrentThread::attachToGroup(merge_list_entry_->thread_group); + CurrentThread::detachFromGroupIfNotDetached(); + CurrentThread::attachToGroup(thread_group); } ThreadGroupSwitcher::~ThreadGroupSwitcher() @@ -27,37 +28,10 @@ ThreadGroupSwitcher::~ThreadGroupSwitcher() if (!prev_thread_group) return; - if (!merge_list_entry) - return; - - CurrentThread::detachGroup(); + CurrentThread::detachFromGroupIfNotDetached(); CurrentThread::attachToGroup(prev_thread_group); } -ThreadGroupSwitcher::ThreadGroupSwitcher(ThreadGroupSwitcher && other) noexcept -{ - this->swap(other); -} - -ThreadGroupSwitcher& ThreadGroupSwitcher::operator=(ThreadGroupSwitcher && other) noexcept -{ - if (this != &other) - { - auto tmp = ThreadGroupSwitcher(); - tmp.swap(other); - this->swap(tmp); - } - return *this; -} - -void ThreadGroupSwitcher::swap(ThreadGroupSwitcher & other) noexcept -{ - std::swap(merge_list_entry, other.merge_list_entry); - std::swap(prev_thread_group, other.prev_thread_group); - std::swap(prev_query_id, other.prev_query_id); -} - - MergeListElement::MergeListElement( const StorageID & table_id_, FutureMergedMutatedPartPtr future_part, @@ -90,18 +64,13 @@ MergeListElement::MergeListElement( is_mutation = (result_part_info.getDataVersion() != source_data_version); } - thread_group = std::make_shared(); - - thread_group->query_context = CurrentThread::get().getQueryContext(); - thread_group->global_context = CurrentThread::get().getGlobalContext(); + thread_group = ThreadGroupStatus::createForQuery(CurrentThread::get().getQueryContext(), {}); auto * p_counters = CurrentThread::get().current_performance_counters; while (p_counters && p_counters->level != VariableContext::Process) p_counters = p_counters->getParent(); thread_group->performance_counters.setParent(p_counters); - thread_group->master_thread_id = CurrentThread::get().thread_id; - auto & memory_tracker = thread_group->memory_tracker; memory_tracker.setDescription(description.c_str()); diff --git a/src/Storages/MergeTree/MergeList.h b/src/Storages/MergeTree/MergeList.h index 2a166470203..37587d9d517 100644 --- a/src/Storages/MergeTree/MergeList.h +++ b/src/Storages/MergeTree/MergeList.h @@ -69,15 +69,23 @@ struct Settings; class ThreadGroupSwitcher : private boost::noncopyable { public: - explicit ThreadGroupSwitcher(MergeListEntry * merge_list_entry_); - ThreadGroupSwitcher(ThreadGroupSwitcher && other) noexcept; - ThreadGroupSwitcher& operator=(ThreadGroupSwitcher && other) noexcept; - ~ThreadGroupSwitcher(); -private: ThreadGroupSwitcher() noexcept = default; - void swap(ThreadGroupSwitcher & other) noexcept; + explicit ThreadGroupSwitcher(ThreadGroupStatusPtr thread_group); + ThreadGroupSwitcher(ThreadGroupSwitcher && other) noexcept + : prev_thread_group(std::move(other.prev_thread_group)) + { + other.prev_thread_group = nullptr; + } + ThreadGroupSwitcher & operator=(ThreadGroupSwitcher && other) noexcept + { + chassert(this != &other); + prev_thread_group = std::move(other.prev_thread_group); + other.prev_thread_group = nullptr; + return *this; + } + ~ThreadGroupSwitcher(); - MergeListEntry * merge_list_entry = nullptr; +private: ThreadGroupStatusPtr prev_thread_group; }; diff --git a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp index 052c6467b37..709a681619a 100644 --- a/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MergePlainMergeTreeTask.cpp @@ -34,7 +34,7 @@ bool MergePlainMergeTreeTask::executeStep() /// Make out memory tracker a parent of current thread memory tracker std::optional switcher; if (merge_list_entry) - switcher = ThreadGroupSwitcher(merge_list_entry.get()); + switcher = ThreadGroupSwitcher((*merge_list_entry)->thread_group); switch (state) { diff --git a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp index 673bfaa0d47..822098e4352 100644 --- a/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp +++ b/src/Storages/MergeTree/MutatePlainMergeTreeTask.cpp @@ -70,7 +70,7 @@ bool MutatePlainMergeTreeTask::executeStep() /// Make out memory tracker a parent of current thread memory tracker std::optional switcher; if (merge_list_entry) - switcher = ThreadGroupSwitcher(merge_list_entry.get()); + switcher = ThreadGroupSwitcher((*merge_list_entry)->thread_group); switch (state) { diff --git a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp index fbc6522170e..9368f7d8c51 100644 --- a/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeMutateTaskBase.cpp @@ -130,7 +130,7 @@ bool ReplicatedMergeMutateTaskBase::executeImpl() { std::optional switcher; if (merge_mutate_entry) - switcher = ThreadGroupSwitcher(merge_mutate_entry.get()); + switcher = ThreadGroupSwitcher((*merge_mutate_entry)->thread_group); auto remove_processed_entry = [&] () -> bool { From 22da93e239ffd4402ba27aee4c982742082cc9fc Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Thu, 23 Mar 2023 21:41:01 +0000 Subject: [PATCH 07/68] Cosmetics --- src/Functions/formatDateTime.cpp | 99 +++++++++++++++----------------- src/Functions/parseDateTime.cpp | 26 ++++----- 2 files changed, 58 insertions(+), 67 deletions(-) diff --git a/src/Functions/formatDateTime.cpp b/src/Functions/formatDateTime.cpp index bbb4c3ba5b0..daea8b3a7b0 100644 --- a/src/Functions/formatDateTime.cpp +++ b/src/Functions/formatDateTime.cpp @@ -39,21 +39,17 @@ namespace ErrorCodes namespace { -struct FormatDateTimeTraits +enum class SupportInteger { - enum class SupportInteger - { - Yes, - No - }; - - enum class FormatSyntax - { - MySQL, - Joda - }; + Yes, + No }; +enum class FormatSyntax +{ + MySQL, + Joda +}; template struct InstructionValueTypeMap {}; template <> struct InstructionValueTypeMap { using InstructionValueType = UInt32; }; @@ -85,11 +81,9 @@ constexpr std::string_view weekdaysFull[] = {"Sunday", "Monday", "Tuesday", "Wed constexpr std::string_view weekdaysShort[] = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"}; -constexpr std::string_view monthsFull[] - = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"}; +constexpr std::string_view monthsFull[] = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"}; -constexpr std::string_view monthsShort[] - = {"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}; +constexpr std::string_view monthsShort[] = {"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}; /** formatDateTime(time, 'format') * Performs formatting of time, according to provided format. @@ -129,7 +123,7 @@ constexpr std::string_view monthsShort[] * * PS. We can make this function to return FixedString. Currently it returns String. */ -template +template class FunctionFormatDateTimeImpl : public IFunction { private: @@ -157,7 +151,7 @@ private: /// This is the reason why we use raw function pointer in MySQL format and std::function /// in Joda format. using Func = std::conditional_t< - format_syntax == FormatDateTimeTraits::FormatSyntax::MySQL, + format_syntax == FormatSyntax::MySQL, size_t (*)(char *, Time, UInt64, UInt32, const DateLUTImpl &), std::function>; @@ -257,7 +251,10 @@ private: return pos; } public: - static size_t mysqlNoop(char *, Time, UInt64, UInt32, const DateLUTImpl &) { return 0; } + static size_t mysqlNoop(char *, Time, UInt64, UInt32, const DateLUTImpl &) + { + return 0; + } static size_t mysqlCentury(char * dest, Time source, UInt64, UInt32, const DateLUTImpl & timezone) { @@ -430,8 +427,7 @@ private: return writeNumber2(dest, ToSecondImpl::execute(source, timezone)); } - static size_t - mysqlFractionalSecond(char * dest, Time /*source*/, UInt64 fractional_second, UInt32 scale, const DateLUTImpl & /*timezone*/) + static size_t mysqlFractionalSecond(char * dest, Time /*source*/, UInt64 fractional_second, UInt32 scale, const DateLUTImpl & /*timezone*/) { if (scale == 0) scale = 1; @@ -672,7 +668,7 @@ public: DataTypePtr getReturnTypeImpl(const ColumnsWithTypeAndName & arguments) const override { - if constexpr (support_integer == FormatDateTimeTraits::SupportInteger::Yes) + if constexpr (support_integer == SupportInteger::Yes) { if (arguments.size() != 1 && arguments.size() != 2 && arguments.size() != 3) throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, @@ -718,7 +714,7 @@ public: ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr & result_type, [[maybe_unused]] size_t input_rows_count) const override { ColumnPtr res; - if constexpr (support_integer == FormatDateTimeTraits::SupportInteger::Yes) + if constexpr (support_integer == SupportInteger::Yes) { if (arguments.size() == 1) { @@ -793,7 +789,7 @@ public: using T = typename InstructionValueTypeMap::InstructionValueType; std::vector> instructions; String out_template; - auto result_size = parseFormat(format, instructions, scale, out_template); + size_t out_template_size = parseFormat(format, instructions, scale, out_template); const DateLUTImpl * time_zone_tmp = nullptr; if (castType(arguments[0].type.get(), [&]([[maybe_unused]] const auto & type) { return true; })) @@ -807,26 +803,26 @@ public: const auto & vec = times->getData(); auto col_res = ColumnString::create(); - auto & dst_data = col_res->getChars(); - auto & dst_offsets = col_res->getOffsets(); - dst_data.resize(vec.size() * (result_size + 1)); - dst_offsets.resize(vec.size()); + auto & res_data = col_res->getChars(); + auto & res_offsets = col_res->getOffsets(); + res_data.resize(vec.size() * (out_template_size + 1)); + res_offsets.resize(vec.size()); - if constexpr (format_syntax == FormatDateTimeTraits::FormatSyntax::MySQL) + if constexpr (format_syntax == FormatSyntax::MySQL) { - /// Fill result with literals. + /// Fill result with template. { - UInt8 * begin = dst_data.data(); - UInt8 * end = begin + dst_data.size(); - UInt8 * pos = begin; + const UInt8 * const begin = res_data.data(); + const UInt8 * const end = res_data.data() + res_data.size(); + UInt8 * pos = res_data.data(); if (pos < end) { - memcpy(pos, out_template.data(), result_size + 1); /// With zero terminator. - pos += result_size + 1; + memcpy(pos, out_template.data(), out_template_size + 1); /// With zero terminator. mystring[mystring.size()] = '\0' is guaranteed since C++11. + pos += out_template_size + 1; } - /// Fill by copying exponential growing ranges. + /// Copy exponentially growing ranges. while (pos < end) { size_t bytes_to_copy = std::min(pos - begin, end - pos); @@ -836,7 +832,7 @@ public: } } - auto * begin = reinterpret_cast(dst_data.data()); + auto * begin = reinterpret_cast(res_data.data()); auto * pos = begin; for (size_t i = 0; i < vec.size(); ++i) { @@ -844,9 +840,7 @@ public: { const auto c = DecimalUtils::split(vec[i], scale); for (auto & instruction : instructions) - { instruction.perform(pos, static_cast(c.whole), c.fractional, scale, time_zone); - } } else { @@ -855,21 +849,19 @@ public: } *pos++ = '\0'; - dst_offsets[i] = pos - begin; + res_offsets[i] = pos - begin; } - dst_data.resize(pos - begin); + res_data.resize(pos - begin); return col_res; } template size_t parseFormat(const String & format, std::vector> & instructions, UInt32 scale, String & out_template) const { - static_assert( - format_syntax == FormatDateTimeTraits::FormatSyntax::MySQL || format_syntax == FormatDateTimeTraits::FormatSyntax::Joda, - "format syntax must be one of MySQL or Joda"); + static_assert(format_syntax == FormatSyntax::MySQL || format_syntax == FormatSyntax::Joda); - if constexpr (format_syntax == FormatDateTimeTraits::FormatSyntax::MySQL) + if constexpr (format_syntax == FormatSyntax::MySQL) return parseMySQLFormat(format, instructions, scale, out_template); else return parseJodaFormat(format, instructions, scale, out_template); @@ -914,13 +906,13 @@ public: switch (*pos) { - // Abbreviated weekday [Mon...Sun] + // Abbreviated weekday [Mon-Sun] case 'a': instructions.emplace_back(&Instruction::mysqlDayOfWeekTextShort); out_template += "Mon"; break; - // Abbreviated month [Jan...Dec] + // Abbreviated month [Jan-Dec] case 'b': instructions.emplace_back(&Instruction::mysqlMonthOfYearTextShort); out_template += "Jan"; @@ -958,12 +950,10 @@ public: // Fractional seconds case 'f': - { /// If the time data type has no fractional part, then we print '0' as the fractional part. instructions.emplace_back(&Instruction::mysqlFractionalSecond); out_template += String(std::max(1, scale), '0'); break; - } // Short YYYY-MM-DD date, equivalent to %Y-%m-%d 2001-08-23 case 'F': @@ -1013,7 +1003,7 @@ public: out_template += "0"; break; - // Full weekday [Monday...Sunday] + // Full weekday [Monday-Sunday] case 'W': instructions.emplace_back(&Instruction::mysqlDayOfWeekTextLong); out_template += "Monday"; @@ -1186,6 +1176,7 @@ public: size_t reserve_size = 0; const char * pos = format.data(); const char * end = format.data() + format.size(); + while (pos < end) { const char * cur_token = pos; @@ -1392,10 +1383,10 @@ struct NameFromUnixTimeInJodaSyntax }; -using FunctionFormatDateTime = FunctionFormatDateTimeImpl; -using FunctionFromUnixTimestamp = FunctionFormatDateTimeImpl; -using FunctionFormatDateTimeInJodaSyntax = FunctionFormatDateTimeImpl; -using FunctionFromUnixTimestampInJodaSyntax = FunctionFormatDateTimeImpl; +using FunctionFormatDateTime = FunctionFormatDateTimeImpl; +using FunctionFromUnixTimestamp = FunctionFormatDateTimeImpl; +using FunctionFormatDateTimeInJodaSyntax = FunctionFormatDateTimeImpl; +using FunctionFromUnixTimestampInJodaSyntax = FunctionFormatDateTimeImpl; } diff --git a/src/Functions/parseDateTime.cpp b/src/Functions/parseDateTime.cpp index abee7e0d8f8..cd3c0d993d0 100644 --- a/src/Functions/parseDateTime.cpp +++ b/src/Functions/parseDateTime.cpp @@ -101,16 +101,16 @@ namespace bool is_year_of_era = false; /// If true, year is calculated from era and year of era, the latter cannot be zero or negative. bool has_year = false; /// Whether year was explicitly specified. - /// If is_clock_hour = true, is_hour_of_half_day = true, hour's range is [1, 12] - /// If is_clock_hour = true, is_hour_of_half_day = false, hour's range is [1, 24] - /// If is_clock_hour = false, is_hour_of_half_day = true, hour's range is [0, 11] - /// If is_clock_hour = false, is_hour_of_half_day = false, hour's range is [0, 23] + /// If hour_starts_at_1 = true, is_hour_of_half_day = true, hour's range is [1, 12] + /// If hour_starts_at_1 = true, is_hour_of_half_day = false, hour's range is [1, 24] + /// If hour_starts_at_1 = false, is_hour_of_half_day = true, hour's range is [0, 11] + /// If hour_starts_at_1 = false, is_hour_of_half_day = false, hour's range is [0, 23] Int32 hour = 0; Int32 minute = 0; /// range [0, 59] Int32 second = 0; /// range [0, 59] bool is_am = true; /// If is_hour_of_half_day = true and is_am = false (i.e. pm) then add 12 hours to the result DateTime - bool is_clock_hour = false; /// Whether the hour is clockhour + bool hour_starts_at_1 = false; /// Whether the hour is clockhour bool is_hour_of_half_day = false; /// Whether the hour is of half day bool has_time_zone_offset = false; /// If true, time zone offset is explicitly specified. @@ -137,7 +137,7 @@ namespace second = 0; is_am = true; - is_clock_hour = false; + hour_starts_at_1 = false; is_hour_of_half_day = false; has_time_zone_offset = false; @@ -275,23 +275,23 @@ namespace throw Exception(ErrorCodes::CANNOT_PARSE_DATETIME, "Unknown half day of day: {}", text); } - void setHour(Int32 hour_, bool is_hour_of_half_day_ = false, bool is_clock_hour_ = false) + void setHour(Int32 hour_, bool is_hour_of_half_day_ = false, bool hour_starts_at_1_ = false) { Int32 max_hour; Int32 min_hour; Int32 new_hour = hour_; - if (!is_hour_of_half_day_ && !is_clock_hour_) + if (!is_hour_of_half_day_ && !hour_starts_at_1_) { max_hour = 23; min_hour = 0; } - else if (!is_hour_of_half_day_ && is_clock_hour_) + else if (!is_hour_of_half_day_ && hour_starts_at_1_) { max_hour = 24; min_hour = 1; new_hour = hour_ % 24; } - else if (is_hour_of_half_day_ && !is_clock_hour_) + else if (is_hour_of_half_day_ && !hour_starts_at_1_) { max_hour = 11; min_hour = 0; @@ -306,16 +306,16 @@ namespace if (hour_ < min_hour || hour_ > max_hour) throw Exception( ErrorCodes::CANNOT_PARSE_DATETIME, - "Value {} for hour must be in the range [{}, {}] if_hour_of_half_day={} and is_clock_hour={}", + "Value {} for hour must be in the range [{}, {}] if_hour_of_half_day={} and hour_starts_at_1={}", hour, max_hour, min_hour, is_hour_of_half_day_, - is_clock_hour_); + hour_starts_at_1_); hour = new_hour; is_hour_of_half_day = is_hour_of_half_day_; - is_clock_hour = is_clock_hour_; + hour_starts_at_1 = hour_starts_at_1_; } void setMinute(Int32 minute_) From 3db38dbb5a1a227c749549faf4ec0f140bc267b7 Mon Sep 17 00:00:00 2001 From: Robert Schulze Date: Thu, 23 Mar 2023 22:16:10 +0000 Subject: [PATCH 08/68] Replace mySQL date formatter M behavior from minutes to month name --- .../functions/date-time-functions.md | 8 +- src/Functions/formatDateTime.cpp | 576 +++++++++++++----- src/Functions/parseDateTime.cpp | 40 +- .../00718_format_datetime.reference | 4 +- .../0_stateless/00718_format_datetime.sql | 3 + ...00921_datetime64_compatibility_long.python | 2 +- ...21_datetime64_compatibility_long.reference | 2 +- .../0_stateless/01411_from_unixtime.reference | 2 +- .../0_stateless/02564_date_format.reference | 2 +- .../02668_parse_datetime.reference | 7 + .../0_stateless/02668_parse_datetime.sql | 6 +- 11 files changed, 493 insertions(+), 159 deletions(-) diff --git a/docs/en/sql-reference/functions/date-time-functions.md b/docs/en/sql-reference/functions/date-time-functions.md index d06ab253cf7..425d67ed5a0 100644 --- a/docs/en/sql-reference/functions/date-time-functions.md +++ b/docs/en/sql-reference/functions/date-time-functions.md @@ -1276,16 +1276,16 @@ Using replacement fields, you can define a pattern for the resulting string. “ | %k | hour in 24h format (00-23) | 22 | | %l | hour in 12h format (01-12) | 09 | | %m | month as an integer number (01-12) | 01 | -| %M | minute (00-59) | 33 | +| %M | full month name (January-December) | January | | %n | new-line character (‘’) | | | %p | AM or PM designation | PM | | %Q | Quarter (1-4) | 1 | -| %r | 12-hour HH:MM AM/PM time, equivalent to %H:%M %p | 10:30 PM | -| %R | 24-hour HH:MM time, equivalent to %H:%M | 22:33 | +| %r | 12-hour HH:MM AM/PM time, equivalent to %H:%i %p | 10:30 PM | +| %R | 24-hour HH:MM time, equivalent to %H:%i | 22:33 | | %s | second (00-59) | 44 | | %S | second (00-59) | 44 | | %t | horizontal-tab character (’) | | -| %T | ISO 8601 time format (HH:MM:SS), equivalent to %H:%M:%S | 22:33:44 | +| %T | ISO 8601 time format (HH:MM:SS), equivalent to %H:%i:%S | 22:33:44 | | %u | ISO 8601 weekday as number with Monday as 1 (1-7) | 2 | | %V | ISO 8601 week number (01-53) | 01 | | %w | weekday as a integer number with Sunday as 0 (0-6) | 2 | diff --git a/src/Functions/formatDateTime.cpp b/src/Functions/formatDateTime.cpp index daea8b3a7b0..c243222db91 100644 --- a/src/Functions/formatDateTime.cpp +++ b/src/Functions/formatDateTime.cpp @@ -109,13 +109,13 @@ constexpr std::string_view monthsShort[] = {"Jan", "Feb", "Mar", "Apr", "May", " * * Performance on Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz: * - * WITH formatDateTime(now() + number, '%H:%M:%S') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x); + * WITH formatDateTime(now() + number, '%H:%i:%S') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x); * - 97 million rows per second per core; * * WITH formatDateTime(toDateTime('2018-01-01 00:00:00') + number, '%F %T') AS x SELECT count() FROM system.numbers WHERE NOT ignore(x) * - 71 million rows per second per core; * - * select count() from (select formatDateTime(t, '%m/%d/%Y %H:%M:%S') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); + * select count() from (select formatDateTime(t, '%m/%d/%Y %H:%i:%S') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); * - 53 million rows per second per core; * * select count() from (select formatDateTime(t, 'Hello %Y World') from (select toDateTime('2018-01-01 00:00:00')+number as t from numbers(100000000))); @@ -146,26 +146,34 @@ private: class Instruction { public: - /// Using std::function will cause performance degradation in MySQL format by 0.45x. - /// But std::function is required for Joda format to capture extra variables. - /// This is the reason why we use raw function pointer in MySQL format and std::function - /// in Joda format. - using Func = std::conditional_t< - format_syntax == FormatSyntax::MySQL, - size_t (*)(char *, Time, UInt64, UInt32, const DateLUTImpl &), - std::function>; + /// Joda format generally requires capturing extra variables (i.e. holding state) which is more convenient with + /// std::function and std::bind. Unfortunately, std::function causes a performance degradation by 0.45x compared to raw function + /// pointers. For MySQL format, we generally prefer raw function pointers. Because of the special case that not all formatters are + /// fixed-width formatters (see mysqlLiteral), we still need to be able to store state. For that reason, we use member function + /// pointers instead of static function pointers. + using FuncMysql = size_t (Instruction