From 6c1ff47080ec1694eb93b81ee863d6aa1529df63 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Fri, 19 Jan 2024 12:36:33 -0800 Subject: [PATCH] Upgrade fmt to 10.1.1 (from 8.0.1) (#8434) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/8434 fmt 8.0.1 is not upwards compatible to 9+ but backwards compatibility should not be an issue from what I could see in the logs and have done during https://github.com/facebookincubator/velox/issues/7700 (newst folly is compatible with fmt 10 but we built it with 8.0.1). Closes https://github.com/facebookincubator/velox/issues/7896 Pull Request resolved: https://github.com/facebookincubator/velox/pull/7941 Reviewed By: Yuhta Differential Revision: D52880145 Pulled By: kgpai fbshipit-source-id: 68db12f6c67e968562122d324d0a9f9e06ea1f62 --- .circleci/dist_compile.yml | 2 +- .github/workflows/benchmark.yml | 8 +- CMake/resolve_dependency_modules/fmt.cmake | 8 +- .../fmt/no-targets.patch | 12 -- CMakeLists.txt | 16 +-- scripts/setup-centos8.sh | 2 +- scripts/setup-circleci.sh | 39 ++++--- scripts/setup-macos.sh | 2 +- scripts/setup-ubuntu.sh | 7 +- scripts/setup-velox-torcharrow.sh | 2 +- velox/benchmarks/basic/LikeTpchBenchmark.cpp | 4 + velox/common/base/Exceptions.h | 1 + velox/common/base/FmtStdFormatters.h | 106 ++++++++++++++++++ velox/common/base/RuntimeMetrics.h | 6 + velox/common/caching/AsyncDataCache.cpp | 1 + velox/common/caching/SsdCache.cpp | 1 + velox/common/compression/Compression.cpp | 3 +- velox/common/compression/Compression.h | 12 ++ velox/common/file/FileSystems.cpp | 8 +- velox/common/memory/ByteStream.cpp | 10 +- velox/common/memory/MemoryAllocator.h | 10 ++ velox/common/memory/MemoryPool.h | 12 ++ velox/common/memory/tests/CMakeLists.txt | 1 + .../memory/tests/MemoryAllocatorTest.cpp | 5 +- .../memory/tests/MemoryArbitratorTest.cpp | 2 +- .../common/memory/tests/MemoryManagerTest.cpp | 6 +- velox/common/memory/tests/MemoryPoolTest.cpp | 5 +- .../memory/tests/MockSharedArbitratorTest.cpp | 8 +- .../connectors/fuzzer/FuzzerConnectorSplit.h | 22 ++++ velox/connectors/hive/HiveDataSink.h | 23 ++++ .../hive/storage_adapters/gcs/CMakeLists.txt | 3 +- .../storage_adapters/gcs/GCSFileSystem.cpp | 1 + .../hive/storage_adapters/s3fs/S3Util.h | 9 +- .../hive/tests/HiveDataSinkTest.cpp | 4 +- velox/connectors/tpch/CMakeLists.txt | 3 +- velox/connectors/tpch/TpchConnectorSplit.h | 22 ++++ velox/core/CMakeLists.txt | 1 + velox/core/Expressions.h | 8 +- velox/core/PlanFragment.h | 10 ++ velox/core/PlanNode.h | 20 +++- velox/core/tests/PlanFragmentTest.cpp | 2 +- velox/core/tests/TypedExprSerdeTest.cpp | 2 + velox/dwio/common/Options.cpp | 4 + velox/dwio/common/Options.h | 1 + velox/dwio/common/Writer.cpp | 11 +- velox/dwio/dwrf/common/FileMetadata.h | 7 ++ velox/dwio/dwrf/test/ColumnWriterTest.cpp | 7 +- velox/dwio/dwrf/test/CompressionTest.cpp | 7 +- velox/dwio/dwrf/writer/WriterContext.cpp | 4 +- velox/dwio/parquet/thrift/CMakeLists.txt | 3 +- .../dwio/parquet/thrift/ParquetThriftTypes.h | 79 +++++++++++++ velox/exec/Driver.h | 9 ++ velox/exec/HashBuild.h | 9 ++ velox/exec/HashTable.cpp | 1 + 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/Task.cpp | 5 +- velox/exec/fuzzer/PrestoQueryRunner.cpp | 4 +- velox/exec/tests/AggregationTest.cpp | 9 +- velox/exec/tests/HashJoinTest.cpp | 39 +++---- velox/exec/tests/OrderByTest.cpp | 5 +- velox/exec/tests/SharedArbitratorTest.cpp | 3 +- velox/exec/tests/TableWriteTest.cpp | 11 +- velox/exec/tests/utils/QueryAssertions.cpp | 8 ++ velox/functions/lib/DateTimeFormatter.h | 20 ++++ velox/functions/lib/Re2Functions.h | 8 ++ velox/substrait/SubstraitParser.cpp | 3 +- velox/substrait/SubstraitToVeloxExpr.h | 39 +++++++ .../substrait/tests/JsonToProtoConverter.cpp | 5 +- velox/type/Subfield.h | 20 ++++ velox/type/Timestamp.h | 21 ++++ velox/vector/VectorSaver.cpp | 7 ++ 76 files changed, 703 insertions(+), 131 deletions(-) delete mode 100644 CMake/resolve_dependency_modules/fmt/no-targets.patch create mode 100644 velox/common/base/FmtStdFormatters.h diff --git a/.circleci/dist_compile.yml b/.circleci/dist_compile.yml index b681c538bfded..c793f0ea43e63 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 diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 4bbd3d0c66db8..46428d529edff 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 diff --git a/CMake/resolve_dependency_modules/fmt.cmake b/CMake/resolve_dependency_modules/fmt.cmake index 55be2629962ed..88d8d674d3a3d 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 5ec962d32973c..0000000000000 --- 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) diff --git a/CMakeLists.txt b/CMakeLists.txt index 30381860b155e..9422ff17d147a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -252,14 +252,6 @@ 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}) add_compile_definitions(CODEGEN_ENABLED=1) @@ -457,6 +449,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() diff --git a/scripts/setup-centos8.sh b/scripts/setup-centos8.sh index fe6042301d8fb..e6808097509f3 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 bd275f48869ef..b49d21eaee95d 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 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,29 +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/8.0.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://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 & +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 & +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. @@ -113,9 +118,13 @@ 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 +cmake_install mvfst -DBUILD_TESTS=OFF cmake_install fbthrift -Denable_tests=OFF # cmake_install ranges-v3 diff --git a/scripts/setup-macos.sh b/scripts/setup-macos.sh index 83d8990603eae..00195e30a291e 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 94c34aaa68ab1..061b65c4a46dd 100755 --- a/scripts/setup-ubuntu.sh +++ b/scripts/setup-ubuntu.sh @@ -46,6 +46,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 \ @@ -86,11 +87,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 @@ -119,7 +115,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 a31e35515d214..72186bc7158c6 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 diff --git a/velox/benchmarks/basic/LikeTpchBenchmark.cpp b/velox/benchmarks/basic/LikeTpchBenchmark.cpp index 41477afd7f626..a9521dda195d9 100644 --- a/velox/benchmarks/basic/LikeTpchBenchmark.cpp +++ b/velox/benchmarks/basic/LikeTpchBenchmark.cpp @@ -45,6 +45,10 @@ enum class TpchBenchmarkCase { TpchQuery20, }; +auto format_as(TpchBenchmarkCase f) { + return fmt::underlying(f); +} + class LikeFunctionsBenchmark : public FunctionBaseTest, public FunctionBenchmarkBase { public: diff --git a/velox/common/base/Exceptions.h b/velox/common/base/Exceptions.h index 8206462789c4f..3396fa6348a82 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::velox { diff --git a/velox/common/base/FmtStdFormatters.h b/velox/common/base/FmtStdFormatters.h new file mode 100644 index 0000000000000..90784d8ce461a --- /dev/null +++ b/velox/common/base/FmtStdFormatters.h @@ -0,0 +1,106 @@ +/* + * 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 +#if FMT_VERSION >= 100100 +#include +#endif + +#include +#include +#include +#include +#include +#include + +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 + +#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 {}; + +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 diff --git a/velox/common/base/RuntimeMetrics.h b/velox/common/base/RuntimeMetrics.h index 9a81ff1837916..a14478c79673f 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/AsyncDataCache.cpp b/velox/common/caching/AsyncDataCache.cpp index 3cd0245f2d5f6..1362befdcae5c 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" diff --git a/velox/common/caching/SsdCache.cpp b/velox/common/caching/SsdCache.cpp index a6aa26284dc67..5b11fe89eebfc 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" diff --git a/velox/common/compression/Compression.cpp b/velox/common/compression/Compression.cpp index 3843f5902a980..e17ff941ba99f 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/compression/Compression.h b/velox/common/compression/Compression.h index 317f9717a2ae5..82658976571c9 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/common/file/FileSystems.cpp b/velox/common/file/FileSystems.cpp index eac9c01337b74..28e66dc2bd11c 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/ByteStream.cpp b/velox/common/memory/ByteStream.cpp index 90c86199465f0..50bcf2b7cc609 100644 --- a/velox/common/memory/ByteStream.cpp +++ b/velox/common/memory/ByteStream.cpp @@ -97,7 +97,10 @@ void ByteInputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteInputStream: {}", position); + static_assert(sizeof(std::streamsize) <= sizeof(long long)); + VELOX_FAIL( + "Seeking past end of ByteInputStream: {}", + static_cast(position)); } void ByteInputStream::next(bool throwIfPastEnd) { @@ -286,7 +289,10 @@ void ByteOutputStream::seekp(std::streampos position) { } toSkip -= range.size; } - VELOX_FAIL("Seeking past end of ByteOutputStream: {}", position); + static_assert(sizeof(std::streamsize) <= sizeof(long long)); + VELOX_FAIL( + "Seeking past end of ByteOutputStream: {}", + static_cast(position)); } void ByteOutputStream::flush(OutputStream* out) { diff --git a/velox/common/memory/MemoryAllocator.h b/velox/common/memory/MemoryAllocator.h index 215c58c91e97a..048dbd770ab31 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" @@ -503,3 +504,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 0d496c85d6754..80da4cb2b5f35 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" @@ -1066,3 +1067,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/CMakeLists.txt b/velox/common/memory/tests/CMakeLists.txt index f243b3f4be7d5..c58b0dd86821f 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/common/memory/tests/MemoryAllocatorTest.cpp b/velox/common/memory/tests/MemoryAllocatorTest.cpp index c146cc142cce2..6d752e7318901 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 @@ -770,8 +771,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/MemoryArbitratorTest.cpp b/velox/common/memory/tests/MemoryArbitratorTest.cpp index c6acdd96af201..cc9851e8a95d0 100644 --- a/velox/common/memory/tests/MemoryArbitratorTest.cpp +++ b/velox/common/memory/tests/MemoryArbitratorTest.cpp @@ -417,7 +417,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 62d05b3dc9dc8..17d83ef9cfcbe 100644 --- a/velox/common/memory/tests/MemoryManagerTest.cpp +++ b/velox/common/memory/tests/MemoryManagerTest.cpp @@ -14,7 +14,9 @@ * limitations under the License. */ +#include #include +#include #include #include "velox/common/base/VeloxException.h" @@ -567,8 +569,8 @@ TEST_F(MemoryManagerTest, quotaEnforcement) { for (const auto& testData : testSettings) { 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)); const int alignment = 32; MemoryManagerOptions options; diff --git a/velox/common/memory/tests/MemoryPoolTest.cpp b/velox/common/memory/tests/MemoryPoolTest.cpp index 6ef24923a104a..a9635e6e2c270 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 @@ -1126,8 +1127,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/common/memory/tests/MockSharedArbitratorTest.cpp b/velox/common/memory/tests/MockSharedArbitratorTest.cpp index 1d86011d64acd..c257e9de2c32a 100644 --- a/velox/common/memory/tests/MockSharedArbitratorTest.cpp +++ b/velox/common/memory/tests/MockSharedArbitratorTest.cpp @@ -16,8 +16,10 @@ #include +#include #include #include +#include #include "folly/experimental/EventCount.h" #include "folly/futures/Barrier.h" #include "velox/common/base/tests/GTestUtils.h" @@ -879,7 +881,7 @@ 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)); setupMemory(); @@ -920,7 +922,7 @@ 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)); setupMemory(); @@ -955,7 +957,7 @@ 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)); setupMemory(memoryCapacity, minPoolCapacity); diff --git a/velox/connectors/fuzzer/FuzzerConnectorSplit.h b/velox/connectors/fuzzer/FuzzerConnectorSplit.h index b82749e0be9b6..1c4b6fef87cc4 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 0e859f2cf4e00..3dc73471360d4 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); @@ -585,3 +587,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/hive/storage_adapters/gcs/CMakeLists.txt b/velox/connectors/hive/storage_adapters/gcs/CMakeLists.txt index 02d3035ff7a46..3158d42b1d264 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 8b1113e398bf5..6eaada548701b 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" diff --git a/velox/connectors/hive/storage_adapters/s3fs/S3Util.h b/velox/connectors/hive/storage_adapters/s3fs/S3Util.h index c5a51d28c7afd..09f42a47ffb5d 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(), \ + static_cast(error.GetErrorType()), \ 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/connectors/hive/tests/HiveDataSinkTest.cpp b/velox/connectors/hive/tests/HiveDataSinkTest.cpp index 648ec1f8c5081..ce2e2ed626c8a 100644 --- a/velox/connectors/hive/tests/HiveDataSinkTest.cpp +++ b/velox/connectors/hive/tests/HiveDataSinkTest.cpp @@ -582,7 +582,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), @@ -722,7 +722,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/connectors/tpch/CMakeLists.txt b/velox/connectors/tpch/CMakeLists.txt index b7123f2f6f0e8..6d1bcddcd0529 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 298a36ac85df5..029d07a841351 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/CMakeLists.txt b/velox/core/CMakeLists.txt index eb4c18cdbe802..fe3b2da46db3b 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/Expressions.h b/velox/core/Expressions.h index 459b0f5808ede..33f2281d5498b 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 a2990a7eda23c..2ccd933fc78bb 100644 --- a/velox/core/PlanFragment.h +++ b/velox/core/PlanFragment.h @@ -81,3 +81,13 @@ struct PlanFragment { }; } // namespace facebook::velox::core + +template <> +struct fmt::formatter + : formatter { + auto format( + const 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 4ecb664d0e3cc..38d4f1f26fe9e 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/core/tests/PlanFragmentTest.cpp b/velox/core/tests/PlanFragmentTest.cpp index 7505f40600647..8f0b7c67abad0 100644 --- a/velox/core/tests/PlanFragmentTest.cpp +++ b/velox/core/tests/PlanFragmentTest.cpp @@ -149,7 +149,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/core/tests/TypedExprSerdeTest.cpp b/velox/core/tests/TypedExprSerdeTest.cpp index f2c30b1a449a9..22214b5877675 100644 --- a/velox/core/tests/TypedExprSerdeTest.cpp +++ b/velox/core/tests/TypedExprSerdeTest.cpp @@ -58,6 +58,7 @@ TEST_F(TypedExprSerDeTest, fieldAccess) { std::shared_ptr expression = std::make_shared(BIGINT(), "a"); testSerde(expression); + ASSERT_EQ(expression->toString(), "\"a\""); expression = std::make_shared( VARCHAR(), @@ -65,6 +66,7 @@ TEST_F(TypedExprSerDeTest, fieldAccess) { ROW({"a", "b"}, {VARCHAR(), BOOLEAN()}), "ab"), 0); testSerde(expression); + ASSERT_EQ(expression->toString(), "\"ab\"[\"a\"]"); } TEST_F(TypedExprSerDeTest, constant) { diff --git a/velox/dwio/common/Options.cpp b/velox/dwio/common/Options.cpp index 32672da0d88d9..5e4c20bfe33c8 100644 --- a/velox/dwio/common/Options.cpp +++ b/velox/dwio/common/Options.cpp @@ -21,6 +21,10 @@ namespace velox { namespace dwio { namespace common { +std::string format_as(FileFormat f) { + return toString(f); +} + FileFormat toFileFormat(std::string s) { if (s == "dwrf") { return FileFormat::DWRF; diff --git a/velox/dwio/common/Options.h b/velox/dwio/common/Options.h index 5f2a688765f82..430f4750758dd 100644 --- a/velox/dwio/common/Options.h +++ b/velox/dwio/common/Options.h @@ -49,6 +49,7 @@ enum class FileFormat { FileFormat toFileFormat(std::string s); std::string toString(FileFormat fmt); +std::string format_as(FileFormat f); FOLLY_ALWAYS_INLINE std::ostream& operator<<( std::ostream& output, diff --git a/velox/dwio/common/Writer.cpp b/velox/dwio/common/Writer.cpp index 421d825c54a3e..8edea5a122c52 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 {}", oldState, 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(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/common/FileMetadata.h b/velox/dwio/dwrf/common/FileMetadata.h index 2ea21628a595a..ac8bb09e6ebb9 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/dwio/dwrf/test/ColumnWriterTest.cpp b/velox/dwio/dwrf/test/ColumnWriterTest.cpp index 94033c06bd840..f7a2b1249d4fa 100644 --- a/velox/dwio/dwrf/test/ColumnWriterTest.cpp +++ b/velox/dwio/dwrf/test/ColumnWriterTest.cpp @@ -104,12 +104,7 @@ 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())); + 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 733d73796b0c5..349d2df74db63 100644 --- a/velox/dwio/dwrf/test/CompressionTest.cpp +++ b/velox/dwio/dwrf/test/CompressionTest.cpp @@ -23,6 +23,7 @@ #include #include +#include "velox/common/compression/Compression.h" #include @@ -398,8 +399,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(), kind_)); + SCOPED_TRACE(fmt::format( + "{} compression {}", + testData.debugString(), + compressionKindToString(kind_))); auto config = std::make_shared(); config->set(Config::COMPRESSION, kind_); diff --git a/velox/dwio/dwrf/writer/WriterContext.cpp b/velox/dwio/dwrf/writer/WriterContext.cpp index 1030697c3ea4f..ef3f7f4c34bd3 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 "velox/common/compression/Compression.h" #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/dwio/parquet/thrift/CMakeLists.txt b/velox/dwio/parquet/thrift/CMakeLists.txt index 08df5614f92fc..8b21b15a2e60c 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) diff --git a/velox/dwio/parquet/thrift/ParquetThriftTypes.h b/velox/dwio/parquet/thrift/ParquetThriftTypes.h index 517c425b7f012..2bb89ca530c81 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/exec/Driver.h b/velox/exec/Driver.h index fcac70efbbaef..61da304b68a62 100644 --- a/velox/exec/Driver.h +++ b/velox/exec/Driver.h @@ -671,3 +671,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 327341679b280..1f09f85f6752a 100644 --- a/velox/exec/HashBuild.h +++ b/velox/exec/HashBuild.h @@ -336,3 +336,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.cpp b/velox/exec/HashTable.cpp index d4cbbdb0cbf1f..1e0bbd6ffb3d0 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" diff --git a/velox/exec/HashTable.h b/velox/exec/HashTable.h index 23cf2d2ae5a5c..5dc6e128934cd 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 2b640fe7c5865..d286eb5aff1f3 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 1ba9ef443fb4d..eb0d2474c9356 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 7fef6baaba0aa..21ea9d17388e1 100644 --- a/velox/exec/Spill.h +++ b/velox/exec/Spill.h @@ -447,3 +447,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 1a5d08b19a635..3c0e9d28cdd5e 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 b1a3df7ca228e..899bc00b69c08 100644 --- a/velox/exec/Spiller.h +++ b/velox/exec/Spiller.h @@ -318,3 +318,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/Task.cpp b/velox/exec/Task.cpp index 06dec197dcacb..a6f840bc9b815 100644 --- a/velox/exec/Task.cpp +++ b/velox/exec/Task.cpp @@ -528,7 +528,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 5507a1f5de3dd..3486682732305 100644 --- a/velox/exec/fuzzer/PrestoQueryRunner.cpp +++ b/velox/exec/fuzzer/PrestoQueryRunner.cpp @@ -99,7 +99,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 89d68224a9608..578a16cba3b5c 100644 --- a/velox/exec/tests/AggregationTest.cpp +++ b/velox/exec/tests/AggregationTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -2277,7 +2278,7 @@ 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)); @@ -2403,9 +2404,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)); + auto tempDirectory = exec::test::TempDirectoryPath::create(); auto queryCtx = std::make_shared(executor_.get()); queryCtx->testingOverrideMemoryPool(memory::memoryManager()->addRootPool( @@ -2698,9 +2700,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)); + 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 59bc3aea0f713..86519d4c271db 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" @@ -915,8 +916,8 @@ 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)); HashJoinBuilder(*pool_, duckDbQueryRunner_, driverExecutor_.get()) @@ -1371,7 +1372,7 @@ 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)); @@ -1442,8 +1443,8 @@ 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)); std::vector probeVectors = @@ -1551,8 +1552,8 @@ 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)); // probeVectors size is greater than buildVector size. @@ -1962,8 +1963,8 @@ 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)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { @@ -2239,8 +2240,8 @@ 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)); auto probeVectors = makeBatches(4, [&](int32_t /*unused*/) { @@ -2351,8 +2352,8 @@ 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)); // Left side keys are [0, 1, 2,..10]. @@ -2712,8 +2713,8 @@ 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)); // Left side keys are [0, 1, 2,..10]. @@ -2946,8 +2947,8 @@ 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)); // Left side keys are [0, 1, 2,..10]. @@ -5374,7 +5375,7 @@ 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)); @@ -5506,7 +5507,7 @@ 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)); auto tempDirectory = exec::test::TempDirectoryPath::create(); diff --git a/velox/exec/tests/OrderByTest.cpp b/velox/exec/tests/OrderByTest.cpp index 74f935fc6b686..63c2a10065e75 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" @@ -862,7 +863,7 @@ 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)); auto tempDirectory = exec::test::TempDirectoryPath::create(); @@ -992,7 +993,7 @@ 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)); auto tempDirectory = exec::test::TempDirectoryPath::create(); diff --git a/velox/exec/tests/SharedArbitratorTest.cpp b/velox/exec/tests/SharedArbitratorTest.cpp index 8103e121d303a..bee4143c6bd03 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" @@ -174,7 +175,7 @@ class FakeMemoryOperator : public Operator { VELOX_CHECK_GE(totalBytes_, 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 4898a925dced0..52972f41ee763 100644 --- a/velox/exec/tests/TableWriteTest.cpp +++ b/velox/exec/tests/TableWriteTest.cpp @@ -32,6 +32,7 @@ #include #include +#include "velox/dwio/common/Options.h" using namespace facebook::velox; using namespace facebook::velox::core; @@ -187,10 +188,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())); @@ -2572,7 +2573,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); } } diff --git a/velox/exec/tests/utils/QueryAssertions.cpp b/velox/exec/tests/utils/QueryAssertions.cpp index e224320edaf67..5d5b8d3746b29 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" // @manual #include "velox/duckdb/conversion/DuckConversion.h" #include "velox/exec/tests/utils/Cursor.h" #include "velox/exec/tests/utils/QueryAssertions.h" @@ -1514,3 +1515,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 c3d597e460bae..37940171ea326 100644 --- a/velox/functions/lib/DateTimeFormatter.h +++ b/velox/functions/lib/DateTimeFormatter.h @@ -205,3 +205,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 94f05d8a5726e..8c2fecd7c12ba 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/substrait/SubstraitParser.cpp b/velox/substrait/SubstraitParser.cpp index cafeac8baea05..c2a22c68180f4 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)); } } diff --git a/velox/substrait/SubstraitToVeloxExpr.h b/velox/substrait/SubstraitToVeloxExpr.h index dd5957e8e7857..2dc85c31a28ea 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,41 @@ 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); + } +}; + +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); + } +}; diff --git a/velox/substrait/tests/JsonToProtoConverter.cpp b/velox/substrait/tests/JsonToProtoConverter.cpp index 356e296b0c027..e00f6dd90708c 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 5aa74c5c8c630..0bc77a54523f2 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,22 @@ 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); + } +}; + +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); + } +}; diff --git a/velox/type/Timestamp.h b/velox/type/Timestamp.h index 93aa0aca9e64d..7e170d837418e 100644 --- a/velox/type/Timestamp.h +++ b/velox/type/Timestamp.h @@ -411,3 +411,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 21aa035a7f1f4..827e73ec64d37 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); + } +};