Merge branch 'master' into ci-compiled-junit-tests

Jenkinsfile

@@ -165,7 +165,7 @@ pipeline {
                     agent {
                         docker {
                             image 'noisepage:focal'
-                            label 'h0-only'
+                            label 'dgb'
                             args '--cap-add sys_ptrace -v /jenkins/ccache:/home/jenkins/.ccache'
                         }
                     }
@@ -518,6 +518,7 @@ pipeline {
                     agent {
                         docker {
                             image 'noisepage:focal'
+                            label 'dgb'
                             args '--cap-add sys_ptrace -v /jenkins/ccache:/home/jenkins/.ccache'
                         }
                     }
@@ -594,6 +595,10 @@ pipeline {
 
                     }
                     post {
+                        always {
+                            archiveArtifacts(artifacts: 'build/Testing/**/*.xml', fingerprint: true)
+                            xunit reduceLog: false, tools: [CTest(deleteOutputFiles: false, failIfNotNew: false, pattern: 'build/Testing/**/*.xml', skipNoTestFiles: false, stopProcessingIfError: false)]
+                        }
                         cleanup {
                             deleteDir()
                         }

docs/cpp_guidelines_code_style.md

@@ -190,4 +190,56 @@ in the header or explicit template instantiation must be used. Exposing implemen
 adding more `#include`s to the header, which can lead to a painful blowup of compile time, so the latter is preferred.
 
 You may also be interested in the cppreference for [std::enable_if](https://en.cppreference.com/w/cpp/types/enable_if),
-which allows you to compile out functions entirely. However, this may lead to mystery linking errors in tests.
+which allows you to compile out functions entirely. However, this may lead to mystery linking errors in tests.
+
+### Defining enumerations
+
+1. C++ enums serialize as a number. This is annoying for debugging purposes.
+2. Writing a ToString and FromString for every member of an enum also sucks.
+3. We can't use the version provided by our JSON library (NLOHMANN_JSON_SERIALIZE_ENUM) because that requires including
+   a HUGE header, blowing up compile time.
+4. By using the macro in `enum_defs.h`, you will get an automatic ToString and FromString that is kept in sync.
+
+The macro is `ENUM_DEFINE(EnumName, EnumCppType, EnumValMacro)`:
+
+- `EnumName` is the name of your enum.
+- `EnumCppType` is the underlying C++ type of your enum, for example, `uint8_t`.
+- `EnumValMacro` is a macro that you must define, taking exactly one macro T as an argument, and applying T to the (
+  EnumName, EnumMemberName).
+
+The macro defines the enum with one additional field (NUM_ENUM_ENTRIES) and two corresponding EnumNameToString and
+EnumNameFromString functions.
+
+This is illustrated in the following truncated example:
+
+```
+// First, define a macro-taking-a-macro with all your enum members.
+#define EXPRESSION_TYPE_ENUM(T)                     \
+    T(ExpressionType, INVALID)                      \
+                                                    \
+    T(ExpressionType, OPERATOR_UNARY_MINUS)         \
+    T(ExpressionType, OPERATOR_PLUS)                \
+    T(ExpressionType, OPERATOR_MINUS)               \
+    T(ExpressionType, OPERATOR_MULTIPLY)            \
+    T(ExpressionType, OPERATOR_DIVIDE)              \
+    T(ExpressionType, OPERATOR_CONCAT)              \
+    T(ExpressionType, OPERATOR_MOD)                 \
+    ...
+
+// Next, define the enum itself.
+ENUM_DEFINE(ExpressionType, uint8_t, EXPRESSION_TYPE_ENUM);
+
+// This is equivalent to
+enum ExpressionType : uint8_t { INVALID, OPERATOR_UNARY_MINUS, ..., NUM_ENUM_ENTRIES }
+std::string ExpressionTypeToString(ExpressionType type);
+ExpressionType ExpressionTypeFromString(const std::string &str);
+
+// Finally, clean up your macro for macro hygiene.
+#undef EXPRESSION_TYPE_ENUM
+```
+
+Note that unfortunately `enum_defs.h` leaks the following unrelated macro definitions:
+
+- `ENUM_DEFINE_ITEM`
+- `ENUM_TO_STRING`
+- `ENUM_FROM_STRING`

src/execution/compiler/codegen.cpp

@@ -772,7 +772,7 @@ ast::Expr *CodeGen::VPIFilter(ast::Expr *exec_ctx, ast::Expr *vp, parser::Expres
       break;
     default:
       throw NOT_IMPLEMENTED_EXCEPTION(fmt::format("CodeGen: Vector filter type {} from VPI not supported.",
-                                                  parser::ExpressionTypeToString(comp_type, true)));
+                                                  parser::ExpressionTypeToString(comp_type)));
   }
   ast::Expr *call = CallBuiltin(builtin, {exec_ctx, vp, Const32(col_idx), filter_val, tids});
   call->SetType(ast::BuiltinType::Get(context_, ast::BuiltinType::Nil));

src/execution/compiler/compilation_context.cpp

@@ -388,9 +388,8 @@ void CompilationContext::Prepare(const parser::AbstractExpression &expression) {
       break;
     }
     default: {
-      throw NOT_IMPLEMENTED_EXCEPTION(
-          fmt::format("Code generation for expression type '{}' not supported.",
-                      parser::ExpressionTypeToString(expression.GetExpressionType(), false)));
+      throw NOT_IMPLEMENTED_EXCEPTION(fmt::format("Code generation for expression type '{}' not supported.",
+                                                  parser::ExpressionTypeToString(expression.GetExpressionType())));
     }
   }
 

src/execution/compiler/expression/arithmetic_translator.cpp

@@ -35,7 +35,7 @@ ast::Expr *ArithmeticTranslator::DeriveValue(WorkContext *ctx, const ColumnValue
       return codegen->BinaryOp(parsing::Token::Type::PERCENT, left_val, right_val);
     default: {
       throw NOT_IMPLEMENTED_EXCEPTION(
-          fmt::format("Translation of arithmetic type {}", parser::ExpressionTypeToString(expr_type, true)));
+          fmt::format("Translation of arithmetic type {}", parser::ExpressionTypeToString(expr_type)));
     }
   }
 }

