From d77300461782fd55e869b8acfe8542b20ef32323 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 8 Dec 2023 19:30:32 +0100 Subject: [PATCH 01/40] bump fmt to 10.1.1 --- CMake/resolve_dependency_modules/fmt.cmake | 8 +++----- .../resolve_dependency_modules/fmt/no-targets.patch | 12 ------------ 2 files changed, 3 insertions(+), 17 deletions(-) delete mode 100644 CMake/resolve_dependency_modules/fmt/no-targets.patch diff --git a/CMake/resolve_dependency_modules/fmt.cmake b/CMake/resolve_dependency_modules/fmt.cmake index 55be2629962e..88d8d674d3a3 100644 --- a/CMake/resolve_dependency_modules/fmt.cmake +++ b/CMake/resolve_dependency_modules/fmt.cmake @@ -13,9 +13,9 @@ # limitations under the License. include_guard(GLOBAL) -set(VELOX_FMT_VERSION 8.0.1) +set(VELOX_FMT_VERSION 10.1.1) set(VELOX_FMT_BUILD_SHA256_CHECKSUM - b06ca3130158c625848f3fb7418f235155a4d389b2abc3a6245fb01cb0eb1e01) + 78b8c0a72b1c35e4443a7e308df52498252d1cefc2b08c9a97bc9ee6cfe61f8b) set(VELOX_FMT_SOURCE_URL "https://github.com/fmtlib/fmt/archive/${VELOX_FMT_VERSION}.tar.gz") @@ -25,9 +25,7 @@ message(STATUS "Building fmt from source") FetchContent_Declare( fmt URL ${VELOX_FMT_SOURCE_URL} - URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM} - PATCH_COMMAND git apply ${CMAKE_CURRENT_LIST_DIR}/fmt/no-targets.patch) - + URL_HASH ${VELOX_FMT_BUILD_SHA256_CHECKSUM}) # Force fmt to create fmt-config.cmake which can be found by other dependecies # (e.g. folly) set(FMT_INSTALL ON) diff --git a/CMake/resolve_dependency_modules/fmt/no-targets.patch b/CMake/resolve_dependency_modules/fmt/no-targets.patch deleted file mode 100644 index 5ec962d32973..000000000000 --- a/CMake/resolve_dependency_modules/fmt/no-targets.patch +++ /dev/null @@ -1,12 +0,0 @@ -This can be removed once we upgrade to fmt >= 9 ---- a/support/cmake/fmt-config.cmake.in -+++ b/support/cmake/fmt-config.cmake.in -@@ -1,4 +1,7 @@ - @PACKAGE_INIT@ - --include(${CMAKE_CURRENT_LIST_DIR}/@targets_export_name@.cmake) -+if (NOT TARGET fmt::fmt) -+ include(${CMAKE_CURRENT_LIST_DIR}/@targets_export_name@.cmake) -+endif () -+ - check_required_components(fmt) From 78ff5c37ef642fc8cae80722e433db040f24535f Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Tue, 12 Dec 2023 23:01:37 +0100 Subject: [PATCH 02/40] add formatters and other fixes for fmt 8.1+ --- velox/common/base/RuntimeMetrics.h | 6 +++++ velox/common/caching/SsdCache.cpp | 2 +- velox/common/compression/Compression.cpp | 3 ++- velox/common/file/FileSystems.cpp | 8 +++---- velox/common/memory/MemoryAllocator.h | 10 ++++++++ velox/common/memory/MemoryPool.h | 12 ++++++++++ .../memory/tests/MemoryAllocatorTest.cpp | 5 ++-- velox/common/memory/tests/MemoryPoolTest.cpp | 5 ++-- .../connectors/fuzzer/FuzzerConnectorSplit.h | 22 ++++++++++++++++++ velox/connectors/hive/HiveDataSink.h | 23 +++++++++++++++++++ velox/connectors/tpch/CMakeLists.txt | 3 ++- velox/connectors/tpch/TpchConnectorSplit.h | 22 ++++++++++++++++++ velox/core/Expressions.h | 8 +++---- velox/core/PlanFragment.h | 8 +++++++ velox/core/PlanNode.h | 20 +++++++++++++++- velox/dwio/dwrf/common/FileMetadata.h | 7 ++++++ velox/exec/Driver.h | 9 ++++++++ velox/exec/HashBuild.h | 9 ++++++++ velox/exec/HashTable.h | 11 +++++++++ velox/exec/Operator.h | 9 ++++++++ velox/exec/ProbeOperatorState.h | 10 ++++++++ velox/exec/Spill.h | 8 +++++++ velox/exec/SpillOperatorGroup.h | 11 +++++++++ velox/exec/Spiller.h | 7 ++++++ velox/exec/tests/utils/QueryAssertions.cpp | 8 +++++++ velox/functions/lib/DateTimeFormatter.h | 20 ++++++++++++++++ velox/functions/lib/Re2Functions.h | 8 +++++++ velox/functions/prestosql/Arithmetic.h | 7 ++++++ .../substrait/tests/JsonToProtoConverter.cpp | 5 ++-- velox/type/Subfield.h | 10 ++++++++ velox/type/Timestamp.h | 21 +++++++++++++++++ velox/vector/VectorSaver.cpp | 7 ++++++ 32 files changed, 304 insertions(+), 20 deletions(-) diff --git a/velox/common/base/RuntimeMetrics.h b/velox/common/base/RuntimeMetrics.h index 9a81ff183791..a14478c79673 100644 --- a/velox/common/base/RuntimeMetrics.h +++ b/velox/common/base/RuntimeMetrics.h @@ -111,3 +111,9 @@ class RuntimeStatWriterScopeGuard { }; } // namespace facebook::velox +template <> +struct fmt::formatter : formatter { + auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/common/caching/SsdCache.cpp b/velox/common/caching/SsdCache.cpp index f0de8069ebe0..8685150584a1 100644 --- a/velox/common/caching/SsdCache.cpp +++ b/velox/common/caching/SsdCache.cpp @@ -82,7 +82,7 @@ bool SsdCache::startWrite() { } void SsdCache::write(std::vector pins) { - VELOX_CHECK_LE(numShards_, writesInProgress_); + VELOX_CHECK_LE(numShards_, writesInProgress_.load()); TestValue::adjust("facebook::velox::cache::SsdCache::write", this); diff --git a/velox/common/compression/Compression.cpp b/velox/common/compression/Compression.cpp index 3843f5902a98..e17ff941ba99 100644 --- a/velox/common/compression/Compression.cpp +++ b/velox/common/compression/Compression.cpp @@ -56,7 +56,8 @@ CompressionKind codecTypeToCompressionKind(folly::io::CodecType type) { case folly::io::CodecType::GZIP: return CompressionKind_GZIP; default: - VELOX_UNSUPPORTED("Not support folly codec type {}", type); + VELOX_UNSUPPORTED( + "Not support folly codec type {}", folly::to(type)); } } diff --git a/velox/common/file/FileSystems.cpp b/velox/common/file/FileSystems.cpp index eac9c01337b7..28e66dc2bd11 100644 --- a/velox/common/file/FileSystems.cpp +++ b/velox/common/file/FileSystems.cpp @@ -155,8 +155,8 @@ class LocalFileSystem : public FileSystem { 0, ec.value(), "Mkdir {} failed: {}, message: {}", - path, - ec, + std::string(path), + ec.value(), ec.message()); VLOG(1) << "LocalFileSystem::mkdir " << path; } @@ -168,8 +168,8 @@ class LocalFileSystem : public FileSystem { 0, ec.value(), "Rmdir {} failed: {}, message: {}", - path, - ec, + std::string(path), + ec.value(), ec.message()); VLOG(1) << "LocalFileSystem::rmdir " << path; } diff --git a/velox/common/memory/MemoryAllocator.h b/velox/common/memory/MemoryAllocator.h index 78751c6c735d..6779f6c6e5f3 100644 --- a/velox/common/memory/MemoryAllocator.h +++ b/velox/common/memory/MemoryAllocator.h @@ -23,6 +23,7 @@ #include #include +#include #include #include "velox/common/base/CheckedArithmetic.h" #include "velox/common/base/Exceptions.h" @@ -529,3 +530,12 @@ class MemoryAllocator : public std::enable_shared_from_this { std::ostream& operator<<(std::ostream& out, const MemoryAllocator::Kind& kind); } // namespace facebook::velox::memory +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + facebook::velox::memory::MemoryAllocator::InjectedFailure s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/common/memory/MemoryPool.h b/velox/common/memory/MemoryPool.h index 20a8c12cb1d1..a5d24c1a0154 100644 --- a/velox/common/memory/MemoryPool.h +++ b/velox/common/memory/MemoryPool.h @@ -22,6 +22,7 @@ #include #include +#include #include "velox/common/base/BitUtil.h" #include "velox/common/base/Exceptions.h" #include "velox/common/base/Portability.h" @@ -1074,3 +1075,14 @@ class StlAllocator { } }; } // namespace facebook::velox::memory + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::memory::MemoryPool::Kind s, + format_context& ctx) { + return formatter::format( + facebook::velox::memory::MemoryPool::kindString(s), ctx); + } +}; diff --git a/velox/common/memory/tests/MemoryAllocatorTest.cpp b/velox/common/memory/tests/MemoryAllocatorTest.cpp index dcf9ede9b061..16002d9643f5 100644 --- a/velox/common/memory/tests/MemoryAllocatorTest.cpp +++ b/velox/common/memory/tests/MemoryAllocatorTest.cpp @@ -23,6 +23,7 @@ #include "velox/common/memory/MmapArena.h" #include "velox/common/testutil/TestValue.h" +#include #include #include #include @@ -776,8 +777,8 @@ TEST_P(MemoryAllocatorTest, nonContiguousFailure) { std::string debugString() const { return fmt::format( "numOldPages:{}, numNewPages:{}, injectedFailure:{}", - numOldPages, - numNewPages, + static_cast(numOldPages), + static_cast(numNewPages), injectedFailure); } } testSettings[] = {// Cap failure injection. diff --git a/velox/common/memory/tests/MemoryPoolTest.cpp b/velox/common/memory/tests/MemoryPoolTest.cpp index d1315a93b2d1..f7dac9e99c43 100644 --- a/velox/common/memory/tests/MemoryPoolTest.cpp +++ b/velox/common/memory/tests/MemoryPoolTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -1128,8 +1129,8 @@ TEST_P(MemoryPoolTest, persistentNonContiguousAllocateFailure) { std::string debugString() const { return fmt::format( "numOldPages:{}, numNewPages:{}, injectedFailure:{}", - numOldPages, - numNewPages, + static_cast(numOldPages), + static_cast(numNewPages), injectedFailure); } } testSettings[] = {// Cap failure injection. diff --git a/velox/connectors/fuzzer/FuzzerConnectorSplit.h b/velox/connectors/fuzzer/FuzzerConnectorSplit.h index b82749e0be9b..1c4b6fef87cc 100644 --- a/velox/connectors/fuzzer/FuzzerConnectorSplit.h +++ b/velox/connectors/fuzzer/FuzzerConnectorSplit.h @@ -28,3 +28,25 @@ struct FuzzerConnectorSplit : public connector::ConnectorSplit { }; } // namespace facebook::velox::connector::fuzzer + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::connector::fuzzer::FuzzerConnectorSplit s, + format_context& ctx) { + return formatter::format(s.toString(), ctx); + } +}; + +template <> +struct fmt::formatter< + std::shared_ptr> + : formatter { + auto format( + std::shared_ptr + s, + format_context& ctx) { + return formatter::format(s->toString(), ctx); + } +}; diff --git a/velox/connectors/hive/HiveDataSink.h b/velox/connectors/hive/HiveDataSink.h index a4ea8f2d7dc3..fad79bd0007a 100644 --- a/velox/connectors/hive/HiveDataSink.h +++ b/velox/connectors/hive/HiveDataSink.h @@ -435,6 +435,8 @@ class HiveDataSink : public DataSink { private: enum class State { kRunning = 0, kAborted = 1, kClosed = 2 }; + friend struct fmt::formatter< + facebook::velox::connector::hive::HiveDataSink::State>; static std::string stateString(State state); @@ -584,3 +586,24 @@ class HiveDataSink : public DataSink { }; } // namespace facebook::velox::connector::hive + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::connector::hive::HiveDataSink::State s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; + +template <> +struct fmt::formatter< + facebook::velox::connector::hive::LocationHandle::TableType> + : formatter { + auto format( + facebook::velox::connector::hive::LocationHandle::TableType s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/connectors/tpch/CMakeLists.txt b/velox/connectors/tpch/CMakeLists.txt index b7123f2f6f0e..6d1bcddcd052 100644 --- a/velox/connectors/tpch/CMakeLists.txt +++ b/velox/connectors/tpch/CMakeLists.txt @@ -14,7 +14,8 @@ add_library(velox_tpch_connector OBJECT TpchConnector.cpp) -target_link_libraries(velox_tpch_connector velox_connector velox_tpch_gen) +target_link_libraries(velox_tpch_connector velox_connector velox_tpch_gen + fmt::fmt) if(${VELOX_BUILD_TESTING}) add_subdirectory(tests) diff --git a/velox/connectors/tpch/TpchConnectorSplit.h b/velox/connectors/tpch/TpchConnectorSplit.h index 298a36ac85df..029d07a84135 100644 --- a/velox/connectors/tpch/TpchConnectorSplit.h +++ b/velox/connectors/tpch/TpchConnectorSplit.h @@ -15,6 +15,7 @@ */ #pragma once +#include #include "velox/connectors/Connector.h" namespace facebook::velox::connector::tpch { @@ -40,3 +41,24 @@ struct TpchConnectorSplit : public connector::ConnectorSplit { }; } // namespace facebook::velox::connector::tpch + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::connector::tpch::TpchConnectorSplit s, + format_context& ctx) { + return formatter::format(s.toString(), ctx); + } +}; + +template <> +struct fmt::formatter< + std::shared_ptr> + : formatter { + auto format( + std::shared_ptr s, + format_context& ctx) { + return formatter::format(s->toString(), ctx); + } +}; diff --git a/velox/core/Expressions.h b/velox/core/Expressions.h index 459b0f5808ed..33f2281d5498 100644 --- a/velox/core/Expressions.h +++ b/velox/core/Expressions.h @@ -320,11 +320,10 @@ class FieldAccessTypedExpr : public ITypedExpr { std::string toString() const override { if (inputs().empty()) { - return fmt::format("{}", std::quoted(name(), '"', '"')); + return fmt::format("{:?}", name()); } - return fmt::format( - "{}[{}]", inputs()[0]->toString(), std::quoted(name(), '"', '"')); + return fmt::format("{}[{:?}]", inputs()[0]->toString(), name()); } size_t localHash() const override { @@ -401,8 +400,7 @@ class DereferenceTypedExpr : public ITypedExpr { } std::string toString() const override { - return fmt::format( - "{}[{}]", inputs()[0]->toString(), std::quoted(name(), '"', '"')); + return fmt::format("{}[{:?}]", inputs()[0]->toString(), name()); } size_t localHash() const override { diff --git a/velox/core/PlanFragment.h b/velox/core/PlanFragment.h index a2990a7eda23..fda37e86b6f2 100644 --- a/velox/core/PlanFragment.h +++ b/velox/core/PlanFragment.h @@ -81,3 +81,11 @@ struct PlanFragment { }; } // namespace facebook::velox::core + +template <> +struct fmt::formatter + : formatter { + auto format(facebook::velox::core::ExecutionStrategy s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/core/PlanNode.h b/velox/core/PlanNode.h index deb3e0adba68..54f79ef8ad33 100644 --- a/velox/core/PlanNode.h +++ b/velox/core/PlanNode.h @@ -14,7 +14,7 @@ * limitations under the License. */ #pragma once - +#include #include "velox/connectors/Connector.h" #include "velox/core/Expressions.h" #include "velox/core/QueryConfig.h" @@ -2386,3 +2386,21 @@ class TopNRowNumberNode : public PlanNode { }; } // namespace facebook::velox::core + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::core::PartitionedOutputNode::Kind s, + format_context& ctx) { + return formatter::format( + facebook::velox::core::PartitionedOutputNode::kindString(s), ctx); + } +}; + +template <> +struct fmt::formatter : formatter { + auto format(facebook::velox::core::JoinType s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/dwio/dwrf/common/FileMetadata.h b/velox/dwio/dwrf/common/FileMetadata.h index 2ea21628a595..ac8bb09e6ebb 100644 --- a/velox/dwio/dwrf/common/FileMetadata.h +++ b/velox/dwio/dwrf/common/FileMetadata.h @@ -495,3 +495,10 @@ class FooterWrapper : public ProtoWrapperBase { }; } // namespace facebook::velox::dwrf + +template <> +struct fmt::formatter : formatter { + auto format(facebook::velox::dwrf::DwrfFormat s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/exec/Driver.h b/velox/exec/Driver.h index aa614afe4126..20fa75a69d30 100644 --- a/velox/exec/Driver.h +++ b/velox/exec/Driver.h @@ -636,3 +636,12 @@ class ScopedDriverThreadContext { DriverThreadContext* driverThreadContext(); } // namespace facebook::velox::exec + +template <> +struct fmt::formatter + : formatter { + auto format(facebook::velox::exec::StopReason s, format_context& ctx) { + return formatter::format( + facebook::velox::exec::stopReasonString(s), ctx); + } +}; diff --git a/velox/exec/HashBuild.h b/velox/exec/HashBuild.h index 4033767efbfb..fa9e6dbf03dd 100644 --- a/velox/exec/HashBuild.h +++ b/velox/exec/HashBuild.h @@ -333,3 +333,12 @@ inline std::ostream& operator<<(std::ostream& os, HashBuild::State state) { return os; } } // namespace facebook::velox::exec + +template <> +struct fmt::formatter + : formatter { + auto format(facebook::velox::exec::HashBuild::State s, format_context& ctx) { + return formatter::format( + facebook::velox::exec::HashBuild::stateName(s), ctx); + } +}; diff --git a/velox/exec/HashTable.h b/velox/exec/HashTable.h index 23cf2d2ae5a5..5dc6e128934c 100644 --- a/velox/exec/HashTable.h +++ b/velox/exec/HashTable.h @@ -953,3 +953,14 @@ class HashTable : public BaseHashTable { }; } // namespace facebook::velox::exec + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::exec::BaseHashTable::HashMode s, + format_context& ctx) { + return formatter::format( + facebook::velox::exec::BaseHashTable::modeString(s), ctx); + } +}; diff --git a/velox/exec/Operator.h b/velox/exec/Operator.h index 2b640fe7c586..d286eb5aff1f 100644 --- a/velox/exec/Operator.h +++ b/velox/exec/Operator.h @@ -761,3 +761,12 @@ class SourceOperator : public Operator { } }; } // namespace facebook::velox::exec + +template <> +struct fmt::formatter : formatter { + auto format(std::thread::id s, format_context& ctx) { + std::ostringstream oss; + oss << s; + return formatter::format(oss.str(), ctx); + } +}; diff --git a/velox/exec/ProbeOperatorState.h b/velox/exec/ProbeOperatorState.h index 1ba9ef443fb4..eb0d2474c935 100644 --- a/velox/exec/ProbeOperatorState.h +++ b/velox/exec/ProbeOperatorState.h @@ -55,3 +55,13 @@ enum class ProbeOperatorState { std::string probeOperatorStateName(ProbeOperatorState state); } // namespace facebook::velox::exec + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::exec::ProbeOperatorState s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/exec/Spill.h b/velox/exec/Spill.h index f98b4eada46f..7c5b5de2c2d0 100644 --- a/velox/exec/Spill.h +++ b/velox/exec/Spill.h @@ -442,3 +442,11 @@ struct hash<::facebook::velox::exec::SpillPartitionId> { } }; } // namespace std + +template <> +struct fmt::formatter + : formatter { + auto format(facebook::velox::exec::SpillPartitionId s, format_context& ctx) { + return formatter::format(s.toString(), ctx); + } +}; diff --git a/velox/exec/SpillOperatorGroup.h b/velox/exec/SpillOperatorGroup.h index 1a5d08b19a63..3c0e9d28cdd5 100644 --- a/velox/exec/SpillOperatorGroup.h +++ b/velox/exec/SpillOperatorGroup.h @@ -188,3 +188,14 @@ inline std::ostream& operator<<( } } // namespace facebook::velox::exec + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::exec::SpillOperatorGroup::State s, + format_context& ctx) { + return formatter::format( + facebook::velox::exec::SpillOperatorGroup::stateName(s), ctx); + } +}; diff --git a/velox/exec/Spiller.h b/velox/exec/Spiller.h index c732480801de..e33fdcdd9091 100644 --- a/velox/exec/Spiller.h +++ b/velox/exec/Spiller.h @@ -335,3 +335,10 @@ class Spiller { std::vector spillRuns_; }; } // namespace facebook::velox::exec + +template <> +struct fmt::formatter : formatter { + auto format(facebook::velox::exec::Spiller::Type s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/exec/tests/utils/QueryAssertions.cpp b/velox/exec/tests/utils/QueryAssertions.cpp index 3e5b3c26bece..c3417a48c1d4 100644 --- a/velox/exec/tests/utils/QueryAssertions.cpp +++ b/velox/exec/tests/utils/QueryAssertions.cpp @@ -17,6 +17,7 @@ #include #include +#include "duckdb/common/types.hpp" #include "velox/duckdb/conversion/DuckConversion.h" #include "velox/exec/tests/utils/Cursor.h" #include "velox/exec/tests/utils/QueryAssertions.h" @@ -1477,3 +1478,10 @@ void printResults(const RowVectorPtr& result, std::ostream& out) { } } // namespace facebook::velox::exec::test + +template <> +struct fmt::formatter<::duckdb::LogicalTypeId> : formatter { + auto format(::duckdb::LogicalTypeId s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/functions/lib/DateTimeFormatter.h b/velox/functions/lib/DateTimeFormatter.h index 179a642ad09a..e0ac6bd2f8be 100644 --- a/velox/functions/lib/DateTimeFormatter.h +++ b/velox/functions/lib/DateTimeFormatter.h @@ -193,3 +193,23 @@ std::shared_ptr buildJodaDateTimeFormatter( const std::string_view& format); } // namespace facebook::velox::functions + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::functions::DateTimeFormatterType s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; + +template <> +struct fmt::formatter + : formatter { + auto format( + facebook::velox::functions::DateTimeFormatSpecifier s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/functions/lib/Re2Functions.h b/velox/functions/lib/Re2Functions.h index e2723dd6682c..71879c6f9d39 100644 --- a/velox/functions/lib/Re2Functions.h +++ b/velox/functions/lib/Re2Functions.h @@ -220,3 +220,11 @@ struct Re2RegexpReplace { }; } // namespace facebook::velox::functions + +template <> +struct fmt::formatter + : formatter { + auto format(facebook::velox::functions::PatternKind s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/functions/prestosql/Arithmetic.h b/velox/functions/prestosql/Arithmetic.h index ab1543cbb87c..4fac64da6651 100644 --- a/velox/functions/prestosql/Arithmetic.h +++ b/velox/functions/prestosql/Arithmetic.h @@ -646,3 +646,10 @@ struct CosineSimilarityFunction { } // namespace } // namespace facebook::velox::functions + +template <> +struct fmt::formatter : formatter { + auto format(std::errc s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/substrait/tests/JsonToProtoConverter.cpp b/velox/substrait/tests/JsonToProtoConverter.cpp index 356e296b0c02..e00f6dd90708 100644 --- a/velox/substrait/tests/JsonToProtoConverter.cpp +++ b/velox/substrait/tests/JsonToProtoConverter.cpp @@ -32,7 +32,6 @@ void JsonToProtoConverter::readFromFile( auto status = google::protobuf::util::JsonStringToMessage(msgData, &msg); VELOX_CHECK( status.ok(), - "Failed to parse Substrait JSON: {} {}", - status.code(), - status.message()); + "Failed to parse Substrait JSON: {}", + status.message().ToString()); } diff --git a/velox/type/Subfield.h b/velox/type/Subfield.h index 5aa74c5c8c63..93a47e412e9d 100644 --- a/velox/type/Subfield.h +++ b/velox/type/Subfield.h @@ -16,6 +16,7 @@ #pragma once #include +#include #include #include "velox/common/base/Exceptions.h" @@ -313,3 +314,12 @@ struct hash<::facebook::velox::common::Subfield> { } }; } // namespace std + +template <> +struct fmt::formatter<::facebook::velox::common::Subfield> + : formatter { + auto format(const ::facebook::velox::common::Subfield& s, format_context& ctx) + const { + return formatter::format(s.toString(), ctx); + } +}; diff --git a/velox/type/Timestamp.h b/velox/type/Timestamp.h index ff644edbae37..763732590d83 100644 --- a/velox/type/Timestamp.h +++ b/velox/type/Timestamp.h @@ -394,3 +394,24 @@ struct hasher<::facebook::velox::Timestamp> { }; } // namespace folly + +namespace fmt { +template <> +struct formatter + : formatter { + auto format( + facebook::velox::TimestampToStringOptions::Precision s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; +template <> +struct formatter + : formatter { + auto format( + facebook::velox::TimestampToStringOptions::Mode s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; +} // namespace fmt diff --git a/velox/vector/VectorSaver.cpp b/velox/vector/VectorSaver.cpp index 21aa035a7f1f..827e73ec64d3 100644 --- a/velox/vector/VectorSaver.cpp +++ b/velox/vector/VectorSaver.cpp @@ -779,3 +779,10 @@ SelectivityVector restoreSelectivityVector(std::istream& in) { } } // namespace facebook::velox + +template <> +struct fmt::formatter : formatter { + auto format(facebook::velox::Encoding s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; From ca6d5a9cdc045e916b291a36f7ddf3d71a1b1b6a Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 03:59:53 +0100 Subject: [PATCH 03/40] more changes --- velox/common/caching/AsyncDataCache.cpp | 6 +++--- velox/common/memory/ByteStream.cpp | 4 ++-- velox/common/memory/tests/MemoryArbitratorTest.cpp | 2 +- velox/common/memory/tests/MemoryManagerTest.cpp | 2 +- velox/common/memory/tests/MockSharedArbitratorTest.cpp | 4 ++-- velox/core/CMakeLists.txt | 1 + velox/core/PlanFragment.h | 2 +- velox/core/tests/PlanFragmentTest.cpp | 2 +- velox/dwio/common/ScanSpec.cpp | 2 +- velox/dwio/common/Writer.cpp | 4 ++-- velox/dwio/dwrf/test/ColumnWriterTest.cpp | 7 ++----- velox/dwio/dwrf/test/CompressionTest.cpp | 3 ++- 12 files changed, 19 insertions(+), 20 deletions(-) diff --git a/velox/common/caching/AsyncDataCache.cpp b/velox/common/caching/AsyncDataCache.cpp index 81ba57072e33..c65ceaee4c16 100644 --- a/velox/common/caching/AsyncDataCache.cpp +++ b/velox/common/caching/AsyncDataCache.cpp @@ -68,7 +68,7 @@ void AsyncDataCacheEntry::setExclusiveToShared() { } void AsyncDataCacheEntry::release() { - VELOX_CHECK_NE(0, numPins_); + VELOX_CHECK_NE(0, static_cast(numPins_)); if (numPins_ == kExclusive) { // Dereferencing an exclusive entry without converting to shared means that // the content could not be shared, e.g. error in loading. @@ -135,7 +135,7 @@ std::string AsyncDataCacheEntry::toString() const { key_.fileNum.id(), key_.offset, size_, - numPins_); + static_cast(numPins_)); } std::unique_ptr CacheShard::getFreeEntry() { @@ -639,7 +639,7 @@ bool AsyncDataCache::makeSpace( VELOX_CHECK( numThreadsInAllocate_ >= 0 && numThreadsInAllocate_ < 10000, "Leak in numThreadsInAllocate_: {}", - numThreadsInAllocate_); + static_cast(numThreadsInAllocate_)); if (numThreadsInAllocate_) { rank = ++numThreadsInAllocate_; isCounted = true; diff --git a/velox/common/memory/ByteStream.cpp b/velox/common/memory/ByteStream.cpp index 8fce27881688..eff8f0bc3566 100644 --- a/velox/common/memory/ByteStream.cpp +++ b/velox/common/memory/ByteStream.cpp @@ -97,7 +97,7 @@ void ByteInputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteInputStream: {}", position); + VELOX_FAIL("Seeking past end of ByteInputStream: {}", static_cast(position)); } void ByteInputStream::next(bool throwIfPastEnd) { @@ -283,7 +283,7 @@ void ByteOutputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteOutputStream: {}", position); + VELOX_FAIL("Seeking past end of ByteOutputStream: {}", static_cast(position)); } void ByteOutputStream::flush(OutputStream* out) { diff --git a/velox/common/memory/tests/MemoryArbitratorTest.cpp b/velox/common/memory/tests/MemoryArbitratorTest.cpp index f5501eef2f01..9b6c0d4f42fd 100644 --- a/velox/common/memory/tests/MemoryArbitratorTest.cpp +++ b/velox/common/memory/tests/MemoryArbitratorTest.cpp @@ -391,7 +391,7 @@ class MockLeafMemoryReclaimer : public MemoryReclaimer { void free(const Allocation& allocation) { pool_->free(allocation.buffer, allocation.size); totalUsedBytes_ -= allocation.size; - VELOX_CHECK_GE(totalUsedBytes_, 0); + VELOX_CHECK_GE(static_cast(totalUsedBytes_), 0); } uint64_t reclaimableBytes() const { diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index 7e19cb448824..da5f9675d9f8 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -591,7 +591,7 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { SCOPED_TRACE(testData.debugString()); std::vector contiguousAllocations = {false, true}; for (const auto& contiguousAlloc : contiguousAllocations) { - SCOPED_TRACE(fmt::format("contiguousAlloc {}", contiguousAlloc)); + SCOPED_TRACE(fmt::format("contiguousAlloc {}", static_cast(contiguousAlloc))); const int alignment = 32; MemoryManagerOptions options; options.alignment = alignment; diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 59b0a693941a..0d44ef15a1cb 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -885,7 +885,7 @@ TEST_F(MockSharedArbitrationTest, failedArbitration) { TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", static_cast(isLeafReclaimable))); setupMemory(); auto op = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = MB; @@ -926,7 +926,7 @@ TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", static_cast(isLeafReclaimable))); setupMemory(); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int reclaimedOpCapacity = kMemoryCapacity * 2 / 3; diff --git a/velox/core/CMakeLists.txt b/velox/core/CMakeLists.txt index eb4c18cdbe80..fe3b2da46db3 100644 --- a/velox/core/CMakeLists.txt +++ b/velox/core/CMakeLists.txt @@ -34,4 +34,5 @@ target_link_libraries( velox_vector Boost::headers Folly::folly + fmt::fmt PRIVATE velox_encode) diff --git a/velox/core/PlanFragment.h b/velox/core/PlanFragment.h index fda37e86b6f2..6b6c514ee843 100644 --- a/velox/core/PlanFragment.h +++ b/velox/core/PlanFragment.h @@ -85,7 +85,7 @@ struct PlanFragment { template <> struct fmt::formatter : formatter { - auto format(facebook::velox::core::ExecutionStrategy s, format_context& ctx) { + auto format(const facebook::velox::core::ExecutionStrategy& s, format_context& ctx) { return formatter::format(static_cast(s), ctx); } }; diff --git a/velox/core/tests/PlanFragmentTest.cpp b/velox/core/tests/PlanFragmentTest.cpp index daf88bb8a483..6f8af1ffbd4d 100644 --- a/velox/core/tests/PlanFragmentTest.cpp +++ b/velox/core/tests/PlanFragmentTest.cpp @@ -144,7 +144,7 @@ TEST_F(PlanFragmentTest, aggregationCanSpill) { std::string debugString() const { return fmt::format( "aggregationStep:{} isSpillEnabled:{} isAggregationSpillEnabled:{} isDistinct:{} hasPreAggregation:{} expectedCanSpill:{}", - aggregationStep, + AggregationNode::stepName(aggregationStep), isSpillEnabled, isAggregationSpillEnabled, isDistinct, diff --git a/velox/dwio/common/ScanSpec.cpp b/velox/dwio/common/ScanSpec.cpp index 4845b3242a77..a3bedb6a18e6 100644 --- a/velox/dwio/common/ScanSpec.cpp +++ b/velox/dwio/common/ScanSpec.cpp @@ -48,7 +48,7 @@ ScanSpec* ScanSpec::getOrCreateChild(const Subfield& subfield) { auto& path = subfield.path(); for (size_t depth = 0; depth < path.size(); ++depth) { auto element = path[depth].get(); - VELOX_CHECK_EQ(element->kind(), kNestedField); + VELOX_CHECK_EQ(element->kind().toString(), kNestedField); auto* nestedField = static_cast(element); auto it = container->childByFieldName_.find(nestedField->name()); if (it != container->childByFieldName_.end()) { diff --git a/velox/dwio/common/Writer.cpp b/velox/dwio/common/Writer.cpp index 421d825c54a3..828a3ec4cbee 100644 --- a/velox/dwio/common/Writer.cpp +++ b/velox/dwio/common/Writer.cpp @@ -36,7 +36,7 @@ void Writer::checkStateTransition(State oldState, State newState) { default: break; } - VELOX_FAIL("Unexpected state transition from {} to {}", oldState, newState); + VELOX_FAIL("Unexpected state transition from {} to {}", Writer::stateString(oldState), Writer::stateString(newState)); } std::string Writer::stateString(State state) { @@ -59,7 +59,7 @@ bool Writer::isRunning() const { } void Writer::checkRunning() const { - VELOX_CHECK_EQ(state_, State::kRunning, "Writer is not running: {}", state_); + VELOX_CHECK_EQ(static_cast(state_), static_cast(State::kRunning), "Writer is not running: {}", Writer::stateString(state_)); } void Writer::setState(State state) { diff --git a/velox/dwio/dwrf/test/ColumnWriterTest.cpp b/velox/dwio/dwrf/test/ColumnWriterTest.cpp index 0a931d5b71b1..f2d025638e46 100644 --- a/velox/dwio/dwrf/test/ColumnWriterTest.cpp +++ b/velox/dwio/dwrf/test/ColumnWriterTest.cpp @@ -105,11 +105,8 @@ class TestStripeStreams : public StripeStreamsBase { if (!stream || stream->isSuppressed()) { if (throwIfNotFound) { DWIO_RAISE(fmt::format( - "stream (node = {}, seq = {}, column = {}, kind = {}) not found", - si.encodingKey().node(), - si.encodingKey().sequence(), - si.column(), - si.kind())); + "stream {} not found", + si.toString())); } else { return nullptr; } diff --git a/velox/dwio/dwrf/test/CompressionTest.cpp b/velox/dwio/dwrf/test/CompressionTest.cpp index d5d63cc76e46..d14a1b5dca19 100644 --- a/velox/dwio/dwrf/test/CompressionTest.cpp +++ b/velox/dwio/dwrf/test/CompressionTest.cpp @@ -21,6 +21,7 @@ #include "velox/dwio/dwrf/common/wrap/dwrf-proto-wrapper.h" #include "velox/dwio/dwrf/test/OrcTest.h" +#include #include #include @@ -396,7 +397,7 @@ TEST_P(CompressionTest, getCompressionBufferOOM) { for (const auto& testData : testSettings) { SCOPED_TRACE( - fmt::format("{} compression {}", testData.debugString(), kind_)); + fmt::format("{} compression {}", testData.debugString(), compressionKindToString(kind_))); auto config = std::make_shared(); config->set(Config::COMPRESSION, kind_); From 6f8976d5e7b1afe22880d20de2bc0a9505cb4cae Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 04:03:43 +0100 Subject: [PATCH 04/40] format --- velox/common/memory/ByteStream.cpp | 6 ++++-- velox/common/memory/tests/MemoryManagerTest.cpp | 3 ++- .../common/memory/tests/MockSharedArbitratorTest.cpp | 6 ++++-- velox/core/PlanFragment.h | 4 +++- velox/dwio/common/ScanSpec.cpp | 2 +- velox/dwio/common/Writer.cpp | 11 +++++++++-- velox/dwio/dwrf/test/ColumnWriterTest.cpp | 4 +--- velox/dwio/dwrf/test/CompressionTest.cpp | 6 ++++-- velox/type/Subfield.h | 9 +++++++++ 9 files changed, 37 insertions(+), 14 deletions(-) diff --git a/velox/common/memory/ByteStream.cpp b/velox/common/memory/ByteStream.cpp index eff8f0bc3566..acf12dd8a567 100644 --- a/velox/common/memory/ByteStream.cpp +++ b/velox/common/memory/ByteStream.cpp @@ -97,7 +97,8 @@ void ByteInputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteInputStream: {}", static_cast(position)); + VELOX_FAIL( + "Seeking past end of ByteInputStream: {}", static_cast(position)); } void ByteInputStream::next(bool throwIfPastEnd) { @@ -283,7 +284,8 @@ void ByteOutputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteOutputStream: {}", static_cast(position)); + VELOX_FAIL( + "Seeking past end of ByteOutputStream: {}", static_cast(position)); } void ByteOutputStream::flush(OutputStream* out) { diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index da5f9675d9f8..3682e30ee8ca 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -591,7 +591,8 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { SCOPED_TRACE(testData.debugString()); std::vector contiguousAllocations = {false, true}; for (const auto& contiguousAlloc : contiguousAllocations) { - SCOPED_TRACE(fmt::format("contiguousAlloc {}", static_cast(contiguousAlloc))); + SCOPED_TRACE(fmt::format( + "contiguousAlloc {}", static_cast(contiguousAlloc))); const int alignment = 32; MemoryManagerOptions options; options.alignment = alignment; diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 0d44ef15a1cb..95d39ba5d09e 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -885,7 +885,8 @@ TEST_F(MockSharedArbitrationTest, failedArbitration) { TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", static_cast(isLeafReclaimable))); + SCOPED_TRACE(fmt::format( + "isLeafReclaimable {}", static_cast(isLeafReclaimable))); setupMemory(); auto op = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = MB; @@ -926,7 +927,8 @@ TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", static_cast(isLeafReclaimable))); + SCOPED_TRACE(fmt::format( + "isLeafReclaimable {}", static_cast(isLeafReclaimable))); setupMemory(); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int reclaimedOpCapacity = kMemoryCapacity * 2 / 3; diff --git a/velox/core/PlanFragment.h b/velox/core/PlanFragment.h index 6b6c514ee843..2ccd933fc78b 100644 --- a/velox/core/PlanFragment.h +++ b/velox/core/PlanFragment.h @@ -85,7 +85,9 @@ struct PlanFragment { template <> struct fmt::formatter : formatter { - auto format(const facebook::velox::core::ExecutionStrategy& s, format_context& ctx) { + auto format( + const facebook::velox::core::ExecutionStrategy& s, + format_context& ctx) { return formatter::format(static_cast(s), ctx); } }; diff --git a/velox/dwio/common/ScanSpec.cpp b/velox/dwio/common/ScanSpec.cpp index a3bedb6a18e6..4845b3242a77 100644 --- a/velox/dwio/common/ScanSpec.cpp +++ b/velox/dwio/common/ScanSpec.cpp @@ -48,7 +48,7 @@ ScanSpec* ScanSpec::getOrCreateChild(const Subfield& subfield) { auto& path = subfield.path(); for (size_t depth = 0; depth < path.size(); ++depth) { auto element = path[depth].get(); - VELOX_CHECK_EQ(element->kind().toString(), kNestedField); + VELOX_CHECK_EQ(element->kind(), kNestedField); auto* nestedField = static_cast(element); auto it = container->childByFieldName_.find(nestedField->name()); if (it != container->childByFieldName_.end()) { diff --git a/velox/dwio/common/Writer.cpp b/velox/dwio/common/Writer.cpp index 828a3ec4cbee..8edea5a122c5 100644 --- a/velox/dwio/common/Writer.cpp +++ b/velox/dwio/common/Writer.cpp @@ -36,7 +36,10 @@ void Writer::checkStateTransition(State oldState, State newState) { default: break; } - VELOX_FAIL("Unexpected state transition from {} to {}", Writer::stateString(oldState), Writer::stateString(newState)); + VELOX_FAIL( + "Unexpected state transition from {} to {}", + Writer::stateString(oldState), + Writer::stateString(newState)); } std::string Writer::stateString(State state) { @@ -59,7 +62,11 @@ bool Writer::isRunning() const { } void Writer::checkRunning() const { - VELOX_CHECK_EQ(static_cast(state_), static_cast(State::kRunning), "Writer is not running: {}", Writer::stateString(state_)); + VELOX_CHECK_EQ( + static_cast(state_), + static_cast(State::kRunning), + "Writer is not running: {}", + Writer::stateString(state_)); } void Writer::setState(State state) { diff --git a/velox/dwio/dwrf/test/ColumnWriterTest.cpp b/velox/dwio/dwrf/test/ColumnWriterTest.cpp index f2d025638e46..e0faf8e4ae06 100644 --- a/velox/dwio/dwrf/test/ColumnWriterTest.cpp +++ b/velox/dwio/dwrf/test/ColumnWriterTest.cpp @@ -104,9 +104,7 @@ class TestStripeStreams : public StripeStreamsBase { } if (!stream || stream->isSuppressed()) { if (throwIfNotFound) { - DWIO_RAISE(fmt::format( - "stream {} not found", - si.toString())); + DWIO_RAISE(fmt::format("stream {} not found", si.toString())); } else { return nullptr; } diff --git a/velox/dwio/dwrf/test/CompressionTest.cpp b/velox/dwio/dwrf/test/CompressionTest.cpp index d14a1b5dca19..3f2e9bbc9bb9 100644 --- a/velox/dwio/dwrf/test/CompressionTest.cpp +++ b/velox/dwio/dwrf/test/CompressionTest.cpp @@ -396,8 +396,10 @@ TEST_P(CompressionTest, getCompressionBufferOOM) { {true, true}, {true, false}, {false, true}, {false, false}}; for (const auto& testData : testSettings) { - SCOPED_TRACE( - fmt::format("{} compression {}", testData.debugString(), compressionKindToString(kind_))); + SCOPED_TRACE(fmt::format( + "{} compression {}", + testData.debugString(), + compressionKindToString(kind_))); auto config = std::make_shared(); config->set(Config::COMPRESSION, kind_); diff --git a/velox/type/Subfield.h b/velox/type/Subfield.h index 93a47e412e9d..f49b4fe50ffd 100644 --- a/velox/type/Subfield.h +++ b/velox/type/Subfield.h @@ -323,3 +323,12 @@ struct fmt::formatter<::facebook::velox::common::Subfield> return formatter::format(s.toString(), ctx); } }; + +template <> +struct fmt::formatter<::facebook::velox::common::SubfieldKind> + : formatter { + auto format(const ::facebook::velox::common::SubfieldKind& s, format_context& ctx) + const { + return formatter::format(static_cast(s), ctx); + } +}; From 2509d1817f60ee9632adbff8a4ef7361d966b40f Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 15:13:49 +0100 Subject: [PATCH 05/40] add suggestions from review --- velox/common/caching/AsyncDataCache.cpp | 6 +++--- velox/common/memory/ByteStream.cpp | 8 ++++++-- velox/common/memory/tests/MemoryAllocatorTest.cpp | 4 ++-- velox/common/memory/tests/MemoryManagerTest.cpp | 4 ++-- velox/common/memory/tests/MemoryPoolTest.cpp | 4 ++-- velox/common/memory/tests/MockSharedArbitratorTest.cpp | 8 ++++---- velox/type/Subfield.h | 5 +++-- 7 files changed, 22 insertions(+), 17 deletions(-) diff --git a/velox/common/caching/AsyncDataCache.cpp b/velox/common/caching/AsyncDataCache.cpp index c65ceaee4c16..da340415d1f4 100644 --- a/velox/common/caching/AsyncDataCache.cpp +++ b/velox/common/caching/AsyncDataCache.cpp @@ -68,7 +68,7 @@ void AsyncDataCacheEntry::setExclusiveToShared() { } void AsyncDataCacheEntry::release() { - VELOX_CHECK_NE(0, static_cast(numPins_)); + VELOX_CHECK_NE(0, numPins_.load()); if (numPins_ == kExclusive) { // Dereferencing an exclusive entry without converting to shared means that // the content could not be shared, e.g. error in loading. @@ -135,7 +135,7 @@ std::string AsyncDataCacheEntry::toString() const { key_.fileNum.id(), key_.offset, size_, - static_cast(numPins_)); + numPins_.load()); } std::unique_ptr CacheShard::getFreeEntry() { @@ -639,7 +639,7 @@ bool AsyncDataCache::makeSpace( VELOX_CHECK( numThreadsInAllocate_ >= 0 && numThreadsInAllocate_ < 10000, "Leak in numThreadsInAllocate_: {}", - static_cast(numThreadsInAllocate_)); + numThreadsInAllocate_.load()); if (numThreadsInAllocate_) { rank = ++numThreadsInAllocate_; isCounted = true; diff --git a/velox/common/memory/ByteStream.cpp b/velox/common/memory/ByteStream.cpp index acf12dd8a567..7cf24a96a476 100644 --- a/velox/common/memory/ByteStream.cpp +++ b/velox/common/memory/ByteStream.cpp @@ -97,8 +97,10 @@ void ByteInputStream::seekp(std::streampos position) { } toSkip -= range.size; } + static_assert(sizeof(std::streamsize) <= sizeof(long long)); VELOX_FAIL( - "Seeking past end of ByteInputStream: {}", static_cast(position)); + "Seeking past end of ByteInputStream: {}", + static_cast(position)); } void ByteInputStream::next(bool throwIfPastEnd) { @@ -284,8 +286,10 @@ void ByteOutputStream::seekp(std::streampos position) { } toSkip -= range.size; } + static_assert(sizeof(std::streamsize) <= sizeof(long long)); VELOX_FAIL( - "Seeking past end of ByteOutputStream: {}", static_cast(position)); + "Seeking past end of ByteOutputStream: {}", + static_cast(position)); } void ByteOutputStream::flush(OutputStream* out) { diff --git a/velox/common/memory/tests/MemoryAllocatorTest.cpp b/velox/common/memory/tests/MemoryAllocatorTest.cpp index 16002d9643f5..3eb970214fbf 100644 --- a/velox/common/memory/tests/MemoryAllocatorTest.cpp +++ b/velox/common/memory/tests/MemoryAllocatorTest.cpp @@ -777,8 +777,8 @@ TEST_P(MemoryAllocatorTest, nonContiguousFailure) { std::string debugString() const { return fmt::format( "numOldPages:{}, numNewPages:{}, injectedFailure:{}", - static_cast(numOldPages), - static_cast(numNewPages), + static_cast(numOldPages), + static_cast(numNewPages), injectedFailure); } } testSettings[] = {// Cap failure injection. diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index 3682e30ee8ca..243c9d022b92 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -591,8 +591,8 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { SCOPED_TRACE(testData.debugString()); std::vector contiguousAllocations = {false, true}; for (const auto& contiguousAlloc : contiguousAllocations) { - SCOPED_TRACE(fmt::format( - "contiguousAlloc {}", static_cast(contiguousAlloc))); + SCOPED_TRACE( + fmt::format("contiguousAlloc {}", contiguousAlloc.operator bool())); const int alignment = 32; MemoryManagerOptions options; options.alignment = alignment; diff --git a/velox/common/memory/tests/MemoryPoolTest.cpp b/velox/common/memory/tests/MemoryPoolTest.cpp index f7dac9e99c43..2e3218a66cfe 100644 --- a/velox/common/memory/tests/MemoryPoolTest.cpp +++ b/velox/common/memory/tests/MemoryPoolTest.cpp @@ -1129,8 +1129,8 @@ TEST_P(MemoryPoolTest, persistentNonContiguousAllocateFailure) { std::string debugString() const { return fmt::format( "numOldPages:{}, numNewPages:{}, injectedFailure:{}", - static_cast(numOldPages), - static_cast(numNewPages), + static_cast(numOldPages), + static_cast(numNewPages), injectedFailure); } } testSettings[] = {// Cap failure injection. diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 95d39ba5d09e..5d36f80730d8 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -885,8 +885,8 @@ TEST_F(MockSharedArbitrationTest, failedArbitration) { TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format( - "isLeafReclaimable {}", static_cast(isLeafReclaimable))); + SCOPED_TRACE( + fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); setupMemory(); auto op = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = MB; @@ -927,8 +927,8 @@ TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format( - "isLeafReclaimable {}", static_cast(isLeafReclaimable))); + SCOPED_TRACE( + fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); setupMemory(); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int reclaimedOpCapacity = kMemoryCapacity * 2 / 3; diff --git a/velox/type/Subfield.h b/velox/type/Subfield.h index f49b4fe50ffd..0bc77a54523f 100644 --- a/velox/type/Subfield.h +++ b/velox/type/Subfield.h @@ -327,8 +327,9 @@ struct fmt::formatter<::facebook::velox::common::Subfield> template <> struct fmt::formatter<::facebook::velox::common::SubfieldKind> : formatter { - auto format(const ::facebook::velox::common::SubfieldKind& s, format_context& ctx) - const { + auto format( + const ::facebook::velox::common::SubfieldKind& s, + format_context& ctx) const { return formatter::format(static_cast(s), ctx); } }; From 8dc32b2c9f0ed6d763b79e4b18417940a8ff572d Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 16:41:28 +0100 Subject: [PATCH 06/40] additional changes --- .../hive/tests/HiveDataSinkTest.cpp | 4 +-- velox/dwio/dwrf/writer/WriterContext.cpp | 4 ++- velox/exec/HashTable.cpp | 3 +- velox/exec/Task.cpp | 5 ++- velox/exec/fuzzer/PrestoQueryRunner.cpp | 4 ++- velox/exec/tests/AggregationTest.cpp | 11 +++++-- velox/exec/tests/HashJoinTest.cpp | 33 ++++++++++++------- velox/exec/tests/OrderByTest.cpp | 6 ++-- velox/exec/tests/SharedArbitratorTest.cpp | 6 ++-- velox/exec/tests/TableWriteTest.cpp | 11 ++++--- 10 files changed, 57 insertions(+), 30 deletions(-) diff --git a/velox/connectors/hive/tests/HiveDataSinkTest.cpp b/velox/connectors/hive/tests/HiveDataSinkTest.cpp index 545520aa4ddf..f06a5cd6e721 100644 --- a/velox/connectors/hive/tests/HiveDataSinkTest.cpp +++ b/velox/connectors/hive/tests/HiveDataSinkTest.cpp @@ -572,7 +572,7 @@ TEST_F(HiveDataSinkTest, memoryReclaim) { std::string debugString() const { return fmt::format( "format: {}, sortWriter: {}, writerSpillEnabled: {}, writerFlushThreshold: {}, expectedWriterReclaimEnabled: {}, expectedWriterReclaimed: {}", - format, + dwio::common::toString(format), sortWriter, writerSpillEnabled, succinctBytes(writerFlushThreshold), @@ -713,7 +713,7 @@ TEST_F(HiveDataSinkTest, memoryReclaimAfterClose) { std::string debugString() const { return fmt::format( "format: {}, sortWriter: {}, writerSpillEnabled: {}, close: {}, expectedWriterReclaimEnabled: {}", - format, + dwio::common::toString(format), sortWriter, writerSpillEnabled, close, diff --git a/velox/dwio/dwrf/writer/WriterContext.cpp b/velox/dwio/dwrf/writer/WriterContext.cpp index 1030697c3ea4..c9d0edccaa2e 100644 --- a/velox/dwio/dwrf/writer/WriterContext.cpp +++ b/velox/dwio/dwrf/writer/WriterContext.cpp @@ -15,6 +15,7 @@ */ #include "velox/dwio/dwrf/writer/WriterContext.h" +#include #include "velox/exec/MemoryReclaimer.h" namespace facebook::velox::dwrf { @@ -61,7 +62,8 @@ WriterContext::WriterContext( handler_ = std::make_unique(); } validateConfigs(); - VLOG(2) << fmt::format("Compression config: {}", compression_); + VLOG(2) << fmt::format( + "Compression config: {}", common::compressionKindToString(compression_)); } WriterContext::~WriterContext() { diff --git a/velox/exec/HashTable.cpp b/velox/exec/HashTable.cpp index d4cbbdb0cbf1..80e41bd5ff61 100644 --- a/velox/exec/HashTable.cpp +++ b/velox/exec/HashTable.cpp @@ -955,7 +955,8 @@ void HashTable::parallelJoinBuild() { hashes); insertForJoin(overflows.data(), hashes.data(), overflows.size(), nullptr); auto table = i == 0 ? this : otherTables_[i - 1].get(); - VELOX_CHECK_EQ(table->rows()->numRows(), table->numParallelBuildRows_); + VELOX_CHECK_EQ( + table->rows()->numRows(), table->numParallelBuildRows_.load()); } } diff --git a/velox/exec/Task.cpp b/velox/exec/Task.cpp index 9ec1f5ec1577..d5a47d56fc5c 100644 --- a/velox/exec/Task.cpp +++ b/velox/exec/Task.cpp @@ -508,7 +508,10 @@ RowVectorPtr Task::next(ContinueFuture* future) { } } - VELOX_CHECK_EQ(state_, kRunning, "Task has already finished processing."); + VELOX_CHECK_EQ( + static_cast(state_), + static_cast(kRunning), + "Task has already finished processing."); // On first call, create the drivers. if (driverFactories_.empty()) { diff --git a/velox/exec/fuzzer/PrestoQueryRunner.cpp b/velox/exec/fuzzer/PrestoQueryRunner.cpp index d6ea1d59bfbf..bd68f0b1fc40 100644 --- a/velox/exec/fuzzer/PrestoQueryRunner.cpp +++ b/velox/exec/fuzzer/PrestoQueryRunner.cpp @@ -98,7 +98,9 @@ class ServerResponse { const auto& error = response_["error"]; VELOX_FAIL( - "Presto query failed: {} {}", error["errorCode"], error["message"]); + "Presto query failed: {} {}", + error["errorCode"].asInt(), + error["message"].asString()); } std::string queryId() const { diff --git a/velox/exec/tests/AggregationTest.cpp b/velox/exec/tests/AggregationTest.cpp index c6ca94e6f26e..d3acaf636475 100644 --- a/velox/exec/tests/AggregationTest.cpp +++ b/velox/exec/tests/AggregationTest.cpp @@ -2276,7 +2276,8 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringAllocation) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); @@ -2403,7 +2404,9 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringOutputProcessing) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); + auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( @@ -2700,7 +2703,9 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimWithEmptyAggregationTable) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); + auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( diff --git a/velox/exec/tests/HashJoinTest.cpp b/velox/exec/tests/HashJoinTest.cpp index d07966b90aad..cd3e4296a2b2 100644 --- a/velox/exec/tests/HashJoinTest.cpp +++ b/velox/exec/tests/HashJoinTest.cpp @@ -902,7 +902,8 @@ TEST_P(MultiThreadedHashJoinTest, outOfJoinKeyColumnOrder) { TEST_P(MultiThreadedHashJoinTest, emptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); HashJoinBuilder(*pool_, duckDbQueryRunner_, driverExecutor_.get()) .hashProbeFinishEarlyOnEmptyBuild(finishOnEmpty) @@ -1358,7 +1359,8 @@ TEST_P(MultiThreadedHashJoinTest, joinSidesDifferentSchema) { TEST_P(MultiThreadedHashJoinTest, innerJoinWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); std::vector probeVectors = makeBatches(5, [&](int32_t batch) { return makeRowVector({ @@ -1429,7 +1431,8 @@ TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilter) { TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilterWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); std::vector probeVectors = makeBatches(10, [&](int32_t /*unused*/) { @@ -1538,7 +1541,8 @@ TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilter) { TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilterWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); // probeVectors size is greater than buildVector size. std::vector probeVectors = @@ -1949,7 +1953,8 @@ TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilter) { TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilterAndEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2226,7 +2231,8 @@ TEST_P(MultiThreadedHashJoinTest, antiJoin) { TEST_P(MultiThreadedHashJoinTest, antiJoinWithFilterAndEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2338,7 +2344,8 @@ TEST_P(MultiThreadedHashJoinTest, leftJoin) { TEST_P(MultiThreadedHashJoinTest, leftJoinWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); // Left side keys are [0, 1, 2,..10]. // Use 3-rd column as row number to allow for asserting the order of @@ -2699,7 +2706,8 @@ TEST_P(MultiThreadedHashJoinTest, rightJoin) { TEST_P(MultiThreadedHashJoinTest, rightJoinWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -2933,7 +2941,8 @@ TEST_P(MultiThreadedHashJoinTest, fullJoin) { TEST_P(MultiThreadedHashJoinTest, fullJoinWithEmptyBuild) { std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE( + fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -5221,7 +5230,8 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringAllocation) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool("", kMaxBytes); @@ -5353,7 +5363,8 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringOutputProcessing) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool( "", kMaxBytes, memory::MemoryReclaimer::create()); diff --git a/velox/exec/tests/OrderByTest.cpp b/velox/exec/tests/OrderByTest.cpp index 013570d6a0e7..8c69ad40a03e 100644 --- a/velox/exec/tests/OrderByTest.cpp +++ b/velox/exec/tests/OrderByTest.cpp @@ -866,7 +866,8 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringAllocation) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( @@ -997,7 +998,8 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringOutputProcessing) { std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE( + fmt::format("enableSpilling {}", enableSpilling.operator bool())); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( diff --git a/velox/exec/tests/SharedArbitratorTest.cpp b/velox/exec/tests/SharedArbitratorTest.cpp index 059356037c90..1d315d7da978 100644 --- a/velox/exec/tests/SharedArbitratorTest.cpp +++ b/velox/exec/tests/SharedArbitratorTest.cpp @@ -190,17 +190,17 @@ class FakeMemoryOperator : public Operator { pool()->free(allocIt->buffer, allocIt->size); allocIt = allocations_.erase(allocIt); } - VELOX_CHECK_GE(totalBytes_, 0); + VELOX_CHECK_GE(totalBytes_.load(), 0); } private: void clear() { for (auto& allocation : allocations_) { totalBytes_ -= allocation.free(); - VELOX_CHECK_GE(totalBytes_, 0); + VELOX_CHECK_GE(totalBytes_.load(), 0); } allocations_.clear(); - VELOX_CHECK_EQ(totalBytes_, 0); + VELOX_CHECK_EQ(totalBytes_.load(), 0); } const bool canReclaim_; diff --git a/velox/exec/tests/TableWriteTest.cpp b/velox/exec/tests/TableWriteTest.cpp index 27ca50f499e9..2a64937fff69 100644 --- a/velox/exec/tests/TableWriteTest.cpp +++ b/velox/exec/tests/TableWriteTest.cpp @@ -30,6 +30,7 @@ #include "velox/functions/prestosql/aggregates/RegisterAggregateFunctions.h" #include "velox/vector/fuzzer/VectorFuzzer.h" +#include #include #include @@ -170,10 +171,10 @@ struct TestParam { std::string toString() const { return fmt::format( "FileFormat[{}] TestMode[{}] commitStrategy[{}] bucketKind[{}] bucketSort[{}] multiDrivers[{}] compression[{}]", - fileFormat(), - testMode(), - commitStrategy(), - bucketKind(), + dwio::common::toString((fileFormat())), + testModeString(testMode()), + commitStrategyToString(commitStrategy()), + HiveBucketProperty::kindString(bucketKind()), bucketSort(), multiDrivers(), compressionKindToString(compressionKind())); @@ -2455,7 +2456,7 @@ TEST_P(AllTableWriterTest, tableWriteOutputCheck) { if (!commitContextVector->isNullAt(i)) { ASSERT_TRUE(RE2::FullMatch( commitContextVector->valueAt(i).getString(), - fmt::format(".*{}.*", commitStrategy_))) + fmt::format(".*{}.*", commitStrategyToString(commitStrategy_)))) << commitContextVector->valueAt(i); } } From a5e2fc42c75f1bb6c777bab8391f32465ba43d66 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 19:26:23 +0100 Subject: [PATCH 07/40] add fmt std formatter header --- velox/common/base/Exceptions.h | 1 + velox/common/base/FmtStdFormatters.h | 29 ++++++++++++++++++++++++++ velox/functions/prestosql/Arithmetic.h | 7 ------- 3 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 velox/common/base/FmtStdFormatters.h diff --git a/velox/common/base/Exceptions.h b/velox/common/base/Exceptions.h index 1a96c5e59da5..8bf4edadbece 100644 --- a/velox/common/base/Exceptions.h +++ b/velox/common/base/Exceptions.h @@ -26,6 +26,7 @@ #include #include #include +#include "FmtStdFormatters.h" #include "velox/common/base/VeloxException.h" namespace facebook { diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h new file mode 100644 index 000000000000..d54ffd650966 --- /dev/null +++ b/velox/common/base/FmtStdFormatters.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * 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 +#include + +#include + +template <> +struct fmt::formatter : formatter { + auto format(std::errc s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/functions/prestosql/Arithmetic.h b/velox/functions/prestosql/Arithmetic.h index 4fac64da6651..ab1543cbb87c 100644 --- a/velox/functions/prestosql/Arithmetic.h +++ b/velox/functions/prestosql/Arithmetic.h @@ -646,10 +646,3 @@ struct CosineSimilarityFunction { } // namespace } // namespace facebook::velox::functions - -template <> -struct fmt::formatter : formatter { - auto format(std::errc s, format_context& ctx) { - return formatter::format(static_cast(s), ctx); - } -}; From 68f634ebaa54b599b07fcdb6fe6551acf155dccb Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Wed, 13 Dec 2023 21:26:31 +0100 Subject: [PATCH 08/40] Update velox/common/base/FmtStdFormatters.h --- velox/common/base/FmtStdFormatters.h | 1 - 1 file changed, 1 deletion(-) diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h index d54ffd650966..fbc36978ab11 100644 --- a/velox/common/base/FmtStdFormatters.h +++ b/velox/common/base/FmtStdFormatters.h @@ -17,7 +17,6 @@ #pragma once #include -#include #include From 5b19954bc9cda937f5a8e9bd1ad92062324c8f65 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Thu, 14 Dec 2023 14:46:40 +0100 Subject: [PATCH 09/40] temporarily remove local fmt --- .circleci/dist_compile.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 920d4f5e5798..554340f5f013 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -72,6 +72,9 @@ commands: if [ -e /opt/rh/gcc-toolset-9/enable ]; then source /opt/rh/gcc-toolset-9/enable fi + if [ -z "$VELOX_DEPENDENCY_SOURCE"]; then + rm -rf /usr/local/include/fmt || echo "fmt not found" + fi - restore_cache: name: "Restore CCache Cache" keys: @@ -247,6 +250,7 @@ jobs: executor: build environment: DuckDB_SOURCE: SYSTEM + folly_SOURCE: BUNDLED steps: - pre-steps - install-duckdb @@ -461,6 +465,7 @@ jobs: environment: VELOX_DEPENDENCY_SOURCE: SYSTEM simdjson_SOURCE: BUNDLED + folly_SOURCE: BUNDLED DuckDB_SOURCE: BUNDLED steps: - fuzzer-run: @@ -601,6 +606,8 @@ jobs: linux-pr-fuzzer-run: executor: build + environment: + folly_SOURCE: BUNDLED steps: - pre-steps - run: From c46a45699d2c4e0e131a9dac154182c6e5d75c35 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Thu, 14 Dec 2023 14:55:01 +0100 Subject: [PATCH 10/40] fix typo --- .circleci/dist_compile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 554340f5f013..23fe316e9a7b 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -72,7 +72,7 @@ commands: if [ -e /opt/rh/gcc-toolset-9/enable ]; then source /opt/rh/gcc-toolset-9/enable fi - if [ -z "$VELOX_DEPENDENCY_SOURCE"]; then + if [ -z "$VELOX_DEPENDENCY_SOURCE" ]; then rm -rf /usr/local/include/fmt || echo "fmt not found" fi - restore_cache: From b98c8d3289bc7b2632d4ac0c99db79ee8e7bcdb9 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Thu, 14 Dec 2023 15:44:12 +0100 Subject: [PATCH 11/40] always delete and use fmt bundled fix adapters job rm fmt cmake config --- .circleci/dist_compile.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 23fe316e9a7b..62777a992b35 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -72,9 +72,8 @@ commands: if [ -e /opt/rh/gcc-toolset-9/enable ]; then source /opt/rh/gcc-toolset-9/enable fi - if [ -z "$VELOX_DEPENDENCY_SOURCE" ]; then - rm -rf /usr/local/include/fmt || echo "fmt not found" - fi + rm -rf /usr/local/include/fmt || echo "fmt not found" + rm -rf /usr/local/lib64/cmake/fmt|| echo "fmt not found" - restore_cache: name: "Restore CCache Cache" keys: @@ -251,6 +250,7 @@ jobs: environment: DuckDB_SOURCE: SYSTEM folly_SOURCE: BUNDLED + fmt_SOURCE: BUNDLED steps: - pre-steps - install-duckdb @@ -396,6 +396,8 @@ jobs: ICU_SOURCE: BUNDLED simdjson_SOURCE: BUNDLED DuckDB_SOURCE: SYSTEM + fmt_SOURCE: BUNDLED + folly_SOURCE: BUNDLED steps: - pre-steps - install-duckdb @@ -466,6 +468,7 @@ jobs: VELOX_DEPENDENCY_SOURCE: SYSTEM simdjson_SOURCE: BUNDLED folly_SOURCE: BUNDLED + fmt_SOURCE: BUNDLED DuckDB_SOURCE: BUNDLED steps: - fuzzer-run: @@ -608,6 +611,7 @@ jobs: executor: build environment: folly_SOURCE: BUNDLED + fmt_SOURCE: BUNDLED steps: - pre-steps - run: From 174f4a81073a64396489847bde7b1a6b1632b912 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Thu, 14 Dec 2023 15:50:34 +0100 Subject: [PATCH 12/40] update setup scripts --- scripts/setup-centos8.sh | 2 +- scripts/setup-circleci.sh | 2 +- scripts/setup-macos.sh | 2 +- scripts/setup-ubuntu.sh | 7 +------ scripts/setup-velox-torcharrow.sh | 2 +- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/scripts/setup-centos8.sh b/scripts/setup-centos8.sh index fe6042301d8f..e6808097509f 100755 --- a/scripts/setup-centos8.sh +++ b/scripts/setup-centos8.sh @@ -68,7 +68,7 @@ wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog & wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo & wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & -wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt & +wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & wait # For cmake and source downloads to complete. diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index bd275f48869e..90432bf83b76 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -70,7 +70,7 @@ wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog & wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo & wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & -wget_and_untar https://github.com/fmtlib/fmt/archive/8.0.1.tar.gz fmt & +wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & # wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & diff --git a/scripts/setup-macos.sh b/scripts/setup-macos.sh index 83d8990603ea..00195e30a291 100755 --- a/scripts/setup-macos.sh +++ b/scripts/setup-macos.sh @@ -88,7 +88,7 @@ function install_build_prerequisites { } function install_fmt { - github_checkout fmtlib/fmt 8.0.1 + github_checkout fmtlib/fmt 10.1.1 cmake_install -DFMT_TEST=OFF } diff --git a/scripts/setup-ubuntu.sh b/scripts/setup-ubuntu.sh index 42054e593092..4e961b336fdb 100755 --- a/scripts/setup-ubuntu.sh +++ b/scripts/setup-ubuntu.sh @@ -45,6 +45,7 @@ sudo --preserve-env apt update && sudo --preserve-env apt install -y libunwind-d libboost-all-dev \ libicu-dev \ libdouble-conversion-dev \ + libfmt-dev \ libgoogle-glog-dev \ libbz2-dev \ libgflags-dev \ @@ -85,11 +86,6 @@ function prompt { ) 2> /dev/null } -function install_fmt { - github_checkout fmtlib/fmt 8.0.1 - cmake_install -DFMT_TEST=OFF -} - function install_folly { github_checkout facebook/folly "${FB_OS_VERSION}" cmake_install -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON @@ -118,7 +114,6 @@ function install_conda { } function install_velox_deps { - run_and_time install_fmt run_and_time install_folly run_and_time install_fizz run_and_time install_wangle diff --git a/scripts/setup-velox-torcharrow.sh b/scripts/setup-velox-torcharrow.sh index a31e35515d21..72186bc7158c 100755 --- a/scripts/setup-velox-torcharrow.sh +++ b/scripts/setup-velox-torcharrow.sh @@ -80,7 +80,7 @@ wget_and_untar https://github.com/gflags/gflags/archive/refs/tags/v2.2.2.tar.gz wget_and_untar https://ftp.openssl.org/source/openssl-1.1.1k.tar.gz openssl & wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.69.0/source/boost_1_69_0.tar.gz boost & wget_and_untar https://github.com/facebook/folly/archive/v2022.11.14.00.tar.gz folly & -wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/8.0.1.tar.gz fmt & +wget_and_untar https://github.com/fmtlib/fmt/archive/refs/tags/10.1.1.tar.gz fmt & wait From e0eba43845bd1b17771860b44fbc45ffbd9def82 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 01:02:37 +0100 Subject: [PATCH 13/40] use operator bool() --- velox/common/memory/tests/MockSharedArbitratorTest.cpp | 2 +- velox/exec/tests/OutputBufferManagerTest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 5d36f80730d8..31f2a33ade43 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -963,7 +963,7 @@ TEST_F(MockSharedArbitrationTest, arbitrateWithMemoryReclaim) { const uint64_t minPoolCapacity = 8 * MB; const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); setupMemory(memoryCapacity, minPoolCapacity); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = 8 * MB; diff --git a/velox/exec/tests/OutputBufferManagerTest.cpp b/velox/exec/tests/OutputBufferManagerTest.cpp index 254f3c713bac..92c054dbcc3d 100644 --- a/velox/exec/tests/OutputBufferManagerTest.cpp +++ b/velox/exec/tests/OutputBufferManagerTest.cpp @@ -1061,7 +1061,7 @@ TEST_F(OutputBufferManagerTest, updateBrodcastBufferOnFailedTask) { TEST_P(AllOutputBufferManagerTest, multiFetchers) { const std::vector earlyTerminations = {false, true}; for (const auto earlyTermination : earlyTerminations) { - SCOPED_TRACE(fmt::format("earlyTermination {}", earlyTermination)); + SCOPED_TRACE(fmt::format("earlyTermination {}", earlyTermination.operator bool())); const vector_size_t size = 10; const std::string taskId = "t0"; From 17e63aea3b0e45a293551015bd9e28c58ddb43df Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 01:16:51 +0100 Subject: [PATCH 14/40] add missing deps to setup-circleci.sh --- scripts/setup-circleci.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 90432bf83b76..36e82e51d88c 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -30,7 +30,7 @@ function dnf_install { dnf_install epel-release dnf-plugins-core # For ccache, ninja dnf config-manager --set-enabled powertools -dnf_install ninja-build ccache gcc-toolset-9 git wget which libevent-devel \ +dnf_install ninja-build cmake ccache gcc-toolset-9 git wget which libevent-devel \ openssl-devel re2-devel libzstd-devel lz4-devel double-conversion-devel \ libdwarf-devel curl-devel libicu-devel @@ -81,6 +81,7 @@ wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & +wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst wait # For cmake and source downloads to complete. @@ -116,6 +117,7 @@ cmake_install folly -DFOLLY_HAVE_INT128_T=ON cmake_install fizz/fizz -DBUILD_TESTS=OFF cmake_install wangle/wangle -DBUILD_TESTS=OFF +cmake_install mvfst -DBUILD_TESTS=OFF cmake_install fbthrift -Denable_tests=OFF # cmake_install ranges-v3 From 86787f70f5b3d1fbf7b800a48dbdfc75dbe12e6a Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 02:55:11 +0100 Subject: [PATCH 15/40] remove operator bool() --- .../common/memory/tests/MemoryManagerTest.cpp | 6 +- .../memory/tests/MockSharedArbitratorTest.cpp | 12 ++-- velox/exec/tests/AggregationTest.cpp | 12 ++-- velox/exec/tests/HashJoinTest.cpp | 60 +++++++++---------- velox/exec/tests/OrderByTest.cpp | 8 +-- velox/exec/tests/OutputBufferManagerTest.cpp | 2 +- 6 files changed, 50 insertions(+), 50 deletions(-) diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index 243c9d022b92..04f561528e34 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -589,10 +589,10 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { std::make_shared(testData.memoryQuotaBytes); MemoryAllocator::setDefaultInstance(allocator.get()); SCOPED_TRACE(testData.debugString()); - std::vector contiguousAllocations = {false, true}; - for (const auto& contiguousAlloc : contiguousAllocations) { + const std::vector contiguousAllocations = {false, true}; + for (const auto contiguousAlloc : contiguousAllocations) { SCOPED_TRACE( - fmt::format("contiguousAlloc {}", contiguousAlloc.operator bool())); + fmt::format("contiguousAlloc {}", contiguousAlloc)); const int alignment = 32; MemoryManagerOptions options; options.alignment = alignment; diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 31f2a33ade43..b54e12dadcd9 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -883,10 +883,10 @@ TEST_F(MockSharedArbitrationTest, failedArbitration) { } TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { - std::vector isLeafReclaimables = {true, false}; + const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { SCOPED_TRACE( - fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); + fmt::format("isLeafReclaimable {}", isLeafReclaimable)); setupMemory(); auto op = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = MB; @@ -925,10 +925,10 @@ TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { } TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { - std::vector isLeafReclaimables = {true, false}; + const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { SCOPED_TRACE( - fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); + fmt::format("isLeafReclaimable {}", isLeafReclaimable)); setupMemory(); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int reclaimedOpCapacity = kMemoryCapacity * 2 / 3; @@ -961,9 +961,9 @@ TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { TEST_F(MockSharedArbitrationTest, arbitrateWithMemoryReclaim) { const uint64_t memoryCapacity = 256 * MB; const uint64_t minPoolCapacity = 8 * MB; - const std::vector isLeafReclaimables = {true, false}; + const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable.operator bool())); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); setupMemory(memoryCapacity, minPoolCapacity); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = 8 * MB; diff --git a/velox/exec/tests/AggregationTest.cpp b/velox/exec/tests/AggregationTest.cpp index d3acaf636475..ec0c55996e1e 100644 --- a/velox/exec/tests/AggregationTest.cpp +++ b/velox/exec/tests/AggregationTest.cpp @@ -2274,10 +2274,10 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringAllocation) { auto rowType = ROW({"c0", "c1", "c2"}, {INTEGER(), INTEGER(), VARCHAR()}); auto batches = makeVectors(rowType, 1000, 10); - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); @@ -2402,10 +2402,10 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringOutputProcessing) { auto rowType = ROW({"c0", "c1", "c2"}, {INTEGER(), INTEGER(), INTEGER()}); auto batches = makeVectors(rowType, 1000, 10); - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); @@ -2701,10 +2701,10 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimWithEmptyAggregationTable) { auto rowType = ROW({"c0", "c1", "c2"}, {INTEGER(), INTEGER(), INTEGER()}); auto batches = makeVectors(rowType, 1000, 10); - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); diff --git a/velox/exec/tests/HashJoinTest.cpp b/velox/exec/tests/HashJoinTest.cpp index cd3e4296a2b2..ad7159ef3477 100644 --- a/velox/exec/tests/HashJoinTest.cpp +++ b/velox/exec/tests/HashJoinTest.cpp @@ -900,10 +900,10 @@ TEST_P(MultiThreadedHashJoinTest, outOfJoinKeyColumnOrder) { } TEST_P(MultiThreadedHashJoinTest, emptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); HashJoinBuilder(*pool_, duckDbQueryRunner_, driverExecutor_.get()) .hashProbeFinishEarlyOnEmptyBuild(finishOnEmpty) @@ -1357,10 +1357,10 @@ TEST_P(MultiThreadedHashJoinTest, joinSidesDifferentSchema) { } TEST_P(MultiThreadedHashJoinTest, innerJoinWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; + const std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); std::vector probeVectors = makeBatches(5, [&](int32_t batch) { return makeRowVector({ @@ -1429,10 +1429,10 @@ TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilter) { } TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilterWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); std::vector probeVectors = makeBatches(10, [&](int32_t /*unused*/) { @@ -1539,10 +1539,10 @@ TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilter) { } TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilterWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); // probeVectors size is greater than buildVector size. std::vector probeVectors = @@ -1951,10 +1951,10 @@ TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilter) { } TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilterAndEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2229,10 +2229,10 @@ TEST_P(MultiThreadedHashJoinTest, antiJoin) { } TEST_P(MultiThreadedHashJoinTest, antiJoinWithFilterAndEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2342,10 +2342,10 @@ TEST_P(MultiThreadedHashJoinTest, leftJoin) { } TEST_P(MultiThreadedHashJoinTest, leftJoinWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. // Use 3-rd column as row number to allow for asserting the order of @@ -2704,10 +2704,10 @@ TEST_P(MultiThreadedHashJoinTest, rightJoin) { } TEST_P(MultiThreadedHashJoinTest, rightJoinWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -2939,10 +2939,10 @@ TEST_P(MultiThreadedHashJoinTest, fullJoin) { } TEST_P(MultiThreadedHashJoinTest, fullJoinWithEmptyBuild) { - std::vector finishOnEmptys = {false, true}; - for (auto finishOnEmpty : finishOnEmptys) { + const std::vector finishOnEmptys = {false, true}; + for (const auto finishOnEmpty : finishOnEmptys) { SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty.operator bool())); + fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -5228,10 +5228,10 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringAllocation) { createDuckDbTable("t", probeVectors); createDuckDbTable("u", buildVectors); - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool("", kMaxBytes); @@ -5361,10 +5361,10 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringOutputProcessing) { createDuckDbTable("t", probeVectors); createDuckDbTable("u", buildVectors); - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool( "", kMaxBytes, memory::MemoryReclaimer::create()); diff --git a/velox/exec/tests/OrderByTest.cpp b/velox/exec/tests/OrderByTest.cpp index 8c69ad40a03e..38080bd40536 100644 --- a/velox/exec/tests/OrderByTest.cpp +++ b/velox/exec/tests/OrderByTest.cpp @@ -864,10 +864,10 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringAllocation) { batches.push_back(fuzzer.fuzzRow(rowType)); } - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( @@ -996,10 +996,10 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringOutputProcessing) { batches.push_back(fuzzer.fuzzRow(rowType)); } - std::vector enableSpillings = {false, true}; + const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling.operator bool())); + fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( diff --git a/velox/exec/tests/OutputBufferManagerTest.cpp b/velox/exec/tests/OutputBufferManagerTest.cpp index 92c054dbcc3d..254f3c713bac 100644 --- a/velox/exec/tests/OutputBufferManagerTest.cpp +++ b/velox/exec/tests/OutputBufferManagerTest.cpp @@ -1061,7 +1061,7 @@ TEST_F(OutputBufferManagerTest, updateBrodcastBufferOnFailedTask) { TEST_P(AllOutputBufferManagerTest, multiFetchers) { const std::vector earlyTerminations = {false, true}; for (const auto earlyTermination : earlyTerminations) { - SCOPED_TRACE(fmt::format("earlyTermination {}", earlyTermination.operator bool())); + SCOPED_TRACE(fmt::format("earlyTermination {}", earlyTermination)); const vector_size_t size = 10; const std::string taskId = "t0"; From 328095ddb142c04e7b85c2747b2ad42fdec3642c Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:03:39 +0100 Subject: [PATCH 16/40] add fmt/format.h explicitly, format --- .../common/memory/tests/MemoryManagerTest.cpp | 4 +-- .../memory/tests/MockSharedArbitratorTest.cpp | 7 ++-- velox/exec/tests/AggregationTest.cpp | 10 +++--- velox/exec/tests/HashJoinTest.cpp | 34 +++++++------------ velox/exec/tests/OrderByTest.cpp | 7 ++-- 5 files changed, 24 insertions(+), 38 deletions(-) diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index 04f561528e34..2df090bb6b0d 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include "velox/common/base/VeloxException.h" @@ -591,8 +592,7 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { SCOPED_TRACE(testData.debugString()); const std::vector contiguousAllocations = {false, true}; for (const auto contiguousAlloc : contiguousAllocations) { - SCOPED_TRACE( - fmt::format("contiguousAlloc {}", contiguousAlloc)); + SCOPED_TRACE(fmt::format("contiguousAlloc {}", contiguousAlloc)); const int alignment = 32; MemoryManagerOptions options; options.alignment = alignment; diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index b54e12dadcd9..184df69aceb1 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -16,6 +16,7 @@ #include +#include #include #include #include "folly/experimental/EventCount.h" @@ -885,8 +886,7 @@ TEST_F(MockSharedArbitrationTest, failedArbitration) { TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE( - fmt::format("isLeafReclaimable {}", isLeafReclaimable)); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); setupMemory(); auto op = addMemoryOp(nullptr, isLeafReclaimable); const int allocateSize = MB; @@ -927,8 +927,7 @@ TEST_F(MockSharedArbitrationTest, singlePoolGrowCapacityWithArbitration) { TEST_F(MockSharedArbitrationTest, arbitrateWithCapacityShrink) { const std::vector isLeafReclaimables = {true, false}; for (const auto isLeafReclaimable : isLeafReclaimables) { - SCOPED_TRACE( - fmt::format("isLeafReclaimable {}", isLeafReclaimable)); + SCOPED_TRACE(fmt::format("isLeafReclaimable {}", isLeafReclaimable)); setupMemory(); auto* reclaimedOp = addMemoryOp(nullptr, isLeafReclaimable); const int reclaimedOpCapacity = kMemoryCapacity * 2 / 3; diff --git a/velox/exec/tests/AggregationTest.cpp b/velox/exec/tests/AggregationTest.cpp index ec0c55996e1e..8a1a3ea659ed 100644 --- a/velox/exec/tests/AggregationTest.cpp +++ b/velox/exec/tests/AggregationTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -2276,8 +2277,7 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringAllocation) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); @@ -2404,8 +2404,7 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimDuringOutputProcessing) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); @@ -2703,8 +2702,7 @@ DEBUG_ONLY_TEST_F(AggregationTest, reclaimWithEmptyAggregationTable) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); diff --git a/velox/exec/tests/HashJoinTest.cpp b/velox/exec/tests/HashJoinTest.cpp index ad7159ef3477..0be156e09cfa 100644 --- a/velox/exec/tests/HashJoinTest.cpp +++ b/velox/exec/tests/HashJoinTest.cpp @@ -16,6 +16,7 @@ #include +#include #include "folly/experimental/EventCount.h" #include "velox/common/base/tests/GTestUtils.h" #include "velox/common/testutil/TestValue.h" @@ -902,8 +903,7 @@ TEST_P(MultiThreadedHashJoinTest, outOfJoinKeyColumnOrder) { TEST_P(MultiThreadedHashJoinTest, emptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); HashJoinBuilder(*pool_, duckDbQueryRunner_, driverExecutor_.get()) .hashProbeFinishEarlyOnEmptyBuild(finishOnEmpty) @@ -1359,8 +1359,7 @@ TEST_P(MultiThreadedHashJoinTest, joinSidesDifferentSchema) { TEST_P(MultiThreadedHashJoinTest, innerJoinWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); std::vector probeVectors = makeBatches(5, [&](int32_t batch) { return makeRowVector({ @@ -1431,8 +1430,7 @@ TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilter) { TEST_P(MultiThreadedHashJoinTest, leftSemiJoinFilterWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); std::vector probeVectors = makeBatches(10, [&](int32_t /*unused*/) { @@ -1541,8 +1539,7 @@ TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilter) { TEST_P(MultiThreadedHashJoinTest, rightSemiJoinFilterWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); // probeVectors size is greater than buildVector size. std::vector probeVectors = @@ -1953,8 +1950,7 @@ TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilter) { TEST_P(MultiThreadedHashJoinTest, nullAwareAntiJoinWithFilterAndEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2231,8 +2227,7 @@ TEST_P(MultiThreadedHashJoinTest, antiJoin) { TEST_P(MultiThreadedHashJoinTest, antiJoinWithFilterAndEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { return makeRowVector( @@ -2344,8 +2339,7 @@ TEST_P(MultiThreadedHashJoinTest, leftJoin) { TEST_P(MultiThreadedHashJoinTest, leftJoinWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. // Use 3-rd column as row number to allow for asserting the order of @@ -2706,8 +2700,7 @@ TEST_P(MultiThreadedHashJoinTest, rightJoin) { TEST_P(MultiThreadedHashJoinTest, rightJoinWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -2941,8 +2934,7 @@ TEST_P(MultiThreadedHashJoinTest, fullJoin) { TEST_P(MultiThreadedHashJoinTest, fullJoinWithEmptyBuild) { const std::vector finishOnEmptys = {false, true}; for (const auto finishOnEmpty : finishOnEmptys) { - SCOPED_TRACE( - fmt::format("finishOnEmpty: {}", finishOnEmpty)); + SCOPED_TRACE(fmt::format("finishOnEmpty: {}", finishOnEmpty)); // Left side keys are [0, 1, 2,..10]. std::vector probeVectors = mergeBatches( @@ -5230,8 +5222,7 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringAllocation) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool("", kMaxBytes); @@ -5363,8 +5354,7 @@ DEBUG_ONLY_TEST_F(HashJoinTest, reclaimDuringOutputProcessing) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryPool = memory::defaultMemoryManager().addRootPool( "", kMaxBytes, memory::MemoryReclaimer::create()); diff --git a/velox/exec/tests/OrderByTest.cpp b/velox/exec/tests/OrderByTest.cpp index 38080bd40536..e572d90c9369 100644 --- a/velox/exec/tests/OrderByTest.cpp +++ b/velox/exec/tests/OrderByTest.cpp @@ -15,6 +15,7 @@ */ #include +#include #include "folly/experimental/EventCount.h" #include "velox/common/base/tests/GTestUtils.h" #include "velox/common/file/FileSystems.h" @@ -866,8 +867,7 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringAllocation) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( @@ -998,8 +998,7 @@ DEBUG_ONLY_TEST_F(OrderByTest, reclaimDuringOutputProcessing) { const std::vector enableSpillings = {false, true}; for (const auto enableSpilling : enableSpillings) { - SCOPED_TRACE( - fmt::format("enableSpilling {}", enableSpilling)); + SCOPED_TRACE(fmt::format("enableSpilling {}", enableSpilling)); auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool( From cfb794d4bc596f38cff3fa229e285dfd90aee28e Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:06:27 +0100 Subject: [PATCH 17/40] install deps after checking out baseline --- .github/workflows/benchmark.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 4bbd3d0c66db..46428d529edf 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -69,10 +69,6 @@ jobs: fetch-depth: 0 submodules: 'recursive' - - name: "Install dependencies" - if: ${{ github.event_name == 'pull_request' }} - run: source velox/scripts/setup-ubuntu.sh - - name: "Checkout Merge Base" if: ${{ github.event_name == 'pull_request' }} working-directory: velox @@ -90,6 +86,10 @@ jobs: git submodule update --init --recursive echo $(git log -n 1) + - name: "Install dependencies" + if: ${{ github.event_name == 'pull_request' }} + run: source velox/scripts/setup-ubuntu.sh + - name: Build Baseline Benchmarks if: ${{ github.event_name == 'pull_request' }} working-directory: velox From 6150585dae06a193c83d1bef2ac1873320a75310 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:13:15 +0100 Subject: [PATCH 18/40] add missing & --- scripts/setup-circleci.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 36e82e51d88c..466524f82609 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -72,7 +72,7 @@ wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/ wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & # wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & -wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop +wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & FB_OS_VERSION="v2022.11.14.00" @@ -81,7 +81,7 @@ wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & -wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst +wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & wait # For cmake and source downloads to complete. From 442b6a982e1b3a526d380a6e4680f325a26b266f Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:33:58 +0100 Subject: [PATCH 19/40] move fbthirft find_package behind folly --- CMakeLists.txt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f4c55ba4a011..39e48eca956c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -258,13 +258,7 @@ if(VELOX_ENABLE_PARQUET) set(VELOX_ENABLE_ARROW ON) endif() -if(VELOX_ENABLE_REMOTE_FUNCTIONS) - # TODO: Move this to use resolve_dependency(). For some reason, FBThrift - # requires clients to explicitly install fizz and wangle. - find_package(fizz CONFIG REQUIRED) - find_package(wangle CONFIG REQUIRED) - find_package(FBThrift CONFIG REQUIRED) -endif() + # define processor variable for conditional compilation if(${VELOX_CODEGEN_SUPPORT}) @@ -463,6 +457,14 @@ add_compile_definitions(FOLLY_HAVE_INT128_T=1) set_source(folly) resolve_dependency(folly) +if(VELOX_ENABLE_REMOTE_FUNCTIONS) + # TODO: Move this to use resolve_dependency(). For some reason, FBThrift + # requires clients to explicitly install fizz and wangle. + find_package(fizz CONFIG REQUIRED) + find_package(wangle CONFIG REQUIRED) + find_package(FBThrift CONFIG REQUIRED) +endif() + if(DEFINED FOLLY_BENCHMARK_STATIC_LIB) set(FOLLY_BENCHMARK ${FOLLY_BENCHMARK_STATIC_LIB}) else() From badddd2d49bff7f9124cdb36aeda176fd374a846 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:42:41 +0100 Subject: [PATCH 20/40] iwyu ? --- CMakeLists.txt | 2 -- velox/common/memory/tests/MemoryManagerTest.cpp | 1 + velox/common/memory/tests/MockSharedArbitratorTest.cpp | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 39e48eca956c..c891392efecd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -258,8 +258,6 @@ if(VELOX_ENABLE_PARQUET) set(VELOX_ENABLE_ARROW ON) endif() - - # define processor variable for conditional compilation if(${VELOX_CODEGEN_SUPPORT}) add_compile_definitions(CODEGEN_ENABLED=1) diff --git a/velox/common/memory/tests/MemoryManagerTest.cpp b/velox/common/memory/tests/MemoryManagerTest.cpp index 2df090bb6b0d..5712c15e23be 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -16,6 +16,7 @@ #include #include +#include #include "velox/common/base/VeloxException.h" #include "velox/common/base/tests/GTestUtils.h" diff --git a/velox/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 184df69aceb1..323418fb17a0 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include "folly/experimental/EventCount.h" #include "folly/futures/Barrier.h" #include "velox/common/base/tests/GTestUtils.h" From 2d72661c21fbf4b43f9e87aabe7988e45b2d7f4c Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 05:44:27 +0100 Subject: [PATCH 21/40] wait to stop throttling? --- scripts/setup-circleci.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 466524f82609..097525a76f69 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -71,6 +71,9 @@ wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & + +wait # For cmake and source downloads to complete. + # wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & From 4815d4fb1ead9bb5791ed6d82ed461553eb284e6 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 06:12:15 +0100 Subject: [PATCH 22/40] remove & --- scripts/setup-circleci.sh | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 097525a76f69..84a11841f3a6 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -65,26 +65,24 @@ function wget_and_untar { wget_and_untar https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz /usr & # Fetch sources. -wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags & -wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog & -wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo & -wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & -wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & -wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & +wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags +wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog +wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo +wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost +wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy +wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt -wait # For cmake and source downloads to complete. - -# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & -wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & -wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & +# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 +wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop +wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf FB_OS_VERSION="v2022.11.14.00" -wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly & -wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & -wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & -wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & -wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & +wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly +wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz +wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle +wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift +wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst wait # For cmake and source downloads to complete. From 93da4737d5a96eafa77c144951831d82ae876cf1 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 15 Dec 2023 07:10:52 +0100 Subject: [PATCH 23/40] add .load() --- velox/common/memory/tests/CMakeLists.txt | 1 + velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/velox/common/memory/tests/CMakeLists.txt b/velox/common/memory/tests/CMakeLists.txt index f243b3f4be7d..c58b0dd86821 100644 --- a/velox/common/memory/tests/CMakeLists.txt +++ b/velox/common/memory/tests/CMakeLists.txt @@ -39,6 +39,7 @@ target_link_libraries( velox_test_util velox_vector_fuzzer Folly::folly + fmt::fmt gflags::gflags glog::glog gmock diff --git a/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp b/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp index 8b1113e398bf..63d1f1396770 100644 --- a/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp +++ b/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp @@ -83,7 +83,7 @@ class GCSReadFile final : public ReadFile { key_); } length_ = (*metadata).size(); - VELOX_CHECK_GE(length_, 0); + VELOX_CHECK_GE(length_.load(), 0); } std::string_view pread(uint64_t offset, uint64_t length, void* buffer) From 024f5074f6effa044f4ea976dba1ee4aad7467b9 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sat, 16 Dec 2023 03:04:31 +0100 Subject: [PATCH 24/40] Revert "remove &" This reverts commit 4815d4fb1ead9bb5791ed6d82ed461553eb284e6. --- scripts/setup-circleci.sh | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 84a11841f3a6..097525a76f69 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -65,24 +65,26 @@ function wget_and_untar { wget_and_untar https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz /usr & # Fetch sources. -wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags -wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog -wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo -wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost -wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy -wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt +wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags & +wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog & +wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo & +wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & +wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & +wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & -# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 -wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop -wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf +wait # For cmake and source downloads to complete. + +# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & +wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & +wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & FB_OS_VERSION="v2022.11.14.00" -wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly -wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz -wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle -wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift -wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst +wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly & +wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & +wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & +wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & +wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & wait # For cmake and source downloads to complete. From e74ce8f5142a32075fa02ad7aa6dbd6709086df4 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sat, 16 Dec 2023 03:04:48 +0100 Subject: [PATCH 25/40] Revert "wait to stop throttling?" This reverts commit 2d72661c21fbf4b43f9e87aabe7988e45b2d7f4c. --- scripts/setup-circleci.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 097525a76f69..466524f82609 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -71,9 +71,6 @@ wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & - -wait # For cmake and source downloads to complete. - # wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & From 1811482a0ff0e19ea03b00da53e05983d35195fb Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sat, 16 Dec 2023 03:05:52 +0100 Subject: [PATCH 26/40] fix mvfst version --- scripts/setup-circleci.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 466524f82609..fb82dcebb97c 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -81,6 +81,7 @@ wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & +FB_OS_VERSION="v2023.12.04.00" #mvfast doesn't have a v2022 tag wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & wait # For cmake and source downloads to complete. From ba22d82ea329e5353ced4fcb7c747d76d3089336 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sat, 16 Dec 2023 05:14:44 +0100 Subject: [PATCH 27/40] add more formatters --- velox/benchmarks/basic/LikeTpchBenchmark.cpp | 7 ++ velox/common/base/FmtStdFormatters.h | 2 +- velox/common/compression/Compression.h | 12 +++ .../hive/storage_adapters/s3fs/S3Util.h | 9 ++- .../dwio/parquet/thrift/ParquetThriftTypes.h | 79 +++++++++++++++++++ velox/substrait/SubstraitToVeloxExpr.h | 10 +++ 6 files changed, 117 insertions(+), 2 deletions(-) diff --git a/velox/benchmarks/basic/LikeTpchBenchmark.cpp b/velox/benchmarks/basic/LikeTpchBenchmark.cpp index 1931585fdc9a..600697ef8a0d 100644 --- a/velox/benchmarks/basic/LikeTpchBenchmark.cpp +++ b/velox/benchmarks/basic/LikeTpchBenchmark.cpp @@ -240,3 +240,10 @@ int main(int argc, char* argv[]) { return 0; } + +template <> +struct fmt::formatter : formatter { + auto format(const TpchBenchmarkCase& s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h index fbc36978ab11..003139a750a9 100644 --- a/velox/common/base/FmtStdFormatters.h +++ b/velox/common/base/FmtStdFormatters.h @@ -22,7 +22,7 @@ template <> struct fmt::formatter : formatter { - auto format(std::errc s, format_context& ctx) { + auto format(const std::errc& s, format_context& ctx) { return formatter::format(static_cast(s), ctx); } }; diff --git a/velox/common/compression/Compression.h b/velox/common/compression/Compression.h index 317f9717a2ae..82658976571c 100644 --- a/velox/common/compression/Compression.h +++ b/velox/common/compression/Compression.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include @@ -46,3 +47,14 @@ CompressionKind stringToCompressionKind(const std::string& kind); constexpr uint64_t DEFAULT_COMPRESSION_BLOCK_SIZE = 256 * 1024; } // namespace facebook::velox::common + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::common::CompressionKind& s, + format_context& ctx) { + return formatter::format( + facebook::velox::common::compressionKindToString(s), ctx); + } +}; diff --git a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h index c5a51d28c7af..d0979b2ccb00 100644 --- a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h +++ b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h @@ -163,7 +163,7 @@ inline std::string getRequestID( errorMsgPrefix, \ getErrorStringFromS3Error(error), \ s3URI(bucket, key), \ - error.GetErrorType(), \ + error.GetExceptionName(), \ error.GetResponseCode(), \ getS3BackendService(error.GetResponseHeaders()), \ error.GetMessage(), \ @@ -172,3 +172,10 @@ inline std::string getRequestID( } } // namespace facebook::velox + +template <> +struct fmt::formatter : formatter { + auto format(Aws::Http::HttpResponseCode s, format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; diff --git a/velox/dwio/parquet/thrift/ParquetThriftTypes.h b/velox/dwio/parquet/thrift/ParquetThriftTypes.h index 517c425b7f01..2bb89ca530c8 100644 --- a/velox/dwio/parquet/thrift/ParquetThriftTypes.h +++ b/velox/dwio/parquet/thrift/ParquetThriftTypes.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -3742,3 +3743,81 @@ void swap(FileCryptoMetaData& a, FileCryptoMetaData& b); std::ostream& operator<<(std::ostream& out, const FileCryptoMetaData& obj); } // namespace facebook::velox::parquet::thrift + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::Type::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::CompressionCodec::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::ConvertedType::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter< + facebook::velox::parquet::thrift::FieldRepetitionType::type> + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::FieldRepetitionType::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::Encoding::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::PageType::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; + +template <> +struct fmt::formatter + : fmt::formatter { + auto format( + const facebook::velox::parquet::thrift::BoundaryOrder::type& s, + format_context& ctx) { + return formatter::format( + facebook::velox::parquet::thrift::to_string(s), ctx); + } +}; diff --git a/velox/substrait/SubstraitToVeloxExpr.h b/velox/substrait/SubstraitToVeloxExpr.h index dd5957e8e785..fdfef92d67a5 100644 --- a/velox/substrait/SubstraitToVeloxExpr.h +++ b/velox/substrait/SubstraitToVeloxExpr.h @@ -16,6 +16,7 @@ #pragma once +#include #include "velox/core/Expressions.h" #include "velox/substrait/SubstraitParser.h" #include "velox/vector/ComplexVector.h" @@ -81,3 +82,12 @@ class SubstraitVeloxExprConverter { }; } // namespace facebook::velox::substrait + +template <> +struct fmt::formatter : formatter { + auto format( + const substrait::Expression::RexTypeCase& s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; From 33bc29db471242aaa446eb9d24588eb1089ca668 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 02:34:33 +0100 Subject: [PATCH 28/40] backport bool formatter form fmt 10.1 format --- velox/common/base/FmtStdFormatters.h | 49 ++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h index 003139a750a9..ca18f537dde3 100644 --- a/velox/common/base/FmtStdFormatters.h +++ b/velox/common/base/FmtStdFormatters.h @@ -18,7 +18,10 @@ #include +#include #include +#include +#include template <> struct fmt::formatter : formatter { @@ -26,3 +29,49 @@ struct fmt::formatter : formatter { return formatter::format(static_cast(s), ctx); } }; + +// -- Backport from fmt 10 see fmtlib/fmt#3570 +#if FMT_VERSION < 100100 +namespace fmt::detail { +template +struct has_flip : std::false_type {}; + +template +struct has_flip().flip())>> + : std::true_type {}; + +template +struct is_bit_reference_like { + static constexpr const bool value = std::is_convertible::value && + std::is_nothrow_assignable::value && has_flip::value; +}; + +#ifdef _LIBCPP_VERSION + +// Workaround for libc++ incompatibility with C++ standard. +// According to the Standard, `bitset::operator[] const` returns bool. +template +struct is_bit_reference_like> { + static constexpr const bool value = true; +}; + +#endif +} // namespace fmt::detail + +// We can't use std::vector::reference and +// std::bitset::reference because the compiler can't deduce Allocator and N +// in partial specialization. +template +struct fmt::formatter< + BitRef, + Char, + std::enable_if_t::value>> + : formatter { + template + FMT_CONSTEXPR auto format(const BitRef& v, FormatContext& ctx) const + -> decltype(ctx.out()) { + return formatter::format(v, ctx); + } +}; +#endif +// -- End of backport From 4718b1fec7abb200b711a2cf3fbc7995cc3c4264 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 02:34:54 +0100 Subject: [PATCH 29/40] link fmt --- velox/dwio/parquet/thrift/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/velox/dwio/parquet/thrift/CMakeLists.txt b/velox/dwio/parquet/thrift/CMakeLists.txt index 08df5614f92f..8b21b15a2e60 100644 --- a/velox/dwio/parquet/thrift/CMakeLists.txt +++ b/velox/dwio/parquet/thrift/CMakeLists.txt @@ -13,4 +13,5 @@ # limitations under the License. add_library(velox_dwio_parquet_thrift ParquetThriftTypes.cpp) -target_link_libraries(velox_dwio_parquet_thrift arrow thrift Boost::headers) +target_link_libraries(velox_dwio_parquet_thrift arrow thrift Boost::headers + fmt::fmt) From 90e6a8a2526f46c1e0ea393efd61b0f582de4fc3 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 02:42:48 +0100 Subject: [PATCH 30/40] use earliest mvfst version --- scripts/setup-circleci.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index fb82dcebb97c..ffef106c3578 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -81,7 +81,7 @@ wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & -FB_OS_VERSION="v2023.12.04.00" #mvfast doesn't have a v2022 tag +FB_OS_VERSION="v2023.07.24.00" #mvfast doesn't have a v2022 tag wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & wait # For cmake and source downloads to complete. From f309e251c41a56b541559e7cc1e6a3978a72085a Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 03:33:33 +0100 Subject: [PATCH 31/40] add backports for atomic and errc --- velox/common/base/FmtStdFormatters.h | 44 ++++++++++++++++--- velox/common/caching/AsyncDataCache.cpp | 7 +-- velox/common/caching/SsdCache.cpp | 3 +- .../hive/storage_adapters/gcs/CMakeLists.txt | 3 +- .../storage_adapters/gcs/GCSFileSystem.cpp | 3 +- velox/exec/HashTable.cpp | 4 +- velox/exec/tests/SharedArbitratorTest.cpp | 5 ++- velox/substrait/SubstraitParser.cpp | 3 +- 8 files changed, 54 insertions(+), 18 deletions(-) diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h index ca18f537dde3..2e73a985bb4d 100644 --- a/velox/common/base/FmtStdFormatters.h +++ b/velox/common/base/FmtStdFormatters.h @@ -17,21 +17,52 @@ #pragma once #include +#if FMT_VERSION >= 100100 +#include +#endif +#include #include -#include +#include +#include #include #include -template <> -struct fmt::formatter : formatter { - auto format(const std::errc& s, format_context& ctx) { - return formatter::format(static_cast(s), ctx); +template +struct fmt::formatter + : formatter::type, Char> { + template + auto format(std::errc v, FormatContext& ctx) const + -> decltype(ctx.out()) { + using underlying_type = std::underlying_type::type; + return formatter::format( + static_cast(v), ctx); + } +}; + +#if FMT_VERSION < 100100 +// This should be 100101 but FMT_VERSION was not bumped in 10.1.1 +// but under a month has passed since 10.1.0 release so we can assume 10.1.1 +// +// Backport from fmt 10.1.1 see fmtlib/fmt#3574 +// Formats std::atomic +template +struct fmt::formatter< + std::atomic, + Char, + std::enable_if_t::value>> + : formatter { + template + auto format(const std::atomic& v, FormatContext& ctx) const + -> decltype(ctx.out()) { + return formatter::format(v.load(), ctx); } }; +#endif -// -- Backport from fmt 10 see fmtlib/fmt#3570 #if FMT_VERSION < 100100 +// Backport from fmt 10.1 see fmtlib/fmt#3570 +// Formats std::vector namespace fmt::detail { template struct has_flip : std::false_type {}; @@ -74,4 +105,3 @@ struct fmt::formatter< } }; #endif -// -- End of backport diff --git a/velox/common/caching/AsyncDataCache.cpp b/velox/common/caching/AsyncDataCache.cpp index da340415d1f4..cd1822d5eed4 100644 --- a/velox/common/caching/AsyncDataCache.cpp +++ b/velox/common/caching/AsyncDataCache.cpp @@ -19,6 +19,7 @@ #include "velox/common/caching/SsdCache.h" #include "velox/common/base/Counters.h" +#include "velox/common/base/Exceptions.h" #include "velox/common/base/StatsReporter.h" #include "velox/common/base/SuccinctPrinter.h" #include "velox/common/caching/FileIds.h" @@ -68,7 +69,7 @@ void AsyncDataCacheEntry::setExclusiveToShared() { } void AsyncDataCacheEntry::release() { - VELOX_CHECK_NE(0, numPins_.load()); + VELOX_CHECK_NE(0, numPins_); if (numPins_ == kExclusive) { // Dereferencing an exclusive entry without converting to shared means that // the content could not be shared, e.g. error in loading. @@ -135,7 +136,7 @@ std::string AsyncDataCacheEntry::toString() const { key_.fileNum.id(), key_.offset, size_, - numPins_.load()); + numPins_); } std::unique_ptr CacheShard::getFreeEntry() { @@ -639,7 +640,7 @@ bool AsyncDataCache::makeSpace( VELOX_CHECK( numThreadsInAllocate_ >= 0 && numThreadsInAllocate_ < 10000, "Leak in numThreadsInAllocate_: {}", - numThreadsInAllocate_.load()); + numThreadsInAllocate_); if (numThreadsInAllocate_) { rank = ++numThreadsInAllocate_; isCounted = true; diff --git a/velox/common/caching/SsdCache.cpp b/velox/common/caching/SsdCache.cpp index 8685150584a1..6d3d90c6a7c9 100644 --- a/velox/common/caching/SsdCache.cpp +++ b/velox/common/caching/SsdCache.cpp @@ -16,6 +16,7 @@ #include "velox/common/caching/SsdCache.h" #include #include +#include "velox/common/base/Exceptions.h" #include "velox/common/caching/FileIds.h" #include "velox/common/file/FileSystems.h" #include "velox/common/testutil/TestValue.h" @@ -82,7 +83,7 @@ bool SsdCache::startWrite() { } void SsdCache::write(std::vector pins) { - VELOX_CHECK_LE(numShards_, writesInProgress_.load()); + VELOX_CHECK_LE(numShards_, writesInProgress_); TestValue::adjust("facebook::velox::cache::SsdCache::write", this); diff --git a/velox/connectors/hive/storage_adapters/gcs/CMakeLists.txt b/velox/connectors/hive/storage_adapters/gcs/CMakeLists.txt index 02d3035ff7a4..3158d42b1d26 100644 --- a/velox/connectors/hive/storage_adapters/gcs/CMakeLists.txt +++ b/velox/connectors/hive/storage_adapters/gcs/CMakeLists.txt @@ -18,7 +18,8 @@ add_library(velox_gcs RegisterGCSFileSystem.cpp) if(VELOX_ENABLE_GCS) target_sources(velox_gcs PRIVATE GCSFileSystem.cpp GCSUtil.cpp) - target_link_libraries(velox_gcs Folly::folly google-cloud-cpp::storage) + target_link_libraries(velox_gcs velox_exception Folly::folly + google-cloud-cpp::storage) if(${VELOX_BUILD_TESTING}) add_subdirectory(tests) diff --git a/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp b/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp index 63d1f1396770..6eaada548701 100644 --- a/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp +++ b/velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.cpp @@ -15,6 +15,7 @@ */ #include "velox/connectors/hive/storage_adapters/gcs/GCSFileSystem.h" +#include "velox/common/base/Exceptions.h" #include "velox/common/file/File.h" #include "velox/connectors/hive/HiveConfig.h" #include "velox/connectors/hive/storage_adapters/gcs/GCSUtil.h" @@ -83,7 +84,7 @@ class GCSReadFile final : public ReadFile { key_); } length_ = (*metadata).size(); - VELOX_CHECK_GE(length_.load(), 0); + VELOX_CHECK_GE(length_, 0); } std::string_view pread(uint64_t offset, uint64_t length, void* buffer) diff --git a/velox/exec/HashTable.cpp b/velox/exec/HashTable.cpp index 80e41bd5ff61..1e0bbd6ffb3d 100644 --- a/velox/exec/HashTable.cpp +++ b/velox/exec/HashTable.cpp @@ -16,6 +16,7 @@ #include "velox/exec/HashTable.h" #include "velox/common/base/AsyncSource.h" +#include "velox/common/base/Exceptions.h" #include "velox/common/base/Portability.h" #include "velox/common/base/SimdUtil.h" #include "velox/common/process/ProcessBase.h" @@ -955,8 +956,7 @@ void HashTable::parallelJoinBuild() { hashes); insertForJoin(overflows.data(), hashes.data(), overflows.size(), nullptr); auto table = i == 0 ? this : otherTables_[i - 1].get(); - VELOX_CHECK_EQ( - table->rows()->numRows(), table->numParallelBuildRows_.load()); + VELOX_CHECK_EQ(table->rows()->numRows(), table->numParallelBuildRows_); } } diff --git a/velox/exec/tests/SharedArbitratorTest.cpp b/velox/exec/tests/SharedArbitratorTest.cpp index 1d315d7da978..8c967eec83be 100644 --- a/velox/exec/tests/SharedArbitratorTest.cpp +++ b/velox/exec/tests/SharedArbitratorTest.cpp @@ -24,6 +24,7 @@ #include #include "folly/experimental/EventCount.h" #include "folly/futures/Barrier.h" +#include "velox/common/base/Exceptions.h" #include "velox/common/base/tests/GTestUtils.h" #include "velox/common/memory/MallocAllocator.h" #include "velox/common/memory/Memory.h" @@ -190,14 +191,14 @@ class FakeMemoryOperator : public Operator { pool()->free(allocIt->buffer, allocIt->size); allocIt = allocations_.erase(allocIt); } - VELOX_CHECK_GE(totalBytes_.load(), 0); + VELOX_CHECK_GE(totalBytes_, 0); } private: void clear() { for (auto& allocation : allocations_) { totalBytes_ -= allocation.free(); - VELOX_CHECK_GE(totalBytes_.load(), 0); + VELOX_CHECK_GE(totalBytes_, 0); } allocations_.clear(); VELOX_CHECK_EQ(totalBytes_.load(), 0); diff --git a/velox/substrait/SubstraitParser.cpp b/velox/substrait/SubstraitParser.cpp index cafeac8baea0..c2a22c68180f 100644 --- a/velox/substrait/SubstraitParser.cpp +++ b/velox/substrait/SubstraitParser.cpp @@ -15,6 +15,7 @@ */ #include "velox/substrait/SubstraitParser.h" +#include #include "velox/common/base/Exceptions.h" #include "velox/substrait/TypeUtils.h" #include "velox/substrait/VeloxSubstraitSignature.h" @@ -103,7 +104,7 @@ int32_t SubstraitParser::parseReferenceSegment( default: VELOX_NYI( "Substrait conversion not supported for ReferenceSegment '{}'", - typeCase); + std::to_string(typeCase)); } } From 28db1322a28fe52aa9c41ecadb2a230d7ff3cde2 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 05:44:22 +0100 Subject: [PATCH 32/40] add formatters for substrait --- velox/common/base/FmtStdFormatters.h | 3 +-- velox/substrait/SubstraitToVeloxExpr.h | 29 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h index 2e73a985bb4d..90784d8ce461 100644 --- a/velox/common/base/FmtStdFormatters.h +++ b/velox/common/base/FmtStdFormatters.h @@ -32,8 +32,7 @@ template struct fmt::formatter : formatter::type, Char> { template - auto format(std::errc v, FormatContext& ctx) const - -> decltype(ctx.out()) { + auto format(std::errc v, FormatContext& ctx) const -> decltype(ctx.out()) { using underlying_type = std::underlying_type::type; return formatter::format( static_cast(v), ctx); diff --git a/velox/substrait/SubstraitToVeloxExpr.h b/velox/substrait/SubstraitToVeloxExpr.h index fdfef92d67a5..2dc85c31a28e 100644 --- a/velox/substrait/SubstraitToVeloxExpr.h +++ b/velox/substrait/SubstraitToVeloxExpr.h @@ -91,3 +91,32 @@ struct fmt::formatter : formatter { return formatter::format(static_cast(s), ctx); } }; + +template <> +struct fmt::formatter + : formatter { + auto format( + const substrait::Expression::Cast::FailureBehavior& s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; +template <> +struct fmt::formatter + : formatter { + auto format( + const substrait::Expression_FieldReference::ReferenceTypeCase& s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; + +template <> +struct fmt::formatter + : formatter { + auto format( + const substrait::Expression_Literal::LiteralTypeCase& s, + format_context& ctx) { + return formatter::format(static_cast(s), ctx); + } +}; From c48bf1bed5bb643f6f80b77bea3d6d5487b9faa2 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 17:06:56 +0100 Subject: [PATCH 33/40] revert *_SOURCE changes --- .circleci/dist_compile.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 62777a992b35..8e91b18e459c 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -396,8 +396,6 @@ jobs: ICU_SOURCE: BUNDLED simdjson_SOURCE: BUNDLED DuckDB_SOURCE: SYSTEM - fmt_SOURCE: BUNDLED - folly_SOURCE: BUNDLED steps: - pre-steps - install-duckdb @@ -609,9 +607,6 @@ jobs: linux-pr-fuzzer-run: executor: build - environment: - folly_SOURCE: BUNDLED - fmt_SOURCE: BUNDLED steps: - pre-steps - run: From d2edfe1384e1267d954f9853b95d4774bbb00b3e Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 17:22:33 +0100 Subject: [PATCH 34/40] no longer remove fmt --- .circleci/dist_compile.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 8e91b18e459c..a41afd966918 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -72,8 +72,6 @@ commands: if [ -e /opt/rh/gcc-toolset-9/enable ]; then source /opt/rh/gcc-toolset-9/enable fi - rm -rf /usr/local/include/fmt || echo "fmt not found" - rm -rf /usr/local/lib64/cmake/fmt|| echo "fmt not found" - restore_cache: name: "Restore CCache Cache" keys: From a884cc900e21d0d8aa75fe98873bd46cbf3a29e7 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 17:25:02 +0100 Subject: [PATCH 35/40] remove additional envvars --- .circleci/dist_compile.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index a41afd966918..920d4f5e5798 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -247,8 +247,6 @@ jobs: executor: build environment: DuckDB_SOURCE: SYSTEM - folly_SOURCE: BUNDLED - fmt_SOURCE: BUNDLED steps: - pre-steps - install-duckdb @@ -463,8 +461,6 @@ jobs: environment: VELOX_DEPENDENCY_SOURCE: SYSTEM simdjson_SOURCE: BUNDLED - folly_SOURCE: BUNDLED - fmt_SOURCE: BUNDLED DuckDB_SOURCE: BUNDLED steps: - fuzzer-run: From aaacd152ce18f666992230dbe6a9071dd7ac4c67 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Sun, 17 Dec 2023 17:36:24 +0100 Subject: [PATCH 36/40] Revert "Revert "wait to stop throttling?"" This reverts commit e74ce8f5142a32075fa02ad7aa6dbd6709086df4. --- scripts/setup-circleci.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index ffef106c3578..1dff52d96317 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -71,6 +71,9 @@ wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & + +wait # For cmake and source downloads to complete. + # wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & From 2b58bb2cfed56685b06c733c8bac9a29e67c2caa Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 18 Dec 2023 01:18:12 +0100 Subject: [PATCH 37/40] fix setup-circleci.sh --- scripts/setup-circleci.sh | 43 +++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/scripts/setup-circleci.sh b/scripts/setup-circleci.sh index 1dff52d96317..d36b743f828c 100755 --- a/scripts/setup-circleci.sh +++ b/scripts/setup-circleci.sh @@ -30,7 +30,7 @@ function dnf_install { dnf_install epel-release dnf-plugins-core # For ccache, ninja dnf config-manager --set-enabled powertools -dnf_install ninja-build cmake ccache gcc-toolset-9 git wget which libevent-devel \ +dnf_install ninja-build cmake curl ccache gcc-toolset-9 git wget which libevent-devel \ openssl-devel re2-devel libzstd-devel lz4-devel double-conversion-devel \ libdwarf-devel curl-devel libicu-devel @@ -58,34 +58,34 @@ function wget_and_untar { local URL=$1 local DIR=$2 mkdir -p "${DIR}" - wget -q --max-redirect 3 -O - "${URL}" | tar -xz -C "${DIR}" --strip-components=1 + pushd "${DIR}" + curl -L "${URL}" > $2.tar.gz + tar -xz --strip-components=1 -f $2.tar.gz + popd } # untar cmake binary release directly to /usr. wget_and_untar https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz /usr & # Fetch sources. -wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags & -wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog & -wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo & -wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost & -wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy & -wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt & +wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags +wget_and_untar https://github.com/google/glog/archive/v0.4.0.tar.gz glog +wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo +wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost +wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy +wget_and_untar https://github.com/fmtlib/fmt/archive/10.1.1.tar.gz fmt -wait # For cmake and source downloads to complete. - -# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 & -wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop & -wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf & +# wget_and_untar https://github.com/ericniebler/range-v3/archive/0.11.0.tar.gz ranges-v3 +wget_and_untar https://archive.apache.org/dist/hadoop/common/hadoop-2.10.1/hadoop-2.10.1.tar.gz hadoop +wget_and_untar https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz protobuf -FB_OS_VERSION="v2022.11.14.00" +FB_OS_VERSION="v2023.12.04.00" -wget_and_untar https://github.com/facebook/folly/archive/${FB_OS_VERSION}.tar.gz folly & -wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz & -wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle & -wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift & -FB_OS_VERSION="v2023.07.24.00" #mvfast doesn't have a v2022 tag -wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst & +wget_and_untar https://github.com/facebookincubator/fizz/archive/refs/tags/${FB_OS_VERSION}.tar.gz fizz +wget_and_untar https://github.com/facebook/folly/archive/refs/tags/${FB_OS_VERSION}.tar.gz folly +wget_and_untar https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz wangle +wget_and_untar https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz fbthrift +wget_and_untar https://github.com/facebook/mvfst/archive/refs/tags/${FB_OS_VERSION}.tar.gz mvfst wait # For cmake and source downloads to complete. @@ -118,6 +118,9 @@ cmake_install glog -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr cmake_install snappy -DSNAPPY_BUILD_TESTS=OFF cmake_install fmt -DFMT_TEST=OFF cmake_install folly -DFOLLY_HAVE_INT128_T=ON +# remove in #7700 +sed -i 's/\[\[deprecated.*\]\]//g' /usr/local/include/folly/init/Init.h + cmake_install fizz/fizz -DBUILD_TESTS=OFF cmake_install wangle/wangle -DBUILD_TESTS=OFF From de9f10261aeb2c6eebb6808b8f1024837267cbea Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 18 Dec 2023 03:17:49 +0100 Subject: [PATCH 38/40] revert use updated container --- .circleci/dist_compile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index 920d4f5e5798..99d1b87ccbe5 100644 --- a/.circleci/dist_compile.yml +++ b/.circleci/dist_compile.yml @@ -150,7 +150,7 @@ commands: executors: build: docker: - - image : ghcr.io/facebookincubator/velox-dev:circleci-avx + - image : ghcr.io/facebookincubator/velox-dev:new-cci resource_class: 2xlarge environment: CC: /opt/rh/gcc-toolset-9/root/bin/gcc From e5d726e549ae55029c54c4a9fd880d3030b2329c Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 18 Dec 2023 18:44:52 +0100 Subject: [PATCH 39/40] revert s3 util and cast to int --- velox/connectors/hive/storage_adapters/s3fs/S3Util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h index d0979b2ccb00..09f42a47ffb5 100644 --- a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h +++ b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h @@ -163,7 +163,7 @@ inline std::string getRequestID( errorMsgPrefix, \ getErrorStringFromS3Error(error), \ s3URI(bucket, key), \ - error.GetExceptionName(), \ + static_cast(error.GetErrorType()), \ error.GetResponseCode(), \ getS3BackendService(error.GetResponseHeaders()), \ error.GetMessage(), \ From 430e425cea67ca185633429076851348607f2703 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Tue, 19 Dec 2023 04:28:49 +0100 Subject: [PATCH 40/40] add test --- velox/core/tests/TypedExprSerdeTest.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/velox/core/tests/TypedExprSerdeTest.cpp b/velox/core/tests/TypedExprSerdeTest.cpp index 37b402a69838..8ba9ded8a423 100644 --- a/velox/core/tests/TypedExprSerdeTest.cpp +++ b/velox/core/tests/TypedExprSerdeTest.cpp @@ -52,8 +52,9 @@ TEST_F(TypedExprSerDeTest, input) { TEST_F(TypedExprSerDeTest, fieldAccess) { std::shared_ptr expression = - std::make_shared(BIGINT(), "a"); + std::make_shared(BIGINT(), "a\"a"); testSerde(expression); + ASSERT_EQ(expression->toString(), "\"a\"\"a\""); expression = std::make_shared( VARCHAR(), @@ -61,6 +62,7 @@ TEST_F(TypedExprSerDeTest, fieldAccess) { ROW({"a", "b"}, {VARCHAR(), BOOLEAN()}), "ab"), 0); testSerde(expression); + ASSERT_EQ(expression->toString(), "\"ab\"[\"a\"]"); } TEST_F(TypedExprSerDeTest, constant) {