Skip to content

Commit 5bb61ea

Browse files
committed
fix: properly propagate errors in MetricsConfig validation
The previous error handling in VerifyReferencedColumns treated all failures the same way. When FindFieldByName() failed, both actual errors (e.g., internal failures during name resolution) and simple "field not found" cases would return the same generic validation error message, losing important error context. FindFieldByName() returns Result<optional<reference_wrapper>>, which can represent three states: 1. Error during lookup (Result contains error) 2. Lookup succeeded but field not found (Result contains empty optional) 3. Lookup succeeded and field found (Result contains optional with value) The original code combined states 1 and 2: if (!field.has_value() || !field.value().has_value()) Changed to use ICEBERG_ASSIGN_OR_RAISE to separate error propagation from the "not found" check: ICEBERG_ASSIGN_OR_RAISE(auto field, schema.FindFieldByName(...)); if (!field.has_value()) The macro unwraps the Result, propagating any errors immediately, then assigns the optional to field. The subsequent has_value() check verifies if the optional contains a value, maintaining the same functionality while properly handling errors. This pattern is already used elsewhere in the codebase (e.g., table_metadata.cc:80-85).
1 parent cf0fd37 commit 5bb61ea

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

src/iceberg/metrics_config.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ Status MetricsConfig::VerifyReferencedColumns(
3636
}
3737
auto field_name =
3838
std::string_view(key).substr(TableProperties::kMetricModeColumnConfPrefix.size());
39-
auto field = schema.FindFieldByName(field_name);
40-
if (!field.has_value() || !field.value().has_value()) {
39+
ICEBERG_ASSIGN_OR_RAISE(auto field, schema.FindFieldByName(field_name));
40+
if (!field.has_value()) {
4141
return ValidationFailed(
4242
"Invalid metrics config, could not find column {} from table prop {} in "
4343
"schema {}",

0 commit comments

Comments
 (0)