Fix review issues and format code

Author: bagrorg

omniscidb/QueryEngine/AnalyticalTemplatesExtractor.cpp

@@ -1,71 +1,60 @@
+/*
+    Copyright (c) 2023 Intel Corporation
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+        http://www.apache.org/licenses/LICENSE-2.0
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
 #include "AnalyticalTemplatesExtractor.h"
 #include <limits>
 #include <stdexcept>
 #include <vector>
 #include "QueryEngine/CostModel/Measurements.h"
 #include "QueryEngine/RelAlgExecutionUnit.h"
 
+std::vector<costmodel::AnalyticalTemplate> AnalyticalTemplatesExtractor::extractTemplates(
+    const RelAlgExecutionUnit& ra_exe_unit) const {
+  Priority current_priority = std::numeric_limits<Priority>::max();
+  std::vector<costmodel::AnalyticalTemplate> templates;
 
-std::vector<costmodel::AnalyticalTemplate> AnalyticalTemplatesExtractor::extractTemplates(const RelAlgExecutionUnit &ra_exe_unit) const {
-    Priority current_priority = std::numeric_limits<Priority>::max();
-    std::vector<costmodel::AnalyticalTemplate> templates;
-
-    for (auto [priority, templ]: priotities_) {
-        if (priority > current_priority) {
-            return templates;
-        }
-
-        if (templSuits(ra_exe_unit, templ)) {
-            current_priority = priority;
-            templates.push_back(templ);
-        }
+  for (auto [priority, templ] : priotities_) {
+    if (priority > current_priority) {
+      return templates;
     }
 
-    return templates;
-}
-
-template <>
-bool AnalyticalTemplatesExtractor::templSuits<costmodel::AnalyticalTemplate::GroupBy>(const RelAlgExecutionUnit &ra_exe_unit) const {
-    return !ra_exe_unit.groupby_exprs.empty();
-}
-
-template <>
-bool AnalyticalTemplatesExtractor::templSuits<costmodel::AnalyticalTemplate::Join>(const RelAlgExecutionUnit &ra_exe_unit) const {
-    return !ra_exe_unit.join_quals.empty();
-}
-
-template <>
-bool AnalyticalTemplatesExtractor::templSuits<costmodel::AnalyticalTemplate::Scan>(const RelAlgExecutionUnit &ra_exe_unit) const {
-    return !ra_exe_unit.quals.empty() || !ra_exe_unit.simple_quals.empty();
-}
-
-template <>
-bool AnalyticalTemplatesExtractor::templSuits<costmodel::AnalyticalTemplate::Sort>(const RelAlgExecutionUnit &ra_exe_unit) const {
-    return !ra_exe_unit.sort_info.order_entries.empty();
-}
+    if (templSuits(ra_exe_unit, templ)) {
+      current_priority = priority;
+      templates.push_back(templ);
+    }
+  }
 
-template <>
-bool AnalyticalTemplatesExtractor::templSuits<costmodel::AnalyticalTemplate::Reduce>(const RelAlgExecutionUnit &ra_exe_unit) const {
-    // TODO(bagrorg): currently we don't even
-    // use Reduce for cost model
-    return false;
+  return templates;
 }
 
-bool AnalyticalTemplatesExtractor::templSuits(const RelAlgExecutionUnit &ra_exe_unit, costmodel::AnalyticalTemplate templ) const {
-    using costmodel::AnalyticalTemplate;
+bool AnalyticalTemplatesExtractor::templSuits(const RelAlgExecutionUnit& ra_exe_unit,
+                                              costmodel::AnalyticalTemplate templ) const {
+  using costmodel::AnalyticalTemplate;
 
-    switch (templ) {
+  switch (templ) {
     case AnalyticalTemplate::GroupBy:
-        return templSuits<AnalyticalTemplate::GroupBy>(ra_exe_unit);
+      return !ra_exe_unit.groupby_exprs.empty();
     case AnalyticalTemplate::Join:
-        return templSuits<AnalyticalTemplate::Join>(ra_exe_unit);
+      return !ra_exe_unit.join_quals.empty();
     case AnalyticalTemplate::Sort:
-        return templSuits<AnalyticalTemplate::Sort>(ra_exe_unit);
+      return !ra_exe_unit.sort_info.order_entries.empty();
     case AnalyticalTemplate::Reduce:
-        return templSuits<AnalyticalTemplate::Reduce>(ra_exe_unit);
+      // TODO(bagrorg): currently we don't even
+      // use Reduce for cost model
+      return false;
     case AnalyticalTemplate::Scan:
-        return templSuits<AnalyticalTemplate::Scan>(ra_exe_unit);
+      return !ra_exe_unit.quals.empty() || !ra_exe_unit.simple_quals.empty();
     default:
-        throw std::runtime_error("Unknown template for check was given");
-    }
+      throw std::runtime_error("Unknown template for check was given");
+  }
 }

omniscidb/QueryEngine/AnalyticalTemplatesExtractor.h

@@ -1,26 +1,38 @@
+/*
+    Copyright (c) 2023 Intel Corporation
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+        http://www.apache.org/licenses/LICENSE-2.0
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
 #pragma once
 
 #include "QueryEngine/CostModel/Measurements.h"
 #include "QueryEngine/RelAlgExecutionUnit.h"
 
 class AnalyticalTemplatesExtractor {
-public:
-    std::vector<costmodel::AnalyticalTemplate> extractTemplates(const RelAlgExecutionUnit &ra_exe_unit) const;
+ public:
+  std::vector<costmodel::AnalyticalTemplate> extractTemplates(
+      const RelAlgExecutionUnit& ra_exe_unit) const;
 
-private:
-    using Priority = uint8_t;
+ private:
+  using Priority = uint8_t;
 
-    template <costmodel::AnalyticalTemplate templ>
-    bool templSuits(const RelAlgExecutionUnit &ra_exe_unit) const;
+  bool templSuits(const RelAlgExecutionUnit& ra_exe_unit,
+                  costmodel::AnalyticalTemplate templ) const;
 
-    bool templSuits(const RelAlgExecutionUnit &ra_exe_unit, costmodel::AnalyticalTemplate templ) const;
-    
-    // We want to greedy extract templates
-    std::map<Priority, costmodel::AnalyticalTemplate> priotities_ = {
-        {0, costmodel::AnalyticalTemplate::GroupBy},
-        {0, costmodel::AnalyticalTemplate::Join},
-        {0, costmodel::AnalyticalTemplate::Sort},
-        {1, costmodel::AnalyticalTemplate::Scan},
-        {1, costmodel::AnalyticalTemplate::Reduce},
-    };
+  // We want to greedy extract templates
+  const std::map<Priority, costmodel::AnalyticalTemplate> priotities_ = {
+      {0, costmodel::AnalyticalTemplate::GroupBy},
+      {0, costmodel::AnalyticalTemplate::Join},
+      {0, costmodel::AnalyticalTemplate::Sort},
+      {1, costmodel::AnalyticalTemplate::Scan},
+      {1, costmodel::AnalyticalTemplate::Reduce},
+  };
 };

omniscidb/QueryEngine/CMakeLists.txt

@@ -120,13 +120,12 @@ set(query_engine_source_files
     DataRecycler/HashingSchemeRecycler.cpp
     Visitors/QueryPlanDagChecker.cpp
     WorkUnitBuilder.cpp
+    AnalyticalTemplatesExtractor.cpp
 
     Codec.h
     Execute.h
     NvidiaKernel.h
-    QueryTemplateGenerator.h
-    
-    AnalyticalTemplatesExtractor.cpp)
+    QueryTemplateGenerator.h)
 
 set(query_engine_cuda_source_files
     TopKSort.cu

omniscidb/QueryEngine/Execute.cpp

@@ -43,6 +43,7 @@
 #include "OSDependent/omnisci_path.h"
 #include "QueryEngine/AggregateUtils.h"
 #include "QueryEngine/AggregatedColRange.h"
+#include "QueryEngine/AnalyticalTemplatesExtractor.h"
 #include "QueryEngine/CodeGenerator.h"
 #include "QueryEngine/ColumnFetcher.h"
 #include "QueryEngine/CostModel/Dispatchers/DefaultExecutionPolicy.h"
@@ -78,7 +79,6 @@
 #include "Shared/misc.h"
 #include "Shared/scope.h"
 #include "ThirdParty/robin_hood.h"
-#include "QueryEngine/AnalyticalTemplatesExtractor.h"
 
 #include "CostModel/IterativeCostModel.h"
 
@@ -2127,7 +2127,8 @@ std::unique_ptr<policy::ExecutionPolicy> Executor::getExecutionPolicy(
     CHECK(cfg.enable_heterogeneous_execution);
 
     AnalyticalTemplatesExtractor extractor;
-    std::vector<costmodel::AnalyticalTemplate> templates = extractor.extractTemplates(ra_exe_unit);
+    std::vector<costmodel::AnalyticalTemplate> templates =
+        extractor.extractTemplates(ra_exe_unit);
     if (config_->exec.enable_cost_model && ra_exe_unit.cost_model != nullptr &&
         !templates.empty()) {
       size_t bytes = 0;