src/execution/compiler/expression/comparison_translator.cpp

@@ -58,7 +58,7 @@ ast::Expr *ComparisonTranslator::DeriveValue(WorkContext *ctx, const ColumnValue
       return codegen->NotLike(left_val, right_val);
     default: {
       throw NOT_IMPLEMENTED_EXCEPTION(
-          fmt::format("Translation of comparison type {}", parser::ExpressionTypeToString(expr_type, true)));
+          fmt::format("Translation of comparison type {}", parser::ExpressionTypeToString(expr_type)));
     }
   }
 }

src/execution/compiler/expression/conjunction_translator.cpp

@@ -30,7 +30,7 @@ ast::Expr *ConjunctionTranslator::DeriveValue(WorkContext *ctx, const ColumnValu
       return codegen->BinaryOp(parsing::Token::Type::OR, left_val, right_val);
     default: {
       throw NOT_IMPLEMENTED_EXCEPTION(
-          fmt::format("Translation of conjunction type {}", parser::ExpressionTypeToString(expr_type, true)));
+          fmt::format("Translation of conjunction type {}", parser::ExpressionTypeToString(expr_type)));
     }
   }
 }

src/execution/compiler/expression/null_check_translator.cpp

@@ -23,8 +23,7 @@ ast::Expr *NullCheckTranslator::DeriveValue(WorkContext *ctx, const ColumnValueP
     case parser::ExpressionType::OPERATOR_IS_NOT_NULL:
       return codegen->UnaryOp(parsing::Token::Type::BANG, codegen->CallBuiltin(ast::Builtin::IsValNull, {input}));
     default:
-      throw NOT_IMPLEMENTED_EXCEPTION(
-          fmt::format("operator expression type {}", parser::ExpressionTypeToString(type, false)));
+      throw NOT_IMPLEMENTED_EXCEPTION(fmt::format("operator expression type {}", parser::ExpressionTypeToString(type)));
   }
 }
 

src/include/common/enum_defs.h

@@ -0,0 +1,54 @@
+#pragma once
+
+#include <string>
+
+#include "common/error/exception.h"
+
+/** Used in defining the actual enum values. */
+#define ENUM_DEFINE_ITEM(EnumType, EnumVal) EnumVal,
+
+/** Used in defining EnumTypeToString(). */
+#define ENUM_TO_STRING(EnumType, EnumVal) \
+  case EnumType::EnumVal: {               \
+    return #EnumVal;                      \
+  }
+
+/** Used in defining EnumTypeFromString(). Hardcoded to expect str to be in scope. */
+#define ENUM_FROM_STRING(EnumType, EnumVal)                             \
+  else if (str == #EnumVal) { /* NOLINT readability/braces confusion */ \
+    return EnumType::EnumVal;                                           \
+  }
+
+/**
+ * Defines:
+ * 1. An enum class named EnumName of type EnumCppType.
+ * 2. An EnumNameToString() function.
+ * 3. An EnumNameFromString() function.
+ *
+ * Please see ExpressionType for example usage.
+ *
+ * The purpose of this macro is to keep the ToString and FromString defined and in-sync.
+ * The string version of the enum is the same as the enum value itself.
+ */
+#define ENUM_DEFINE(EnumName, EnumCppType, EnumValMacro)                                         \
+  enum class EnumName : EnumCppType { EnumValMacro(ENUM_DEFINE_ITEM) NUM_ENUM_ENTRIES };         \
+                                                                                                 \
+  /** @return String version of @p type. */                                                      \
+  inline std::string EnumName##ToString(EnumName type) { /* NOLINT inline usage */               \
+    switch (type) {                                                                              \
+      EnumValMacro(ENUM_TO_STRING);                                                              \
+      default: {                                                                                 \
+        throw CONVERSION_EXCEPTION(                                                              \
+            ("No enum conversion for: " + std::to_string(static_cast<uint64_t>(type))).c_str()); \
+      }                                                                                          \
+    }                                                                                            \
+  }                                                                                              \
+                                                                                                 \
+  /** @return Enum version of @p str. */                                                         \
+  inline EnumName EnumName##FromString(const std::string &str) { /* NOLINT inline usage */       \
+    if (false) { /* This check starts an if-else chain for the macro. */                         \
+    }                                                                                            \
+    EnumValMacro(ENUM_FROM_STRING) else { /* NOLINT readability/braces confusion */              \
+      throw CONVERSION_EXCEPTION("No enum conversion for: " + str);                              \
+    }                                                                                            \
+  }

