From ebc0e07c98b1771cbe46879507163c0d3cd40256 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Sat, 11 Jul 2026 11:24:26 +0900 Subject: [PATCH 01/13] Infra: Group github/codeql-action bumps into a single dependabot PR (#826) Thus, dependabot can update these two jobs in a single PR: https://github.com/apache/iceberg-cpp/blob/f4f6359fa9a95bde4bb2627bd8a5d45273644294/.github/workflows/codeql.yml#L56-L62 - https://github.com/apache/infrastructure-actions/pull/1028 --- .github/dependabot.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fc74cd299..325ea2424 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -26,3 +26,10 @@ updates: day: "sunday" cooldown: default-days: 7 + groups: + # The codeql-action init/autobuild/analyze steps must all run the + # same version - split PRs cause a version mismatch that fails the + # Analyze jobs. Group them so a single PR bumps all of them together. + codeql-action: + patterns: + - "github/codeql-action*" From 4c906d1d631fbc9907b3b0d0c7dbf0bebb90e41b Mon Sep 17 00:00:00 2001 From: Rahul Goel Date: Fri, 10 Jul 2026 22:30:38 -0400 Subject: [PATCH 02/13] feat: make ToLower Unicode-aware via utf8proc (#760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the ASCII-only `StringUtils::ToLower` with a Unicode-aware implementation backed by [utf8proc](https://github.com/JuliaStrings/utf8proc), aligning case-insensitive name handling with Iceberg Java's `toLowerCase(Locale.ROOT)` for simple (1:1) case mappings. The few code points where simple and full case mapping differ (e.g. `İ` U+0130) are tracked in #808. - `ToLower` lower-cases UTF-8 using utf8proc simple (1:1) case mapping (e.g. `CAFÉ` → `café`, `GROẞE` → `große`). Pure-ASCII input takes a byte-wise fast path and never touches utf8proc. - Invalid UTF-8 is handled as a correctness guarantee (raised in review): `ToLower` is total — a byte that does not begin a valid UTF-8 sequence is passed through unchanged and decoding resumes at the next byte, so the valid code points around it are still lower-cased. The comparisons below therefore compare invalid bytes verbatim. - `EqualsIgnoreCase` and `StartsWithIgnoreCase` fold through `ToLower`, so they are case-insensitive for non-ASCII letters too. Both take an allocation-free ASCII fast path and fall back to `ToLower` only when a non-ASCII byte is present. - `StartsWithIgnoreCase` no longer assumes case mapping preserves byte length — the old byte-slice guard mishandled length-changing maps (e.g. `İ` U+0130 → `i`). Now `"İx"` starts with `"i"`, and `"i"` starts with `"İ"`. - `ToUpper` is intentionally left ASCII-only — it only normalizes ASCII enum/codec strings, and simple case mapping would be wrong for some letters (e.g. `ß` would stay unchanged instead of becoming `SS`). - utf8proc is pinned to 2.10.0 and wired into both the CMake (vendored via FetchContent / system package) and Meson (`subprojects/utf8proc.wrap`) builds, with matching source hashes and correct static-vs-shared linkage on Windows. ## Testing - `string_util_test.cc`: `ToLowerUnicode` (incl. invalid / truncated / stray-continuation bytes), `ToUpperAsciiOnly`, and Unicode + invalid-UTF-8 cases for `EqualsIgnoreCase` / `StartsWithIgnoreCase`. - `IgnoreCaseAgreesWithToLowerOracle` exhaustively checks both ASCII fast paths against the `ToLower` oracle over all short strings drawn from an alphabet with length-changing maps (`İ`→`i`, `K`→`k`) and invalid UTF-8. ## Remaining work (tracked in #808) - Full case-mapping parity with Java for the code points where simple ≠ full mapping (e.g. `İ` → `i̇`). - A streaming (allocation-free) non-ASCII path for `EqualsIgnoreCase` / `StartsWithIgnoreCase`. Part of #613 — does not fully fix it; #808 holds the design and remaining plan. Follow-up to #748. --- LICENSE | 92 ++++++++++ README.md | 1 + .../IcebergThirdpartyToolchain.cmake | 71 ++++++++ mkdocs/docs/getting-started.md | 1 + src/iceberg/CMakeLists.txt | 8 +- src/iceberg/meson.build | 20 ++- src/iceberg/test/string_util_test.cc | 161 ++++++++++++++++-- src/iceberg/util/string_util.cc | 44 +++++ src/iceberg/util/string_util.h | 93 +++++++--- subprojects/utf8proc.wrap | 30 ++++ 10 files changed, 484 insertions(+), 37 deletions(-) create mode 100644 subprojects/utf8proc.wrap diff --git a/LICENSE b/LICENSE index 374b7fc58..8d8d5ff4e 100644 --- a/LICENSE +++ b/LICENSE @@ -228,3 +228,95 @@ Home page: https://arrow.apache.org/ License: https://www.apache.org/licenses/LICENSE-2.0 -------------------------------------------------------------------------------- + +This product bundles utf8proc, which is available under the MIT License: + +utf8proc is a software package originally developed by Jan Behrens and the rest +of the Public Software Group, now maintained by the Julia-language developers. +All new work on the utf8proc library is licensed under the MIT "expat" license: + +Copyright (c) 2014-2021 by Steven G. Johnson, Jiahao Chen, Tony Kelman, Jonas +Fonseca, and other contributors listed in the git history. + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. + +The original utf8proc is licensed under the same MIT "expat" license: + +Copyright (c) 2009, 2013 Public Software Group e. V., Berlin, Germany + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. + +utf8proc also contains data derived from the Unicode data files. The following +license applies to that data: + +COPYRIGHT AND PERMISSION NOTICE + +Copyright (c) 1991-2007 Unicode, Inc. All rights reserved. Distributed under +the Terms of Use in http://www.unicode.org/copyright.html. + +Permission is hereby granted, free of charge, to any person obtaining a +copy of the Unicode data files and any associated documentation (the "Data +Files") or Unicode software and any associated documentation (the +"Software") to deal in the Data Files or Software without restriction, +including without limitation the rights to use, copy, modify, merge, +publish, distribute, and/or sell copies of the Data Files or Software, and +to permit persons to whom the Data Files or Software are furnished to do +so, provided that (a) the above copyright notice(s) and this permission +notice appear with all copies of the Data Files or Software, (b) both the +above copyright notice(s) and this permission notice appear in associated +documentation, and (c) there is clear notice in each modified Data File or +in the Software as well as in the documentation associated with the Data +File(s) or Software that the data or software has been modified. + +THE DATA FILES AND SOFTWARE ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY +KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF +THIRD PARTY RIGHTS. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR HOLDERS +INCLUDED IN THIS NOTICE BE LIABLE FOR ANY CLAIM, OR ANY SPECIAL INDIRECT OR +CONSEQUENTIAL DAMAGES, OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF +USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER +TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +PERFORMANCE OF THE DATA FILES OR SOFTWARE. + +Except as contained in this notice, the name of a copyright holder shall +not be used in advertising or otherwise to promote the sale, use or other +dealings in these Data Files or Software without prior written +authorization of the copyright holder. + +Unicode and the Unicode logo are trademarks of Unicode, Inc., and may be +registered in some jurisdictions. All other trademarks and registered +trademarks mentioned herein are the property of their respective owners. + +-------------------------------------------------------------------------------- diff --git a/README.md b/README.md index 7c9a343ec..1318933d9 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,7 @@ If you experience network issues when downloading dependencies, you can customiz - `ICEBERG_AVRO_GIT_URL`: Apache Avro git repository URL - `ICEBERG_NANOARROW_URL`: Nanoarrow tarball URL - `ICEBERG_CROARING_URL`: CRoaring tarball URL +- `ICEBERG_UTF8PROC_URL`: utf8proc tarball URL - `ICEBERG_NLOHMANN_JSON_URL`: nlohmann-json tarball URL - `ICEBERG_SPDLOG_URL`: spdlog tarball URL - `ICEBERG_CPR_URL`: cpr tarball URL diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 34ad6d910..86e840097 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -83,6 +83,7 @@ endfunction() # ICEBERG_AVRO_GIT_URL - Apache Avro git repository URL # ICEBERG_NANOARROW_URL - Nanoarrow tarball URL # ICEBERG_CROARING_URL - CRoaring tarball URL +# ICEBERG_UTF8PROC_URL - utf8proc tarball URL # ICEBERG_NLOHMANN_JSON_URL - nlohmann-json tarball URL # ICEBERG_SPDLOG_URL - spdlog tarball URL # ICEBERG_CPR_URL - cpr tarball URL @@ -120,6 +121,20 @@ else() ) endif() +set(ICEBERG_UTF8PROC_BUILD_VERSION "2.10.0") +set(ICEBERG_UTF8PROC_BUILD_SHA256_CHECKSUM + "276a37dc4d1dd24d7896826a579f4439d1e5fe33603add786bb083cab802e23e") + +if(DEFINED ENV{ICEBERG_UTF8PROC_URL}) + set(UTF8PROC_SOURCE_URL "$ENV{ICEBERG_UTF8PROC_URL}") +else() + # Use the release asset (stable bytes, matching subprojects/utf8proc.wrap) rather + # than the auto-generated tag archive, whose contents GitHub does not guarantee. + set(UTF8PROC_SOURCE_URL + "https://github.com/JuliaStrings/utf8proc/releases/download/v${ICEBERG_UTF8PROC_BUILD_VERSION}/utf8proc-${ICEBERG_UTF8PROC_BUILD_VERSION}.tar.gz" + ) +endif() + # ---------------------------------------------------------------------- # FetchContent @@ -436,6 +451,61 @@ function(resolve_croaring_dependency) PARENT_SCOPE) endfunction() +# ---------------------------------------------------------------------- +# utf8proc + +function(resolve_utf8proc_dependency) + prepare_fetchcontent() + + # The vendored build needs no install rules; without this, CMake < 3.28 (where + # FetchContent has no EXCLUDE_FROM_ALL) would install utf8proc's headers and + # pkg-config file into the iceberg install prefix. + set(UTF8PROC_INSTALL OFF) + + fetchcontent_declare(utf8proc + ${FC_DECLARE_COMMON_OPTIONS} + URL ${UTF8PROC_SOURCE_URL} + URL_HASH "SHA256=${ICEBERG_UTF8PROC_BUILD_SHA256_CHECKSUM}" + FIND_PACKAGE_ARGS + NAMES + utf8proc + CONFIG) + fetchcontent_makeavailable(utf8proc) + + if(utf8proc_SOURCE_DIR) + if(NOT TARGET utf8proc::utf8proc) + add_library(utf8proc::utf8proc INTERFACE IMPORTED) + target_link_libraries(utf8proc::utf8proc INTERFACE utf8proc) + target_include_directories(utf8proc::utf8proc INTERFACE ${utf8proc_SOURCE_DIR}) + endif() + + set(UTF8PROC_VENDORED TRUE) + # utf8proc's CMake puts a raw build-tree path in INTERFACE_INCLUDE_DIRECTORIES, which + # install(EXPORT) rejects. Wrap it in BUILD_INTERFACE so the export is valid; utf8proc + # is a private dependency, so installed consumers never need its headers. + set_target_properties(utf8proc + PROPERTIES OUTPUT_NAME "iceberg_vendored_utf8proc" + POSITION_INDEPENDENT_CODE ON + INTERFACE_INCLUDE_DIRECTORIES + "$") + install(TARGETS utf8proc + EXPORT iceberg_targets + RUNTIME DESTINATION "${ICEBERG_INSTALL_BINDIR}" + ARCHIVE DESTINATION "${ICEBERG_INSTALL_LIBDIR}" + LIBRARY DESTINATION "${ICEBERG_INSTALL_LIBDIR}") + else() + set(UTF8PROC_VENDORED FALSE) + list(APPEND ICEBERG_SYSTEM_DEPENDENCIES utf8proc) + endif() + + set(ICEBERG_SYSTEM_DEPENDENCIES + ${ICEBERG_SYSTEM_DEPENDENCIES} + PARENT_SCOPE) + set(UTF8PROC_VENDORED + ${UTF8PROC_VENDORED} + PARENT_SCOPE) +endfunction() + # ---------------------------------------------------------------------- # nlohmann-json @@ -734,6 +804,7 @@ endfunction() resolve_zlib_dependency() resolve_nanoarrow_dependency() resolve_croaring_dependency() +resolve_utf8proc_dependency() resolve_nlohmann_json_dependency() resolve_spdlog_dependency() diff --git a/mkdocs/docs/getting-started.md b/mkdocs/docs/getting-started.md index 10a1a5c90..52a04e635 100644 --- a/mkdocs/docs/getting-started.md +++ b/mkdocs/docs/getting-started.md @@ -143,6 +143,7 @@ If you experience network issues when downloading dependencies, you can override | `ICEBERG_AVRO_GIT_URL` | Apache Avro git repository | | `ICEBERG_NANOARROW_URL` | Nanoarrow tarball | | `ICEBERG_CROARING_URL` | CRoaring tarball | +| `ICEBERG_UTF8PROC_URL` | utf8proc tarball | | `ICEBERG_NLOHMANN_JSON_URL` | nlohmann-json tarball | | `ICEBERG_CPR_URL` | cpr tarball | diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index 999532c1c..da820c26b 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -154,23 +154,27 @@ list(APPEND "$,nanoarrow::nanoarrow_static,$,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>" nlohmann_json::nlohmann_json spdlog::spdlog + utf8proc::utf8proc ZLIB::ZLIB) list(APPEND ICEBERG_SHARED_BUILD_INTERFACE_LIBS "$,nanoarrow::nanoarrow_static,$,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>" nlohmann_json::nlohmann_json spdlog::spdlog + utf8proc::utf8proc ZLIB::ZLIB) list(APPEND ICEBERG_STATIC_INSTALL_INTERFACE_LIBS "$,iceberg::nanoarrow_static,$,nanoarrow::nanoarrow_static,nanoarrow::nanoarrow_shared>>" "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>" - "$,iceberg::spdlog,spdlog::spdlog>") + "$,iceberg::spdlog,spdlog::spdlog>" + "$,iceberg::utf8proc,utf8proc::utf8proc>") list(APPEND ICEBERG_SHARED_INSTALL_INTERFACE_LIBS "$,iceberg::nanoarrow_static,$,nanoarrow::nanoarrow_shared,nanoarrow::nanoarrow_static>>" "$,iceberg::nlohmann_json,$,nlohmann_json::nlohmann_json,nlohmann_json::nlohmann_json>>" - "$,iceberg::spdlog,spdlog::spdlog>") + "$,iceberg::spdlog,spdlog::spdlog>" + "$,iceberg::utf8proc,utf8proc::utf8proc>") add_iceberg_lib(iceberg SOURCES diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index 4c7960ae6..e5158a4b9 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -198,9 +198,27 @@ croaring_dep = dependency('croaring', static: croaring_needs_static) nanoarrow_dep = dependency('nanoarrow') nlohmann_json_dep = dependency('nlohmann_json') spdlog_dep = dependency('spdlog') +# utf8proc's header declares its functions __declspec(dllimport) on Windows unless +# UTF8PROC_STATIC is defined, and the wrap does not propagate that define to consumers. +# Define it whenever utf8proc is linked statically, so the header's declarations match +# how it is linked. Harmless on other platforms. +utf8proc_needs_static = get_option('default_library') == 'static' +utf8proc_dep = dependency('libutf8proc', static: utf8proc_needs_static) +if utf8proc_needs_static + utf8proc_dep = declare_dependency( + compile_args: ['-DUTF8PROC_STATIC'], + dependencies: utf8proc_dep, + ) +endif zlib_dep = dependency('zlib') -iceberg_deps = [nanoarrow_dep, nlohmann_json_dep, spdlog_dep, zlib_dep] +iceberg_deps = [ + nanoarrow_dep, + nlohmann_json_dep, + spdlog_dep, + utf8proc_dep, + zlib_dep, +] iceberg_lib = library( 'iceberg', diff --git a/src/iceberg/test/string_util_test.cc b/src/iceberg/test/string_util_test.cc index a3fd03760..65d5c23cc 100644 --- a/src/iceberg/test/string_util_test.cc +++ b/src/iceberg/test/string_util_test.cc @@ -41,19 +41,53 @@ TEST(StringUtilsTest, ToUpper) { ASSERT_EQ(StringUtils::ToUpper("123"), "123"); } -// Non-ASCII (multibyte UTF-8) bytes have the high bit set, i.e. are negative when stored -// in a signed char. Only ASCII letters are converted; multibyte bytes pass through -// unchanged. The non-ASCII strings are written as explicit UTF-8 byte escapes so the test -// does not depend on the source-file encoding. See -// https://github.com/apache/iceberg-cpp/issues/613. -TEST(StringUtilsTest, NonAsciiPassThrough) { - // "Naïve" -> "naïve" (ï = U+00EF = 0xC3 0xAF; only the ASCII letters change). - ASSERT_EQ(StringUtils::ToLower("Na\xC3\xAFve"), "na\xC3\xAFve"); - // "café" -> "CAFé" (é = U+00E9 = 0xC3 0xA9 stays unchanged). - ASSERT_EQ(StringUtils::ToUpper("caf\xC3\xA9"), "CAF\xC3\xA9"); - // "日本語" (0xE6 0x97 0xA5 0xE6 0x9C 0xAC 0xE8 0xAA 0x9E) is returned verbatim. +// Non-ASCII strings are written as explicit UTF-8 byte escapes so the test does not +// depend on the source-file encoding. An escape is split before a following hex digit +// (e.g. "...\x9E" "E") so the \x does not absorb it. +// See https://github.com/apache/iceberg-cpp/issues/613. +TEST(StringUtilsTest, ToLowerUnicode) { + // "CAFÉ" -> "café" (É U+00C9 = 0xC3 0x89 -> é U+00E9 = 0xC3 0xA9). + ASSERT_EQ(StringUtils::ToLower("CAF\xC3\x89"), "caf\xC3\xA9"); + // "GROẞE" -> "große": capital sharp S (ẞ U+1E9E) lower-cases to ß (U+00DF), not "ss" + // as casefolding would produce. + ASSERT_EQ(StringUtils::ToLower("GRO\xE1\xBA\x9E" + "E"), + "gro\xC3\x9F" + "e"); + // "日本語" has no case mapping and is returned verbatim. ASSERT_EQ(StringUtils::ToLower("\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E"), "\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E"); + // ASCII prefix before the first non-ASCII byte takes the fast path; the rest goes + // through utf8proc. "ABÉ" -> "abé". + ASSERT_EQ(StringUtils::ToLower("AB\xC3\x89"), "ab\xC3\xA9"); + // An invalid UTF-8 byte (a lone 0xFF) passes through unchanged rather than erroring. + ASSERT_EQ(StringUtils::ToLower("\xFF"), "\xFF"); + // An invalid byte only passes through itself; the valid code points around it are + // still lower-cased ("AB" 0xFF "CÉ" -> "ab" 0xFF "cé"). + ASSERT_EQ(StringUtils::ToLower("AB\xFF" + "C\xC3\x89"), + "ab\xFF" + "c\xC3\xA9"); + // The invalid byte can abut a multi-byte code point with no ASCII between them; 0xFF + // passes through and the adjacent "É" still lower-cases to "é" (0xFF "É" -> 0xFF "é"). + ASSERT_EQ(StringUtils::ToLower("\xFF\xC3\x89"), "\xFF\xC3\xA9"); + // A truncated multi-byte sequence (0xC3 with no continuation byte) passes through + // without consuming the bytes after it. + ASSERT_EQ(StringUtils::ToLower("\xC3" + "AB"), + "\xC3" + "ab"); + // A stray continuation byte (0x80) behaves the same way. + ASSERT_EQ(StringUtils::ToLower("A\x80" + "B"), + "a\x80" + "b"); +} + +// ToUpper is intentionally ASCII-only; non-ASCII (multibyte UTF-8) bytes pass through. +TEST(StringUtilsTest, ToUpperAsciiOnly) { + // "café" -> "CAFé" (é stays unchanged). + ASSERT_EQ(StringUtils::ToUpper("caf\xC3\xA9"), "CAF\xC3\xA9"); ASSERT_EQ(StringUtils::ToUpper("\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E"), "\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E"); } @@ -63,10 +97,109 @@ TEST(StringUtilsTest, EqualsIgnoreCase) { ASSERT_TRUE(StringUtils::EqualsIgnoreCase("", "")); ASSERT_FALSE(StringUtils::EqualsIgnoreCase("abc", "abcd")); ASSERT_FALSE(StringUtils::EqualsIgnoreCase("abc", "abd")); - // ASCII case is folded; non-ASCII bytes are compared as-is. ("Café" vs "café") - ASSERT_TRUE(StringUtils::EqualsIgnoreCase("Caf\xC3\xA9", "caf\xC3\xA9")); - // "café" vs "cafe": the multibyte é differs from ASCII 'e'. + // Unicode-aware: "CAFÉ" matches "café". + ASSERT_TRUE(StringUtils::EqualsIgnoreCase("CAF\xC3\x89", "caf\xC3\xA9")); + // "GROẞE" matches "große" under lowercasing (ẞ -> ß). + ASSERT_TRUE( + StringUtils::EqualsIgnoreCase("GRO\xE1\xBA\x9E" + "E", + "gro\xC3\x9F" + "e")); + // Different letters still differ ("café" vs "cafe"). ASSERT_FALSE(StringUtils::EqualsIgnoreCase("caf\xC3\xA9", "cafe")); + // Fallback correctness: an ASCII operand can equal a non-ASCII one once lower-cased, + // even though their raw byte lengths differ. "İ" (U+0130 = 0xC4 0xB0, two bytes) + // lower-cases to one-byte "i", so it must compare equal to "i" and "I". + ASSERT_TRUE(StringUtils::EqualsIgnoreCase("i", "\xC4\xB0")); + ASSERT_TRUE(StringUtils::EqualsIgnoreCase("\xC4\xB0", "I")); + // The non-ASCII byte can appear after a matching ASCII prefix ("abi" vs "abİ"). + ASSERT_TRUE(StringUtils::EqualsIgnoreCase("abi", "ab\xC4\xB0")); + // Pure-ASCII operands that share a prefix but differ in length are not equal. + ASSERT_FALSE(StringUtils::EqualsIgnoreCase("abc", "ab")); + // Operands containing invalid UTF-8 are still compared case-insensitively on their + // valid parts; the invalid bytes themselves compare verbatim. + ASSERT_TRUE( + StringUtils::EqualsIgnoreCase("AB\xFF" + "C", + "ab\xFF" + "c")); + ASSERT_FALSE(StringUtils::EqualsIgnoreCase("\xFF", "\xFE")); +} + +TEST(StringUtilsTest, StartsWithIgnoreCase) { + ASSERT_TRUE(StringUtils::StartsWithIgnoreCase("AbCdef", "abc")); + ASSERT_TRUE(StringUtils::StartsWithIgnoreCase("abc", "ABC")); + ASSERT_FALSE(StringUtils::StartsWithIgnoreCase("abc", "abd")); + // Empty prefix always matches; a prefix longer than the string does not. + ASSERT_TRUE(StringUtils::StartsWithIgnoreCase("abc", "")); + ASSERT_FALSE(StringUtils::StartsWithIgnoreCase("ab", "abcd")); + // Regression (#760): lower-casing can change byte length, so the prefix must not be + // matched by byte-slicing. "İ" (U+0130 = 0xC4 0xB0) lower-cases to "i", so "İx" + // starts with "i" ... + ASSERT_TRUE(StringUtils::StartsWithIgnoreCase("\xC4\xB0x", "i")); + // ... and "i" starts with "İ" (both lower-case to "i"), which the old byte-length + // guard wrongly rejected. + ASSERT_TRUE(StringUtils::StartsWithIgnoreCase("i", "\xC4\xB0")); + // A matching Unicode prefix: "CAFÉbar" starts with "café". + ASSERT_TRUE( + StringUtils::StartsWithIgnoreCase("CAF\xC3\x89" + "bar", + "caf\xC3\xA9")); + // Invalid UTF-8 bytes compare verbatim in the prefix as well. + ASSERT_TRUE( + StringUtils::StartsWithIgnoreCase("AB\xFF" + "x", + "ab\xFF")); + ASSERT_FALSE(StringUtils::StartsWithIgnoreCase("ab\xFE", "ab\xFF")); +} + +// The ASCII fast paths in EqualsIgnoreCase / StartsWithIgnoreCase must agree with their +// documented ToLower-based semantics for every input, including length-changing case +// mappings and invalid UTF-8. Rather than enumerate cases by hand, exhaustively compare +// both functions against the ToLower oracle over all short strings built from a small +// alphabet that straddles those boundaries. This is the mechanical form of the #760 +// regression, where a fast path disagreed with ToLower on a length-changing mapping. +TEST(StringUtilsTest, IgnoreCaseAgreesWithToLowerOracle) { + // Atoms mix ASCII (upper/lower, including the lowercase targets of the multi-byte + // mappings) with a 2-byte code point that lower-cases to one byte ("İ" U+0130 -> "i"), + // a 3-byte one that also shrinks to one byte ("K" U+212A -> "k"), an ordinary 2-byte + // cased letter ("É"), and an invalid UTF-8 byte. + const std::vector atoms = { + "a", "I", "i", "k", "\xC4\xB0", "\xE2\x84\xAA", "\xC3\x89", "\xFF"}; + + // Build every string of 0..3 atoms, one generation (length) at a time. + std::vector inputs = {""}; + size_t generation_begin = 0; + for (int len = 0; len < 3; ++len) { + const size_t generation_end = inputs.size(); + for (size_t i = generation_begin; i < generation_end; ++i) { + for (const auto& atom : atoms) { + inputs.push_back(inputs[i] + atom); + } + } + generation_begin = generation_end; + } + + // Precompute the oracle so the O(n^2) comparison below does not re-lower each string. + std::vector lowered; + lowered.reserve(inputs.size()); + for (const auto& s : inputs) { + lowered.push_back(StringUtils::ToLower(s)); + } + + for (size_t i = 0; i < inputs.size(); ++i) { + for (size_t j = 0; j < inputs.size(); ++j) { + EXPECT_EQ(StringUtils::EqualsIgnoreCase(inputs[i], inputs[j]), + lowered[i] == lowered[j]) + << "EqualsIgnoreCase disagreed for a=" << testing::PrintToString(inputs[i]) + << " b=" << testing::PrintToString(inputs[j]); + EXPECT_EQ(StringUtils::StartsWithIgnoreCase(inputs[i], inputs[j]), + lowered[i].starts_with(lowered[j])) + << "StartsWithIgnoreCase disagreed for str=" + << testing::PrintToString(inputs[i]) + << " prefix=" << testing::PrintToString(inputs[j]); + } + } } } // namespace iceberg diff --git a/src/iceberg/util/string_util.cc b/src/iceberg/util/string_util.cc index 0454a62b5..8b98397db 100644 --- a/src/iceberg/util/string_util.cc +++ b/src/iceberg/util/string_util.cc @@ -19,10 +19,54 @@ #include "iceberg/util/string_util.h" +#include + +#include + #include "iceberg/util/macros.h" namespace iceberg { +std::string StringUtils::ToLower(std::string_view str) { + std::string result; + result.reserve(str.size()); + + // Lower-case ASCII bytes directly; hand non-ASCII bytes to utf8proc. The common inputs + // (modes, UUIDs, header/property names, enum-like strings) are pure ASCII and never + // touch utf8proc. utf8proc has no string-level helper, so each non-ASCII code point is + // decoded, mapped with utf8proc_tolower (simple 1:1 mapping, not casefolding), and + // re-encoded. + const auto* data = reinterpret_cast(str.data()); + const auto size = static_cast(str.size()); + utf8proc_ssize_t offset = 0; + while (offset < size) { + // An ASCII byte is a complete 1-byte code point (never a UTF-8 continuation byte), + // and utf8proc_tolower agrees with ToLowerAscii on it, so handle it without utf8proc. + if (IsAsciiByte(str[offset])) { + result.push_back(ToLowerAscii(str[offset])); + ++offset; + continue; + } + utf8proc_int32_t code_point = 0; + utf8proc_ssize_t consumed = + utf8proc_iterate(data + offset, size - offset, &code_point); + if (consumed < 0) { + // Invalid UTF-8: pass the offending byte through unchanged and resume decoding at + // the next byte, so the valid code points around it are still lower-cased. + result.push_back(str[offset]); + ++offset; + continue; + } + const utf8proc_int32_t lowered = utf8proc_tolower(code_point); + std::array encoded{}; + const utf8proc_ssize_t written = utf8proc_encode_char(lowered, encoded.data()); + result.append(reinterpret_cast(encoded.data()), + static_cast(written)); + offset += consumed; + } + return result; +} + Result> StringUtils::HexStringToBytes(std::string_view hex) { if (hex.size() % 2 != 0) [[unlikely]] { return InvalidArgument("Hex string must have even length, got: {}", hex.size()); diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index 01b6087b8..62718a0ea 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -20,9 +20,9 @@ #pragma once #include -#include #include #include +#include #include #include #include @@ -41,29 +41,61 @@ concept FromChars = requires(const char* p, T& v) { std::from_chars(p, p, v); }; class ICEBERG_EXPORT StringUtils { public: - // NOTE: These convert ASCII letters only; all other bytes, including non-ASCII - // (multibyte UTF-8) bytes, are passed through unchanged. - // See https://github.com/apache/iceberg-cpp/issues/613. - static std::string ToLower(std::string_view str) { - return str | std::ranges::views::transform(ToLowerAscii) | - std::ranges::to(); - } - + /// \brief Lower-case a UTF-8 string using Unicode simple (1:1) case mapping. + /// + /// Intended for case-insensitive name matching, similar to Iceberg Java's + /// toLowerCase(Locale.ROOT). The mapping is locale-independent, matching the intent + /// of Locale.ROOT. It uses simple (1:1) case mapping rather than Java's full case + /// mapping, so results differ for a few code points; e.g. U+0130 (capital I with dot + /// above) maps to U+0069 ("i") here, but to U+0069 U+0307 ("i" + combining dot above) + /// in Java. For ASCII and the large majority of letters the two agree. + /// + /// Pure-ASCII input takes a byte-wise fast path; utf8proc is only invoked when a + /// non-ASCII byte (>= 0x80) is present. The function is total: it never fails, and + /// input need not be valid UTF-8. A byte that does not begin a valid UTF-8 sequence + /// is copied through unchanged and decoding resumes at the next byte, so the valid + /// code points around it are still lower-cased. + /// See https://github.com/apache/iceberg-cpp/issues/613. + static std::string ToLower(std::string_view str); + + /// \brief Upper-case the ASCII letters (a-z) in a string; all other bytes, including + /// multi-byte UTF-8 sequences, are left unchanged. + /// + /// Deliberately ASCII-only and, unlike ToLower, not Unicode-aware. It is only used to + /// normalize ASCII enum/codec strings (e.g. "gzip" -> "GZIP", "all" -> "ALL") for + /// case-insensitive comparison. A Unicode upper-case is intentionally not provided: + /// simple case mapping would be wrong for some letters (e.g. "ß" (U+00DF) would stay + /// unchanged instead of becoming "SS"), and no caller needs it. static std::string ToUpper(std::string_view str) { return str | std::ranges::views::transform(ToUpperAscii) | std::ranges::to(); } + /// \brief Case-insensitive equality using Unicode simple (1:1) case mapping. + /// + /// Equal when the ToLower forms of both operands are equal, so folding follows + /// ToLower's rules (e.g. "İ" (U+0130) folds to "i"). Defined for any byte sequence: + /// ToLower passes invalid UTF-8 bytes through unchanged, so they compare verbatim. static bool EqualsIgnoreCase(std::string_view lhs, std::string_view rhs) { - return std::ranges::equal( - lhs, rhs, [](char lc, char rc) { return ToLowerAscii(lc) == ToLowerAscii(rc); }); + const std::optional fast = AsciiEqualsIgnoreCase(lhs, rhs); + return fast.has_value() ? *fast : (ToLower(lhs) == ToLower(rhs)); } + /// \brief Case-insensitive prefix test using Unicode simple (1:1) case mapping. + /// + /// True when the ToLower form of str starts with the ToLower form of prefix, so folding + /// follows ToLower's rules (e.g. "İ" (U+0130) folds to "i"). Defined for any byte + /// sequence: ToLower passes invalid UTF-8 bytes through unchanged, so they compare + /// verbatim. static bool StartsWithIgnoreCase(std::string_view str, std::string_view prefix) { - if (str.size() < prefix.size()) { - return false; + if (prefix.size() <= str.size()) { + const std::optional fast = + AsciiEqualsIgnoreCase(str.substr(0, prefix.size()), prefix); + if (fast.has_value()) { + return *fast; + } } - return EqualsIgnoreCase(str.substr(0, prefix.size()), prefix); + return ToLower(str).starts_with(ToLower(prefix)); } /// \brief Count the number of code points in a UTF-8 string. @@ -134,16 +166,37 @@ class ICEBERG_EXPORT StringUtils { } private: - // ASCII-only case conversion using explicit range checks rather than - // std::tolower/std::toupper. This is independent of the current C locale and never - // touches non-ASCII (high-bit) bytes, so multibyte UTF-8 sequences are preserved. It - // also sidesteps the undefined behavior of passing a negative char to . + // ASCII-only case mappings. These avoid std::toupper/std::tolower, which are + // locale-dependent and have undefined behavior for negative char values. + static constexpr char ToUpperAscii(char c) noexcept { + return (c >= 'a' && c <= 'z') ? static_cast(c - 'a' + 'A') : c; + } static constexpr char ToLowerAscii(char c) noexcept { return (c >= 'A' && c <= 'Z') ? static_cast(c - 'A' + 'a') : c; } - static constexpr char ToUpperAscii(char c) noexcept { - return (c >= 'a' && c <= 'z') ? static_cast(c - 'a' + 'A') : c; + // True if c is a 7-bit ASCII byte (< 0x80). The cast is required because char may be + // signed, which would make bytes >= 0x80 compare as negative. + static constexpr bool IsAsciiByte(char c) noexcept { + return (static_cast(c) & 0x80) == 0; + } + + // Case-insensitive equality decided in a single byte-wise pass, without allocating. + // Returns nullopt once a byte of either operand is non-ASCII, because folding can then + // be non-ASCII and length-changing (e.g. "İ" (U+0130) -> "i"), which only ToLower + // knows. + static std::optional AsciiEqualsIgnoreCase(std::string_view a, + std::string_view b) { + const size_t n = std::min(a.size(), b.size()); + for (size_t i = 0; i < n; ++i) { + if (!IsAsciiByte(a[i]) || !IsAsciiByte(b[i])) { + return std::nullopt; + } + if (ToLowerAscii(a[i]) != ToLowerAscii(b[i])) { + return false; + } + } + return a.size() == b.size(); } }; diff --git a/subprojects/utf8proc.wrap b/subprojects/utf8proc.wrap new file mode 100644 index 000000000..9b33b3bea --- /dev/null +++ b/subprojects/utf8proc.wrap @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +[wrap-file] +directory = utf8proc-2.10.0 +source_url = https://github.com/JuliaStrings/utf8proc/releases/download/v2.10.0/utf8proc-2.10.0.tar.gz +source_filename = utf8proc-2.10.0.tar.gz +source_hash = 276a37dc4d1dd24d7896826a579f4439d1e5fe33603add786bb083cab802e23e +patch_filename = utf8proc_2.10.0-1_patch.zip +patch_url = https://wrapdb.mesonbuild.com/v2/utf8proc_2.10.0-1/get_patch +patch_hash = be16c4514603e922f9636045699fe1a6f844d340b9b7c14b809e47253b06a844 +source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/utf8proc_2.10.0-1/utf8proc-2.10.0.tar.gz +wrapdb_version = 2.10.0-1 + +[provide] +libutf8proc = utf8proc_dep From b4737dfd9da028fd82f3a5f2ff6aa66ded0660f7 Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Sat, 11 Jul 2026 10:33:13 +0800 Subject: [PATCH 03/13] fix: generate public API docs correctly (#804) --- Makefile | 1 + mkdocs/Doxyfile | 59 ++----------------- mkdocs/build-docs.sh | 1 + mkdocs/namespace_docs.dox | 54 +++++++++++++++++ src/iceberg/arrow/arrow_io_util.h | 3 + src/iceberg/arrow/s3/s3_properties.h | 3 + src/iceberg/avro/avro_constants.h | 3 + src/iceberg/avro/avro_metrics.h | 3 + src/iceberg/avro/avro_reader.h | 3 + src/iceberg/avro/avro_writer.h | 3 + src/iceberg/catalog.h | 3 + .../catalog/hive/iceberg_hive_export.h | 3 + .../catalog/memory/in_memory_catalog.h | 3 + src/iceberg/catalog/rest/http_request.h | 3 + .../catalog/rest/iceberg_rest_export.h | 3 + src/iceberg/catalog/rest/rest_file_io.h | 3 + src/iceberg/catalog/rest/rest_util.h | 3 + src/iceberg/catalog/session_catalog.h | 3 + src/iceberg/catalog/session_context.h | 3 + .../catalog/sql/iceberg_sql_catalog_export.h | 3 + .../deletes/position_delete_range_consumer.h | 3 + src/iceberg/expression/literal.h | 3 + src/iceberg/file_io.h | 3 + src/iceberg/file_io_registry.h | 3 + src/iceberg/iceberg_bundle_export.h | 3 + src/iceberg/iceberg_data_export.h | 3 + src/iceberg/iceberg_export.h | 3 + src/iceberg/inspect/history_table.h | 3 + src/iceberg/inspect/metadata_table.h | 3 + src/iceberg/inspect/snapshots_table.h | 3 + src/iceberg/location_provider.h | 3 + src/iceberg/metrics/commit_report.h | 3 + src/iceberg/metrics/counter.h | 3 + src/iceberg/metrics/metrics_reporter.h | 3 + src/iceberg/metrics/scan_report.h | 3 + src/iceberg/metrics/timer.h | 3 + src/iceberg/name_mapping.h | 3 + src/iceberg/parquet/parquet_reader.h | 3 + src/iceberg/parquet/parquet_writer.h | 3 + src/iceberg/result.h | 3 + src/iceberg/row/manifest_wrapper.h | 2 +- src/iceberg/schema_util.h | 3 + src/iceberg/snapshot.h | 3 + src/iceberg/sort_order.h | 3 + src/iceberg/storage_credential.h | 3 + src/iceberg/table.h | 3 + src/iceberg/table_properties.h | 3 + src/iceberg/table_scan.h | 3 + src/iceberg/transaction.h | 3 + src/iceberg/update/snapshot_update.h | 3 + src/iceberg/util/bucket_util.h | 3 + src/iceberg/util/config.h | 3 + src/iceberg/util/executor.h | 3 + src/iceberg/util/functional.h | 3 + src/iceberg/util/location_util.h | 3 + src/iceberg/util/macros.h | 3 + src/iceberg/util/partition_value_util.h | 2 +- src/iceberg/util/property_util.h | 3 + src/iceberg/util/retry_util.h | 3 + src/iceberg/util/string_util.h | 3 + src/iceberg/util/task_group.h | 3 + src/iceberg/util/temporal_util.h | 3 + src/iceberg/util/timepoint.h | 3 + src/iceberg/util/transform_util.h | 3 + src/iceberg/util/truncate_util.h | 3 + src/iceberg/util/visitor_generate.h | 3 + 66 files changed, 244 insertions(+), 55 deletions(-) create mode 100644 mkdocs/namespace_docs.dox diff --git a/Makefile b/Makefile index 100069545..99ca5bbaa 100644 --- a/Makefile +++ b/Makefile @@ -33,6 +33,7 @@ install-deps: build-api-docs: cd mkdocs && \ + rm -rf docs/api && \ mkdir -p docs/api && \ doxygen Doxyfile && \ echo "Doxygen output created in docs/api/" diff --git a/mkdocs/Doxyfile b/mkdocs/Doxyfile index 75e5ee232..0237eb479 100644 --- a/mkdocs/Doxyfile +++ b/mkdocs/Doxyfile @@ -966,7 +966,7 @@ WARN_LOGFILE = # spaces. See also FILE_PATTERNS and EXTENSION_MAPPING # Note: If this tag is empty the current directory is searched. -INPUT = ../src ../README.md +INPUT = ../src ../README.md namespace_docs.dox # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses @@ -1006,57 +1006,9 @@ INPUT_FILE_ENCODING = # be provided as doxygen C comment), *.py, *.pyw, *.f90, *.f95, *.f03, *.f08, # *.f18, *.f, *.for, *.vhd, *.vhdl, *.ucf, *.qsf and *.ice. -FILE_PATTERNS = *.c \ - *.cc \ - *.cxx \ - *.cxxm \ - *.cpp \ - *.cppm \ - *.ccm \ - *.c++ \ - *.c++m \ - *.java \ - *.ii \ - *.ixx \ - *.ipp \ - *.i++ \ - *.inl \ - *.idl \ - *.ddl \ - *.odl \ - *.h \ - *.hh \ - *.hxx \ - *.hpp \ - *.h++ \ - *.ixx \ - *.l \ - *.cs \ - *.d \ - *.php \ - *.php4 \ - *.php5 \ - *.phtml \ - *.inc \ - *.m \ - *.markdown \ +FILE_PATTERNS = *.h \ *.md \ - *.mm \ - *.dox \ - *.py \ - *.pyw \ - *.f90 \ - *.f95 \ - *.f03 \ - *.f08 \ - *.f18 \ - *.f \ - *.for \ - *.vhd \ - *.vhdl \ - *.ucf \ - *.qsf \ - *.ice + *.dox # The RECURSIVE tag can be used to specify whether or not subdirectories should # be searched for input files as well. @@ -1071,7 +1023,8 @@ RECURSIVE = YES # Note that relative paths are relative to the directory from which doxygen is # run. -EXCLUDE = +EXCLUDE = ../src/iceberg/test \ + ../src/iceberg/catalog/hive/gen-cpp # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or # directories that are symbolic links (a Unix file system feature) are excluded @@ -1087,7 +1040,7 @@ EXCLUDE_SYMLINKS = NO # Note that the wildcards are matched against the file with absolute path, so to # exclude all test directories for example use the pattern */test/* -EXCLUDE_PATTERNS = +EXCLUDE_PATTERNS = *_internal.h # The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names # (namespaces, classes, functions, etc.) that should be excluded from the diff --git a/mkdocs/build-docs.sh b/mkdocs/build-docs.sh index 6917d339e..70f4bf8f7 100755 --- a/mkdocs/build-docs.sh +++ b/mkdocs/build-docs.sh @@ -41,6 +41,7 @@ if ! command -v doxygen &> /dev/null; then else echo "Building API documentation with Doxygen..." cd mkdocs + rm -rf docs/api mkdir -p docs/api doxygen Doxyfile cd .. diff --git a/mkdocs/namespace_docs.dox b/mkdocs/namespace_docs.dox new file mode 100644 index 000000000..9faa714d6 --- /dev/null +++ b/mkdocs/namespace_docs.dox @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +/// \namespace iceberg +/// \brief Core Apache Iceberg C++ APIs. + +/// \namespace iceberg::arrow +/// \brief Arrow-backed IO and data conversion APIs. + +/// \namespace iceberg::avro +/// \brief Avro reader, writer, and metadata APIs. + +/// \namespace iceberg::hive +/// \brief Hive catalog APIs. + +/// \namespace iceberg::parquet +/// \brief Parquet reader, writer, and registration APIs. + +/// \namespace iceberg::puffin +/// \brief Puffin file metadata, reader, and writer APIs. + +/// \namespace iceberg::rest +/// \brief REST catalog APIs. + +/// \namespace iceberg::rest::auth +/// \brief REST catalog authentication APIs. + +/// \namespace iceberg::retry +/// \brief Retry policy APIs. + +/// \namespace iceberg::sql +/// \brief SQL catalog APIs. + +/// \namespace iceberg::table +/// \brief Table update requirement APIs. + +/// \namespace iceberg::util +/// \brief Utility APIs. diff --git a/src/iceberg/arrow/arrow_io_util.h b/src/iceberg/arrow/arrow_io_util.h index e33826dc6..925ee12a2 100644 --- a/src/iceberg/arrow/arrow_io_util.h +++ b/src/iceberg/arrow/arrow_io_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/arrow/arrow_io_util.h +/// \brief Provide helpers to create Arrow-backed FileIO implementations. + #include #include #include diff --git a/src/iceberg/arrow/s3/s3_properties.h b/src/iceberg/arrow/s3/s3_properties.h index 7b76968a6..03d1492ad 100644 --- a/src/iceberg/arrow/s3/s3_properties.h +++ b/src/iceberg/arrow/s3/s3_properties.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/arrow/s3/s3_properties.h +/// \brief Define S3 configuration property keys. + #include namespace iceberg::arrow { diff --git a/src/iceberg/avro/avro_constants.h b/src/iceberg/avro/avro_constants.h index 6fdfdc5ed..f3f175c1e 100644 --- a/src/iceberg/avro/avro_constants.h +++ b/src/iceberg/avro/avro_constants.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/avro/avro_constants.h +/// \brief Define Avro metadata constants used by Iceberg. + #include namespace iceberg::avro { diff --git a/src/iceberg/avro/avro_metrics.h b/src/iceberg/avro/avro_metrics.h index 799c00ee3..3f055c23b 100644 --- a/src/iceberg/avro/avro_metrics.h +++ b/src/iceberg/avro/avro_metrics.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/avro/avro_metrics.h +/// \brief Compute Iceberg metrics for Avro data files. + #include "iceberg/iceberg_bundle_export.h" #include "iceberg/metrics.h" diff --git a/src/iceberg/avro/avro_reader.h b/src/iceberg/avro/avro_reader.h index 24f95f5d4..c193d00c2 100644 --- a/src/iceberg/avro/avro_reader.h +++ b/src/iceberg/avro/avro_reader.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/avro/avro_reader.h +/// \brief Provide the Avro file reader interface. + #include "iceberg/file_reader.h" #include "iceberg/iceberg_bundle_export.h" diff --git a/src/iceberg/avro/avro_writer.h b/src/iceberg/avro/avro_writer.h index 3bd8110d9..af68fac44 100644 --- a/src/iceberg/avro/avro_writer.h +++ b/src/iceberg/avro/avro_writer.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/avro/avro_writer.h +/// \brief Provide the Avro file writer interface. + #include "iceberg/file_writer.h" #include "iceberg/iceberg_bundle_export.h" diff --git a/src/iceberg/catalog.h b/src/iceberg/catalog.h index 81c9ddd11..79c901fd0 100644 --- a/src/iceberg/catalog.h +++ b/src/iceberg/catalog.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog.h +/// \brief Define the catalog API for table and namespace operations. + #include #include #include diff --git a/src/iceberg/catalog/hive/iceberg_hive_export.h b/src/iceberg/catalog/hive/iceberg_hive_export.h index 229304553..386329753 100644 --- a/src/iceberg/catalog/hive/iceberg_hive_export.h +++ b/src/iceberg/catalog/hive/iceberg_hive_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/hive/iceberg_hive_export.h +/// \brief Define symbol visibility macros for the Hive catalog library. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_HIVE_STATIC # define ICEBERG_HIVE_EXPORT diff --git a/src/iceberg/catalog/memory/in_memory_catalog.h b/src/iceberg/catalog/memory/in_memory_catalog.h index 22a596c10..548fd7afc 100644 --- a/src/iceberg/catalog/memory/in_memory_catalog.h +++ b/src/iceberg/catalog/memory/in_memory_catalog.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/memory/in_memory_catalog.h +/// \brief Provide an in-memory catalog implementation. + #include #include "iceberg/catalog.h" diff --git a/src/iceberg/catalog/rest/http_request.h b/src/iceberg/catalog/rest/http_request.h index 47419c361..4016df66f 100644 --- a/src/iceberg/catalog/rest/http_request.h +++ b/src/iceberg/catalog/rest/http_request.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/rest/http_request.h +/// \brief Define REST HTTP request types. + #include #include #include diff --git a/src/iceberg/catalog/rest/iceberg_rest_export.h b/src/iceberg/catalog/rest/iceberg_rest_export.h index 7e8836f8f..f8b322660 100644 --- a/src/iceberg/catalog/rest/iceberg_rest_export.h +++ b/src/iceberg/catalog/rest/iceberg_rest_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/rest/iceberg_rest_export.h +/// \brief Define symbol visibility macros for the REST catalog library. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_REST_STATIC # define ICEBERG_REST_EXPORT diff --git a/src/iceberg/catalog/rest/rest_file_io.h b/src/iceberg/catalog/rest/rest_file_io.h index 9e5f7a0a9..9bd0a826e 100644 --- a/src/iceberg/catalog/rest/rest_file_io.h +++ b/src/iceberg/catalog/rest/rest_file_io.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/rest/rest_file_io.h +/// \brief Provide helpers to create FileIO instances for REST catalog responses. + #include #include #include diff --git a/src/iceberg/catalog/rest/rest_util.h b/src/iceberg/catalog/rest/rest_util.h index 51a708994..556b2557a 100644 --- a/src/iceberg/catalog/rest/rest_util.h +++ b/src/iceberg/catalog/rest/rest_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/rest/rest_util.h +/// \brief Provide utilities for REST catalog path and config handling. + #include #include #include diff --git a/src/iceberg/catalog/session_catalog.h b/src/iceberg/catalog/session_catalog.h index 2d7aeae25..08ac048a5 100644 --- a/src/iceberg/catalog/session_catalog.h +++ b/src/iceberg/catalog/session_catalog.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/session_catalog.h +/// \brief Define session catalog routing APIs. + #include #include diff --git a/src/iceberg/catalog/session_context.h b/src/iceberg/catalog/session_context.h index 7dee828e1..19eb8fc5f 100644 --- a/src/iceberg/catalog/session_context.h +++ b/src/iceberg/catalog/session_context.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/session_context.h +/// \brief Define session context metadata. + #include #include diff --git a/src/iceberg/catalog/sql/iceberg_sql_catalog_export.h b/src/iceberg/catalog/sql/iceberg_sql_catalog_export.h index 88921779b..d609e5675 100644 --- a/src/iceberg/catalog/sql/iceberg_sql_catalog_export.h +++ b/src/iceberg/catalog/sql/iceberg_sql_catalog_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/catalog/sql/iceberg_sql_catalog_export.h +/// \brief Define symbol visibility macros for the SQL catalog library. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_SQL_CATALOG_STATIC # define ICEBERG_SQL_CATALOG_EXPORT diff --git a/src/iceberg/deletes/position_delete_range_consumer.h b/src/iceberg/deletes/position_delete_range_consumer.h index 69d1c33d7..37f6caeac 100644 --- a/src/iceberg/deletes/position_delete_range_consumer.h +++ b/src/iceberg/deletes/position_delete_range_consumer.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/deletes/position_delete_range_consumer.h +/// \brief Provide bulk application of position deletes. + #include #include #include diff --git a/src/iceberg/expression/literal.h b/src/iceberg/expression/literal.h index dbcabb521..d88ea9248 100644 --- a/src/iceberg/expression/literal.h +++ b/src/iceberg/expression/literal.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/expression/literal.h +/// \brief Define typed literal values used by expressions. + #include #include #include diff --git a/src/iceberg/file_io.h b/src/iceberg/file_io.h index 7d7b31113..3ea4afa49 100644 --- a/src/iceberg/file_io.h +++ b/src/iceberg/file_io.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/file_io.h +/// \brief Define the FileIO abstraction and file stream interfaces. + #include #include #include diff --git a/src/iceberg/file_io_registry.h b/src/iceberg/file_io_registry.h index a0be1b563..1643bf457 100644 --- a/src/iceberg/file_io_registry.h +++ b/src/iceberg/file_io_registry.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/file_io_registry.h +/// \brief Define the FileIO registry. + #include #include #include diff --git a/src/iceberg/iceberg_bundle_export.h b/src/iceberg/iceberg_bundle_export.h index c63f8811d..49407a34a 100644 --- a/src/iceberg/iceberg_bundle_export.h +++ b/src/iceberg/iceberg_bundle_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/iceberg_bundle_export.h +/// \brief Define symbol visibility macros for bundled format integrations. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_BUNDLE_STATIC # define ICEBERG_BUNDLE_EXPORT diff --git a/src/iceberg/iceberg_data_export.h b/src/iceberg/iceberg_data_export.h index 652de97b6..5b710e9cc 100644 --- a/src/iceberg/iceberg_data_export.h +++ b/src/iceberg/iceberg_data_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/iceberg_data_export.h +/// \brief Define symbol visibility macros for data APIs. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_DATA_STATIC # define ICEBERG_DATA_EXPORT diff --git a/src/iceberg/iceberg_export.h b/src/iceberg/iceberg_export.h index 2463fbeaf..5dcfa2f66 100644 --- a/src/iceberg/iceberg_export.h +++ b/src/iceberg/iceberg_export.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/iceberg_export.h +/// \brief Define symbol visibility macros for core Iceberg APIs. + #if defined(_WIN32) || defined(__CYGWIN__) # ifdef ICEBERG_STATIC # define ICEBERG_EXPORT diff --git a/src/iceberg/inspect/history_table.h b/src/iceberg/inspect/history_table.h index 94a26368f..21f1f8002 100644 --- a/src/iceberg/inspect/history_table.h +++ b/src/iceberg/inspect/history_table.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/inspect/history_table.h +/// \brief Define the history metadata table. + #include #include "iceberg/iceberg_export.h" diff --git a/src/iceberg/inspect/metadata_table.h b/src/iceberg/inspect/metadata_table.h index 7d0ac22da..51c5f7920 100644 --- a/src/iceberg/inspect/metadata_table.h +++ b/src/iceberg/inspect/metadata_table.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/inspect/metadata_table.h +/// \brief Define base APIs for metadata tables. + #include #include "iceberg/iceberg_export.h" diff --git a/src/iceberg/inspect/snapshots_table.h b/src/iceberg/inspect/snapshots_table.h index 50017796f..9af1bcacb 100644 --- a/src/iceberg/inspect/snapshots_table.h +++ b/src/iceberg/inspect/snapshots_table.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/inspect/snapshots_table.h +/// \brief Define the snapshots metadata table. + #include #include "iceberg/iceberg_export.h" diff --git a/src/iceberg/location_provider.h b/src/iceberg/location_provider.h index 6b6fa4475..8541b7c11 100644 --- a/src/iceberg/location_provider.h +++ b/src/iceberg/location_provider.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/location_provider.h +/// \brief Define table data location providers. + #include #include #include diff --git a/src/iceberg/metrics/commit_report.h b/src/iceberg/metrics/commit_report.h index 63867c8e0..a644d0b07 100644 --- a/src/iceberg/metrics/commit_report.h +++ b/src/iceberg/metrics/commit_report.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/metrics/commit_report.h +/// \brief Define commit metrics and reports. + #include #include #include diff --git a/src/iceberg/metrics/counter.h b/src/iceberg/metrics/counter.h index 4542b90f0..b44cf844b 100644 --- a/src/iceberg/metrics/counter.h +++ b/src/iceberg/metrics/counter.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/metrics/counter.h +/// \brief Define metrics counters. + #include #include #include diff --git a/src/iceberg/metrics/metrics_reporter.h b/src/iceberg/metrics/metrics_reporter.h index b255a95e8..a45754dc0 100644 --- a/src/iceberg/metrics/metrics_reporter.h +++ b/src/iceberg/metrics/metrics_reporter.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/metrics/metrics_reporter.h +/// \brief Define metrics report dispatch APIs. + #include #include #include diff --git a/src/iceberg/metrics/scan_report.h b/src/iceberg/metrics/scan_report.h index 2d81e7e8f..1ac74c05f 100644 --- a/src/iceberg/metrics/scan_report.h +++ b/src/iceberg/metrics/scan_report.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/metrics/scan_report.h +/// \brief Define scan metrics and reports. + #include #include #include diff --git a/src/iceberg/metrics/timer.h b/src/iceberg/metrics/timer.h index 08bbf8e34..e15c623f3 100644 --- a/src/iceberg/metrics/timer.h +++ b/src/iceberg/metrics/timer.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/metrics/timer.h +/// \brief Define metrics timers. + #include #include #include diff --git a/src/iceberg/name_mapping.h b/src/iceberg/name_mapping.h index 392b573e3..7fd140c0d 100644 --- a/src/iceberg/name_mapping.h +++ b/src/iceberg/name_mapping.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/name_mapping.h +/// \brief Define schema name mapping APIs. + #include #include #include diff --git a/src/iceberg/parquet/parquet_reader.h b/src/iceberg/parquet/parquet_reader.h index 0604230c8..3a8e57cf3 100644 --- a/src/iceberg/parquet/parquet_reader.h +++ b/src/iceberg/parquet/parquet_reader.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/parquet/parquet_reader.h +/// \brief Provide the Parquet file reader interface. + #include "iceberg/file_reader.h" #include "iceberg/iceberg_bundle_export.h" diff --git a/src/iceberg/parquet/parquet_writer.h b/src/iceberg/parquet/parquet_writer.h index 5abe45db9..562ffb801 100644 --- a/src/iceberg/parquet/parquet_writer.h +++ b/src/iceberg/parquet/parquet_writer.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/parquet/parquet_writer.h +/// \brief Provide the Parquet file writer interface. + #include "iceberg/file_writer.h" #include "iceberg/iceberg_bundle_export.h" diff --git a/src/iceberg/result.h b/src/iceberg/result.h index 4b4d0e7fb..88b13c51e 100644 --- a/src/iceberg/result.h +++ b/src/iceberg/result.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/result.h +/// \brief Define Result, Status, and error helpers. + #include #include #include diff --git a/src/iceberg/row/manifest_wrapper.h b/src/iceberg/row/manifest_wrapper.h index 20c2165b2..bd4fc4df6 100644 --- a/src/iceberg/row/manifest_wrapper.h +++ b/src/iceberg/row/manifest_wrapper.h @@ -19,7 +19,7 @@ #pragma once -/// \file iceberg/manifest_wrapper.h +/// \file iceberg/row/manifest_wrapper.h /// Wrapper classes for manifest-related data structures that implement /// StructLike, ArrayLike, and MapLike interfaces for unified data access. diff --git a/src/iceberg/schema_util.h b/src/iceberg/schema_util.h index 9df402336..421d00e0a 100644 --- a/src/iceberg/schema_util.h +++ b/src/iceberg/schema_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/schema_util.h +/// \brief Provide schema projection utilities. + #include #include #include diff --git a/src/iceberg/snapshot.h b/src/iceberg/snapshot.h index c2fd9b8a2..e32f3a715 100644 --- a/src/iceberg/snapshot.h +++ b/src/iceberg/snapshot.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/snapshot.h +/// \brief Define snapshot metadata types. + #include #include #include diff --git a/src/iceberg/sort_order.h b/src/iceberg/sort_order.h index 7c9b799fb..75cabeca7 100644 --- a/src/iceberg/sort_order.h +++ b/src/iceberg/sort_order.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/sort_order.h +/// \brief Define table sort order metadata. + #include #include #include diff --git a/src/iceberg/storage_credential.h b/src/iceberg/storage_credential.h index 604e8ddce..746bad086 100644 --- a/src/iceberg/storage_credential.h +++ b/src/iceberg/storage_credential.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/storage_credential.h +/// \brief Define storage credential metadata. + #include #include diff --git a/src/iceberg/table.h b/src/iceberg/table.h index b52b5d29d..f9b72302a 100644 --- a/src/iceberg/table.h +++ b/src/iceberg/table.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/table.h +/// \brief Define table abstractions and update entry points. + #include #include #include diff --git a/src/iceberg/table_properties.h b/src/iceberg/table_properties.h index 6b92ea1ff..f334272e6 100644 --- a/src/iceberg/table_properties.h +++ b/src/iceberg/table_properties.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/table_properties.h +/// \brief Define table property helpers. + #include #include #include diff --git a/src/iceberg/table_scan.h b/src/iceberg/table_scan.h index e55330f30..f3e2f244d 100644 --- a/src/iceberg/table_scan.h +++ b/src/iceberg/table_scan.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/table_scan.h +/// \brief Define table scan APIs and scan task types. + #include #include #include diff --git a/src/iceberg/transaction.h b/src/iceberg/transaction.h index 47714ba70..284222351 100644 --- a/src/iceberg/transaction.h +++ b/src/iceberg/transaction.h @@ -20,6 +20,9 @@ #pragma once +/// \file iceberg/transaction.h +/// \brief Define transaction APIs. + #include #include #include diff --git a/src/iceberg/update/snapshot_update.h b/src/iceberg/update/snapshot_update.h index 10bb16fbc..e29d96d57 100644 --- a/src/iceberg/update/snapshot_update.h +++ b/src/iceberg/update/snapshot_update.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/update/snapshot_update.h +/// \brief Define base APIs for snapshot updates. + #include #include #include diff --git a/src/iceberg/util/bucket_util.h b/src/iceberg/util/bucket_util.h index 31a574b48..459f487bb 100644 --- a/src/iceberg/util/bucket_util.h +++ b/src/iceberg/util/bucket_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/bucket_util.h +/// \brief Provide bucket transform utilities. + #include #include diff --git a/src/iceberg/util/config.h b/src/iceberg/util/config.h index 089c78f5d..ed847eb63 100644 --- a/src/iceberg/util/config.h +++ b/src/iceberg/util/config.h @@ -18,6 +18,9 @@ */ #pragma once +/// \file iceberg/util/config.h +/// \brief Provide typed configuration helpers. + #include #include #include diff --git a/src/iceberg/util/executor.h b/src/iceberg/util/executor.h index 01aa83d7a..749502b17 100644 --- a/src/iceberg/util/executor.h +++ b/src/iceberg/util/executor.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/executor.h +/// \brief Define task executor interfaces. + #include #include diff --git a/src/iceberg/util/functional.h b/src/iceberg/util/functional.h index c5ab2164b..dbb35a376 100644 --- a/src/iceberg/util/functional.h +++ b/src/iceberg/util/functional.h @@ -22,6 +22,9 @@ #pragma once +/// \file iceberg/util/functional.h +/// \brief Provide move-only function wrappers. + #include #include #include diff --git a/src/iceberg/util/location_util.h b/src/iceberg/util/location_util.h index eb78dece3..c213a3bf7 100644 --- a/src/iceberg/util/location_util.h +++ b/src/iceberg/util/location_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/location_util.h +/// \brief Provide location path utilities. + #include #include "iceberg/iceberg_export.h" diff --git a/src/iceberg/util/macros.h b/src/iceberg/util/macros.h index e391d2bfd..400573d8b 100644 --- a/src/iceberg/util/macros.h +++ b/src/iceberg/util/macros.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/macros.h +/// \brief Define common Iceberg utility macros. + #include #include "iceberg/exception.h" diff --git a/src/iceberg/util/partition_value_util.h b/src/iceberg/util/partition_value_util.h index 0113623ba..abfc1ca87 100644 --- a/src/iceberg/util/partition_value_util.h +++ b/src/iceberg/util/partition_value_util.h @@ -19,7 +19,7 @@ #pragma once -/// \file iceberg/row/partition_values.h +/// \file iceberg/util/partition_value_util.h /// Wrapper classes for partition value related data structures. #include diff --git a/src/iceberg/util/property_util.h b/src/iceberg/util/property_util.h index 4e3e9b125..c46d3a6ab 100644 --- a/src/iceberg/util/property_util.h +++ b/src/iceberg/util/property_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/property_util.h +/// \brief Provide property conversion helpers. + #include #include diff --git a/src/iceberg/util/retry_util.h b/src/iceberg/util/retry_util.h index 08e3fb81d..8d2ed2a18 100644 --- a/src/iceberg/util/retry_util.h +++ b/src/iceberg/util/retry_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/retry_util.h +/// \brief Provide retry policy and runner utilities. + #include #include #include diff --git a/src/iceberg/util/string_util.h b/src/iceberg/util/string_util.h index 62718a0ea..123167324 100644 --- a/src/iceberg/util/string_util.h +++ b/src/iceberg/util/string_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/string_util.h +/// \brief Provide string utility helpers. + #include #include #include diff --git a/src/iceberg/util/task_group.h b/src/iceberg/util/task_group.h index 5263816e3..3d345c240 100644 --- a/src/iceberg/util/task_group.h +++ b/src/iceberg/util/task_group.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/task_group.h +/// \brief Provide grouped task execution utilities. + #include #include #include diff --git a/src/iceberg/util/temporal_util.h b/src/iceberg/util/temporal_util.h index acb1f0e73..4a0642daa 100644 --- a/src/iceberg/util/temporal_util.h +++ b/src/iceberg/util/temporal_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/temporal_util.h +/// \brief Provide date and timestamp conversion utilities. + #include #include #include diff --git a/src/iceberg/util/timepoint.h b/src/iceberg/util/timepoint.h index 34ffedfc7..3a12ccd22 100644 --- a/src/iceberg/util/timepoint.h +++ b/src/iceberg/util/timepoint.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/timepoint.h +/// \brief Provide time point conversion helpers. + #include #include "iceberg/iceberg_export.h" diff --git a/src/iceberg/util/transform_util.h b/src/iceberg/util/transform_util.h index e0918dcf3..77197d6ab 100644 --- a/src/iceberg/util/transform_util.h +++ b/src/iceberg/util/transform_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/transform_util.h +/// \brief Provide partition transform utility helpers. + #include #include diff --git a/src/iceberg/util/truncate_util.h b/src/iceberg/util/truncate_util.h index 7fee86eba..0ed686546 100644 --- a/src/iceberg/util/truncate_util.h +++ b/src/iceberg/util/truncate_util.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/truncate_util.h +/// \brief Provide truncate transform utility helpers. + #include #include #include diff --git a/src/iceberg/util/visitor_generate.h b/src/iceberg/util/visitor_generate.h index ad6f5eb2d..37a9bb6c1 100644 --- a/src/iceberg/util/visitor_generate.h +++ b/src/iceberg/util/visitor_generate.h @@ -19,6 +19,9 @@ #pragma once +/// \file iceberg/util/visitor_generate.h +/// \brief Define macros for generating type visitors. + namespace iceberg { #define ICEBERG_GENERATE_FOR_ALL_TYPES(ACTION) \ From a45bce6326101bc13f80c89912d86b2e8e0b6621 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 11 Jul 2026 14:46:30 +0800 Subject: [PATCH 04/13] chore(deps): bump the codeql-action group with 2 updates (#827) Bumps the codeql-action group with 2 updates: [github/codeql-action/init](https://github.com/github/codeql-action) and [github/codeql-action/analyze](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.36.2 to 4.36.3
Release notes

Sourced from github/codeql-action/init's releases.

v4.36.3

No user facing changes.

Changelog

Sourced from github/codeql-action/init's changelog.

CodeQL Action Changelog

See the releases page for the relevant changes to the CodeQL CLI and language packs.

[UNRELEASED]

  • Upcoming breaking change: Add a deprecation warning for customers using CodeQL version 2.20.6 and earlier. These versions of CodeQL were discontinued on 1 July 2026 alongside GitHub Enterprise Server 3.16, and will be unsupported by the next minor release of the CodeQL Action. #3956

4.37.0 - 08 Jul 2026

  • Update default CodeQL bundle version to 2.26.0. #3995
  • In addition to the existing input format, the config-file input for the codeql-action/init step will soon support a new [owner/]repo[@ref][:path] format. All components except the repository name are optional. If omitted, owner defaults to the same owner as the repository the analysis is running for, ref to main, and path to .github/codeql-action.yaml. Support for this format ships in this version of the CodeQL Action, but will only be enabled over the coming weeks. #3973

4.36.3 - 01 Jul 2026

No user facing changes.

4.36.2 - 04 Jun 2026

  • Cache CodeQL CLI version information across Actions steps. #3943
  • Reduce requests while waiting for analysis processing by using exponential backoff when polling SARIF processing status. #3937
  • Update default CodeQL bundle version to 2.25.6. #3948

4.36.1 - 02 Jun 2026

No user facing changes.

4.36.0 - 22 May 2026

  • Breaking change: Bump the minimum required CodeQL bundle version to 2.19.4. #3894
  • Add support for SHA-256 Git object IDs. #3893
  • Update default CodeQL bundle version to 2.25.5. #3926

4.35.5 - 15 May 2026

  • We have improved how the JavaScript bundles for the CodeQL Action are generated to avoid duplication across bundles and reduce the size of the repository by around 70%. This should have no effect on the runtime behaviour of the CodeQL Action. #3899
  • For performance and accuracy reasons, improved incremental analysis will now only be enabled on a pull request when diff-informed analysis is also enabled for that run. If diff-informed analysis is unavailable (for example, because the PR diff ranges could not be computed), the action will fall back to a full analysis. #3791
  • If multiple inputs are provided for the GitHub-internal analysis-kinds input, only code-scanning will be enabled. The analysis-kinds input is experimental, for GitHub-internal use only, and may change without notice at any time. #3892
  • Added an experimental change which, when running a Code Scanning analysis for a PR with improved incremental analysis enabled, prefers CodeQL CLI versions that have a cached overlay-base database for the configured languages. This speeds up analysis for a repository when there is not yet a cached overlay-base database for the latest CLI version. We expect to roll this change out to everyone in May. #3880

4.35.4 - 07 May 2026

  • Update default CodeQL bundle version to 2.25.4. #3881

4.35.3 - 01 May 2026

  • Upcoming breaking change: Add a deprecation warning for customers using CodeQL version 2.19.3 and earlier. These versions of CodeQL were discontinued on 9 April 2026 alongside GitHub Enterprise Server 3.15, and will be unsupported by the next minor release of the CodeQL Action. #3837
  • Configurations for private registries that use Cloudsmith or GCP OIDC are now accepted. #3850
  • Best-effort connection tests for private registries now use GET requests instead of HEAD for better compatibility with various registry implementations. For NuGet feeds, the test is now always performed against the service index. #3853
  • Fixed a bug where two diagnostics produced within the same millisecond could overwrite each other on disk, causing one of them to be lost. #3852

... (truncated)

Commits
  • 54f647b Merge pull request #3984 from github/update-v4.36.3-1f34ec164
  • e78819e Trigger checks
  • 2c9d3d6 Update changelog for v4.36.3
  • 1f34ec1 Merge pull request #3983 from github/mbg/repo-props/ff-for-config-file-prop
  • d5f0145 Log when repository property has a value but is ignored
  • f27f563 Add test for when the FF is off
  • 0025d0f Use FF
  • f7fa18f Add FF for config file repo property
  • 628fc3f Merge pull request #3979 from github/henrymercer/overlay-db-cleanup-size-tele...
  • 9cfb67b Add clarifying comments
  • Additional commits viewable in compare view

Updates `github/codeql-action/analyze` from 4.36.2 to 4.36.3
Release notes

Sourced from github/codeql-action/analyze's releases.

v4.36.3

No user facing changes.

Changelog

Sourced from github/codeql-action/analyze's changelog.

CodeQL Action Changelog

See the releases page for the relevant changes to the CodeQL CLI and language packs.

[UNRELEASED]

  • Upcoming breaking change: Add a deprecation warning for customers using CodeQL version 2.20.6 and earlier. These versions of CodeQL were discontinued on 1 July 2026 alongside GitHub Enterprise Server 3.16, and will be unsupported by the next minor release of the CodeQL Action. #3956

4.37.0 - 08 Jul 2026

  • Update default CodeQL bundle version to 2.26.0. #3995
  • In addition to the existing input format, the config-file input for the codeql-action/init step will soon support a new [owner/]repo[@ref][:path] format. All components except the repository name are optional. If omitted, owner defaults to the same owner as the repository the analysis is running for, ref to main, and path to .github/codeql-action.yaml. Support for this format ships in this version of the CodeQL Action, but will only be enabled over the coming weeks. #3973

4.36.3 - 01 Jul 2026

No user facing changes.

4.36.2 - 04 Jun 2026

  • Cache CodeQL CLI version information across Actions steps. #3943
  • Reduce requests while waiting for analysis processing by using exponential backoff when polling SARIF processing status. #3937
  • Update default CodeQL bundle version to 2.25.6. #3948

4.36.1 - 02 Jun 2026

No user facing changes.

4.36.0 - 22 May 2026

  • Breaking change: Bump the minimum required CodeQL bundle version to 2.19.4. #3894
  • Add support for SHA-256 Git object IDs. #3893
  • Update default CodeQL bundle version to 2.25.5. #3926

4.35.5 - 15 May 2026

  • We have improved how the JavaScript bundles for the CodeQL Action are generated to avoid duplication across bundles and reduce the size of the repository by around 70%. This should have no effect on the runtime behaviour of the CodeQL Action. #3899
  • For performance and accuracy reasons, improved incremental analysis will now only be enabled on a pull request when diff-informed analysis is also enabled for that run. If diff-informed analysis is unavailable (for example, because the PR diff ranges could not be computed), the action will fall back to a full analysis. #3791
  • If multiple inputs are provided for the GitHub-internal analysis-kinds input, only code-scanning will be enabled. The analysis-kinds input is experimental, for GitHub-internal use only, and may change without notice at any time. #3892
  • Added an experimental change which, when running a Code Scanning analysis for a PR with improved incremental analysis enabled, prefers CodeQL CLI versions that have a cached overlay-base database for the configured languages. This speeds up analysis for a repository when there is not yet a cached overlay-base database for the latest CLI version. We expect to roll this change out to everyone in May. #3880

4.35.4 - 07 May 2026

  • Update default CodeQL bundle version to 2.25.4. #3881

4.35.3 - 01 May 2026

  • Upcoming breaking change: Add a deprecation warning for customers using CodeQL version 2.19.3 and earlier. These versions of CodeQL were discontinued on 9 April 2026 alongside GitHub Enterprise Server 3.15, and will be unsupported by the next minor release of the CodeQL Action. #3837
  • Configurations for private registries that use Cloudsmith or GCP OIDC are now accepted. #3850
  • Best-effort connection tests for private registries now use GET requests instead of HEAD for better compatibility with various registry implementations. For NuGet feeds, the test is now always performed against the service index. #3853
  • Fixed a bug where two diagnostics produced within the same millisecond could overwrite each other on disk, causing one of them to be lost. #3852

... (truncated)

Commits
  • 54f647b Merge pull request #3984 from github/update-v4.36.3-1f34ec164
  • e78819e Trigger checks
  • 2c9d3d6 Update changelog for v4.36.3
  • 1f34ec1 Merge pull request #3983 from github/mbg/repo-props/ff-for-config-file-prop
  • d5f0145 Log when repository property has a value but is ignored
  • f27f563 Add test for when the FF is off
  • 0025d0f Use FF
  • f7fa18f Add FF for config file repo property
  • 628fc3f Merge pull request #3979 from github/henrymercer/overlay-db-cleanup-size-tele...
  • 9cfb67b Add clarifying comments
  • Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore ` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore ` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore ` will remove the ignore condition of the specified dependency and ignore conditions
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/codeql.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 2c03085eb..8220e01ea 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -54,11 +54,11 @@ jobs: persist-credentials: false - name: Initialize CodeQL - uses: github/codeql-action/init@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 + uses: github/codeql-action/init@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3 with: languages: actions - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4.36.2 + uses: github/codeql-action/analyze@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3 with: category: "/language:actions" From 3d5af4751f6b74e3cb588b95c464e69a520b8180 Mon Sep 17 00:00:00 2001 From: wzhuo Date: Sat, 11 Jul 2026 15:31:28 +0800 Subject: [PATCH 05/13] fix: return error for reserved property keys in UpdateProperties::Set() (#817) Previously, reserved table properties (except `format-version`) were silently ignored when set via `UpdateProperties::Set()`. This caused undetected configuration drift compared to Java, which throws exceptions for such attempts. Now attempts to set reserved properties accumulate a validation error via `ICEBERGER_BUILDER_CHECK`, surfaced at `Apply()` time. This matches Java's loud-failure behavior. --- src/iceberg/test/update_properties_test.cc | 47 ++++++++++++++++++++++ src/iceberg/update/update_properties.cc | 8 ++-- src/iceberg/update/update_properties.h | 5 ++- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/iceberg/test/update_properties_test.cc b/src/iceberg/test/update_properties_test.cc index 886558153..2a22ff949 100644 --- a/src/iceberg/test/update_properties_test.cc +++ b/src/iceberg/test/update_properties_test.cc @@ -129,6 +129,53 @@ TEST_F(UpdatePropertiesTest, UpgradeFormatVersionUnsupported) { EXPECT_THAT(result, HasErrorMessage("unsupported format version")); } +TEST_F(UpdatePropertiesTest, SetReservedPropertyUuid) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties()); + update->Set("uuid", "some-uuid"); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property")); +} + +TEST_F(UpdatePropertiesTest, SetReservedPropertyCurrentSchema) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties()); + update->Set("current-schema", R"({"type": "struct"})"); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property")); +} + +TEST_F(UpdatePropertiesTest, SetReservedPropertyCurrentSnapshotId) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties()); + update->Set("current-snapshot-id", "12345"); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property")); +} + +TEST_F(UpdatePropertiesTest, SetFormatVersionStillAllowed) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties()); + update->Set("format-version", "3"); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + EXPECT_TRUE(result.updates.empty()); + ASSERT_TRUE(result.format_version.has_value()); + EXPECT_EQ(result.format_version.value(), 3); +} + +TEST_F(UpdatePropertiesTest, SetValidAndReservedProperties) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties()); + update->Set("valid.key", "valid.value"); + update->Set("snapshot-count", "10"); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property")); +} + TEST_F(UpdatePropertiesTest, CommitSuccess) { ICEBERG_UNWRAP_OR_FAIL(auto empty_update, table_->NewUpdateProperties()); EXPECT_THAT(empty_update->Commit(), IsOk()); diff --git a/src/iceberg/update/update_properties.cc b/src/iceberg/update/update_properties.cc index 1837ab009..d2e75c301 100644 --- a/src/iceberg/update/update_properties.cc +++ b/src/iceberg/update/update_properties.cc @@ -50,10 +50,10 @@ UpdateProperties& UpdateProperties::Set(const std::string& key, "Cannot set property '{}' that is already marked for removal", key); - if (!TableProperties::reserved_properties().contains(key) || - key == TableProperties::kFormatVersion.key()) { - updates_.insert_or_assign(key, value); - } + ICEBERG_BUILDER_CHECK(!TableProperties::reserved_properties().contains(key) || + key == TableProperties::kFormatVersion.key(), + "Cannot set reserved property: '{}'", key); + updates_.insert_or_assign(key, value); return *this; } diff --git a/src/iceberg/update/update_properties.h b/src/iceberg/update/update_properties.h index dee923410..18eba427b 100644 --- a/src/iceberg/update/update_properties.h +++ b/src/iceberg/update/update_properties.h @@ -51,8 +51,9 @@ class ICEBERG_EXPORT UpdateProperties : public PendingUpdate { /// \brief Sets a property key to a specified value. /// - /// The key must not have been previously marked for removal and reserved property keys - /// will be ignored. + /// The key must not have been previously marked for removal and must not be a + /// reserved property key (except `format-version`). Setting a reserved property + /// will result in a validation error at Apply() time. /// /// \param key The property key to set /// \param value The property value to set From 62e1bd8ed5e1ba58b86f8630d10be7cf2ecbb908 Mon Sep 17 00:00:00 2001 From: Xin Huang <42597328+huan233usc@users.noreply.github.com> Date: Sat, 11 Jul 2026 00:32:58 -0700 Subject: [PATCH 06/13] feat: support v3 column default values in UpdateSchema (#793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Part 3 of 4 of Iceberg v3 column default-value support (POC #731), built on the schema-layer support merged in #746. Independent of the read-path PRs (#792 and the Avro follow-up). Adds default-value handling to `UpdateSchema` (schema evolution). ## Changes - **`AddColumn` / `AddRequiredColumn`** take an optional `default_value`. When provided it is set as both the column's `initial-default` and `write-default`. A non-null default also lets a required column be added without `AllowIncompatibleChanges()` — rows written before the change read the default instead of null. - **`UpdateColumnDefault(name, default)`** (new) sets, or clears with `std::nullopt`, a column's `write-default`; the `initial-default` is fixed when the column is added. - Defaults are **cast to the column type** (rejecting uncastable or out-of-range values) and **preserved** across rename / doc / type-promotion updates and nested field-id reassignment. - `RequireColumn` may now mark a column that was added with a default required. The `SchemaField` constructor stores defaults verbatim (it does not coerce them), so the cast/promotion is performed explicitly at each evolution site — the same effect as Java, where `NestedField`'s constructor runs `castDefault`. Same-scale decimal precision widening is handled directly (the unscaled value is unchanged), since `Literal::CastTo` does not cast between decimal types. ## Tests 13 cases in `update_schema_test.cc`: add optional/required/nested column with a default, mismatched/narrowing rejection, `UpdateColumnDefault` (set / clear / cast-to-type / pre-existing column), require-after-default, and preservation across doc updates and type promotion — including same-scale decimal precision promotion. ## Stack 1. #746 — schema: represent / serialize / validate (merged) 2. #792 — read path: Parquet 3. read path: Avro (follows) 4. **this PR** — schema evolution: `addColumn` / `updateColumnDefault` --- src/iceberg/schema_field.cc | 68 +++- src/iceberg/schema_field.h | 42 ++- src/iceberg/test/meson.build | 1 + src/iceberg/test/schema_field_test.cc | 63 ++++ src/iceberg/test/update_schema_test.cc | 410 ++++++++++++++++++++++++- src/iceberg/type_fwd.h | 1 + src/iceberg/update/update_schema.cc | 150 +++++++-- src/iceberg/update/update_schema.h | 53 +++- src/iceberg/util/type_util.cc | 19 +- 9 files changed, 720 insertions(+), 87 deletions(-) diff --git a/src/iceberg/schema_field.cc b/src/iceberg/schema_field.cc index 6c8d10d97..80ea61bce 100644 --- a/src/iceberg/schema_field.cc +++ b/src/iceberg/schema_field.cc @@ -22,9 +22,11 @@ #include #include #include +#include #include "iceberg/expression/literal.h" #include "iceberg/type.h" +#include "iceberg/util/checked_cast.h" #include "iceberg/util/formatter.h" // IWYU pragma: keep #include "iceberg/util/macros.h" @@ -32,8 +34,7 @@ namespace iceberg { namespace { -// A null default value is modeled as the absence of a default (matching Java), so it is -// not stored. +// Treat null defaults as absent. std::shared_ptr DropNullDefault(std::shared_ptr value) { if (value != nullptr && value->IsNull()) { return nullptr; @@ -65,6 +66,55 @@ SchemaField SchemaField::MakeRequired(int32_t field_id, std::string_view name, return {field_id, name, std::move(type), false, doc}; } +SchemaField SchemaField::WithName(std::string_view name) const { + return {field_id_, name, type_, optional_, doc_, initial_default_, write_default_}; +} + +SchemaField SchemaField::WithType(std::shared_ptr type) const { + return {field_id_, name_, std::move(type), optional_, doc_, + initial_default_, write_default_}; +} + +SchemaField SchemaField::WithDoc(std::string_view doc) const { + return {field_id_, name_, type_, optional_, doc, initial_default_, write_default_}; +} + +SchemaField SchemaField::WithInitialDefault( + std::shared_ptr initial_default) const { + return {field_id_, name_, type_, optional_, doc_, std::move(initial_default), + write_default_}; +} + +SchemaField SchemaField::WithWriteDefault( + std::shared_ptr write_default) const { + return {field_id_, + name_, + type_, + optional_, + doc_, + initial_default_, + std::move(write_default)}; +} + +Result SchemaField::CastDefaultValue( + const Literal& value, const std::shared_ptr& target_type) { + if (target_type->type_id() == TypeId::kDecimal && + std::holds_alternative(value.value())) { + const auto& source_type = internal::checked_cast(*value.type()); + const auto& decimal_type = internal::checked_cast(*target_type); + if (source_type.scale() == decimal_type.scale()) { + const auto& decimal_value = std::get(value.value()); + if (!decimal_value.FitsInPrecision(decimal_type.precision())) { + return InvalidArgument("Cannot cast default value to {}: {}", *target_type, + value); + } + return Literal::Decimal(decimal_value.value(), decimal_type.precision(), + decimal_type.scale()); + } + } + return value.CastTo(target_type); +} + int32_t SchemaField::field_id() const { return field_id_; } std::string_view SchemaField::name() const { return name_; } @@ -87,8 +137,7 @@ namespace { Status ValidateDefault(const SchemaField& field, const Literal& value, std::string_view kind) { - // A null default is modeled as absence and dropped at construction, so it never reaches - // here; only the out-of-range cast sentinels need rejecting. + // Null defaults are dropped at construction. if (value.IsAboveMax() || value.IsBelowMin()) { return InvalidSchema("Invalid {} value for {}: value is out of range", kind, field.name()); @@ -97,8 +146,7 @@ Status ValidateDefault(const SchemaField& field, const Literal& value, return InvalidSchema("Invalid {} value for {}: field has no type", kind, field.name()); } - // The spec requires unknown/variant/geometry/geography columns to default to null, so a - // non-null default on them is invalid (a null default was already dropped as absence). + // These types can only use null defaults. switch (field.type()->type_id()) { case TypeId::kUnknown: case TypeId::kVariant: @@ -109,17 +157,13 @@ Status ValidateDefault(const SchemaField& field, const Literal& value, default: break; } - // Defaults are otherwise only supported on primitive fields. The spec also permits JSON - // single-value defaults for struct/list/map (e.g. an empty struct `{}` whose sub-field - // defaults live in field metadata); that matches the current Java model's gap and is - // left as a follow-up. + // Nested defaults are not supported yet. if (!field.type()->is_primitive()) { return InvalidSchema( "Invalid {} value for {}: default values are only supported for primitive types", kind, field.name()); } - // Defaults are stored verbatim (no implicit cast), so a default whose literal type does - // not match the field type is invalid. + // Stored defaults must already match the field type. if (*value.type() != *field.type()) { return InvalidSchema("{} of field {} has type {} but expected {}", kind, field.name(), *value.type(), *field.type()); diff --git a/src/iceberg/schema_field.h b/src/iceberg/schema_field.h index 8066a6406..86bfb92b6 100644 --- a/src/iceberg/schema_field.h +++ b/src/iceberg/schema_field.h @@ -46,10 +46,8 @@ class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable { /// \param[in] type The field type. /// \param[in] optional Whether values of this field are required or nullable. /// \param[in] doc Optional documentation string for the field. - /// \param[in] initial_default The v3 `initial-default` value, or null if absent. The - /// field shares ownership of the (immutable) value. - /// \param[in] write_default The v3 `write-default` value, or null if absent. The field - /// shares ownership of the (immutable) value. + /// \param[in] initial_default The v3 `initial-default`, or null if absent. + /// \param[in] write_default The v3 `write-default`, or null if absent. SchemaField(int32_t field_id, std::string_view name, std::shared_ptr type, bool optional, std::string_view doc = {}, std::shared_ptr initial_default = nullptr, @@ -62,14 +60,33 @@ class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable { static SchemaField MakeRequired(int32_t field_id, std::string_view name, std::shared_ptr type, std::string_view doc = {}); + /// \brief Return a copy with a new name. + SchemaField WithName(std::string_view name) const; + + /// \brief Return a copy with a new type. + SchemaField WithType(std::shared_ptr type) const; + + /// \brief Return a copy with new documentation. + SchemaField WithDoc(std::string_view doc) const; + + /// \brief Return a copy with a new `initial-default`. + SchemaField WithInitialDefault(std::shared_ptr initial_default) const; + + /// \brief Return a copy with a new `write-default`. + SchemaField WithWriteDefault(std::shared_ptr write_default) const; + + /// \brief Cast a default literal to a field type. + static Result CastDefaultValue( + const Literal& value, const std::shared_ptr& target_type); + /// \brief Get the field ID. - [[nodiscard]] int32_t field_id() const; + int32_t field_id() const; /// \brief Get the field name. - [[nodiscard]] std::string_view name() const; + std::string_view name() const; /// \brief Get the field type. - [[nodiscard]] const std::shared_ptr& type() const; + const std::shared_ptr& type() const; /// \brief Get whether the field is optional. [[nodiscard]] bool optional() const; @@ -77,15 +94,13 @@ class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable { /// \brief Get the field documentation. std::string_view doc() const; - /// \brief Get the owning pointer to the default value for this field used when reading - /// rows written before the field existed (v3 `initial-default`), or null if absent. + /// \brief Get the v3 `initial-default`, or null if absent. const std::shared_ptr& initial_default() const; - /// \brief Get the owning pointer to the default value for this field used when a writer - /// does not supply a value (v3 `write-default`), or null if absent. + /// \brief Get the v3 `write-default`, or null if absent. const std::shared_ptr& write_default() const; - [[nodiscard]] std::string ToString() const override; + std::string ToString() const override; Status Validate() const; @@ -107,14 +122,13 @@ class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable { private: /// \brief Compare two fields for equality. - [[nodiscard]] bool Equals(const SchemaField& other) const; + bool Equals(const SchemaField& other) const; int32_t field_id_; std::string name_; std::shared_ptr type_; bool optional_; std::string doc_; - // Immutable default values, shared (not deep-copied) across field copies, like `type_`. std::shared_ptr initial_default_; std::shared_ptr write_default_; }; diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index ba0f6230d..12460237d 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -59,6 +59,7 @@ iceberg_tests = { 'table_requirements_test.cc', 'table_test.cc', 'table_update_test.cc', + 'update_schema_test.cc', ), }, 'logging_test': { diff --git a/src/iceberg/test/schema_field_test.cc b/src/iceberg/test/schema_field_test.cc index c2cd6a758..d757cb0b2 100644 --- a/src/iceberg/test/schema_field_test.cc +++ b/src/iceberg/test/schema_field_test.cc @@ -24,6 +24,7 @@ #include +#include "iceberg/expression/literal.h" #include "iceberg/type.h" #include "iceberg/util/formatter.h" // IWYU pragma: keep @@ -107,4 +108,66 @@ TEST(SchemaFieldTest, WithDoc) { } } +TEST(SchemaFieldTest, WithFieldMetadata) { + auto initial_default = std::make_shared(Literal::Int(1)); + auto write_default = std::make_shared(Literal::Int(2)); + SchemaField field(1, "foo", int32(), false, "doc", initial_default, write_default); + + auto renamed = field.WithName("bar"); + EXPECT_EQ(renamed.name(), "bar"); + EXPECT_EQ(renamed.field_id(), field.field_id()); + EXPECT_EQ(renamed.type(), field.type()); + EXPECT_EQ(renamed.doc(), field.doc()); + EXPECT_EQ(renamed.initial_default(), field.initial_default()); + EXPECT_EQ(renamed.write_default(), field.write_default()); + + auto retyped = field.WithType(int64()); + EXPECT_EQ(retyped.type(), int64()); + EXPECT_EQ(retyped.name(), field.name()); + EXPECT_EQ(retyped.initial_default(), field.initial_default()); + EXPECT_EQ(retyped.write_default(), field.write_default()); + + auto documented = field.WithDoc("new doc"); + EXPECT_EQ(documented.doc(), "new doc"); + EXPECT_EQ(documented.name(), field.name()); + EXPECT_EQ(documented.initial_default(), field.initial_default()); + EXPECT_EQ(documented.write_default(), field.write_default()); + + auto new_initial = std::make_shared(Literal::Int(3)); + auto with_initial = field.WithInitialDefault(new_initial); + EXPECT_EQ(with_initial.initial_default(), new_initial); + EXPECT_EQ(with_initial.write_default(), field.write_default()); + + auto new_write = std::make_shared(Literal::Int(4)); + auto with_write = field.WithWriteDefault(new_write); + EXPECT_EQ(with_write.initial_default(), field.initial_default()); + EXPECT_EQ(with_write.write_default(), new_write); + EXPECT_EQ(field.name(), "foo"); + EXPECT_EQ(*field.write_default(), Literal::Int(2)); +} + +TEST(SchemaFieldTest, CastDefaultValue) { + { + auto result = SchemaField::CastDefaultValue(Literal::Int(5), int64()); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result.value(), Literal::Long(5)); + } + { + auto result = + SchemaField::CastDefaultValue(Literal::Decimal(1234, 9, 2), decimal(18, 2)); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result.value(), Literal::Decimal(1234, 18, 2)); + } + { + auto result = + SchemaField::CastDefaultValue(Literal::Decimal(1234, 9, 3), decimal(18, 2)); + EXPECT_FALSE(result.has_value()); + } + { + auto result = + SchemaField::CastDefaultValue(Literal::Decimal(1234567, 9, 2), decimal(4, 2)); + EXPECT_FALSE(result.has_value()); + } +} + } // namespace iceberg diff --git a/src/iceberg/test/update_schema_test.cc b/src/iceberg/test/update_schema_test.cc index 07872e69a..db6b1d02e 100644 --- a/src/iceberg/test/update_schema_test.cc +++ b/src/iceberg/test/update_schema_test.cc @@ -19,23 +19,88 @@ #include "iceberg/update/update_schema.h" +#include +#include #include +#include +#include +#include #include +#include #include +#include "iceberg/catalog/memory/in_memory_catalog.h" +#include "iceberg/expression/literal.h" #include "iceberg/schema.h" #include "iceberg/schema_field.h" +#include "iceberg/table.h" +#include "iceberg/table_identifier.h" +#include "iceberg/table_metadata.h" #include "iceberg/test/matchers.h" -#include "iceberg/test/update_test_base.h" +#include "iceberg/test/mock_io.h" +#include "iceberg/test/test_resource.h" #include "iceberg/type.h" #include "iceberg/util/checked_cast.h" +#include "iceberg/util/uuid.h" namespace iceberg { using internal::checked_cast; -class UpdateSchemaTest : public UpdateTestBase {}; +class UpdateSchemaTest : public ::testing::Test { + protected: + virtual std::string MetadataResource() const { return "TableMetadataV2Valid.json"; } + virtual std::string TableName() const { return "test_table"; } + + void SetUp() override { + table_ident_ = TableIdentifier{.name = TableName()}; + table_location_ = "/warehouse/" + TableName(); + + InitializeFileIO(); + RegisterTableFromResource(MetadataResource()); + } + + void InitializeFileIO() { + auto mock_file_io = std::make_shared<::testing::NiceMock>(); + auto* mock_file_io_ptr = mock_file_io.get(); + ON_CALL(*mock_file_io, ReadFile(::testing::_, ::testing::_)) + .WillByDefault([mock_file_io_ptr](const std::string& file_location, + std::optional length) { + return mock_file_io_ptr->FileIO::ReadFile(file_location, length); + }); + ON_CALL(*mock_file_io, WriteFile(::testing::_, ::testing::_)) + .WillByDefault([mock_file_io_ptr](const std::string& file_location, + std::string_view content) { + return mock_file_io_ptr->FileIO::WriteFile(file_location, content); + }); + file_io_ = std::move(mock_file_io); + catalog_ = + InMemoryCatalog::Make("test_catalog", file_io_, "/warehouse/", /*properties=*/{}); + } + + void RegisterTable(std::unique_ptr metadata) { + auto metadata_location = std::format("{}/metadata/00001-{}.metadata.json", + table_location_, Uuid::GenerateV7().ToString()); + metadata->location = table_location_; + ASSERT_THAT(TableMetadataUtil::Write(*file_io_, metadata_location, *metadata), + IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(table_, + catalog_->RegisterTable(table_ident_, metadata_location)); + } + + void RegisterTableFromResource(const std::string& resource_name) { + ICEBERG_UNWRAP_OR_FAIL(auto metadata, ReadTableMetadataFromResource(resource_name)); + RegisterTable(std::move(metadata)); + } + + TableIdentifier table_ident_; + std::string table_location_; + std::shared_ptr file_io_; + std::shared_ptr catalog_; + std::shared_ptr table_; +}; TEST_F(UpdateSchemaTest, AddOptionalColumn) { ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); @@ -82,6 +147,347 @@ TEST_F(UpdateSchemaTest, AddRequiredColumnWithAllowIncompatible) { EXPECT_EQ(new_field.doc(), "A required string column"); } +/// Uses v3 metadata for default-value tests. +class UpdateSchemaDefaultValueTest : public UpdateSchemaTest { + protected: + void SetUp() override { + table_ident_ = TableIdentifier{.name = TableName()}; + table_location_ = "/warehouse/" + TableName(); + + InitializeFileIO(); + + ICEBERG_UNWRAP_OR_FAIL(auto metadata, + ReadTableMetadataFromResource("TableMetadataV2Valid.json")); + metadata->format_version = TableMetadata::kSupportedTableFormatVersion; + metadata->next_row_id = TableMetadata::kInitialRowId; + RegisterTable(std::move(metadata)); + } +}; + +TEST_F(UpdateSchemaTest, AddColumnWithDefaultValueRequiresV3) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidSchema)); + EXPECT_THAT(result, HasErrorMessage("is not supported until v3")); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithDefaultValue) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto new_field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(new_field_opt.has_value()); + + const auto& new_field = new_field_opt->get(); + ASSERT_NE(new_field.initial_default(), nullptr); + EXPECT_EQ(*new_field.initial_default(), Literal::Int(42)); + ASSERT_NE(new_field.write_default(), nullptr); + EXPECT_EQ(*new_field.write_default(), Literal::Int(42)); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddRequiredColumnWithDefaultValue) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddRequiredColumn("required_col", string(), "A required string column", + Literal::String("n/a")); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto new_field_opt, + result.schema->FindFieldByName("required_col")); + ASSERT_TRUE(new_field_opt.has_value()); + + const auto& new_field = new_field_opt->get(); + EXPECT_FALSE(new_field.optional()); + ASSERT_NE(new_field.initial_default(), nullptr); + EXPECT_EQ(*new_field.initial_default(), Literal::String("n/a")); + ASSERT_NE(new_field.write_default(), nullptr); + EXPECT_EQ(*new_field.write_default(), Literal::String("n/a")); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithMismatchedDefaultValueFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::String("oops")); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithNarrowingDefaultValueFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", + Literal::Long(std::numeric_limits::max())); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefault) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)) + .UpdateColumnDefault("new_col", Literal::Int(7)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto new_field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(new_field_opt.has_value()); + + const auto& new_field = new_field_opt->get(); + ASSERT_NE(new_field.initial_default(), nullptr); + EXPECT_EQ(*new_field.initial_default(), Literal::Int(42)); + ASSERT_NE(new_field.write_default(), nullptr); + EXPECT_EQ(*new_field.write_default(), Literal::Int(7)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultOnExistingColumn) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->UpdateColumnDefault("x", Literal::Long(0)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("x")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + EXPECT_EQ(field.initial_default(), nullptr); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Long(0)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultClearsWithNullopt) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)) + .UpdateColumnDefault("new_col", std::nullopt); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + ASSERT_NE(field.initial_default(), nullptr); + EXPECT_EQ(*field.initial_default(), Literal::Int(42)); + EXPECT_EQ(field.write_default(), nullptr); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddNestedColumnPreservesNestedDefaults) { + auto nested_type = std::make_shared(std::vector{ + SchemaField(/*field_id=*/100, "inner", int32(), /*optional=*/false, /*doc=*/{}, + std::make_shared(Literal::Int(5)), + std::make_shared(Literal::Int(9)))}); + + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("outer", nested_type, "A nested column"); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto outer_opt, result.schema->FindFieldByName("outer")); + ASSERT_TRUE(outer_opt.has_value()); + + const auto& outer_struct = + internal::checked_cast(*outer_opt->get().type()); + ASSERT_EQ(outer_struct.fields().size(), 1); + const SchemaField& inner = outer_struct.fields()[0]; + ASSERT_NE(inner.initial_default(), nullptr); + EXPECT_EQ(*inner.initial_default(), Literal::Int(5)); + ASSERT_NE(inner.write_default(), nullptr); + EXPECT_EQ(*inner.write_default(), Literal::Int(9)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultCastsToColumnType) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->UpdateColumnDefault("x", Literal::Int(5)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("x")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Long(5)); +} + +TEST_F(UpdateSchemaDefaultValueTest, RequireColumnAddedWithDefault) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)) + .RequireColumn("new_col"); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto new_field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(new_field_opt.has_value()); + EXPECT_FALSE(new_field_opt->get().optional()); +} + +TEST_F(UpdateSchemaDefaultValueTest, RequireNestedMapListColumnAddedWithDefault) { + // Map/list paths omit value/element. + auto map_key_struct = std::make_shared( + std::vector{SchemaField(20, "address", string(), false)}); + auto map_value_struct = std::make_shared( + std::vector{SchemaField(12, "lat", float32(), false), + SchemaField(13, "long", float32(), false)}); + auto map_type = + std::make_shared(SchemaField(10, "key", map_key_struct, false), + SchemaField(11, "value", map_value_struct, false)); + auto list_element_struct = std::make_shared(std::vector{ + SchemaField(15, "x", int64(), false), SchemaField(16, "y", int64(), false)}); + auto list_type = + std::make_shared(SchemaField(14, "element", list_element_struct, true)); + + ICEBERG_UNWRAP_OR_FAIL(auto setup_update, table_->NewUpdateSchema()); + setup_update->AddColumn("locations", map_type, "map of address to coordinate") + .AddColumn("points", list_type, "2-D cartesian points"); + EXPECT_THAT(setup_update->Commit(), IsOk()); + + ICEBERG_UNWRAP_OR_FAIL(auto reloaded, catalog_->LoadTable(table_ident_)); + ICEBERG_UNWRAP_OR_FAIL(auto update, reloaded->NewUpdateSchema()); + update->AddColumn("locations", "alt", float32(), "altitude", Literal::Float(0.0f)) + .RequireColumn("locations.alt") + .AddColumn("points", "z", int64(), "z coordinate", Literal::Long(0)) + .RequireColumn("points.z"); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + + ICEBERG_UNWRAP_OR_FAIL(auto locations_opt, result.schema->FindFieldByName("locations")); + ASSERT_TRUE(locations_opt.has_value()); + const auto& map = checked_cast(*locations_opt->get().type()); + const auto& value_struct = checked_cast(*map.value().type()); + ICEBERG_UNWRAP_OR_FAIL(auto alt_opt, value_struct.GetFieldByName("alt")); + ASSERT_TRUE(alt_opt.has_value()); + EXPECT_FALSE(alt_opt->get().optional()); + ASSERT_NE(alt_opt->get().initial_default(), nullptr); + + ICEBERG_UNWRAP_OR_FAIL(auto points_opt, result.schema->FindFieldByName("points")); + ASSERT_TRUE(points_opt.has_value()); + const auto& list = checked_cast(*points_opt->get().type()); + const auto& element_struct = checked_cast(*list.element().type()); + ICEBERG_UNWRAP_OR_FAIL(auto z_opt, element_struct.GetFieldByName("z")); + ASSERT_TRUE(z_opt.has_value()); + EXPECT_FALSE(z_opt->get().optional()); + ASSERT_NE(z_opt->get().initial_default(), nullptr); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDocPreservesDefaultValues) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)) + .UpdateColumnDoc("new_col", "updated doc"); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + EXPECT_EQ(field.doc(), "updated doc"); + ASSERT_NE(field.initial_default(), nullptr); + EXPECT_EQ(*field.initial_default(), Literal::Int(42)); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Int(42)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnTypePromotesDefaultValues) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", int32(), "An integer column", Literal::Int(42)) + .UpdateColumn("new_col", int64()); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + EXPECT_EQ(field.type(), int64()); + ASSERT_NE(field.initial_default(), nullptr); + EXPECT_EQ(*field.initial_default(), Literal::Long(42)); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Long(42)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnTypePromotesDecimalDefault) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update + ->AddColumn("new_col", decimal(9, 2), "A decimal column", + Literal::Decimal(1234, 9, 2)) + .UpdateColumn("new_col", decimal(18, 2)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + EXPECT_EQ(field.type()->ToString(), decimal(18, 2)->ToString()); + ASSERT_NE(field.initial_default(), nullptr); + EXPECT_EQ(*field.initial_default(), Literal::Decimal(1234, 18, 2)); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2)); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithWiderPrecisionDecimalDefault) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(18, 2), "A decimal column", + Literal::Decimal(1234, 9, 2)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + ASSERT_NE(field.initial_default(), nullptr); + EXPECT_EQ(*field.initial_default(), Literal::Decimal(1234, 18, 2)); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2)); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultWiderPrecisionDecimal) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(18, 2), "A decimal column") + .UpdateColumnDefault("new_col", Literal::Decimal(1234, 9, 2)); + + ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply()); + ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col")); + ASSERT_TRUE(field_opt.has_value()); + + const auto& field = field_opt->get(); + ASSERT_NE(field.write_default(), nullptr); + EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2)); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithDifferentScaleDecimalDefaultFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(18, 2), "A decimal column", + Literal::Decimal(1234, 9, 3)); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + +TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultDifferentScaleDecimalFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(18, 2), "A decimal column") + .UpdateColumnDefault("new_col", Literal::Decimal(1234, 9, 3)); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithOutOfPrecisionDecimalDefaultFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(4, 2), "A decimal column", + Literal::Decimal(1234567, 9, 2)); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + +TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithTypedNullDecimalDefaultFails) { + ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); + update->AddColumn("new_col", decimal(18, 2), "A decimal column", + Literal::Null(decimal(18, 2))); + + auto result = update->Apply(); + EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed)); + EXPECT_THAT(result, HasErrorMessage("Cannot cast default value")); +} + TEST_F(UpdateSchemaTest, AddMultipleColumns) { ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema()); update->AddColumn("col1", int32(), "First column") diff --git a/src/iceberg/type_fwd.h b/src/iceberg/type_fwd.h index 35a493b48..91e18b24c 100644 --- a/src/iceberg/type_fwd.h +++ b/src/iceberg/type_fwd.h @@ -104,6 +104,7 @@ class GeometryType; /// \brief Data values. class Decimal; +class Literal; class Uuid; /// \brief Schema. diff --git a/src/iceberg/update/update_schema.cc b/src/iceberg/update/update_schema.cc index 5c50ee41d..56e167e10 100644 --- a/src/iceberg/update/update_schema.cc +++ b/src/iceberg/update/update_schema.cc @@ -29,6 +29,7 @@ #include #include +#include "iceberg/expression/literal.h" #include "iceberg/json_serde_internal.h" #include "iceberg/name_mapping.h" #include "iceberg/schema.h" @@ -39,6 +40,7 @@ #include "iceberg/type.h" #include "iceberg/util/checked_cast.h" #include "iceberg/util/error_collector.h" +#include "iceberg/util/formatter.h" // IWYU pragma: keep #include "iceberg/util/macros.h" #include "iceberg/util/string_util.h" #include "iceberg/util/type_util.h" @@ -214,11 +216,9 @@ class ApplyChangesVisitor { if (update_it != updates_.end()) { const auto& update_field = update_it->second; - return SchemaField(field_id, update_field->name(), std::move(result_type), - update_field->optional(), update_field->doc()); + return update_field->WithType(std::move(result_type)); } else if (result_type != field.type()) { - return SchemaField(field_id, field.name(), std::move(result_type), field.optional(), - field.doc()); + return field.WithType(std::move(result_type)); } else { return field; } @@ -282,6 +282,10 @@ std::vector ApplyChangesVisitor::MoveFields( return reordered; } +bool ValidateDefaultLiteral(const Literal& value) { + return !value.IsNull() && !value.IsAboveMax() && !value.IsBelowMin(); +} + } // namespace Result> UpdateSchema::Make( @@ -345,39 +349,43 @@ UpdateSchema& UpdateSchema::CaseSensitive(bool case_sensitive) { } UpdateSchema& UpdateSchema::AddColumn(std::string_view name, std::shared_ptr type, - std::string_view doc) { + std::string_view doc, + std::optional default_value) { ICEBERG_BUILDER_CHECK(!name.contains('.'), "Cannot add column with ambiguous name: {}, use " "AddColumn(parent, name, type, doc)", name); - return AddColumnInternal(std::nullopt, name, /*is_optional=*/true, std::move(type), - doc); + return AddColumnInternal(/*parent=*/std::nullopt, name, /*is_optional=*/true, + std::move(type), doc, std::move(default_value)); } UpdateSchema& UpdateSchema::AddColumn(std::optional parent, std::string_view name, std::shared_ptr type, - std::string_view doc) { + std::string_view doc, + std::optional default_value) { return AddColumnInternal(std::move(parent), name, /*is_optional=*/true, std::move(type), - doc); + doc, std::move(default_value)); } UpdateSchema& UpdateSchema::AddRequiredColumn(std::string_view name, std::shared_ptr type, - std::string_view doc) { + std::string_view doc, + std::optional default_value) { ICEBERG_BUILDER_CHECK(!name.contains('.'), "Cannot add column with ambiguous name: {}, use " "AddRequiredColumn(parent, name, type, doc)", name); return AddColumnInternal(std::nullopt, name, /*is_optional=*/false, std::move(type), - doc); + doc, std::move(default_value)); } UpdateSchema& UpdateSchema::AddRequiredColumn(std::optional parent, std::string_view name, std::shared_ptr type, - std::string_view doc) { + std::string_view doc, + std::optional default_value) { return AddColumnInternal(std::move(parent), name, /*is_optional=*/false, - std::move(type), doc); + std::move(type), doc, std::move(default_value)); } UpdateSchema& UpdateSchema::UpdateColumn(std::string_view name, @@ -399,8 +407,25 @@ UpdateSchema& UpdateSchema::UpdateColumn(std::string_view name, "Cannot change column type: {}: {} -> {}", name, field.type()->ToString(), new_type->ToString()); - updates_[field_id] = std::make_shared( - field.field_id(), field.name(), new_type, field.optional(), field.doc()); + // Defaults must follow the promoted type. + auto promote_default = [&](const std::shared_ptr& value) + -> Result> { + if (value == nullptr) { + return nullptr; + } + ICEBERG_ASSIGN_OR_RAISE(Literal promoted, + SchemaField::CastDefaultValue(*value, new_type)); + return std::make_shared(std::move(promoted)); + }; + ICEBERG_BUILDER_ASSIGN_OR_RETURN(std::shared_ptr initial_default, + promote_default(field.initial_default())); + ICEBERG_BUILDER_ASSIGN_OR_RETURN(std::shared_ptr write_default, + promote_default(field.write_default())); + + updates_[field_id] = + std::make_shared(field.WithType(new_type) + .WithInitialDefault(std::move(initial_default)) + .WithWriteDefault(std::move(write_default))); return *this; } @@ -420,9 +445,44 @@ UpdateSchema& UpdateSchema::UpdateColumnDoc(std::string_view name, return *this; } - updates_[field_id] = - std::make_shared(field.field_id(), field.name(), field.type(), - field.optional(), std::string(new_doc)); + updates_[field_id] = std::make_shared(field.WithDoc(new_doc)); + + return *this; +} + +UpdateSchema& UpdateSchema::UpdateColumnDefault(std::string_view name, + std::optional new_default) { + ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto field_opt, FindFieldForUpdate(name)); + ICEBERG_BUILDER_CHECK(field_opt.has_value(), "Cannot update missing column: {}", name); + + const auto& field = field_opt->get(); + int32_t field_id = field.field_id(); + + ICEBERG_BUILDER_CHECK(!deletes_.contains(field_id), + "Cannot update a column that will be deleted: {}", field.name()); + + if (!new_default.has_value()) { + updates_[field_id] = std::make_shared(field.WithWriteDefault(nullptr)); + return *this; + } + + ICEBERG_BUILDER_CHECK(field.type()->is_primitive(), + "Invalid default value for {}: {} (must be null)", *field.type(), + *new_default); + ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR( + Literal typed_default, + SchemaField::CastDefaultValue( + *new_default, internal::checked_pointer_cast(field.type())), + "Cannot cast default value to {}: {}", *field.type(), *new_default); + ICEBERG_BUILDER_CHECK(ValidateDefaultLiteral(typed_default), + "Cannot cast default value to {}: {}", *field.type(), + *new_default); + if (field.write_default() != nullptr && *field.write_default() == typed_default) { + return *this; + } + + updates_[field_id] = std::make_shared( + field.WithWriteDefault(std::make_shared(std::move(typed_default)))); return *this; } @@ -443,9 +503,7 @@ UpdateSchema& UpdateSchema::RenameColumn(std::string_view name, const SchemaField& base_field = update_it != updates_.end() ? *update_it->second : field; - updates_[field_id] = std::make_shared( - base_field.field_id(), std::string(new_name), base_field.type(), - base_field.optional(), base_field.doc()); + updates_[field_id] = std::make_shared(base_field.WithName(new_name)); auto it = std::ranges::find(identifier_field_names_, name); if (it != identifier_field_names_.end()) { @@ -474,9 +532,9 @@ UpdateSchema& UpdateSchema::UpdateColumnRequirementInternal(std::string_view nam return *this; } - // TODO(GuotaoYu): support added column with default value - // bool is_defaulted_add = IsAdded(name) && field.initial_default() != null; - bool is_defaulted_add = false; + // Defaulted adds can become required. + bool is_defaulted_add = added_name_to_id_.contains(CaseSensitivityAwareName(name)) && + field.initial_default() != nullptr; ICEBERG_BUILDER_CHECK(is_optional || is_defaulted_add || allow_incompatible_changes_, "Cannot change column nullability: {}: optional -> required", @@ -625,13 +683,15 @@ Result UpdateSchema::Apply() { .updated_props = std::move(updated_props)}; } -// TODO(Guotao Yu): v3 default value is not yet supported UpdateSchema& UpdateSchema::AddColumnInternal(std::optional parent, std::string_view name, bool is_optional, std::shared_ptr type, - std::string_view doc) { + std::string_view doc, + std::optional default_value) { int32_t parent_id = kTableRootId; std::string full_name; + // For map/list adds, this omits synthetic value/element path segments. + std::string short_name; if (parent.has_value()) { ICEBERG_BUILDER_CHECK(!parent->empty(), "Parent name cannot be empty"); @@ -661,13 +721,13 @@ UpdateSchema& UpdateSchema::AddColumnInternal(std::optional pa ICEBERG_BUILDER_CHECK(!deletes_.contains(parent_id), "Cannot add to a column that will be deleted: {}", *parent); - auto current_name = std::format("{}.{}", *parent, name); - ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto current_field, FindField(current_name)); + short_name = std::format("{}.{}", *parent, name); + ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto current_field, FindField(short_name)); ICEBERG_BUILDER_CHECK( !current_field.has_value() || deletes_.contains(current_field->get().field_id()), "Cannot add column, name already exists: {}.{}", *parent, name); - // Build full name using canonical name of parent + // Use the canonical parent name for schema updates. ICEBERG_BUILDER_ASSIGN_OR_RETURN(auto parent_name_opt, schema_->FindColumnNameById(parent_id)); ICEBERG_BUILDER_CHECK(parent_name_opt.has_value(), @@ -680,16 +740,19 @@ UpdateSchema& UpdateSchema::AddColumnInternal(std::optional pa "Cannot add column, name already exists: {}", name); full_name = std::string(name); + short_name = full_name; } ICEBERG_BUILDER_CHECK( - is_optional || allow_incompatible_changes_, + is_optional || default_value.has_value() || allow_incompatible_changes_, "Incompatible change: cannot add required column without a default value: {}", full_name); int32_t new_id = AssignNewColumnId(); added_name_to_id_[CaseSensitivityAwareName(full_name)] = new_id; + // Needed for same-update lookups by user-facing path. + added_name_to_id_[CaseSensitivityAwareName(short_name)] = new_id; if (parent_id != kTableRootId) { id_to_parent_[new_id] = parent_id; } @@ -697,11 +760,30 @@ UpdateSchema& UpdateSchema::AddColumnInternal(std::optional pa AssignFreshIdVisitor id_assigner([this]() { return AssignNewColumnId(); }); auto type_with_fresh_ids = id_assigner.Visit(type); - auto new_field = std::make_shared(new_id, std::string(name), - std::move(type_with_fresh_ids), - is_optional, std::string(doc)); + std::shared_ptr initial_default; + std::shared_ptr write_default; + if (default_value.has_value()) { + ICEBERG_BUILDER_CHECK(type_with_fresh_ids->is_primitive(), + "Invalid default value for {}: {} (must be null)", + *type_with_fresh_ids, *default_value); + ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR( + Literal typed_default, + SchemaField::CastDefaultValue( + *default_value, + internal::checked_pointer_cast(type_with_fresh_ids)), + "Cannot cast default value to {}: {}", *type_with_fresh_ids, *default_value); + ICEBERG_BUILDER_CHECK(ValidateDefaultLiteral(typed_default), + "Cannot cast default value to {}: {}", *type_with_fresh_ids, + *default_value); + // Add-column defaults set both default kinds. + auto shared_default = std::make_shared(std::move(typed_default)); + initial_default = shared_default; + write_default = std::move(shared_default); + } - updates_[new_id] = std::move(new_field); + updates_[new_id] = std::make_shared( + new_id, name, std::move(type_with_fresh_ids), is_optional, doc, + std::move(initial_default), std::move(write_default)); parent_to_added_ids_[parent_id].push_back(new_id); return *this; diff --git a/src/iceberg/update/update_schema.h b/src/iceberg/update/update_schema.h index 2be3732a0..d0dd944ef 100644 --- a/src/iceberg/update/update_schema.h +++ b/src/iceberg/update/update_schema.h @@ -30,6 +30,7 @@ #include #include +#include "iceberg/expression/literal.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -41,10 +42,6 @@ namespace iceberg { /// /// When committing, these changes will be applied to the current table metadata. /// Commit conflicts will not be resolved and will result in a CommitFailed error. -/// -/// TODO(Guotao Yu): Add support for V3 default values when adding columns. Currently, all -/// added columns use null as the default value, but Iceberg V3 supports custom -/// default values for new columns. class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { public: static Result> Make( @@ -78,15 +75,17 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// If type is a nested type, its field IDs are reassigned when added to the /// existing schema. /// - /// The added column will be optional with a null default value. + /// v3+: `default_value` sets `initial-default` and `write-default`. /// /// \param name Name for the new column. /// \param type Type for the new column. /// \param doc Documentation string for the new column. + /// \param default_value Optional default value for the new column (v3+). /// \return Reference to this for method chaining. /// \note InvalidArgument will be reported if name contains ".". UpdateSchema& AddColumn(std::string_view name, std::shared_ptr type, - std::string_view doc = ""); + std::string_view doc = "", + std::optional default_value = std::nullopt); /// \brief Add a new optional column to a nested struct with documentation. /// @@ -102,22 +101,23 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// If type is a nested type, its field IDs are reassigned when added to the /// existing schema. /// - /// The added column will be optional with a null default value. + /// v3+: `default_value` sets `initial-default` and `write-default`. /// /// \param parent Name of the parent struct to which the column will be added. /// \param name Name for the new column. /// \param type Type for the new column. /// \param doc Documentation string for the new column. + /// \param default_value Optional default value for the new column (v3+). /// \return Reference to this for method chaining. /// \note InvalidArgument will be reported if parent doesn't identify a struct. UpdateSchema& AddColumn(std::optional parent, std::string_view name, - std::shared_ptr type, std::string_view doc = ""); + std::shared_ptr type, std::string_view doc = "", + std::optional default_value = std::nullopt); /// \brief Add a new required top-level column with documentation. /// - /// Adding a required column without a default is an incompatible change that can - /// break reading older data. To suppress exceptions thrown when an incompatible - /// change is detected, call AllowIncompatibleChanges(). + /// Required adds need AllowIncompatibleChanges() unless `default_value` is set. + /// v3+: `default_value` sets `initial-default` and `write-default`. /// /// Because "." may be interpreted as a column path separator or may be used in /// field names, it is not allowed in names passed to this method. To add to nested @@ -130,16 +130,17 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// \param name Name for the new column. /// \param type Type for the new column. /// \param doc Documentation string for the new column. + /// \param default_value Optional default value for the new column (v3+). /// \return Reference to this for method chaining. /// \note InvalidArgument will be reported if name contains ".". UpdateSchema& AddRequiredColumn(std::string_view name, std::shared_ptr type, - std::string_view doc = ""); + std::string_view doc = "", + std::optional default_value = std::nullopt); /// \brief Add a new required column to a nested struct with documentation. /// - /// Adding a required column without a default is an incompatible change that can - /// break reading older data. To suppress exceptions thrown when an incompatible - /// change is detected, call AllowIncompatibleChanges(). + /// Required adds need AllowIncompatibleChanges() unless `default_value` is set. + /// v3+: `default_value` sets `initial-default` and `write-default`. /// /// The parent name is used to find the parent using Schema::FindFieldByName(). If /// the parent name is null or empty, the new column will be added to the root as a @@ -157,11 +158,13 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// \param name Name for the new column. /// \param type Type for the new column. /// \param doc Documentation string for the new column. + /// \param default_value Optional default value for the new column (v3+). /// \return Reference to this for method chaining. /// \note InvalidArgument will be reported if parent doesn't identify a struct. UpdateSchema& AddRequiredColumn(std::optional parent, std::string_view name, std::shared_ptr type, - std::string_view doc = ""); + std::string_view doc = "", + std::optional default_value = std::nullopt); /// \brief Rename a column in the schema. /// @@ -210,6 +213,20 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// the column will be deleted. UpdateSchema& UpdateColumnDoc(std::string_view name, std::string_view new_doc); + /// \brief Update the `write-default` value for a column (v3+). + /// + /// The name is used to find the column to update using Schema::FindFieldByName(). + /// Only `write-default` changes; `initial-default` is fixed. + /// + /// \param name Name of the column to update the default value for. + /// \param new_default Replacement `write-default` value for the column, or + /// `std::nullopt` to clear it. + /// \return Reference to this for method chaining. + /// \note InvalidArgument will be reported if name doesn't identify a column in the + /// schema or if the column will be deleted. + UpdateSchema& UpdateColumnDefault(std::string_view name, + std::optional new_default); + /// \brief Update a column to be optional. /// /// \param name Name of the column to mark optional. @@ -364,10 +381,12 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate { /// \param is_optional Whether the column is optional. /// \param type Type for the new column. /// \param doc Optional documentation string. + /// \param default_value Optional default value for the new column (v3+). /// \return Reference to this for method chaining. UpdateSchema& AddColumnInternal(std::optional parent, std::string_view name, bool is_optional, - std::shared_ptr type, std::string_view doc); + std::shared_ptr type, std::string_view doc, + std::optional default_value); /// \brief Internal implementation for updating column requirement (optional/required). /// diff --git a/src/iceberg/util/type_util.cc b/src/iceberg/util/type_util.cc index 1f1e02747..6656c5636 100644 --- a/src/iceberg/util/type_util.cc +++ b/src/iceberg/util/type_util.cc @@ -385,9 +385,9 @@ std::shared_ptr AssignFreshIdVisitor::Visit(const StructType& type) std::vector fresh_fields; for (size_t i = 0; i < type.fields().size(); ++i) { const auto& field = type.fields()[i]; - fresh_fields.emplace_back(fresh_ids[i], std::string(field.name()), - Visit(field.type()), field.optional(), - std::string(field.doc())); + fresh_fields.emplace_back( + fresh_ids[i], std::string(field.name()), Visit(field.type()), field.optional(), + std::string(field.doc()), field.initial_default(), field.write_default()); } return std::make_shared(std::move(fresh_fields)); } @@ -397,7 +397,8 @@ std::shared_ptr AssignFreshIdVisitor::Visit(const ListType& type) cons int32_t fresh_id = next_id_(); SchemaField fresh_elem_field(fresh_id, std::string(elem_field.name()), Visit(elem_field.type()), elem_field.optional(), - std::string(elem_field.doc())); + std::string(elem_field.doc()), + elem_field.initial_default(), elem_field.write_default()); return std::make_shared(std::move(fresh_elem_field)); } @@ -410,10 +411,12 @@ std::shared_ptr AssignFreshIdVisitor::Visit(const MapType& type) const SchemaField fresh_key_field(fresh_key_id, std::string(key_field.name()), Visit(key_field.type()), key_field.optional(), - std::string(key_field.doc())); - SchemaField fresh_value_field(fresh_value_id, std::string(value_field.name()), - Visit(value_field.type()), value_field.optional(), - std::string(value_field.doc())); + std::string(key_field.doc()), key_field.initial_default(), + key_field.write_default()); + SchemaField fresh_value_field( + fresh_value_id, std::string(value_field.name()), Visit(value_field.type()), + value_field.optional(), std::string(value_field.doc()), + value_field.initial_default(), value_field.write_default()); return std::make_shared(std::move(fresh_key_field), std::move(fresh_value_field)); } From aa36d3584917deb267a47014b9556f07b8eae47e Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Sat, 11 Jul 2026 17:42:26 +0800 Subject: [PATCH 07/13] ci: add RelWithDebInfo build and fix free-nonheap-object error (#812) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding a RelWithDebInfo build revealed a build error. This PR fixes the issue. ``` error: ‘void operator delete(void*, std::size_t)’ called on a pointer to an unallocated object ‘1’ [-Werror=free-nonheap-object] ``` --- .github/workflows/test.yml | 16 ++++++++++------ ci/scripts/build_iceberg.sh | 16 +++++++++++----- src/iceberg/json_serde.cc | 6 +++--- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b46bb924c..4901e8ea9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,11 +39,15 @@ env: jobs: ubuntu: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} - name: AMD64 Ubuntu 26.04 + name: AMD64 Ubuntu 26.04 (${{ matrix.cmake_build_type }}) runs-on: ubuntu-26.04 - timeout-minutes: 30 + timeout-minutes: 60 strategy: fail-fast: false + matrix: + cmake_build_type: + - Debug + - RelWithDebInfo env: SCCACHE_DIR: ${{ github.workspace }}/.sccache SCCACHE_CACHE_SIZE: "2G" @@ -59,9 +63,9 @@ jobs: uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ${{ github.workspace }}/.sccache - key: sccache-test-ubuntu-${{ github.run_id }} + key: sccache-test-ubuntu-${{ matrix.cmake_build_type }}-${{ github.run_id }} restore-keys: | - sccache-test-ubuntu- + sccache-test-ubuntu-${{ matrix.cmake_build_type }}- - name: Setup sccache uses: mozilla-actions/sccache-action@9e7fa8a12102821edf02ca5dbea1acd0f89a2696 # v0.0.10 - name: Build Iceberg @@ -69,7 +73,7 @@ jobs: env: CC: gcc-14 CXX: g++-14 - run: ci/scripts/build_iceberg.sh $(pwd) ON ON + run: ci/scripts/build_iceberg.sh $(pwd) ON ON OFF OFF ON ${{ matrix.cmake_build_type }} - name: Show sccache stats shell: bash run: sccache --show-stats @@ -78,7 +82,7 @@ jobs: uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ${{ github.workspace }}/.sccache - key: sccache-test-ubuntu-${{ github.run_id }} + key: sccache-test-ubuntu-${{ matrix.cmake_build_type }}-${{ github.run_id }} - name: Build Example shell: bash env: diff --git a/ci/scripts/build_iceberg.sh b/ci/scripts/build_iceberg.sh index 6af0802f6..e00b6ca24 100755 --- a/ci/scripts/build_iceberg.sh +++ b/ci/scripts/build_iceberg.sh @@ -17,7 +17,7 @@ # specific language governing permissions and limitations # under the License. # -# Usage: build_iceberg.sh [rest_integration_tests=OFF] [sccache=OFF] [s3=OFF] [sigv4=OFF] [bundle_awssdk=ON] +# Usage: build_iceberg.sh [rest_integration_tests=OFF] [sccache=OFF] [s3=OFF] [sigv4=OFF] [bundle_awssdk=ON] [build_type=Debug] set -eux @@ -37,6 +37,12 @@ is_windows() { [[ "${OSTYPE}" == "msys" || "${OSTYPE}" == "win32" || "${OSTYPE}" == "cygwin" ]] } +if is_windows; then + build_type=${7:-Release} +else + build_type=${7:-Debug} +fi + CMAKE_ARGS=( "-G Ninja" "-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ICEBERG_HOME}}" @@ -65,13 +71,13 @@ fi if is_windows; then CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake") - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Release") + CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") else # Pass an externally provided toolchain (e.g. vcpkg for the SigV4 job) if [[ -n "${CMAKE_TOOLCHAIN_FILE:-}" ]]; then CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}") fi - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Debug") + CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") fi if [[ "${build_enable_sccache}" == "ON" ]]; then @@ -86,9 +92,9 @@ fi cmake "${CMAKE_ARGS[@]}" ${source_dir} if is_windows; then - cmake --build . --config Release --target install + cmake --build . --config "${build_type}" --target install if [[ "${run_tests}" == "ON" ]]; then - ctest --output-on-failure -C Release + ctest --output-on-failure -C "${build_type}" fi else cmake --build . --target install diff --git a/src/iceberg/json_serde.cc b/src/iceberg/json_serde.cc index 297265435..f322996ec 100644 --- a/src/iceberg/json_serde.cc +++ b/src/iceberg/json_serde.cc @@ -1377,9 +1377,9 @@ Result> TableMetadataFromJson(const nlohmann::jso TimePointMs{std::chrono::milliseconds(last_updated_ms)}; if (json.contains(kRefs)) { - ICEBERG_ASSIGN_OR_RAISE( - table_metadata->refs, - FromJsonMap>(json, kRefs, SnapshotRefFromJson)); + ICEBERG_ASSIGN_OR_RAISE(auto refs, FromJsonMap>( + json, kRefs, SnapshotRefFromJson)); + table_metadata->refs = std::move(refs); } else if (table_metadata->current_snapshot_id != kInvalidSnapshotId) { table_metadata->refs["main"] = std::make_unique(SnapshotRef{ .snapshot_id = table_metadata->current_snapshot_id, From 755799d1ba4ae3d63ffa01cb7152cc796ed3ef52 Mon Sep 17 00:00:00 2001 From: Junwang Zhao Date: Sun, 12 Jul 2026 00:04:02 +0800 Subject: [PATCH 08/13] refactor: use decimal high and low for Arrow scalar (#803) --- src/iceberg/arrow/literal_util.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/iceberg/arrow/literal_util.cc b/src/iceberg/arrow/literal_util.cc index c13338199..e8ee4d6c6 100644 --- a/src/iceberg/arrow/literal_util.cc +++ b/src/iceberg/arrow/literal_util.cc @@ -123,9 +123,7 @@ Result> ToArrowScalar(const Literal& literal, return std::make_shared<::arrow::DoubleScalar>(std::get(value)); case TypeId::kDecimal: { const auto& decimal = std::get(value); - ::arrow::Decimal128 arrow_decimal( - static_cast(decimal.value() >> 64), - static_cast(decimal.value() & ~uint64_t{0})); + ::arrow::Decimal128 arrow_decimal(decimal.high(), decimal.low()); return std::make_shared<::arrow::Decimal128Scalar>(arrow_decimal, std::move(arrow_type)); } From 263795e9288fe4421c9248888ddd16aba677b8a4 Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Sat, 11 Jul 2026 18:39:30 -0700 Subject: [PATCH 09/13] build: add benchmark build scaffolding (#825) ## Summary Adds an opt-in benchmark build path so future C++ microbenchmarks can live in-tree without affecting normal builds. Benchmarks stay disabled by default, and this PR keeps the executable intentionally small so reviewers can focus on CMake/Meson wiring and Google Benchmark integration. - Adds `ICEBERG_BUILD_BENCHMARKS` for CMake and a disabled-by-default `benchmarks` Meson feature. - Wires `src/iceberg/benchmark` into both build systems. - Adds Google Benchmark dependency setup for CMake and Meson. - Adds a tiny smoke benchmark so the new target is buildable in isolation. Part of #690. ## Stack This is PR 1 of 2. PR 2 adds the actual metrics evaluator benchmark cases on top of this scaffolding: timothyw553/iceberg-cpp#1. ## Test Plan - [x] Configured benchmark-enabled CMake build with `-DICEBERG_BUILD_BENCHMARKS=ON`. - [x] Built `metrics_evaluator_benchmark` on this branch. - [x] Ran the smoke benchmark with `--benchmark_min_time=0.01s`. - [x] Re-ran the default build and test suite on the top of the stack: 18/18 tests passed. - [ ] Meson wiring is syntax-reviewed but not locally executed because `meson` is not installed in this shell. ## Verification Commands ```bash cmake -S . -B build-bench-stack -G Ninja \ -DFETCHCONTENT_TRY_FIND_PACKAGE_MODE=NEVER \ -DCMAKE_IGNORE_PREFIX_PATH=/opt/homebrew/anaconda3 \ -DCMAKE_PREFIX_PATH='/opt/homebrew/opt/snappy;/opt/homebrew/opt/zstd' \ -DICEBERG_BUILD_BENCHMARKS=ON cmake --build build-bench-stack --target metrics_evaluator_benchmark build-bench-stack/src/iceberg/benchmark/metrics_evaluator_benchmark --benchmark_min_time=0.01s cmake --build build ctest --test-dir build --output-on-failure ``` --- .github/workflows/cpp-linter.yml | 1 + CMakeLists.txt | 1 + meson.options | 7 ++++ src/iceberg/CMakeLists.txt | 4 +++ src/iceberg/benchmark/CMakeLists.txt | 41 +++++++++++++++++++++ src/iceberg/benchmark/benchmark_smoke.cc | 46 ++++++++++++++++++++++++ src/iceberg/benchmark/meson.build | 26 ++++++++++++++ src/iceberg/meson.build | 4 +++ subprojects/google-benchmark.wrap | 30 ++++++++++++++++ 9 files changed, 160 insertions(+) create mode 100644 src/iceberg/benchmark/CMakeLists.txt create mode 100644 src/iceberg/benchmark/benchmark_smoke.cc create mode 100644 src/iceberg/benchmark/meson.build create mode 100644 subprojects/google-benchmark.wrap diff --git a/.github/workflows/cpp-linter.yml b/.github/workflows/cpp-linter.yml index 3dcf7ba4f..44dd6e350 100644 --- a/.github/workflows/cpp-linter.yml +++ b/.github/workflows/cpp-linter.yml @@ -77,6 +77,7 @@ jobs: -DCMAKE_C_COMPILER_LAUNCHER=sccache \ -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \ -DICEBERG_BUILD_SQL_CATALOG=ON \ + -DICEBERG_BUILD_BENCHMARKS=ON \ -DICEBERG_SQL_SQLITE=ON \ -DICEBERG_SQL_POSTGRESQL=ON \ -DICEBERG_SQL_MYSQL=ON diff --git a/CMakeLists.txt b/CMakeLists.txt index 5f8823567..db54b24ed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,6 +40,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON) option(ICEBERG_BUILD_STATIC "Build static library" ON) option(ICEBERG_BUILD_SHARED "Build shared library" OFF) option(ICEBERG_BUILD_TESTS "Build tests" ON) +option(ICEBERG_BUILD_BENCHMARKS "Build benchmarks" OFF) option(ICEBERG_BUILD_BUNDLE "Build the battery included library" ON) option(ICEBERG_BUILD_REST "Build rest catalog client" ON) option(ICEBERG_BUILD_REST_INTEGRATION_TESTS "Build rest catalog integration tests" OFF) diff --git a/meson.options b/meson.options index c53574889..0448c61df 100644 --- a/meson.options +++ b/meson.options @@ -52,3 +52,10 @@ option( ) option('tests', type: 'feature', description: 'Build tests', value: 'enabled') + +option( + 'benchmarks', + type: 'feature', + description: 'Build benchmarks', + value: 'disabled', +) diff --git a/src/iceberg/CMakeLists.txt b/src/iceberg/CMakeLists.txt index da820c26b..049a61256 100644 --- a/src/iceberg/CMakeLists.txt +++ b/src/iceberg/CMakeLists.txt @@ -365,3 +365,7 @@ endif() if(ICEBERG_BUILD_TESTS) add_subdirectory(test) endif() + +if(ICEBERG_BUILD_BENCHMARKS) + add_subdirectory(benchmark) +endif() diff --git a/src/iceberg/benchmark/CMakeLists.txt b/src/iceberg/benchmark/CMakeLists.txt new file mode 100644 index 000000000..0227c3871 --- /dev/null +++ b/src/iceberg/benchmark/CMakeLists.txt @@ -0,0 +1,41 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +set(BENCHMARK_ENABLE_GTEST_TESTS + OFF + CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_INSTALL + OFF + CACHE BOOL "" FORCE) +set(BENCHMARK_ENABLE_TESTING + OFF + CACHE BOOL "" FORCE) + +fetchcontent_declare(google_benchmark + GIT_REPOSITORY https://github.com/google/benchmark.git + GIT_TAG a4cf155615c63e019ae549e31703bf367df5b471 # v1.8.4 + FIND_PACKAGE_ARGS + NAMES + benchmark + CONFIG) +fetchcontent_makeavailable(google_benchmark) + +add_executable(benchmark_smoke benchmark_smoke.cc) +target_link_libraries(benchmark_smoke + PRIVATE "$,iceberg_static,iceberg_shared>" + "$,benchmark::benchmark,benchmark>" +) diff --git a/src/iceberg/benchmark/benchmark_smoke.cc b/src/iceberg/benchmark/benchmark_smoke.cc new file mode 100644 index 000000000..abcc35550 --- /dev/null +++ b/src/iceberg/benchmark/benchmark_smoke.cc @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +#include + +namespace iceberg { +namespace { + +void BM_BenchmarkSmoke(benchmark::State& state) { + while (state.KeepRunning()) { + benchmark::DoNotOptimize(state.iterations()); + } + + state.SetItemsProcessed(state.iterations()); +} + +BENCHMARK(BM_BenchmarkSmoke); + +} // namespace +} // namespace iceberg + +int main(int argc, char** argv) { + benchmark::Initialize(&argc, argv); + if (benchmark::ReportUnrecognizedArguments(argc, argv)) { + return 1; + } + benchmark::RunSpecifiedBenchmarks(); + benchmark::Shutdown(); + return 0; +} diff --git a/src/iceberg/benchmark/meson.build b/src/iceberg/benchmark/meson.build new file mode 100644 index 000000000..affc34e07 --- /dev/null +++ b/src/iceberg/benchmark/meson.build @@ -0,0 +1,26 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +benchmark_dep = dependency('benchmark', required: true) + +benchmark_smoke = executable( + 'benchmark_smoke', + sources: files('benchmark_smoke.cc'), + dependencies: [iceberg_dep, benchmark_dep], +) + +benchmark('benchmark_smoke', benchmark_smoke) diff --git a/src/iceberg/meson.build b/src/iceberg/meson.build index e5158a4b9..7340bfa28 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -337,3 +337,7 @@ subdir('inspect') if get_option('tests').enabled() subdir('test') endif + +if get_option('benchmarks').enabled() + subdir('benchmark') +endif diff --git a/subprojects/google-benchmark.wrap b/subprojects/google-benchmark.wrap new file mode 100644 index 000000000..51f4e24d4 --- /dev/null +++ b/subprojects/google-benchmark.wrap @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +[wrap-file] +directory = benchmark-1.8.4 +source_url = https://github.com/google/benchmark/archive/refs/tags/v1.8.4.tar.gz +source_filename = benchmark-1.8.4.tar.gz +source_hash = 3e7059b6b11fb1bbe28e33e02519398ca94c1818874ebed18e504dc6f709be45 +source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/google-benchmark_1.8.4-5/benchmark-1.8.4.tar.gz +patch_filename = google-benchmark_1.8.4-5_patch.zip +patch_url = https://wrapdb.mesonbuild.com/v2/google-benchmark_1.8.4-5/get_patch +patch_hash = 671ffed65f1e95e8c20edb7a06eb54476797e58169160b255f52dc71f4b83957 +wrapdb_version = 1.8.4-5 + +[provide] +dependency_names = benchmark, benchmark_main From 2191844e14f4efd3d6ee9bf5d0db835e218c1ffd Mon Sep 17 00:00:00 2001 From: Anupam Yadav Date: Sat, 11 Jul 2026 18:50:58 -0700 Subject: [PATCH 10/13] build: enable clang-tidy misc-include-cleaner for include hygiene (#820) --- .clang-tidy | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index e7e8abd33..5b13a0551 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -21,6 +21,7 @@ Checks: | clang-analyzer-*, google-*, modernize-*, + misc-include-cleaner, readability-identifier-naming, readability-isolate-declaration, -modernize-use-nodiscard, @@ -41,5 +42,7 @@ CheckOptions: value: '_' - key: modernize-use-scoped-lock.WarnOnSingleLocks value: 'false' + - key: misc-include-cleaner.IgnoreHeaders + value: 'arrow/.*;avro/.*;aws/.*;cpr/.*;gmock/.*;gtest/.*;nanoarrow/.*;nlohmann/.*;parquet/.*;roaring/.*;spdlog/.*;sqlpp23/.*;sqlite3\.h;thrift/.*;utf8proc\.h;zlib\.h' HeaderFilterRegex: 'src/iceberg|example' From 0825f56c4f9dd535487aba292e99c053570f8d8b Mon Sep 17 00:00:00 2001 From: Abanoub Doss Date: Sat, 11 Jul 2026 21:16:48 -0500 Subject: [PATCH 11/13] ci: retry build dir cleanup on Windows (#807) ## What Retry `rm -rf` cleanup in the CMake CI helper scripts. Windows can briefly keep a just-built executable or DLL open, which can make cleanup fail even after the build and tests have passed. `build_example.sh` retries before configuring and still lets `mkdir` fail if the directory remains, so it will not build in a half-deleted tree. `build_iceberg.sh` retries after the build and does not fail an otherwise successful job if final cleanup is still blocked. --------- Co-authored-by: Abanoub Doss --- ci/scripts/build_example.sh | 13 ++++++++++++- ci/scripts/build_iceberg.sh | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/ci/scripts/build_example.sh b/ci/scripts/build_example.sh index 41714cf32..a71649ec3 100755 --- a/ci/scripts/build_example.sh +++ b/ci/scripts/build_example.sh @@ -23,7 +23,18 @@ source_dir=${1} build_dir=${1}/build run_example=${ICEBERG_RUN_EXAMPLE:-OFF} -rm -rf "${build_dir}" +# Clean up before configuring. If Windows still holds a just-built exe/dll +# after the retries, let mkdir fail rather than reuse a half-deleted tree. +for attempt in 1 2 3; do + if rm -rf "${build_dir}"; then + break + fi + if [[ "${attempt}" != "3" ]]; then + sleep 2 + else + echo "Failed to remove build directory after 3 attempts: ${build_dir}" >&2 + fi +done mkdir "${build_dir}" pushd ${build_dir} diff --git a/ci/scripts/build_iceberg.sh b/ci/scripts/build_iceberg.sh index e00b6ca24..55fb4e532 100755 --- a/ci/scripts/build_iceberg.sh +++ b/ci/scripts/build_iceberg.sh @@ -105,5 +105,15 @@ fi popd -# clean up between builds -rm -rf ${build_dir} +# Clean up after the build. Windows can briefly hold a just-built exe/dll, +# so retry but do not fail an otherwise successful CI job. +for attempt in 1 2 3; do + if rm -rf "${build_dir}"; then + break + fi + if [[ "${attempt}" != "3" ]]; then + sleep 2 + else + echo "Failed to remove build directory after 3 attempts: ${build_dir}" >&2 + fi +done From fee18845357aefa570f3aa067061eb204ea33873 Mon Sep 17 00:00:00 2001 From: Abanoub Doss Date: Tue, 14 Jul 2026 00:36:12 -0500 Subject: [PATCH 12/13] ci: build Windows test leg in Debug (#806) ## What Let the CMake CI scripts use `ICEBERG_BUILD_TYPE` and default to `Debug`, including on Windows. MSVC builds with debug info now use embedded `/Z7` debug info through `CMP0141` and `CMAKE_MSVC_DEBUG_INFORMATION_FORMAT` so Debug objects remain cacheable by sccache. The build scripts also use the same single-config Ninja build path on every platform. Since these jobs pass `-G Ninja`, `CMAKE_BUILD_TYPE` controls the build and `--config Release` / `ctest -C Release` were not needed. ## Why Windows was the only main `Test` workflow leg still forcing the helper scripts through `Release`. After this change, the helper-script CMake test jobs default to `Debug` on pull requests and `main`; `ICEBERG_BUILD_TYPE` remains available if the project wants an override. Release verification and the SQL Catalog workflow's explicit Windows `Release` matrix entry are unchanged. The original Windows Debug blocker in #39 was an `IMPORTED_LOCATION` failure from a multi-config CMake path. That specific failure no longer applies because the helper scripts now use Ninja single-config builds. The remaining Debug-only blocker was MSVC rejecting a `constexpr static PartitionValues` test helper: `PartitionValues` owns a `std::vector`, and MSVC Debug's checked-iterator mode (`_ITERATOR_DEBUG_LEVEL != 0`) makes that construction fail constant evaluation. This PR constructs the empty `PartitionValues` at the call site instead. This follows the CI direction discussed in #799. ## Validation Fork validation passed: - `Test / AMD64 Windows 2025` passed in 23m48s on the pull-request run: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474339/job/84929642907 - `SQL Catalog / AMD64 Windows 2025` passed in 4m13s: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474303/job/84929643407 - `Meson / AMD64 Windows 2025` passed in 3m21s: https://github.com/abnobdoss/iceberg-cpp/actions/runs/28638474339/job/84929642897 The Windows Debug test job reported 45% sccache hits, 385 hits, 477 misses, and 0 errors. The first post-merge Apache `main` run will reseed Windows cache entries because the previous entries were built from Release objects. Closes #39. --------- Co-authored-by: Abanoub Doss --- .github/workflows/sql_catalog_test.yml | 2 +- ci/scripts/build_example.sh | 23 ++++++----------- ci/scripts/build_iceberg.sh | 34 +++++++++----------------- cmake_modules/IcebergSccache.cmake | 21 ++++++---------- src/iceberg/test/scan_test_base.h | 3 +-- 5 files changed, 29 insertions(+), 54 deletions(-) diff --git a/.github/workflows/sql_catalog_test.yml b/.github/workflows/sql_catalog_test.yml index 28bd33173..108ff8ae2 100644 --- a/.github/workflows/sql_catalog_test.yml +++ b/.github/workflows/sql_catalog_test.yml @@ -58,7 +58,7 @@ jobs: cmake_extra_args: "" - title: AMD64 Windows 2025 runs-on: windows-2025 - cmake_build_type: Release + cmake_build_type: Debug cmake_extra_args: -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake env: SCCACHE_DIR: ${{ github.workspace }}/.sccache diff --git a/ci/scripts/build_example.sh b/ci/scripts/build_example.sh index a71649ec3..7ccdf4a8f 100755 --- a/ci/scripts/build_example.sh +++ b/ci/scripts/build_example.sh @@ -49,26 +49,19 @@ CMAKE_ARGS=( if is_windows; then CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake") - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Release") -else - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Debug") fi +build_type="${ICEBERG_BUILD_TYPE:-Debug}" +CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") + cmake "${CMAKE_ARGS[@]}" ${source_dir} -if is_windows; then - cmake --build . --config Release - if [[ "${run_example}" == "ON" ]]; then - if [[ -x ./demo_example.exe ]]; then - ./demo_example.exe +cmake --build . +if [[ "${run_example}" == "ON" ]]; then + if is_windows; then + ./demo_example.exe else - ./Release/demo_example.exe + ./demo_example fi - fi -else - cmake --build . - if [[ "${run_example}" == "ON" ]]; then - ./demo_example - fi fi popd diff --git a/ci/scripts/build_iceberg.sh b/ci/scripts/build_iceberg.sh index 55fb4e532..6a7dc607a 100755 --- a/ci/scripts/build_iceberg.sh +++ b/ci/scripts/build_iceberg.sh @@ -37,11 +37,7 @@ is_windows() { [[ "${OSTYPE}" == "msys" || "${OSTYPE}" == "win32" || "${OSTYPE}" == "cygwin" ]] } -if is_windows; then - build_type=${7:-Release} -else - build_type=${7:-Debug} -fi +build_type=${7:-Debug} CMAKE_ARGS=( "-G Ninja" @@ -70,16 +66,16 @@ else fi if is_windows; then - CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake") - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") -else - # Pass an externally provided toolchain (e.g. vcpkg for the SigV4 job) - if [[ -n "${CMAKE_TOOLCHAIN_FILE:-}" ]]; then - CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}") - fi - CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") + CMAKE_TOOLCHAIN_FILE="${CMAKE_TOOLCHAIN_FILE:-C:/vcpkg/scripts/buildsystems/vcpkg.cmake}" fi +# Pass an externally provided toolchain, or the default Windows vcpkg toolchain. +if [[ -n "${CMAKE_TOOLCHAIN_FILE:-}" ]]; then + CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}") +fi + +CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=${build_type}") + if [[ "${build_enable_sccache}" == "ON" ]]; then CMAKE_ARGS+=("-DCMAKE_CXX_COMPILER_LAUNCHER=sccache") CMAKE_ARGS+=("-DCMAKE_C_COMPILER_LAUNCHER=sccache") @@ -91,16 +87,10 @@ if [[ -n "${ICEBERG_EXTRA_CMAKE_ARGS:-}" ]]; then fi cmake "${CMAKE_ARGS[@]}" ${source_dir} -if is_windows; then - cmake --build . --config "${build_type}" --target install - if [[ "${run_tests}" == "ON" ]]; then - ctest --output-on-failure -C "${build_type}" - fi -else - cmake --build . --target install - if [[ "${run_tests}" == "ON" ]]; then + +cmake --build . --target install +if [[ "${run_tests}" == "ON" ]]; then ctest --output-on-failure - fi fi popd diff --git a/cmake_modules/IcebergSccache.cmake b/cmake_modules/IcebergSccache.cmake index d02e65df5..a618a4862 100644 --- a/cmake_modules/IcebergSccache.cmake +++ b/cmake_modules/IcebergSccache.cmake @@ -18,19 +18,12 @@ if(MSVC_TOOLCHAIN AND "${CMAKE_CXX_COMPILER_LAUNCHER}" STREQUAL "sccache") message(STATUS "Configuring sccache for MSVC") - # Remove /Zi or /ZI - string(REGEX REPLACE "/Z[iI]" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}") - string(REGEX REPLACE "/Z[iI]" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}") + # Keep MSVC Debug objects cacheable by sccache without affecting Release. + set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "$<$:Embedded>") - string(REGEX REPLACE "/Z[iI]" "" CMAKE_C_FLAGS_RELWITHDEBINFO - "${CMAKE_C_FLAGS_RELWITHDEBINFO}") - string(REGEX REPLACE "/Z[iI]" "" CMAKE_CXX_FLAGS_RELWITHDEBINFO - "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}") - - # Add /Z7 - set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} /Z7") - set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /Z7") - - set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} /Z7") - set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} /Z7") + # CMP0141 normally handles these flags; normalize any flags injected elsewhere. + foreach(flag_var CMAKE_C_FLAGS_DEBUG CMAKE_CXX_FLAGS_DEBUG CMAKE_C_FLAGS_RELWITHDEBINFO + CMAKE_CXX_FLAGS_RELWITHDEBINFO) + string(REGEX REPLACE "/Z[iI]" "/Z7" ${flag_var} "${${flag_var}}") + endforeach() endif() diff --git a/src/iceberg/test/scan_test_base.h b/src/iceberg/test/scan_test_base.h index 5bd1222e0..4e32febe2 100644 --- a/src/iceberg/test/scan_test_base.h +++ b/src/iceberg/test/scan_test_base.h @@ -221,7 +221,7 @@ class ScanTestBase : public testing::TestWithParam { std::shared_ptr spec = nullptr) { std::vector> files_with_partitions; for (const auto& path : added_files) { - files_with_partitions.emplace_back(path, kEmptyPartition); + files_with_partitions.emplace_back(path, PartitionValues{}); } return MakeAppendSnapshotWithPartitionValues(format_version, snapshot_id, parent_snapshot_id, sequence_number, @@ -350,7 +350,6 @@ class ScanTestBase : public testing::TestWithParam { private: int manifest_counter_ = 0; int manifest_list_counter_ = 0; - constexpr static PartitionValues kEmptyPartition{}; }; } // namespace iceberg From 2b0d474f02a4b3e46387b36e535ee15b2511553a Mon Sep 17 00:00:00 2001 From: Timothy Wang Date: Wed, 8 Jul 2026 15:00:15 -0700 Subject: [PATCH 13/13] benchmark: add metrics evaluator benchmark --- src/iceberg/benchmark/CMakeLists.txt | 6 + src/iceberg/benchmark/meson.build | 8 + .../benchmark/metrics_evaluator_benchmark.cc | 293 ++++++++++++++++++ 3 files changed, 307 insertions(+) create mode 100644 src/iceberg/benchmark/metrics_evaluator_benchmark.cc diff --git a/src/iceberg/benchmark/CMakeLists.txt b/src/iceberg/benchmark/CMakeLists.txt index 0227c3871..9a97855ff 100644 --- a/src/iceberg/benchmark/CMakeLists.txt +++ b/src/iceberg/benchmark/CMakeLists.txt @@ -39,3 +39,9 @@ target_link_libraries(benchmark_smoke PRIVATE "$,iceberg_static,iceberg_shared>" "$,benchmark::benchmark,benchmark>" ) + +add_executable(metrics_evaluator_benchmark metrics_evaluator_benchmark.cc) +target_link_libraries(metrics_evaluator_benchmark + PRIVATE "$,iceberg_static,iceberg_shared>" + "$,benchmark::benchmark,benchmark>" +) diff --git a/src/iceberg/benchmark/meson.build b/src/iceberg/benchmark/meson.build index affc34e07..e7bd73657 100644 --- a/src/iceberg/benchmark/meson.build +++ b/src/iceberg/benchmark/meson.build @@ -24,3 +24,11 @@ benchmark_smoke = executable( ) benchmark('benchmark_smoke', benchmark_smoke) + +metrics_evaluator_benchmark = executable( + 'metrics_evaluator_benchmark', + sources: files('metrics_evaluator_benchmark.cc'), + dependencies: [iceberg_dep, benchmark_dep], +) + +benchmark('metrics_evaluator_benchmark', metrics_evaluator_benchmark) diff --git a/src/iceberg/benchmark/metrics_evaluator_benchmark.cc b/src/iceberg/benchmark/metrics_evaluator_benchmark.cc new file mode 100644 index 000000000..86406ac1d --- /dev/null +++ b/src/iceberg/benchmark/metrics_evaluator_benchmark.cc @@ -0,0 +1,293 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "iceberg/expression/expressions.h" +#include "iceberg/expression/inclusive_metrics_evaluator.h" +#include "iceberg/expression/literal.h" +#include "iceberg/expression/strict_metrics_evaluator.h" +#include "iceberg/manifest/manifest_entry.h" +#include "iceberg/result.h" +#include "iceberg/schema.h" +#include "iceberg/schema_field.h" +#include "iceberg/type.h" + +namespace iceberg { +namespace { + +enum class ExpressionCase { + kEqualId, + kRangeId, + kStartsWithName, + kSmallInAge, + kMediumInAge, + kNullOrNanSalary, +}; + +template +T Must(Result result) { + if (!result.has_value()) { + throw std::runtime_error(result.error().message); + } + return std::move(result).value(); +} + +std::shared_ptr MakeBenchmarkSchema() { + return std::make_shared( + std::vector{ + SchemaField::MakeRequired(1, "id", int64()), + SchemaField::MakeOptional(2, "name", string()), + SchemaField::MakeRequired(3, "age", int32()), + SchemaField::MakeOptional(4, "salary", float64()), + SchemaField::MakeRequired(5, "active", boolean()), + }, + /*schema_id=*/0); +} + +std::vector MakeAgeLiterals(int32_t size) { + std::vector values; + values.reserve(size); + for (int32_t value = 0; value < size; ++value) { + values.push_back(Literal::Int(20 + value)); + } + return values; +} + +std::shared_ptr MakeExpression(ExpressionCase expression_case) { + switch (expression_case) { + case ExpressionCase::kEqualId: + return Expressions::Equal("id", Literal::Long(150)); + case ExpressionCase::kRangeId: + return Expressions::And(Expressions::GreaterThanOrEqual("id", Literal::Long(120)), + Expressions::LessThan("id", Literal::Long(220))); + case ExpressionCase::kStartsWithName: + return Expressions::StartsWith("name", "name-0"); + case ExpressionCase::kSmallInAge: + return Expressions::In("age", MakeAgeLiterals(4)); + case ExpressionCase::kMediumInAge: + return Expressions::In("age", MakeAgeLiterals(64)); + case ExpressionCase::kNullOrNanSalary: + return Expressions::Or(Expressions::IsNull("salary"), + Expressions::IsNaN("salary")); + } + std::unreachable(); +} + +std::vector Serialize(const Literal& literal) { + return Must(literal.Serialize()); +} + +DataFile MakeDataFile(int64_t index) { + auto id_lower = 100 + index % 75; + auto age_lower = static_cast(20 + index % 40); + auto name_suffix = std::to_string(1000 + index % 100); + auto salary_lower = 1.0 + static_cast(index % 25); + + DataFile file; + file.content = DataFile::Content::kData; + file.file_path = "file://" + std::to_string(index) + ".parquet"; + file.file_format = FileFormatType::kParquet; + file.record_count = 1024; + file.file_size_in_bytes = 64 * 1024; + file.value_counts = {{1, file.record_count}, {2, file.record_count}, + {3, file.record_count}, {4, file.record_count}, + {5, file.record_count}}; + file.null_value_counts = {{1, 0}, {2, 0}, {3, 0}, {4, index % 7 == 0 ? 64 : 0}, + {5, 0}}; + file.nan_value_counts = {{4, index % 11 == 0 ? 32 : 0}}; + file.lower_bounds = { + {1, Serialize(Literal::Long(id_lower))}, + {2, Serialize(Literal::String("name-" + name_suffix))}, + {3, Serialize(Literal::Int(age_lower))}, + {4, Serialize(Literal::Double(salary_lower))}, + {5, Serialize(Literal::Boolean(index % 2 == 0))}, + }; + file.upper_bounds = { + {1, Serialize(Literal::Long(id_lower + 100))}, + {2, Serialize(Literal::String("name-" + std::to_string(1999 + index % 100)))}, + {3, Serialize(Literal::Int(age_lower + 10))}, + {4, Serialize(Literal::Double(salary_lower + 100.0))}, + {5, Serialize(Literal::Boolean(true))}, + }; + file.split_offsets = {4}; + file.sort_order_id = 0; + return file; +} + +std::vector MakeDataFiles(int64_t size) { + std::vector files; + files.reserve(size); + for (int64_t index = 0; index < size; ++index) { + files.push_back(MakeDataFile(index)); + } + return files; +} + +void BM_InclusiveMetricsEvaluatorMake(benchmark::State& state, + ExpressionCase expression_case) { + auto schema = MakeBenchmarkSchema(); + auto expression = MakeExpression(expression_case); + + while (state.KeepRunning()) { + auto evaluator = + Must(InclusiveMetricsEvaluator::Make(expression, *schema, /*case_sensitive=*/true)); + benchmark::DoNotOptimize(evaluator); + } + + state.SetItemsProcessed(state.iterations()); +} + +void BM_StrictMetricsEvaluatorMake(benchmark::State& state, + ExpressionCase expression_case) { + auto schema = MakeBenchmarkSchema(); + auto expression = MakeExpression(expression_case); + + while (state.KeepRunning()) { + auto evaluator = + Must(StrictMetricsEvaluator::Make(expression, schema, /*case_sensitive=*/true)); + benchmark::DoNotOptimize(evaluator); + } + + state.SetItemsProcessed(state.iterations()); +} + +void BM_InclusiveMetricsEvaluatorEvaluate(benchmark::State& state, + ExpressionCase expression_case) { + auto schema = MakeBenchmarkSchema(); + auto expression = MakeExpression(expression_case); + auto evaluator = + Must(InclusiveMetricsEvaluator::Make(expression, *schema, /*case_sensitive=*/true)); + auto files = MakeDataFiles(state.range(0)); + + while (state.KeepRunning()) { + int64_t matching_files = 0; + for (const auto& file : files) { + matching_files += Must(evaluator->Evaluate(file)) ? 1 : 0; + } + benchmark::DoNotOptimize(matching_files); + } + + state.SetItemsProcessed(state.iterations() * static_cast(files.size())); +} + +void BM_StrictMetricsEvaluatorEvaluate(benchmark::State& state, + ExpressionCase expression_case) { + auto schema = MakeBenchmarkSchema(); + auto expression = MakeExpression(expression_case); + auto evaluator = Must(StrictMetricsEvaluator::Make(expression, schema, + /*case_sensitive=*/true)); + auto files = MakeDataFiles(state.range(0)); + + while (state.KeepRunning()) { + int64_t matching_files = 0; + for (const auto& file : files) { + matching_files += Must(evaluator->Evaluate(file)) ? 1 : 0; + } + benchmark::DoNotOptimize(matching_files); + } + + state.SetItemsProcessed(state.iterations() * static_cast(files.size())); +} + +void ApplyFileCounts(benchmark::internal::Benchmark* benchmark) { + benchmark->ArgName("files")->Arg(1)->Arg(100)->Arg(10000); +} + +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, equal_id, + ExpressionCase::kEqualId); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, range_id, + ExpressionCase::kRangeId); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, starts_with_name, + ExpressionCase::kStartsWithName); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, small_in_age, + ExpressionCase::kSmallInAge); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, medium_in_age, + ExpressionCase::kMediumInAge); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorMake, null_or_nan_salary, + ExpressionCase::kNullOrNanSalary); + +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, equal_id, ExpressionCase::kEqualId); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, range_id, ExpressionCase::kRangeId); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, starts_with_name, + ExpressionCase::kStartsWithName); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, small_in_age, + ExpressionCase::kSmallInAge); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, medium_in_age, + ExpressionCase::kMediumInAge); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorMake, null_or_nan_salary, + ExpressionCase::kNullOrNanSalary); + +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, equal_id, + ExpressionCase::kEqualId) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, range_id, + ExpressionCase::kRangeId) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, starts_with_name, + ExpressionCase::kStartsWithName) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, small_in_age, + ExpressionCase::kSmallInAge) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, medium_in_age, + ExpressionCase::kMediumInAge) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_InclusiveMetricsEvaluatorEvaluate, null_or_nan_salary, + ExpressionCase::kNullOrNanSalary) + ->Apply(ApplyFileCounts); + +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, equal_id, + ExpressionCase::kEqualId) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, range_id, + ExpressionCase::kRangeId) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, starts_with_name, + ExpressionCase::kStartsWithName) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, small_in_age, + ExpressionCase::kSmallInAge) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, medium_in_age, + ExpressionCase::kMediumInAge) + ->Apply(ApplyFileCounts); +BENCHMARK_CAPTURE(BM_StrictMetricsEvaluatorEvaluate, null_or_nan_salary, + ExpressionCase::kNullOrNanSalary) + ->Apply(ApplyFileCounts); + +} // namespace +} // namespace iceberg + +int main(int argc, char** argv) { + benchmark::Initialize(&argc, argv); + if (benchmark::ReportUnrecognizedArguments(argc, argv)) { + return 1; + } + benchmark::RunSpecifiedBenchmarks(); + benchmark::Shutdown(); + return 0; +}