diff --git a/export/planloader/CMakeLists.txt b/export/planloader/CMakeLists.txt index e08e839c..cc256930 100644 --- a/export/planloader/CMakeLists.txt +++ b/export/planloader/CMakeLists.txt @@ -1,5 +1,6 @@ # SPDX-License-Identifier: Apache-2.0 +set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) add_library(planloader SHARED planloader.cpp) add_dependencies(planloader substrait_io) diff --git a/scripts/run-clang-tidy.sh b/scripts/run-clang-tidy.sh index 293ef869..89b27492 100755 --- a/scripts/run-clang-tidy.sh +++ b/scripts/run-clang-tidy.sh @@ -6,7 +6,7 @@ WORKDIR="$( cd $SCRIPTDIR/.. && pwd )" # Make compile_command.json rm -rf tmp && mkdir tmp && cmake -Btmp -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DSUBSTRAIT_CPP_ROUNDTRIP_TESTING=ON # Build substrait protobuf -pushd tmp/src/substrait/proto && make -j && popd || exit +pushd tmp/src/substrait/proto && make -j 2 && popd || exit # Build textplan grammar pushd tmp/src/substrait/textplan/parser/grammar && make -j antlr4_runtime textplan_grammar_headers && popd || exit # Run clang-tidy diff --git a/scripts/setup-ubuntu.sh b/scripts/setup-ubuntu.sh index e503d536..9be1fbf8 100755 --- a/scripts/setup-ubuntu.sh +++ b/scripts/setup-ubuntu.sh @@ -18,10 +18,16 @@ sudo --preserve-env apt install -y \ clang-tidy \ git \ wget \ - protobuf-compiler \ clang-format \ uuid-dev \ default-jre \ libcurl4-openssl-dev +# Install the currently supported version of protobuf: +PB_REL="https://github.com/protocolbuffers/protobuf/releases" +PB_VER="28.2" +curl -LO $PB_REL/download/v$PB_VER/protoc-$PB_VER-linux-x86_64.zip +unzip protoc-$PB_VER-linux-x86_64.zip -d $HOME/.local +export PATH="$PATH:$HOME/.local/bin" + pip install cmake-format diff --git a/src/substrait/common/tests/IoTest.cpp b/src/substrait/common/tests/IoTest.cpp index 183594bd..e97ea063 100644 --- a/src/substrait/common/tests/IoTest.cpp +++ b/src/substrait/common/tests/IoTest.cpp @@ -7,7 +7,10 @@ #include #include #include + +#ifndef _WIN32 #include +#endif using ::protobuf_matchers::EqualsProto; using ::protobuf_matchers::Partially; @@ -45,8 +48,9 @@ TEST_F(IoTest, LoadMissingFile) { class SaveAndLoadTestFixture : public ::testing::TestWithParam { public: void SetUp() override { - testFileDirectory_ = std::filesystem::temp_directory_path() / - std::filesystem::path("my_temp_dir"); + testFileDirectory_ = (std::filesystem::temp_directory_path() / + std::filesystem::path("my_temp_dir")) + .string(); if (!std::filesystem::create_directory(testFileDirectory_)) { ASSERT_TRUE(false) << "Failed to create temporary directory."; diff --git a/src/substrait/textplan/converter/LoadBinary.cpp b/src/substrait/textplan/converter/LoadBinary.cpp index c5d9f4ce..6691b507 100644 --- a/src/substrait/textplan/converter/LoadBinary.cpp +++ b/src/substrait/textplan/converter/LoadBinary.cpp @@ -23,10 +23,13 @@ namespace { class StringErrorCollector : public google::protobuf::io::ErrorCollector { public: - void AddError(int line, int column, const std::string& message) override { + void RecordError( + int line, + google::protobuf::io::ColumnNumber column, + absl::string_view message) override { errors_.push_back( std::to_string(line + 1) + ":" + std::to_string(column + 1) + " → " + - message); + std::string(message)); } [[nodiscard]] std::vector getErrors() const { diff --git a/src/substrait/textplan/converter/ReferenceNormalizer.cpp b/src/substrait/textplan/converter/ReferenceNormalizer.cpp index 61754a61..d668385f 100644 --- a/src/substrait/textplan/converter/ReferenceNormalizer.cpp +++ b/src/substrait/textplan/converter/ReferenceNormalizer.cpp @@ -18,7 +18,7 @@ bool compareExtensionFunctions( return make_tuple( // First sort so that extension functions precede any other kind of // extension. - not decl.has_extension_function(), + !decl.has_extension_function(), // Next sort by space. decl.extension_function().extension_uri_reference(), // Finally sort by name within a space. diff --git a/src/substrait/textplan/parser/CMakeLists.txt b/src/substrait/textplan/parser/CMakeLists.txt index e593908e..5c95f5b2 100644 --- a/src/substrait/textplan/parser/CMakeLists.txt +++ b/src/substrait/textplan/parser/CMakeLists.txt @@ -36,12 +36,13 @@ target_link_libraries( absl::status absl::statusor) -add_executable(planparser Tool.cpp) - -target_link_libraries(planparser substrait_textplan_loader error_listener) +if(UNIX) + add_executable(planparser Tool.cpp) + target_link_libraries(planparser substrait_textplan_loader error_listener) + install(TARGETS planparser EXPORT SubstraitTargets) +endif() +install(TARGETS substrait_textplan_loader EXPORT SubstraitTargets) if(${SUBSTRAIT_CPP_BUILD_TESTING}) add_subdirectory(tests) endif() - -install(TARGETS planparser substrait_textplan_loader EXPORT SubstraitTargets) diff --git a/src/substrait/textplan/tests/CMakeLists.txt b/src/substrait/textplan/tests/CMakeLists.txt index a2815a27..1f9c08fd 100644 --- a/src/substrait/textplan/tests/CMakeLists.txt +++ b/src/substrait/textplan/tests/CMakeLists.txt @@ -39,6 +39,9 @@ add_test_case( cmake_path(GET CMAKE_CURRENT_SOURCE_DIR PARENT_PATH TEXTPLAN_SOURCE_DIR) +# Get all JSON files in the data directory +file(GLOB JSON_FILES "${TEXTPLAN_SOURCE_DIR}/data/*.json") + add_custom_command( TARGET substrait_textplan_round_trip_test POST_BUILD @@ -48,9 +51,15 @@ add_custom_command( COMMAND ${CMAKE_COMMAND} -E copy "${TEXTPLAN_SOURCE_DIR}/converter/data/q6_first_stage.json" - "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/q6_first_stage.json" - COMMAND ${CMAKE_COMMAND} -E copy "${TEXTPLAN_SOURCE_DIR}/data/*.json" - "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/") + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/q6_first_stage.json") + +foreach(json_file ${TEXTPLAN_JSON_FILES}) + add_custom_command( + TARGET substrait_textplan_round_trip_test + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy "${json_file}" + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/") +endforeach() message( STATUS "test data will be here: ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data" diff --git a/src/substrait/textplan/tests/RoundtripTest.cpp b/src/substrait/textplan/tests/RoundtripTest.cpp index 2ccc7176..08ea9d97 100644 --- a/src/substrait/textplan/tests/RoundtripTest.cpp +++ b/src/substrait/textplan/tests/RoundtripTest.cpp @@ -59,7 +59,7 @@ std::vector getTestCases() { testDataPath.append("data"); for (auto const& dirEntry : std::filesystem::recursive_directory_iterator{testDataPath}) { - std::string pathName = dirEntry.path(); + std::string pathName = dirEntry.path().string(); if (endsWith(pathName, ".json") && !endsWith(pathName, "q6_first_stage.json")) { filenames.push_back(pathName); @@ -69,6 +69,8 @@ std::vector getTestCases() { return filenames; } +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(RoundTripBinaryToTextFixture); + TEST_P(RoundTripBinaryToTextFixture, RoundTrip) { auto filename = GetParam(); auto jsonOrError = readFromFile(filename); diff --git a/third_party/protobuf-matchers b/third_party/protobuf-matchers index a76296f5..bd0b02a6 160000 --- a/third_party/protobuf-matchers +++ b/third_party/protobuf-matchers @@ -1 +1 @@ -Subproject commit a76296f5d5369d57f44126acf59c290a397490c6 +Subproject commit bd0b02a6b648676011732a4dae13d8782fc8d82c diff --git a/third_party/protobuf.cmake b/third_party/protobuf.cmake index e7942cf3..ef05a6ae 100644 --- a/third_party/protobuf.cmake +++ b/third_party/protobuf.cmake @@ -12,7 +12,7 @@ FetchContent_Declare(GTest ) FetchContent_Declare(Protobuf GIT_REPOSITORY https://github.com/protocolbuffers/protobuf.git - GIT_TAG v23.4 + GIT_TAG v28.2 OVERRIDE_FIND_PACKAGE ) set(protobuf_BUILD_TESTS OFF CACHE INTERNAL "")