amogh-jahagirdar commented on code in PR #17960:
URL: https://github.com/apache/iceberg/pull/17960#discussion_r4139229874
##########
core/src/test/java/org/apache/iceberg/TestMetricsConfig.java:
##########
@@ -22,14 +22,80 @@
import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;
+import java.io.File;
+import java.io.IOException;
import java.util.Map;
import java.util.Set;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
public class TestMetricsConfig {
+ @TempDir private File temp;
+
+ private static final int ID = 1;
+ private static final int EVENT_TIME = 2;
+ private static final int CATEGORY = 3;
+ private static final int DATA = 4;
+
+ private static final Schema PARTITIONED_SCHEMA =
+ new Schema(
+ required(ID, "id", Types.IntegerType.get()),
+ optional(EVENT_TIME, "event_time",
Types.TimestampType.withoutZone()),
+ optional(CATEGORY, "category", Types.StringType.get()),
+ optional(DATA, "data", Types.StringType.get()));
+
+ @Test
+ public void testConfigCannotDisablePartitionSourceMetrics() throws
IOException {
+ PartitionSpec spec =
PartitionSpec.builderFor(PARTITIONED_SCHEMA).identity("category").build();
+ Map<String, String> props =
+ ImmutableMap.of(TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX +
"category", "none");
+ Table table = TestTables.create(temp, "identity-override",
PARTITIONED_SCHEMA, spec, 4, props);
+
+ assertThat(MetricsConfig.forTable(table).columnMode(CATEGORY))
+ .as("column config should not be able to disable metrics for a
partition source column")
+ .isEqualTo(MetricsModes.Full.get());
+ }
+
+ @Test
+ public void testBucketPartitionColumnIgnored() throws IOException {
+ PartitionSpec spec =
PartitionSpec.builderFor(PARTITIONED_SCHEMA).bucket("category", 4).build();
+ Table table = TestTables.create(temp, "bucket", PARTITIONED_SCHEMA, spec,
4, ImmutableMap.of());
+
+ assertThat(MetricsConfig.forTable(table).columnMode(CATEGORY))
+ .as("non-order-preserving partition transform should not promote its
source column")
+ .isEqualTo(MetricsModes.Truncate.withLength(16));
+ }
+
+ @Test
+ public void testColumnModeAndFieldIdsFromPartitionSpec() throws IOException {
Review Comment:
Done. Let me know if the 2 partition fields for the same source case is as
realistic as possible. I do think fundamentally that use case isn't realistic
though it is allowed (and we can't have multiple time transforms on the same
source columns at least from this implementation). So the test that was added
was an identity + truncate on the same source column.
--
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]
