wgtmac commented on code in PR #794:
URL: https://github.com/apache/iceberg-cpp/pull/794#discussion_r3503948989
##########
src/iceberg/util/error_collector.h:
##########
@@ -159,14 +159,20 @@ class ICEBERG_EXPORT ErrorCollector {
/// in Build(), Apply(), or Commit() methods) to validate that no errors
/// were accumulated during the builder method calls.
///
- /// \return Status indicating success if no errors, or a ValidationFailed
- /// error with all accumulated error messages
+ /// \return Status indicating success if no errors, a
RetryableValidationFailed if
+ /// all accumulated errors are retryable validations, or a
ValidationFailed
+ /// error with all accumulated error messages otherwise
[[nodiscard]] Status CheckErrors() const {
if (!errors_.empty()) {
std::string error_msg = "Validation failed due to the following
errors:\n";
+ bool all_retryable = true;
Review Comment:
The `all_retryable` check is intentional because C++ builders accumulate
errors, while Java is fail-fast. In Java, a `RetryableValidationException`
stops evaluation immediately, so there is no mixed-error result to classify.
In C++, if we treated the result as retryable when only the first error is
retryable, a chained builder call could retry even when another collected error
is deterministic and cannot be fixed by refreshing metadata. Requiring all
collected errors to be retryable keeps retry limited to cases where refreshing
can plausibly fix the whole update.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
