From 74bf673bc9f1e7c603aa69d0035aa35fcde6ea37 Mon Sep 17 00:00:00 2001 From: David Sisson Date: Mon, 5 Feb 2024 22:34:14 -0800 Subject: [PATCH] Added a planloader test. --- export/planloader/CMakeLists.txt | 12 +++++-- .../{textplan.cpp => planloader.cpp} | 15 +++----- export/planloader/planloader.h | 30 ++++++++++++++++ export/planloader/tests/CMakeLists.txt | 34 +++++++++++++++++++ export/planloader/tests/PlanLoaderTest.cpp | 32 +++++++++++++++++ 5 files changed, 110 insertions(+), 13 deletions(-) rename export/planloader/{textplan.cpp => planloader.cpp} (87%) create mode 100644 export/planloader/planloader.h create mode 100644 export/planloader/tests/CMakeLists.txt create mode 100644 export/planloader/tests/PlanLoaderTest.cpp diff --git a/export/planloader/CMakeLists.txt b/export/planloader/CMakeLists.txt index 563578de..7d9547b9 100644 --- a/export/planloader/CMakeLists.txt +++ b/export/planloader/CMakeLists.txt @@ -1,12 +1,18 @@ # SPDX-License-Identifier: Apache-2.0 -# MEGAHACK -- Given that the compiler options include lots of undesirable stuff in Debug mode, either cleanse it or refuse to build unless we're in release mode. +if(NOT BUILD_SUBDIR_NAME EQUAL "release") + message(SEND_ERROR, "The planloader library does not work in Debug mode due to its dependencies.") +endif() -add_library(planloader SHARED textplan.cpp) +add_library(planloader SHARED planloader.cpp) add_compile_options(-FPIC) set_target_properties(planloader PROPERTIES SUBVERSION 1) add_dependencies(planloader substrait_io) target_link_libraries(planloader substrait_io) -install(TARGETS planloader LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) +install(TARGETS planloader LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} PRIVATE_HEADER DESTINATION ${CMAKE_INSTALL_INCDIR}) + +if(${SUBSTRAIT_CPP_BUILD_TESTING}) + add_subdirectory(tests) +endif() diff --git a/export/planloader/textplan.cpp b/export/planloader/planloader.cpp similarity index 87% rename from export/planloader/textplan.cpp rename to export/planloader/planloader.cpp index 59653cf8..6c53e760 100644 --- a/export/planloader/textplan.cpp +++ b/export/planloader/planloader.cpp @@ -1,21 +1,16 @@ /* SPDX-License-Identifier: Apache-2.0 */ +#include "planloader.h" + #include extern "C" { -typedef struct { - char *buffer; - uint32_t size; - char *errorMessage; -} SerializedPlan; - // Load a Substrait plan (in any format) from disk. // Stores the Substrait plan in planBuffer in serialized form. // Returns a SerializedPlan structure containing either the serialized plan or -// an error message. +// an error message. errorMessage is nullptr upon success. SerializedPlan* load_substrait_plan(const char* filename) { - // MEGAHACK -- Consider putting this initialization into a separate function. auto newPlan = new SerializedPlan(); newPlan->buffer = nullptr; newPlan->size = 0; @@ -48,7 +43,7 @@ void free_substrait_plan(SerializedPlan* plan) { // Write a serialized Substrait plan to disk in the specified format. // On error returns a non-empty error message. -// On success an empty string is returned. +// On success a nullptr is returned. const char* save_substrait_plan( const char* planData, uint32_t planDataLength, @@ -61,7 +56,7 @@ const char* save_substrait_plan( if (!result.ok()) { return result.message().data(); } - return ""; + return nullptr; } } // extern "C" diff --git a/export/planloader/planloader.h b/export/planloader/planloader.h new file mode 100644 index 00000000..ce5fb4ab --- /dev/null +++ b/export/planloader/planloader.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +#include + +extern "C" { + +typedef struct { + char *buffer; + uint32_t size; + char *errorMessage; +} SerializedPlan; + +// Load a Substrait plan (in any format) from disk. +// Stores the Substrait plan in planBuffer in serialized form. +// Returns a SerializedPlan structure containing either the serialized plan or +// an error message. +SerializedPlan* load_substrait_plan(const char* filename); + +void free_substrait_plan(SerializedPlan* plan); + +// Write a serialized Substrait plan to disk in the specified format. +// On error returns a non-empty error message. +// On success an empty string is returned. +const char* save_substrait_plan( + const char* planData, + uint32_t planDataLength, + const char* filename, + io::substrait::PlanFileFormat format); + +} // extern "C" diff --git a/export/planloader/tests/CMakeLists.txt b/export/planloader/tests/CMakeLists.txt new file mode 100644 index 00000000..1989bf4e --- /dev/null +++ b/export/planloader/tests/CMakeLists.txt @@ -0,0 +1,34 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_path(GET CMAKE_CURRENT_BINARY_DIR PARENT_PATH + CMAKE_CURRENT_BINARY_PARENT_DIR) +cmake_path(GET CMAKE_CURRENT_BINARY_PARENT_DIR PARENT_PATH + CMAKE_CURRENT_BINARY_TOPLEVEL_DIR) + +set(CMAKE_RUNTIME_OUTPUT_DIRECTORY + "${CMAKE_CURRENT_BINARY_TOPLEVEL_DIR}/${BUILD_SUBDIR_NAME}") + +add_test_case( + planloader_test + SOURCES + PlanLoaderTest.cpp + EXTRA_LINK_LIBS + planloader + gtest + gtest_main) + +set(TEXTPLAN_SOURCE_DIR "${CMAKE_SOURCE_DIR}/src/substrait/textplan") + +add_custom_command( + TARGET planloader_test + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E echo "Copying unit test data.." + COMMAND ${CMAKE_COMMAND} -E make_directory + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data" + COMMAND + ${CMAKE_COMMAND} -E copy + "${TEXTPLAN_SOURCE_DIR}/converter/data/q6_first_stage.json" + "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data/q6_first_stage.json") + +message( + STATUS "test data will be here: ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/tests/data") diff --git a/export/planloader/tests/PlanLoaderTest.cpp b/export/planloader/tests/PlanLoaderTest.cpp new file mode 100644 index 00000000..4216bf10 --- /dev/null +++ b/export/planloader/tests/PlanLoaderTest.cpp @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +#include +#include + +#include "../planloader.h" +#include "substrait/proto/plan.pb.h" + +namespace io::substrait::textplan { +namespace { + +TEST(PlanLoaderTest, LoadAndSave) { + auto serializedPlan = load_substrait_plan("data/q6_first_stage.json"); + ASSERT_EQ(serializedPlan->errorMessage, nullptr); + + ::substrait::proto::Plan plan; + bool parseStatus = + plan.ParseFromArray(serializedPlan->buffer, serializedPlan->size); + ASSERT_TRUE(parseStatus) << "Failed to parse the plan."; + + const char* saveStatus = save_substrait_plan( + serializedPlan->buffer, + serializedPlan->size, + "outfile.splan", + PlanFileFormat::kText); + ASSERT_EQ(saveStatus, nullptr); + + free_substrait_plan(serializedPlan); +} + +} // namespace +} // namespace io::substrait::textplan