From 6bc27b50bb4c81e949d86aa550207c5f59249fa4 Mon Sep 17 00:00:00 2001 From: Gabriel Igliozzi Date: Mon, 9 Sep 2024 11:29:06 -0700 Subject: [PATCH] Add safe-guard to omit void partitions --- .../common/server/properties/Config.java | 7 +++ .../server/properties/DefaultConfigImpl.java | 5 +++ .../server/properties/MetacatProperties.java | 3 ++ .../properties/PartitionColumnProperties.java | 43 +++++++++++++++++++ .../connector/hive/HiveConnectorPlugin.java | 3 +- .../HiveConnectorInfoConverter.java | 2 +- .../hive/converters/HiveTypeConverter.java | 23 +++++++++- .../sql/HiveConnectorFastTableService.java | 3 +- .../HiveConnectorInfoConvertorSpec.groovy | 3 +- .../converters/HiveTypeConverterSpec.groovy | 15 ++++--- .../polaris/PolarisConnectorPlugin.java | 3 +- .../PolarisConnectorTableServiceTest.java | 2 +- .../metacat-test-cluster/docker-compose.yml | 3 +- 13 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/PartitionColumnProperties.java diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Config.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Config.java index c9669e737..bcea5a08f 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Config.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Config.java @@ -642,5 +642,12 @@ public interface Config { * @return parentChildRelationshipProperties */ ParentChildRelationshipProperties getParentChildRelationshipProperties(); + + /** + * Whether we want to omit void transform partition from current partitions. + * + * @return True if it should be omitted + */ + boolean omitVoidTransformEnabled(); } diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/DefaultConfigImpl.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/DefaultConfigImpl.java index 5f223ed7a..75bc4af86 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/DefaultConfigImpl.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/DefaultConfigImpl.java @@ -703,4 +703,9 @@ public boolean isParentChildDropEnabled() { public ParentChildRelationshipProperties getParentChildRelationshipProperties() { return this.metacatProperties.getParentChildRelationshipProperties(); } + + @Override + public boolean omitVoidTransformEnabled() { + return this.metacatProperties.getPartitionColumnProperties().isOmitVoidPartitionEnabled(); + } } diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/MetacatProperties.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/MetacatProperties.java index 6e9505ce9..745e97ef0 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/MetacatProperties.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/MetacatProperties.java @@ -72,6 +72,8 @@ public class MetacatProperties { private RateLimiterProperties rateLimiterProperties = new RateLimiterProperties(); @NonNull private ParentChildRelationshipProperties parentChildRelationshipProperties; + @NonNull + private PartitionColumnProperties partitionColumnProperties; /** * Constructor for MetacatProperties. @@ -81,5 +83,6 @@ public class MetacatProperties { public MetacatProperties(final Environment env) { this.env = env; this.parentChildRelationshipProperties = new ParentChildRelationshipProperties(env); + this.partitionColumnProperties = new PartitionColumnProperties(env); } } diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/PartitionColumnProperties.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/PartitionColumnProperties.java new file mode 100644 index 000000000..62f40348e --- /dev/null +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/PartitionColumnProperties.java @@ -0,0 +1,43 @@ +package com.netflix.metacat.common.server.properties; + +import lombok.Data; +import lombok.extern.slf4j.Slf4j; +import org.springframework.core.env.Environment; +import javax.annotation.Nullable; + +/** + * Partition Column service properties. + * + * @author gabeiglio + */ +@Data +@Slf4j +public class PartitionColumnProperties { + private static final String OMIT_VOID_TRANSFORM_PARTITION_FIELDS_PROPERTY_NAME = + "metacat.partitionColumnProperties.omitVoidTransformPartitionFields"; + private boolean omitVoidPartitionEnabled; + + /** + * Constructor. + * + * @param env Spring environment + */ + public PartitionColumnProperties(@Nullable final Environment env) { + if (env != null) { + setOmitVoidTransformPartitionFields( + env.getProperty(OMIT_VOID_TRANSFORM_PARTITION_FIELDS_PROPERTY_NAME, + Boolean.class, true) + ); + } + } + + /** + * set omitVoidPartitionEnabled based on boolean config. + * + * @param configBool configBool + */ + public void setOmitVoidTransformPartitionFields(final boolean configBool) { + this.omitVoidPartitionEnabled = configBool; + } + +} diff --git a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/HiveConnectorPlugin.java b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/HiveConnectorPlugin.java index 0dcc2b1a5..33781e0dc 100644 --- a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/HiveConnectorPlugin.java +++ b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/HiveConnectorPlugin.java @@ -32,7 +32,8 @@ */ public class HiveConnectorPlugin implements ConnectorPlugin { private static final String CONNECTOR_TYPE = "hive"; - private static final HiveTypeConverter HIVE_TYPE_CONVERTER = new HiveTypeConverter(); + private static final HiveTypeConverter HIVE_TYPE_CONVERTER = + new HiveTypeConverter(ConnectorContext.builder().build().getConfig()); private static final HiveConnectorInfoConverter INFO_CONVERTER_HIVE = new HiveConnectorInfoConverter(HIVE_TYPE_CONVERTER); diff --git a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConverter.java b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConverter.java index 8a897056e..01253be9f 100644 --- a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConverter.java +++ b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConverter.java @@ -65,7 +65,7 @@ public class HiveConnectorInfoConverter implements ConnectorInfoConverter icebergSchemaTofieldDtos(final Schema schema, final List partitionFields) { final List fields = Lists.newArrayList(); - final List partitionNames = - partitionFields.stream() + final List partitionNames; + + if (config.omitVoidTransformEnabled()) { + partitionNames = partitionFields.stream() .filter(f -> f.transform() != null && !f.transform().toString().equals("void")) .map(f -> schema.findField(f.sourceId()).name()) .collect(Collectors.toList()); + } else { + partitionNames = partitionFields.stream() + .map(f -> schema.findField(f.sourceId()).name()) + .collect(Collectors.toList()); + } for (Types.NestedField field : schema.columns()) { final FieldInfo fieldInfo = new FieldInfo(); diff --git a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/sql/HiveConnectorFastTableService.java b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/sql/HiveConnectorFastTableService.java index 82f9af081..049e5f5f1 100644 --- a/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/sql/HiveConnectorFastTableService.java +++ b/metacat-connector-hive/src/main/java/com/netflix/metacat/connector/hive/sql/HiveConnectorFastTableService.java @@ -154,7 +154,8 @@ public TableInfo get(final ConnectorRequestContext requestContext, final Qualifi && HiveTableUtil.isCommonView(info)) { final String tableLoc = HiveTableUtil.getCommonViewMetadataLocation(info); return hiveConnectorFastTableServiceProxy.getCommonViewTableInfo(name, tableLoc, info, - new HiveTypeConverter(), connectorContext.getConfig().isIcebergCacheEnabled()); + new HiveTypeConverter(connectorContext.getConfig()), + connectorContext.getConfig().isIcebergCacheEnabled()); } if (!connectorContext.getConfig().isIcebergEnabled() || !HiveTableUtil.isIcebergTable(info)) { return info; diff --git a/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConvertorSpec.groovy b/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConvertorSpec.groovy index 0ebbe6f13..90eb4e666 100644 --- a/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConvertorSpec.groovy +++ b/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveConnectorInfoConvertorSpec.groovy @@ -59,7 +59,8 @@ class HiveConnectorInfoConvertorSpec extends Specification{ def setup() { // Stub this to always return true config.isEpochInSeconds() >> true - converter = new HiveConnectorInfoConverter( new HiveTypeConverter()) + config.omitVoidTransformEnabled() >> true + converter = new HiveConnectorInfoConverter(new HiveTypeConverter(config)) } def 'test date to epoch seconds'() { diff --git a/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveTypeConverterSpec.groovy b/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveTypeConverterSpec.groovy index 2ca1efec7..73bfbb3f1 100644 --- a/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveTypeConverterSpec.groovy +++ b/metacat-connector-hive/src/test/groovy/com/netflix/metacat/connector/hive/converters/HiveTypeConverterSpec.groovy @@ -13,6 +13,7 @@ package com.netflix.metacat.connector.hive.converters +import com.netflix.metacat.common.server.properties.Config import org.apache.iceberg.PartitionField import org.apache.iceberg.transforms.Identity import org.apache.iceberg.transforms.VoidTransform @@ -28,8 +29,14 @@ import spock.lang.Unroll * @since 1.0.0 */ class HiveTypeConverterSpec extends Specification { - @Shared - HiveTypeConverter converter = new HiveTypeConverter() + Config config = Mock(Config) + HiveTypeConverter converter; + + def setup() { + // Stub omitVoidPartitionEnabled to always return true + this.config.omitVoidTransformEnabled() >> true + this.converter = new HiveTypeConverter(this.config) + } @Unroll def 'can convert "#typeString" to a presto type and back'(String typeString) { @@ -276,10 +283,8 @@ class HiveTypeConverterSpec extends Specification { new PartitionField(2, 2, "field2", new VoidTransform()), ] - def hiveTypeConverter = new HiveTypeConverter() - when: - def fieldDtos = hiveTypeConverter.icebergSchemaTofieldDtos(initialSchema, initialPartitionFields) + def fieldDtos = this.converter.icebergSchemaTofieldDtos(initialSchema, initialPartitionFields) then: fieldDtos.size() == 3 diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorPlugin.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorPlugin.java index 5dd60de69..a57130397 100644 --- a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorPlugin.java +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorPlugin.java @@ -16,7 +16,8 @@ public class PolarisConnectorPlugin implements ConnectorPlugin { private static final String CONNECTOR_TYPE = "polaris"; - private static final HiveTypeConverter TYPE_CONVERTER = new HiveTypeConverter(); + private static final HiveTypeConverter TYPE_CONVERTER = + new HiveTypeConverter(ConnectorContext.builder().build().getConfig()); private static final HiveConnectorInfoConverter INFO_CONVERTER = new HiveConnectorInfoConverter(TYPE_CONVERTER); diff --git a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorTableServiceTest.java b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorTableServiceTest.java index 27c4f918e..8a8143cd2 100644 --- a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorTableServiceTest.java +++ b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorTableServiceTest.java @@ -93,7 +93,7 @@ public void init() { polarisStoreService, CATALOG_NAME, polarisDBService, - new HiveConnectorInfoConverter(new HiveTypeConverter()), + new HiveConnectorInfoConverter(new HiveTypeConverter(connectorContext.getConfig())), new IcebergTableHandler(connectorContext, new IcebergTableCriteriaImpl(connectorContext), new IcebergTableOpWrapper(connectorContext, serviceManager), diff --git a/metacat-functional-tests/metacat-test-cluster/docker-compose.yml b/metacat-functional-tests/metacat-test-cluster/docker-compose.yml index bdf867b91..ae5967042 100644 --- a/metacat-functional-tests/metacat-test-cluster/docker-compose.yml +++ b/metacat-functional-tests/metacat-test-cluster/docker-compose.yml @@ -77,7 +77,8 @@ services: -Dmetacat.parentChildRelationshipProperties.createEnabled=true -Dmetacat.parentChildRelationshipProperties.getEnabled=true -Dmetacat.parentChildRelationshipProperties.renameEnabled=true - -Dmetacat.parentChildRelationshipProperties.dropEnabled=true' + -Dmetacat.parentChildRelationshipProperties.dropEnabled=true + -Dmetacat.partitionColumnProperties.omitVoidPartitionEnabled=true' labels: - "com.netflix.metacat.oss.test" - "com.netflix.metacat.oss.test.war"