Core: Support metrics with nested field in AppendBenchmark and FileGenerationUtil - #17391
Core: Support metrics with nested field in AppendBenchmark and FileGenerationUtil#17391dramaticlly wants to merge 2 commits into
Conversation
| List<Types.NestedField> leaves = Lists.newArrayList(); | ||
| TypeUtil.visit( | ||
| schema, | ||
| new TypeUtil.SchemaVisitor<Void>() { |
There was a problem hiding this comment.
We have struct / list / map here, but how about variant? With v3 adoption growing, a schema containing variant will throw rather than skip. Let's at least consider adding
@Override
public Void variant(Types.VariantType v) {
return null;
}
There was a problem hiding this comment.
I think current behavior for leafFields will throw UnsupportedOperationException if running into a V3 table with variant type. To me, it seems reasonable as exception serve as a reminder to add variant support instead of silently missing metrics or bound. There's also change like #15629 to add dedicated benchmark for variant.
Prefer to keep the existing scope and revisit v3+ type later in follow up PR
There was a problem hiding this comment.
there is a variand benchmark, we need reviews of it!
|
|
||
| @Test | ||
| public void testBoundsForNestedTypes() { | ||
| MetricsConfig metricsConfig = MetricsConfig.getDefault(); |
There was a problem hiding this comment.
Here we only use MetricsConfig.getDefault(), but the new key code path for a nested field is never exercised under non-default modes (none, counts, truncate).
There was a problem hiding this comment.
Good call, parametrized on all metrics mode for better coverage
|
@huaxingao @nastra if you want to take a look as well :) |
Adding schema support for nested fields in AppendBenchmark and FileGenerationUtil. Previous it will only handle the primitive type but real production workload often involves complex type.