Clint

.clang-tidy

@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: Apache-2.0
+---
+Checks: "*, \
+  -abseil-*, \
+  -cert-env33-c, \
+  -cert-err58-cpp, \
+  -clang-diagnostic-padded, \
+  -clang-analyzer-deadcode.DeadStores, \
+  -cppcoreguidelines-avoid-magic-numbers, \
+  -cppcoreguidelines-pro-bounds-constant-array-index, \
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic, \
+  -cppcoreguidelines-pro-type-reinterpret-cast, \
+  -cppcoreguidelines-no-malloc, \
+  -cppcoreguidelines-owning-memory, \
+  -cppcoreguidelines-macro-usage, \
+  -cppcoreguidelines-pro-type-vararg, \
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay, \
+  -fuchsia-overloaded-operator, \
+  -fuchsia-default-arguments, \  
+  -fuchsia-multiple-inheritance, \
+  -fuchsia-default-arguments-calls, \
+  -fuchsia-trailing-return, \
+  -fuchsia-default-arguments-declarations, \
+  -fuchsia-statically-constructed-objects, \
+  -google-runtime-references, \
+  -google-runtime-int, \
+  -google-explicit-constructor, \
+  -hicpp-no-malloc, \
+  -hicpp-vararg, \
+  -hicpp-invalid-access-moved, \
+  -hicpp-no-array-decay, \
+  -hicpp-signed-bitwise, \
+  -llvm-header-guard, \
+  -modernize-use-trailing-return-type, \
+  -misc-definitions-in-headers, \
+  -misc-unused-alias-decls, \
+  -modernize-concat-nested-namespaces, \
+  -modernize-raw-string-literal, \
+  -readability-magic-numbers"
+HeaderFilterRegex: ""
+...

.github/workflows/build_test.yml

@@ -1,3 +1,4 @@
+# SPDX-License-Identifier: Apache-2.0
 name: Ubuntu Build & Test
 
 on:
@@ -7,6 +8,22 @@ on:
     branches: [main]
 
 jobs:
+  check:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          submodules: recursive
+      - name: Setup Ubuntu
+        run: ./scripts/setup-ubuntu.sh
+      - name: Check License Header
+        uses: apache/skywalking-eyes/[email protected]
+      - name: Check CMake files
+        run: find . \( -name '*.cmake' -o -name 'CMakeLists.txt' \) -exec cmake-format $* {} +
+      - name: Clang-tidy
+        run: python3 scripts/run-clang-tidy.py "." "build" "third_party,scripts,docker,cmake_modules" "h,hpp,cc,cpp"
+      - run: mkdir build
   build:
     runs-on: ubuntu-latest
 

.licenserc.yaml

@@ -0,0 +1,22 @@
+header:
+  license:
+    spdx-id: Apache-2.0
+    content: |
+      SPDX-License-Identifier: Apache-2.0
+
+  paths-ignore:
+    - '.github'
+    - '.gitignore'
+    - '.gitmodules'
+    - '.clang-format'
+    - '.clang-tidy'
+    - '.licenserc.yaml'
+    - 'third_party/fmt'
+    - 'third_party/googletest'
+    - 'third_party/substrait'
+    - 'third_party/yaml-cpp'
+    - '**/*.md'
+    - '**/*.json'
+    - '**/*.log'
+
+  comment: never

cmake_modules/BuildUtils.cmake

@@ -41,5 +41,6 @@ function(ADD_TEST_CASE TEST_NAME)
     add_dependencies(${TEST_NAME} ${ARG_EXTRA_DEPENDENCIES})
   endif()
 
-  add_test(NAME ${TEST_NAME} COMMAND $<TARGET_FILE:${TEST_NAME}>)
+  add_test(NAME ${TEST_NAME} COMMAND $<TARGET_FILE:${TEST_NAME}>
+                             WORKING_DIRECTORY "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests")
 endfunction()

