You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "manuzhang (via GitHub)" <gi...@apache.org> on 2026/10/16 10:20:51 UTC
[PR] fix: qualify Iceberg names in public error macros [iceberg-cpp]
manuzhang opened a new pull request, #1026:
URL: https://github.com/apache/iceberg-cpp/pull/1026
## What
- Qualify the Iceberg names that `ICEBERG_RETURN_UNEXPECTED`, `ICEBERG_ASSIGN_OR_RAISE`, `ICEBERG_PRECHECK`, and `ICEBERG_CHECK` in `iceberg/util/macros.h`, and the `ICEBERG_BUILDER_*` macros in `iceberg/util/error_collector.h`, use with `::iceberg::`.
- Change the `iceberg::` prefixes in the exception macros (`ERROR_TO_EXCEPTION`, used by `ICEBERG_THROW_NOT_OK` and `ICEBERG_ASSIGN_OR_THROW`) to `::iceberg::`, as the logging macros already do.
- Add `error_macros_test.cc`, which uses each of these macros from code outside namespace iceberg.
## Why
The macros named `Error`, `InvalidArgument`, `ValidationFailed`, and `ErrorKind` without their namespace. Those names resolve only in code inside namespace iceberg, so an application that includes the installed headers could not use the macros. For example, `ICEBERG_ASSIGN_OR_RAISE` in an application failed with `'Error' was not declared in this scope`.
## Behavior change
Code outside namespace iceberg can now use these macros. Code inside namespace iceberg compiles and behaves as before, because the qualified names refer to the same functions and types.
## Testing
- `util_test` passes (419 tests), including the new `ErrorMacrosTest` cases for `ICEBERG_ASSIGN_OR_RAISE`, `ICEBERG_RETURN_UNEXPECTED`, `ICEBERG_PRECHECK`, `ICEBERG_CHECK`, `ICEBERG_ASSIGN_OR_THROW`, and the builder macros. The new test file does not compile against the previous headers.
- `metadata_table_test` passes (10 tests), which also compiles the bundle library against the changed headers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: qualify Iceberg names in public error macros [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #1026:
URL: https://github.com/apache/iceberg-cpp/pull/1026#discussion_r4324344524
##########
src/iceberg/util/error_collector.h:
##########
@@ -31,6 +31,8 @@
namespace iceberg {
Review Comment:
I think we can move the namespace declaration just above ErrorCollector then, leaving the macros outside it, WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: qualify Iceberg names in public error macros [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #1026:
URL: https://github.com/apache/iceberg-cpp/pull/1026
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: qualify Iceberg names in public error macros [iceberg-cpp]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #1026:
URL: https://github.com/apache/iceberg-cpp/pull/1026#discussion_r4320241162
##########
src/iceberg/util/macros.h:
##########
@@ -21,15 +21,18 @@
/// \file iceberg/util/macros.h
/// \brief Define common Iceberg utility macros.
+///
+/// The macros name Iceberg symbols with their full namespace, so code outside namespace
Review Comment:
Done in 81321d16, thanks. `ICEBERG_CHECK_OR_DIE` now throws `::iceberg::IcebergError`. I also moved the test helpers into a namespace with its own nested `iceberg`, so the test fails to compile if any of these macros drops the leading `::`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: qualify Iceberg names in public error macros [iceberg-cpp]
Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #1026:
URL: https://github.com/apache/iceberg-cpp/pull/1026#discussion_r4304187832
##########
src/iceberg/util/macros.h:
##########
@@ -21,15 +21,18 @@
/// \file iceberg/util/macros.h
/// \brief Define common Iceberg utility macros.
+///
+/// The macros name Iceberg symbols with their full namespace, so code outside namespace
Review Comment:
ICEBERG_CHECK_OR_DIE in exception.h:47 is the one public error macro left out of this pass; it still expands to iceberg::IcebergError without the leading colons. Worth folding in so the rule this comment states holds for every macro a caller outside the namespace can reach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org