src/include/parser/expression_defs.h

@@ -2,89 +2,85 @@
 
 #include <string>
 
-#include "common/macros.h"
+#include "common/enum_defs.h"
 
 namespace noisepage::parser {
 
+#define EXPRESSION_TYPE_ENUM(T)                       \
+  T(ExpressionType, INVALID)                          \
+                                                      \
+  T(ExpressionType, OPERATOR_UNARY_MINUS)             \
+  T(ExpressionType, OPERATOR_PLUS)                    \
+  T(ExpressionType, OPERATOR_MINUS)                   \
+  T(ExpressionType, OPERATOR_MULTIPLY)                \
+  T(ExpressionType, OPERATOR_DIVIDE)                  \
+  T(ExpressionType, OPERATOR_CONCAT)                  \
+  T(ExpressionType, OPERATOR_MOD)                     \
+  T(ExpressionType, OPERATOR_CAST)                    \
+  T(ExpressionType, OPERATOR_NOT)                     \
+  T(ExpressionType, OPERATOR_IS_NULL)                 \
+  T(ExpressionType, OPERATOR_IS_NOT_NULL)             \
+  T(ExpressionType, OPERATOR_EXISTS)                  \
+                                                      \
+  T(ExpressionType, COMPARE_EQUAL)                    \
+  T(ExpressionType, COMPARE_NOT_EQUAL)                \
+  T(ExpressionType, COMPARE_LESS_THAN)                \
+  T(ExpressionType, COMPARE_GREATER_THAN)             \
+  T(ExpressionType, COMPARE_LESS_THAN_OR_EQUAL_TO)    \
+  T(ExpressionType, COMPARE_GREATER_THAN_OR_EQUAL_TO) \
+  T(ExpressionType, COMPARE_LIKE)                     \
+  T(ExpressionType, COMPARE_NOT_LIKE)                 \
+  T(ExpressionType, COMPARE_IN)                       \
+  T(ExpressionType, COMPARE_IS_DISTINCT_FROM)         \
+                                                      \
+  T(ExpressionType, CONJUNCTION_AND)                  \
+  T(ExpressionType, CONJUNCTION_OR)                   \
+                                                      \
+  T(ExpressionType, COLUMN_VALUE)                     \
+                                                      \
+  T(ExpressionType, VALUE_CONSTANT)                   \
+  T(ExpressionType, VALUE_PARAMETER)                  \
+  T(ExpressionType, VALUE_TUPLE)                      \
+  T(ExpressionType, VALUE_TUPLE_ADDRESS)              \
+  T(ExpressionType, VALUE_NULL)                       \
+  T(ExpressionType, VALUE_VECTOR)                     \
+  T(ExpressionType, VALUE_SCALAR)                     \
+  T(ExpressionType, VALUE_DEFAULT)                    \
+                                                      \
+  T(ExpressionType, AGGREGATE_COUNT)                  \
+  T(ExpressionType, AGGREGATE_SUM)                    \
+  T(ExpressionType, AGGREGATE_MIN)                    \
+  T(ExpressionType, AGGREGATE_MAX)                    \
+  T(ExpressionType, AGGREGATE_AVG)                    \
+  T(ExpressionType, AGGREGATE_TOP_K)                  \
+  T(ExpressionType, AGGREGATE_HISTOGRAM)              \
+                                                      \
+  T(ExpressionType, FUNCTION)                         \
+                                                      \
+  T(ExpressionType, HASH_RANGE)                       \
+                                                      \
+  T(ExpressionType, OPERATOR_CASE_EXPR)               \
+  T(ExpressionType, OPERATOR_NULL_IF)                 \
+  T(ExpressionType, OPERATOR_COALESCE)                \
+                                                      \
+  T(ExpressionType, ROW_SUBQUERY)                     \
+                                                      \
+  T(ExpressionType, STAR)                             \
+  T(ExpressionType, TABLE_STAR)                       \
+  T(ExpressionType, PLACEHOLDER)                      \
+  T(ExpressionType, COLUMN_REF)                       \
+  T(ExpressionType, FUNCTION_REF)                     \
+  T(ExpressionType, TABLE_REF)
+
 /**
  * All possible expression types.
  * TODO(WAN): Do we really need some of this stuff? e.g. HASH_RANGE for elastic
  * TODO(LING): The binder via OperatorExpression now assumes a certain ordering on the ExpressionType.
  */
-enum class ExpressionType : uint8_t {
-  INVALID,
-
-  OPERATOR_UNARY_MINUS,
-  OPERATOR_PLUS,
-  OPERATOR_MINUS,
-  OPERATOR_MULTIPLY,
-  OPERATOR_DIVIDE,
-  OPERATOR_CONCAT,
-  OPERATOR_MOD,
-  OPERATOR_CAST,
-  OPERATOR_NOT,
-  OPERATOR_IS_NULL,
-  OPERATOR_IS_NOT_NULL,
-  OPERATOR_EXISTS,
-
-  COMPARE_EQUAL,
-  COMPARE_NOT_EQUAL,
-  COMPARE_LESS_THAN,
-  COMPARE_GREATER_THAN,
-  COMPARE_LESS_THAN_OR_EQUAL_TO,
-  COMPARE_GREATER_THAN_OR_EQUAL_TO,
-  COMPARE_LIKE,
-  COMPARE_NOT_LIKE,
-  COMPARE_IN,
-  COMPARE_IS_DISTINCT_FROM,
-
-  CONJUNCTION_AND,
-  CONJUNCTION_OR,
-
-  COLUMN_VALUE,
-
-  VALUE_CONSTANT,
-  VALUE_PARAMETER,
-  VALUE_TUPLE,
-  VALUE_TUPLE_ADDRESS,
-  VALUE_NULL,
-  VALUE_VECTOR,
-  VALUE_SCALAR,
-  VALUE_DEFAULT,
-
-  AGGREGATE_COUNT,
-  AGGREGATE_SUM,
-  AGGREGATE_MIN,
-  AGGREGATE_MAX,
-  AGGREGATE_AVG,
-  AGGREGATE_TOP_K,
-  AGGREGATE_HISTOGRAM,
+ENUM_DEFINE(ExpressionType, uint8_t, EXPRESSION_TYPE_ENUM);
+#undef EXPRESSION_TYPE_ENUM
 
-  FUNCTION,
-
-  HASH_RANGE,
-
-  OPERATOR_CASE_EXPR,
-  OPERATOR_NULL_IF,
-  OPERATOR_COALESCE,
-
-  ROW_SUBQUERY,
-
-  STAR,
-  TABLE_STAR,
-  PLACEHOLDER,
-  COLUMN_REF,
-  FUNCTION_REF,
-  TABLE_REF
-};
-
-/**
- * When short_str is true, return a short version of ExpressionType string
- * For example, + instead of Operator_Plus. It's used to generate the expression name
- * @param type Expression Type
- * @param short_str Flag if a short version of the Expression Type should be returned
- * @return String representation of the Expression Type
- */
-std::string ExpressionTypeToString(ExpressionType type, bool short_str);
+/** @return A short representation of @p type, e.g., "+" over "OPERATOR_PLUS". Used for expression name generation. */
+std::string ExpressionTypeToShortString(ExpressionType type);
 
 }  // namespace noisepage::parser

src/optimizer/statistics/selectivity.cpp

@@ -30,7 +30,7 @@ double Selectivity::ComputeSelectivity(common::ManagedPointer<TableStats> stats,
       return DistinctFrom(stats, condition);
     default:
       OPTIMIZER_LOG_WARN("Expression type {0} not supported for computing selectivity",
-                         ExpressionTypeToString(condition.GetType(), false).c_str());
+                         ExpressionTypeToString(condition.GetType()).c_str());
       return DEFAULT_SELECTIVITY;
   }
 }

src/optimizer/statistics/selectivity_util.cpp

@@ -30,7 +30,7 @@ double SelectivityUtil::ComputeSelectivity(common::ManagedPointer<NewColumnStats
       return DistinctFrom(column_stats, condition);
     default:
       OPTIMIZER_LOG_WARN("Expression type {0} not supported for computing selectivity",
-                         ExpressionTypeToString(condition.GetType(), false).c_str());
+                         ExpressionTypeToString(condition.GetType()).c_str());
       return DEFAULT_SELECTIVITY_VALUE;
   }
 }