include/substrait/common/Exceptions.h

@@ -2,9 +2,9 @@
 
 #pragma once
 
-#include <fmt/format.h>
 #include <memory>
 #include <utility>
+#include <fmt/format.h>
 
 namespace io::substrait::common {
 namespace error_code {
@@ -39,12 +39,14 @@ class SubstraitException : public std::exception {
     // objects.
     kUser = 0,
 
-    // Errors where the root cause of the problem is some unreliable aspect of the
-    // system are classified with SYSTEM.
+    // Errors where the root cause of the problem is some unreliable aspect of
+    // the system are classified with SYSTEM.
     kSystem = 1
   };
-
   SubstraitException(
+      const char* file,
+      size_t line,
+      const char* function,
       const std::string& exceptionCode,
       const std::string& exceptionMessage,
       Type exceptionType = Type::kSystem,
@@ -62,10 +64,16 @@ class SubstraitException : public std::exception {
 class SubstraitUserError : public SubstraitException {
  public:
   SubstraitUserError(
+      const char* file,
+      size_t line,
+      const char* function,
       const std::string& exceptionCode,
       const std::string& exceptionMessage,
       const std::string& exceptionName = "SubstraitUserError")
       : SubstraitException(
+            file,
+            line,
+            function,
             exceptionCode,
             exceptionMessage,
             Type::kUser,
@@ -75,10 +83,16 @@ class SubstraitUserError : public SubstraitException {
 class SubstraitRuntimeError final : public SubstraitException {
  public:
   SubstraitRuntimeError(
+      const char* file,
+      size_t line,
+      const char* function,
       const std::string& exceptionCode,
       const std::string& exceptionMessage,
       const std::string& exceptionName = "SubstraitRuntimeError")
       : SubstraitException(
+            file,
+            line,
+            function,
             exceptionCode,
             exceptionMessage,
             Type::kSystem,
@@ -90,10 +104,10 @@ std::string errorMessage(fmt::string_view fmt, const Args&... args) {
   return fmt::vformat(fmt, fmt::make_format_args(args...));
 }
 
-#define SUBSTRAIT_THROW(exception, errorCode, ...)                     \
-  {                                                                    \
-    auto message = ::io::substrait::common::errorMessage(__VA_ARGS__); \
-    throw exception(errorCode, message);                               \
+#define SUBSTRAIT_THROW(exception, errorCode, ...)                         \
+  {                                                                        \
+    auto message = ::io::substrait::common::errorMessage(__VA_ARGS__);     \
+    throw exception(__FILE__, __LINE__, __FUNCTION__, errorCode, message); \
   }
 
 #define SUBSTRAIT_UNSUPPORTED(...)                       \

scripts/run-clang-tidy.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: Apache-2.0
+
+# Run clang-tidy recursively and parallel on directory
+# Usage: run-clang-tidy sourcedir builddir excludedirs extensions
+# extensions and excludedirs are specified as comma-separated
+# string without dot, e.g. 'c,cpp'
+# e.g. run-clang-tidy . build test,other c,cpp file
+
+import os, sys, subprocess, multiprocessing
+manager = multiprocessing.Manager()
+failedfiles = manager.list()
+
+# Get absolute current path and remove trailing seperators
+currentdir = os.path.realpath(os.getcwd()).rstrip(os.sep)
+print("Arguments: " + str(sys.argv))
+# Get absolute source dir after removing leading and trailing seperators from input.
+sourcedir = currentdir + sys.argv[1].lstrip(os.sep).rstrip(os.sep)
+print("Source directory: " + sourcedir)
+builddir = sourcedir + os.sep + sys.argv[2].rstrip(os.sep)
+print("Build directory: " + builddir)
+# Split exclude dirs into a tuple
+excludedirs = tuple([(sourcedir + os.sep + s).rstrip(os.sep) for s in sys.argv[3].split(',')])
+# If the build directory is not the same as the source directory, exclude it
+if not sourcedir == builddir:
+    excludedirs = excludedirs + (builddir,)
+print("Exclude directories: " + str(excludedirs))
+# Split extensions into a tuple
+extensions = tuple([("." + s) for s in sys.argv[4].split(',')])
+print("Extensions: " + str(extensions))
+
+def runclangtidy(filepath):
+    print("Checking: " + filepath)
+    proc = subprocess.Popen("clang-tidy --quiet -p=" + builddir + " " + filepath, shell=True)
+    if proc.wait() != 0:
+        failedfiles.append(filepath)
+
+def collectfiles(dir, exclude, exts):
+    collectedfiles = []
+    for root, dirs, files in os.walk(dir):
+        for file in files:
+            filepath = root + os.sep + file
+            if (len(exclude) == 0 or not filepath.startswith(exclude)) and filepath.endswith(exts):
+                collectedfiles.append(filepath)
+    return collectedfiles
+
+# Define the pool AFTER the global variables and subprocess function because multiprocessing
+# has stricter requirements on member ordering
+# See: https://stackoverflow.com/questions/41385708/multiprocessing-example-giving-attributeerror
+pool = multiprocessing.Pool()
+pool.map(runclangtidy, collectfiles(sourcedir, excludedirs, extensions))
+pool.close()
+pool.join()
+if len(failedfiles) > 0:
+    print("Errors in " + len(failedfiles) + " files")
+    sys.exit(1)
+print("No errors found")
+sys.exit(0)

scripts/setup-ubuntu.sh

@@ -19,8 +19,11 @@ sudo --preserve-env apt install -y \
   ccache \
   ninja-build \
   checkinstall \
+  clang-tidy \
   git \
   wget \
   libprotobuf-dev \
   libprotobuf23 \
   protobuf-compiler
+
+pip install cmake-format

src/substrait/CMakeLists.txt

@@ -12,3 +12,4 @@ add_subdirectory(common)
 add_subdirectory(type)
 add_subdirectory(function)
 add_subdirectory(proto)
+add_subdirectory(textplan)

src/substrait/common/Exceptions.cpp

@@ -6,19 +6,22 @@
 namespace io::substrait::common {
 
 SubstraitException::SubstraitException(
+    const char* file,
+    size_t line,
+    const char* function,
     const std::string& exceptionCode,
     const std::string& exceptionMessage,
     Type exceptionType,
     const std::string& exceptionName)
     : msg_(fmt::format(
-          "Exception: {}\nError Code: {}\nType: {}\nReason: {}\n"
-          "Function: {}\nFile: {}\n:Line: {}\n",
+          "Exception: {}\nError Code: {}\nError Type: {}\nReason: {}\n"
+          "Function: {}\nLocation: {}(Line:{})\n",
           exceptionName,
           exceptionCode,
           exceptionType == Type::kSystem ? "system" : "user",
           exceptionMessage,
-          __FUNCTION__,
-          __FILE__,
-          std::to_string(__LINE__))) {}
+          function,
+          file,
+          std::to_string(line))) {}
 
 } // namespace io::substrait::common

src/substrait/proto/CMakeLists.txt

@@ -41,7 +41,8 @@ set(PROTO_SRCS)
 set(PROTO_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
 cmake_path(GET PROTO_OUTPUT_DIR PARENT_PATH PROTO_OUTPUT_PARENT_DIR)
-cmake_path(GET PROTO_OUTPUT_PARENT_DIR PARENT_PATH PROTO_OUTPUT_TOPLEVEL_DIR)
+cmake_path(GET PROTO_OUTPUT_PARENT_DIR PARENT_PATH PROTO_OUTPUT_MIDLEVEL_DIR)
+cmake_path(GET PROTO_OUTPUT_MIDLEVEL_DIR PARENT_PATH PROTO_OUTPUT_TOPLEVEL_DIR)
 
 foreach(PROTO_FILE IN LISTS PROTOBUF_FILELIST)
     file(RELATIVE_PATH RELATIVE_PROTO_PATH "${CMAKE_SOURCE_DIR}/third_party/substrait/proto/substrait" "${PROTO_FILE}")
@@ -56,7 +57,7 @@ foreach(PROTO_FILE IN LISTS PROTOBUF_FILELIST)
             OUTPUT ${PROTO_SRC} ${PROTO_HDR}
             COMMAND protobuf::protoc
             "--proto_path=${GENERATED_PROTO_TOPLEVEL_DIR}"
-            "--cpp_out=${PROTO_OUTPUT_TOPLEVEL_DIR}"
+            "--cpp_out=${PROTO_OUTPUT_MIDLEVEL_DIR}"
             ${GENERATED_PROTO_FILE}
             DEPENDS ${GENERATED_PROTOBUF_LIST} protobuf::protoc
             COMMENT "Generated C++ protobuf module for ${PROTO_FILE}"
@@ -66,7 +67,15 @@ foreach(PROTO_FILE IN LISTS PROTOBUF_FILELIST)
 endforeach()
 
 # Add the generated protobuf C++ files to our exported library.
-add_library(substrait_proto ${PROTO_SRCS})
+add_library(
+        substrait_proto
+        ${PROTO_SRCS} ${PROTO_HDRS}
+        ProtoUtils.cpp ProtoUtils.h)
+
+# Include the protobuf library as a dependency to use this class.
+target_link_libraries(substrait_proto ${PROTOBUF_LIBRARIES})
 
 # Make sure we can see our own generated include files.
-target_include_directories(substrait_proto PUBLIC "${PROTO_OUTPUT_TOPLEVEL_DIR}")
+target_include_directories(
+        substrait_proto
+        PUBLIC "${PROTO_OUTPUT_TOPLEVEL_DIR}/src")

src/substrait/proto/ProtoUtils.cpp

@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: Apache-2.0 */
+
+#include "substrait/proto/ProtoUtils.h"
+
+namespace substrait::proto {
+
+std::string PlanRelTypeCaseName(::substrait::proto::PlanRel::RelTypeCase num) {
+  static std::vector<std::string> case_names = {
+      "unknown",
+      "rel",
+      "root",
+  };
+
+  if (num >= case_names.size()) {
+    return "unknown";
+  }
+
+  return case_names[num];
+}
+
+std::string RelTypeCaseName(::substrait::proto::Rel::RelTypeCase num) {
+  static std::vector<std::string> case_names = {
+      "unknown",
+      "read",
+      "filter",
+      "fetch",
+      "aggregate",
+      "sort",
+      "join",
+      "project",
+      "set",
+      "extensionsingle",
+      "extensionmulti",
+      "extensionleaf",
+      "cross",
+      "hashjoin",
+      "mergejoin",
+  };
+
+  if (num >= case_names.size()) {
+    return "unknown";
+  }
+
+  return case_names[num];
+}
+
+} // namespace substrait::proto

src/substrait/proto/ProtoUtils.h

@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: Apache-2.0 */
+
+#pragma once
+
+#include <string>
+
+#include "substrait/proto/algebra.pb.h"
+#include "substrait/proto/plan.pb.h"
+
+namespace substrait::proto {
+
+std::string PlanRelTypeCaseName(::substrait::proto::PlanRel::RelTypeCase num);
+
+std::string RelTypeCaseName(::substrait::proto::Rel::RelTypeCase num);
+
+} // namespace substrait::proto

src/substrait/proto/update_proto_package.pl

@@ -1,4 +1,5 @@
 #!/bin/perl -w
+# SPDX-License-Identifier: Apache-2.0
 
 # Renames package declarations for protobuffers from substrait to substrait.proto.
 # This allows us to modify where the generated C++ have their definitions without