From 19527372c3f9598b1a29cbd68b66ae859e59cde5 Mon Sep 17 00:00:00 2001 From: mygrsun Date: Wed, 5 Jun 2024 22:21:05 +0800 Subject: [PATCH 1/4] [#3403] fix(hive-catalog): add hive catalog property list-all-tables (#3703) ### What changes were proposed in this pull request? Add a Hive catalog property "list-all-tables". Using this property to control whether the Iceberg table is displayed in the Hive table list. ### Why are the changes needed? The bug is a schema has the Iceberg tables in the Hive catalog Fix: #3403 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? 1.create a hive catalog with "list-all-tables " property. 2.crate a database and a iceberg table in the catalog by hive beeline 3.check whether the table is displayed in the catalog . --------- Co-authored-by: ericqin --- .../catalog/hive/HiveCatalogOperations.java | 39 ++++++++++++++++++- .../hive/HiveCatalogPropertiesMeta.java | 14 +++++++ .../gravitino/catalog/hive/HiveTable.java | 2 + .../hive/TestHiveCatalogOperations.java | 4 +- docs/apache-hive-catalog.md | 23 +++++------ 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java index c9cbb6d19bf..13dd6740f55 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java @@ -6,9 +6,12 @@ import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.CLIENT_POOL_SIZE; +import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.LIST_ALL_TABLES; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.METASTORE_URIS; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.PRINCIPAL; +import static com.datastrato.gravitino.catalog.hive.HiveTable.ICEBERG_TABLE_TYPE_VALUE; import static com.datastrato.gravitino.catalog.hive.HiveTable.SUPPORT_TABLE_TYPES; +import static com.datastrato.gravitino.catalog.hive.HiveTable.TABLE_TYPE_PROP; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.COMMENT; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TABLE_TYPE; import static com.datastrato.gravitino.connector.BaseCatalog.CATALOG_BYPASS_PREFIX; @@ -99,6 +102,7 @@ public class HiveCatalogOperations implements CatalogOperations, SupportsSchemas private ScheduledThreadPoolExecutor checkTgtExecutor; private String kerberosRealm; private ProxyPlugin proxyPlugin; + boolean listAllTables = true; // Map that maintains the mapping of keys in Gravitino to that in Hive, for example, users // will only need to set the configuration 'METASTORE_URL' in Gravitino and Gravitino will change @@ -150,6 +154,8 @@ public void initialize( this.clientPool = new CachedClientPool(getClientPoolSize(conf), hiveConf, getCacheEvictionInterval(conf)); + + this.listAllTables = enableListAllTables(conf); } private void initKerberosIfNecessary(Map conf, Configuration hadoopConf) { @@ -275,6 +281,10 @@ long getCacheEvictionInterval(Map conf) { .getOrDefault(conf, CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS); } + boolean enableListAllTables(Map conf) { + return (boolean) + propertiesMetadata.catalogPropertiesMetadata().getOrDefault(conf, LIST_ALL_TABLES); + } /** Closes the Hive catalog and releases the associated client pool. */ @Override public void close() { @@ -534,7 +544,18 @@ public NameIdentifier[] listTables(Namespace namespace) throws NoSuchSchemaExcep return clientPool.run( c -> c.getTableObjectsByName(schemaIdent.name(), allTables).stream() - .filter(tb -> SUPPORT_TABLE_TYPES.contains(tb.getTableType())) + .filter( + tb -> { + boolean isSupportTable = SUPPORT_TABLE_TYPES.contains(tb.getTableType()); + if (!isSupportTable) { + return false; + } + if (!listAllTables) { + Map parameters = tb.getParameters(); + return isHiveTable(parameters); + } + return true; + }) .map(tb -> NameIdentifier.of(namespace, tb.getTableName())) .toArray(NameIdentifier[]::new)); } catch (UnknownDBException e) { @@ -550,6 +571,22 @@ public NameIdentifier[] listTables(Namespace namespace) throws NoSuchSchemaExcep } } + boolean isHiveTable(Map tableParameters) { + if (isIcebergTable(tableParameters)) return false; + return true; + } + + boolean isIcebergTable(Map tableParameters) { + if (tableParameters != null) { + boolean isIcebergTable = + ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(tableParameters.get(TABLE_TYPE_PROP)); + if (isIcebergTable) { + return true; + } + } + return false; + } + /** * Loads a table from the Hive Metastore. * diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java index 29cb01b1266..a1d4baac8ac 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogPropertiesMeta.java @@ -36,6 +36,10 @@ public class HiveCatalogPropertiesMeta extends BaseCatalogPropertiesMetadata { public static final String FETCH_TIMEOUT_SEC = "kerberos.keytab-fetch-timeout-sec"; + public static final String LIST_ALL_TABLES = "list-all-tables"; + + public static final boolean DEFAULT_LIST_ALL_TABLES = false; + private static final Map> HIVE_CATALOG_PROPERTY_ENTRIES = ImmutableMap.>builder() .put( @@ -88,6 +92,16 @@ public class HiveCatalogPropertiesMeta extends BaseCatalogPropertiesMetadata { FETCH_TIMEOUT_SEC, PropertyEntry.integerOptionalPropertyEntry( FETCH_TIMEOUT_SEC, "The timeout to fetch key tab", true, 60, false)) + .put( + LIST_ALL_TABLES, + PropertyEntry.booleanPropertyEntry( + LIST_ALL_TABLES, + "Lists all tables in a database, including non-Hive tables, such as Iceberg, etc.", + false, + false, + DEFAULT_LIST_ALL_TABLES, + false, + false)) .putAll(BASIC_CATALOG_PROPERTY_ENTRIES) .build(); diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java index f33ec12d4b0..267b8265eda 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java @@ -62,6 +62,8 @@ public class HiveTable extends BaseTable { // A set of supported Hive table types. public static final Set SUPPORT_TABLE_TYPES = Sets.newHashSet(MANAGED_TABLE.name(), EXTERNAL_TABLE.name()); + public static final String ICEBERG_TABLE_TYPE_VALUE = "ICEBERG"; + public static final String TABLE_TYPE_PROP = "table_type"; private String schemaName; private CachedClientPool clientPool; private StorageDescriptor sd; diff --git a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/TestHiveCatalogOperations.java b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/TestHiveCatalogOperations.java index 27aae03327a..ed85b7819be 100644 --- a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/TestHiveCatalogOperations.java +++ b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/TestHiveCatalogOperations.java @@ -11,6 +11,7 @@ import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.FETCH_TIMEOUT_SEC; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.IMPERSONATION_ENABLE; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.KEY_TAB_URI; +import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.LIST_ALL_TABLES; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.METASTORE_URIS; import static com.datastrato.gravitino.catalog.hive.HiveCatalogPropertiesMeta.PRINCIPAL; import static com.datastrato.gravitino.catalog.hive.TestHiveCatalog.HIVE_PROPERTIES_METADATA; @@ -67,12 +68,13 @@ void testPropertyMeta() { Map> propertyEntryMap = HIVE_PROPERTIES_METADATA.catalogPropertiesMetadata().propertyEntries(); - Assertions.assertEquals(11, propertyEntryMap.size()); + Assertions.assertEquals(12, propertyEntryMap.size()); Assertions.assertTrue(propertyEntryMap.containsKey(METASTORE_URIS)); Assertions.assertTrue(propertyEntryMap.containsKey(Catalog.PROPERTY_PACKAGE)); Assertions.assertTrue(propertyEntryMap.containsKey(BaseCatalog.CATALOG_OPERATION_IMPL)); Assertions.assertTrue(propertyEntryMap.containsKey(CLIENT_POOL_SIZE)); Assertions.assertTrue(propertyEntryMap.containsKey(IMPERSONATION_ENABLE)); + Assertions.assertTrue(propertyEntryMap.containsKey(LIST_ALL_TABLES)); Assertions.assertTrue(propertyEntryMap.get(METASTORE_URIS).isRequired()); Assertions.assertFalse(propertyEntryMap.get(Catalog.PROPERTY_PACKAGE).isRequired()); diff --git a/docs/apache-hive-catalog.md b/docs/apache-hive-catalog.md index 98f19cec3ef..91e7252e031 100644 --- a/docs/apache-hive-catalog.md +++ b/docs/apache-hive-catalog.md @@ -28,17 +28,18 @@ The Hive catalog supports creating, updating, and deleting databases and tables ### Catalog properties -| Property Name | Description | Default Value | Required | Since Version | -|------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|------------------------------|---------------| -| `metastore.uris` | The Hive metastore service URIs, separate multiple addresses with commas. Such as `thrift://127.0.0.1:9083` | (none) | Yes | 0.2.0 | -| `client.pool-size` | The maximum number of Hive metastore clients in the pool for Gravitino. | 1 | No | 0.2.0 | -| `gravitino.bypass.` | Property name with this prefix passed down to the underlying HMS client for use. Such as `gravitino.bypass.hive.metastore.failure.retries = 3` indicate 3 times of retries upon failure of Thrift metastore calls | (none) | No | 0.2.0 | -| `client.pool-cache.eviction-interval-ms` | The cache pool eviction interval. | 300000 | No | 0.4.0 | -| `impersonation-enable` | Enable user impersonation for Hive catalog. | false | No | 0.4.0 | -| `kerberos.principal` | The Kerberos principal for the catalog. You should configure `gravitino.bypass.hadoop.security.authentication`, `gravitino.bypass.hive.metastore.kerberos.principal` and `gravitino.bypass.hive.metastore.sasl.enabled`if you want to use Kerberos. | (none) | required if you use kerberos | 0.4.0 | -| `kerberos.keytab-uri` | The uri of key tab for the catalog. Now supported protocols are `https`, `http`, `ftp`, `file`. | (none) | required if you use kerberos | 0.4.0 | -| `kerberos.check-interval-sec` | The interval to check validness of the principal | 60 | No | 0.4.0 | -| `kerberos.keytab-fetch-timeout-sec` | The timeout to fetch key tab | 60 | No | 0.4.0 | +| Property Name | Description | Default Value | Required | Since Version | +|------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|------------------------------|---------------| +| `metastore.uris` | The Hive metastore service URIs, separate multiple addresses with commas. Such as `thrift://127.0.0.1:9083` | (none) | Yes | 0.2.0 | +| `client.pool-size` | The maximum number of Hive metastore clients in the pool for Gravitino. | 1 | No | 0.2.0 | +| `gravitino.bypass.` | Property name with this prefix passed down to the underlying HMS client for use. Such as `gravitino.bypass.hive.metastore.failure.retries = 3` indicate 3 times of retries upon failure of Thrift metastore calls | (none) | No | 0.2.0 | +| `client.pool-cache.eviction-interval-ms` | The cache pool eviction interval. | 300000 | No | 0.4.0 | +| `impersonation-enable` | Enable user impersonation for Hive catalog. | false | No | 0.4.0 | +| `kerberos.principal` | The Kerberos principal for the catalog. You should configure `gravitino.bypass.hadoop.security.authentication`, `gravitino.bypass.hive.metastore.kerberos.principal` and `gravitino.bypass.hive.metastore.sasl.enabled`if you want to use Kerberos. | (none) | required if you use kerberos | 0.4.0 | +| `kerberos.keytab-uri` | The uri of key tab for the catalog. Now supported protocols are `https`, `http`, `ftp`, `file`. | (none) | required if you use kerberos | 0.4.0 | +| `kerberos.check-interval-sec` | The interval to check validness of the principal | 60 | No | 0.4.0 | +| `kerberos.keytab-fetch-timeout-sec` | The timeout to fetch key tab | 60 | No | 0.4.0 | +| `list-all-tables` | Lists all tables in a database, including non-Hive tables, such as Iceberg, etc | false | No | 0.5.1 | When you use the Gravitino with Trino. You can pass the Trino Hive connector configuration using prefix `trino.bypass.`. For example, using `trino.bypass.hive.config.resources` to pass the `hive.config.resources` to the Gravitino Hive catalog in Trino runtime. From c5c9bef45ce0fbfb2cf28c9eae749cfd5dc55c65 Mon Sep 17 00:00:00 2001 From: Shaofeng Shi Date: Thu, 6 Jun 2024 02:16:39 +0800 Subject: [PATCH 2/4] [#3702]refactor(API): Refactoring SupportsCatalogs.listCatalogs() method to return String[] (#3741) ### What changes were proposed in this pull request? Currently, the SupportsCatalogs.listCatalogs() method returns an array of NameIdentifier to represents the catalogs. Actually a String[] will be more clear and easier to use. ### Why are the changes needed? To make the API more clear to use. Fix: #3702 ### Does this PR introduce _any_ user-facing change? Almost not; Just return type changed, will be more simple. ### How was this patch tested? Yes, many test cases cover this API. --- .../com/datastrato/gravitino/SupportsCatalogs.java | 8 ++++---- .../hive/integration/test/CatalogHiveIT.java | 4 ++-- .../kafka/integration/test/CatalogKafkaIT.java | 4 ++-- .../gravitino/client/GravitinoClient.java | 3 +-- .../gravitino/client/GravitinoMetalake.java | 8 ++++---- .../gravitino/client/TestGravitinoMetalake.java | 8 ++++---- .../connector/catalog/GravitinoCatalogManager.java | 7 +++---- .../integration/test/trino/TrinoQueryIT.java | 4 ++-- .../connector/catalog/CatalogConnectorManager.java | 13 +++++-------- .../trino/connector/GravitinoMockServer.java | 8 +++----- 10 files changed, 30 insertions(+), 37 deletions(-) diff --git a/api/src/main/java/com/datastrato/gravitino/SupportsCatalogs.java b/api/src/main/java/com/datastrato/gravitino/SupportsCatalogs.java index ea91a2c0cca..7b773ff6ed8 100644 --- a/api/src/main/java/com/datastrato/gravitino/SupportsCatalogs.java +++ b/api/src/main/java/com/datastrato/gravitino/SupportsCatalogs.java @@ -18,12 +18,12 @@ public interface SupportsCatalogs { /** - * List all catalogs in the metalake. + * List the name of all catalogs in the metalake. * - * @return The list of catalog's name identifiers. - * @throws NoSuchMetalakeException If the metalake with namespace does not exist. + * @return The list of catalog's names. + * @throws NoSuchMetalakeException If the metalake does not exist. */ - NameIdentifier[] listCatalogs() throws NoSuchMetalakeException; + String[] listCatalogs() throws NoSuchMetalakeException; /** * List all catalogs with their information in the metalake. diff --git a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java index f6fcb1e971b..e76fbd65ea3 100644 --- a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java +++ b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/integration/test/CatalogHiveIT.java @@ -207,8 +207,8 @@ public static void stop() throws IOException { })); Arrays.stream(metalake.listCatalogs()) .forEach( - (ident -> { - metalake.dropCatalog(ident.name()); + (catalogName -> { + metalake.dropCatalog(catalogName); })); if (client != null) { client.dropMetalake(metalakeName); diff --git a/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java b/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java index 0fb14e2ec2e..8cf185ff289 100644 --- a/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java +++ b/catalogs/catalog-kafka/src/test/java/com/datastrato/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java @@ -108,8 +108,8 @@ public static void shutdown() { })); Arrays.stream(metalake.listCatalogs()) .forEach( - (ident -> { - metalake.dropCatalog(ident.name()); + (catalogName -> { + metalake.dropCatalog(catalogName); })); client.dropMetalake(METALAKE_NAME); if (adminClient != null) { diff --git a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoClient.java b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoClient.java index 598156d1c6d..6a200cb0a4d 100644 --- a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoClient.java +++ b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoClient.java @@ -7,7 +7,6 @@ import com.datastrato.gravitino.Catalog; import com.datastrato.gravitino.CatalogChange; -import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.SupportsCatalogs; import com.datastrato.gravitino.exceptions.CatalogAlreadyExistsException; import com.datastrato.gravitino.exceptions.NoSuchCatalogException; @@ -58,7 +57,7 @@ private GravitinoMetalake getMetalake() { } @Override - public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException { + public String[] listCatalogs() throws NoSuchMetalakeException { return getMetalake().listCatalogs(); } diff --git a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoMetalake.java b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoMetalake.java index 5b8cd070230..0ffa3c683bb 100644 --- a/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoMetalake.java +++ b/clients/client-java/src/main/java/com/datastrato/gravitino/client/GravitinoMetalake.java @@ -52,11 +52,11 @@ public class GravitinoMetalake extends MetalakeDTO implements SupportsCatalogs { /** * List all the catalogs under this metalake. * - * @return A list of {@link NameIdentifier} of the catalogs under the specified namespace. - * @throws NoSuchMetalakeException if the metalake with specified namespace does not exist. + * @return A list of the catalog names under the current metalake. + * @throws NoSuchMetalakeException If the metalake does not exist. */ @Override - public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException { + public String[] listCatalogs() throws NoSuchMetalakeException { EntityListResponse resp = restClient.get( @@ -66,7 +66,7 @@ public NameIdentifier[] listCatalogs() throws NoSuchMetalakeException { ErrorHandlers.catalogErrorHandler()); resp.validate(); - return resp.identifiers(); + return Arrays.stream(resp.identifiers()).map(NameIdentifier::name).toArray(String[]::new); } /** diff --git a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestGravitinoMetalake.java b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestGravitinoMetalake.java index 1969b8fff0d..2792ef9f6d5 100644 --- a/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestGravitinoMetalake.java +++ b/clients/client-java/src/test/java/com/datastrato/gravitino/client/TestGravitinoMetalake.java @@ -76,16 +76,16 @@ public void testListCatalogs() throws JsonProcessingException { EntityListResponse resp = new EntityListResponse(new NameIdentifier[] {ident1, ident2}); buildMockResource(Method.GET, path, null, resp, HttpStatus.SC_OK); - NameIdentifier[] catalogs = gravitinoClient.listCatalogs(); + String[] catalogs = gravitinoClient.listCatalogs(); Assertions.assertEquals(2, catalogs.length); - Assertions.assertEquals(ident1, catalogs[0]); - Assertions.assertEquals(ident2, catalogs[1]); + Assertions.assertEquals(ident1.name(), catalogs[0]); + Assertions.assertEquals(ident2.name(), catalogs[1]); // Test return empty catalog list EntityListResponse resp1 = new EntityListResponse(new NameIdentifier[] {}); buildMockResource(Method.GET, path, null, resp1, HttpStatus.SC_OK); - NameIdentifier[] catalogs1 = gravitinoClient.listCatalogs(); + String[] catalogs1 = gravitinoClient.listCatalogs(); Assertions.assertEquals(0, catalogs1.length); // Test return internal error diff --git a/flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/GravitinoCatalogManager.java b/flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/GravitinoCatalogManager.java index ed1cb527cc8..81ae254c323 100644 --- a/flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/GravitinoCatalogManager.java +++ b/flink-connector/src/main/java/com/datastrato/gravitino/flink/connector/catalog/GravitinoCatalogManager.java @@ -5,14 +5,13 @@ package com.datastrato.gravitino.flink.connector.catalog; import com.datastrato.gravitino.Catalog; -import com.datastrato.gravitino.NameIdentifier; import com.datastrato.gravitino.client.GravitinoAdminClient; import com.datastrato.gravitino.client.GravitinoMetalake; import com.google.common.base.Preconditions; +import com.google.common.collect.Sets; import java.util.Arrays; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -128,12 +127,12 @@ public boolean dropCatalog(String catalogName) { * @return Set of catalog names */ public Set listCatalogs() { - NameIdentifier[] catalogNames = metalake.listCatalogs(); + String[] catalogNames = metalake.listCatalogs(); LOG.info( "Load metalake {}'s catalogs. catalogs: {}.", metalake.name(), Arrays.toString(catalogNames)); - return Arrays.stream(catalogNames).map(NameIdentifier::name).collect(Collectors.toSet()); + return Sets.newHashSet(catalogNames); } /** diff --git a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoQueryIT.java b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoQueryIT.java index 312aefcdd4d..f95d0aa7eeb 100644 --- a/integration-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoQueryIT.java +++ b/integration-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoQueryIT.java @@ -79,8 +79,8 @@ public static void setup() throws Exception { private static void cleanupTestEnv() throws Exception { try { Arrays.stream(TrinoQueryITBase.metalake.listCatalogs()) - .filter(catalog -> catalog.name().startsWith("gt_")) - .forEach(catalog -> TrinoQueryITBase.dropCatalog(catalog.name())); + .filter(catalog -> catalog.startsWith("gt_")) + .forEach(TrinoQueryITBase::dropCatalog); await() .atMost(30, TimeUnit.SECONDS) diff --git a/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorManager.java b/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorManager.java index 3ab755b4389..fb76cfdaeaa 100644 --- a/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorManager.java +++ b/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorManager.java @@ -142,7 +142,7 @@ private GravitinoMetalake retrieveMetalake(String metalakeName) { } private void loadCatalogs(GravitinoMetalake metalake) { - NameIdentifier[] catalogNames; + String[] catalogNames; try { catalogNames = metalake.listCatalogs(); } catch (Exception e) { @@ -159,10 +159,7 @@ private void loadCatalogs(GravitinoMetalake metalake) { Set catalogNameStrings = Arrays.stream(catalogNames) .map( - id -> - config.simplifyCatalogNames() - ? id.name() - : getTrinoCatalogName(metalake.name(), id.name())) + id -> config.simplifyCatalogNames() ? id : getTrinoCatalogName(metalake.name(), id)) .collect(Collectors.toSet()); for (Map.Entry entry : catalogConnectors.entrySet()) { @@ -181,9 +178,9 @@ private void loadCatalogs(GravitinoMetalake metalake) { // Load new catalogs belows to the metalake. Arrays.stream(catalogNames) .forEach( - (NameIdentifier nameIdentifier) -> { + (String catalogName) -> { try { - Catalog catalog = metalake.loadCatalog(nameIdentifier.name()); + Catalog catalog = metalake.loadCatalog(catalogName); GravitinoCatalog gravitinoCatalog = new GravitinoCatalog(metalake.name(), catalog); if (catalogConnectors.containsKey(getTrinoCatalogName(gravitinoCatalog))) { // Reload catalogs that have been updated in Gravitino server. @@ -195,7 +192,7 @@ private void loadCatalogs(GravitinoMetalake metalake) { } } catch (Exception e) { LOG.error( - "Failed to load metalake {}'s catalog {}.", metalake.name(), nameIdentifier, e); + "Failed to load metalake {}'s catalog {}.", metalake.name(), catalogName, e); } }); } diff --git a/trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/GravitinoMockServer.java b/trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/GravitinoMockServer.java index 444b2f47360..875621c7999 100644 --- a/trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/GravitinoMockServer.java +++ b/trino-connector/src/test/java/com/datastrato/gravitino/trino/connector/GravitinoMockServer.java @@ -129,12 +129,10 @@ private GravitinoMetalake createMetalake(String metalakeName) { when(metaLake.name()).thenReturn(metalakeName); when(metaLake.listCatalogs()) .thenAnswer( - new Answer() { + new Answer() { @Override - public NameIdentifier[] answer(InvocationOnMock invocation) throws Throwable { - return metalakes.get(metalakeName).catalogs.keySet().stream() - .map(catalogName -> NameIdentifier.of(metalakeName, catalogName)) - .toArray(NameIdentifier[]::new); + public String[] answer(InvocationOnMock invocation) throws Throwable { + return metalakes.get(metalakeName).catalogs.keySet().toArray(String[]::new); }; }); From 42ba4025aded9d528c555e11cc1853ed9a17cbf1 Mon Sep 17 00:00:00 2001 From: mchades Date: Thu, 6 Jun 2024 05:07:17 +0800 Subject: [PATCH 3/4] [#3589] improvement(relational catalog, core): add data type converter api (#3590) ### What changes were proposed in this pull request? - Add `DataTypeConverter` interface for catalogs usage ### Why are the changes needed? Fix: #3589 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests Co-authored-by: Jerry Shao --- .../catalog/hive/HiveCatalogOperations.java | 7 +- .../gravitino/catalog/hive/HiveTable.java | 13 +- ...veType.java => HiveDataTypeConverter.java} | 114 +++++++++++++----- .../catalog/hive/converter/ToHiveType.java | 100 --------------- .../hive/converter/TestTypeConverter.java | 16 +-- .../jdbc/converter/JdbcTypeConverter.java | 22 +--- .../jdbc/operation/JdbcTableOperations.java | 2 +- .../jdbc/converter/SqliteTypeConverter.java | 6 +- .../jdbc/operation/SqliteTableOperations.java | 2 +- .../gravitino/catalog/doris/DorisCatalog.java | 2 +- .../doris/converter/DorisTypeConverter.java | 6 +- .../doris/operation/DorisTableOperations.java | 7 +- .../gravitino/catalog/mysql/MysqlCatalog.java | 4 +- .../mysql/converter/MysqlTypeConverter.java | 6 +- .../mysql/operation/MysqlTableOperations.java | 7 +- .../converter/TestMysqlTypeConverter.java | 6 +- .../catalog/postgresql/PostgreSqlCatalog.java | 2 +- .../converter/PostgreSqlTypeConverter.java | 10 +- .../operation/PostgreSqlTableOperations.java | 9 +- .../TestPostgreSqlTypeConverter.java | 8 +- .../iceberg/converter/ConvertUtil.java | 25 +--- .../converter/IcebergDataTypeConverter.java | 23 ++++ .../iceberg/ops/IcebergTableOpsHelper.java | 10 +- .../iceberg/converter/TestConvertUtil.java | 86 ++++++------- common/src/main/resources/project.properties | 7 ++ .../connector/DataTypeConverter.java | 33 +++++ 26 files changed, 250 insertions(+), 283 deletions(-) rename catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/{FromHiveType.java => HiveDataTypeConverter.java} (54%) delete mode 100644 catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/ToHiveType.java create mode 100644 catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/IcebergDataTypeConverter.java create mode 100644 common/src/main/resources/project.properties create mode 100644 core/src/main/java/com/datastrato/gravitino/connector/DataTypeConverter.java diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java index 13dd6740f55..4cecf5dded7 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java @@ -14,6 +14,7 @@ import static com.datastrato.gravitino.catalog.hive.HiveTable.TABLE_TYPE_PROP; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.COMMENT; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TABLE_TYPE; +import static com.datastrato.gravitino.catalog.hive.converter.HiveDataTypeConverter.CONVERTER; import static com.datastrato.gravitino.connector.BaseCatalog.CATALOG_BYPASS_PREFIX; import static org.apache.hadoop.hive.metastore.TableType.EXTERNAL_TABLE; @@ -21,7 +22,6 @@ import com.datastrato.gravitino.Namespace; import com.datastrato.gravitino.SchemaChange; import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TableType; -import com.datastrato.gravitino.catalog.hive.converter.ToHiveType; import com.datastrato.gravitino.connector.CatalogInfo; import com.datastrato.gravitino.connector.CatalogOperations; import com.datastrato.gravitino.connector.HasPropertyMetadata; @@ -983,7 +983,7 @@ private void doAddColumn(List cols, TableChange.AddColumn change) { targetPosition, new FieldSchema( change.fieldName()[0], - ToHiveType.convert(change.getDataType()).getQualifiedName(), + CONVERTER.fromGravitino(change.getDataType()).getQualifiedName(), change.getComment())); } @@ -1031,7 +1031,8 @@ private void doUpdateColumnType(List cols, TableChange.UpdateColumn if (indexOfColumn == -1) { throw new IllegalArgumentException("UpdateColumnType does not exist: " + columnName); } - cols.get(indexOfColumn).setType(ToHiveType.convert(change.getNewDataType()).getQualifiedName()); + cols.get(indexOfColumn) + .setType(CONVERTER.fromGravitino(change.getNewDataType()).getQualifiedName()); } /** diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java index 267b8265eda..c390deec366 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveTable.java @@ -16,11 +16,10 @@ import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TABLE_TYPE; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TableType.EXTERNAL_TABLE; import static com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TableType.MANAGED_TABLE; +import static com.datastrato.gravitino.catalog.hive.converter.HiveDataTypeConverter.CONVERTER; import static com.datastrato.gravitino.rel.expressions.transforms.Transforms.identity; import com.datastrato.gravitino.catalog.hive.HiveTablePropertiesMetadata.TableType; -import com.datastrato.gravitino.catalog.hive.converter.FromHiveType; -import com.datastrato.gravitino.catalog.hive.converter.ToHiveType; import com.datastrato.gravitino.connector.BaseTable; import com.datastrato.gravitino.connector.PropertiesMetadata; import com.datastrato.gravitino.connector.TableOperations; @@ -114,7 +113,7 @@ public static HiveTable.Builder fromHiveTable(Table table) { f -> HiveColumn.builder() .withName(f.getName()) - .withType(FromHiveType.convert(f.getType())) + .withType(CONVERTER.toGravitino(f.getType())) .withComment(f.getComment()) .build()), table.getPartitionKeys().stream() @@ -122,7 +121,7 @@ public static HiveTable.Builder fromHiveTable(Table table) { p -> HiveColumn.builder() .withName(p.getName()) - .withType(FromHiveType.convert(p.getType())) + .withType(CONVERTER.toGravitino(p.getType())) .withComment(p.getComment()) .build())) .toArray(Column[]::new); @@ -241,7 +240,7 @@ private FieldSchema getPartitionKey(String[] fieldName) { .collect(Collectors.toList()); return new FieldSchema( partitionColumns.get(0).name(), - ToHiveType.convert(partitionColumns.get(0).dataType()).getQualifiedName(), + CONVERTER.fromGravitino(partitionColumns.get(0).dataType()).getQualifiedName(), partitionColumns.get(0).comment()); } @@ -256,7 +255,9 @@ private StorageDescriptor buildStorageDescriptor( .map( c -> new FieldSchema( - c.name(), ToHiveType.convert(c.dataType()).getQualifiedName(), c.comment())) + c.name(), + CONVERTER.fromGravitino(c.dataType()).getQualifiedName(), + c.comment())) .collect(Collectors.toList())); // `location` must not be null, otherwise it will result in an NPE when calling HMS `alterTable` diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/FromHiveType.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/HiveDataTypeConverter.java similarity index 54% rename from catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/FromHiveType.java rename to catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/HiveDataTypeConverter.java index 6d3e6e3477e..009dd229dac 100644 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/FromHiveType.java +++ b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/HiveDataTypeConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 Datastrato Pvt Ltd. + * Copyright 2024 Datastrato Pvt Ltd. * This software is licensed under the Apache License version 2. */ package com.datastrato.gravitino.catalog.hive.converter; @@ -17,12 +17,23 @@ import static org.apache.hadoop.hive.serde.serdeConstants.STRING_TYPE_NAME; import static org.apache.hadoop.hive.serde.serdeConstants.TIMESTAMP_TYPE_NAME; import static org.apache.hadoop.hive.serde.serdeConstants.TINYINT_TYPE_NAME; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getCharTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getDecimalTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getListTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getMapTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getPrimitiveTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getStructTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getUnionTypeInfo; +import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getVarcharTypeInfo; import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoUtils.getTypeInfoFromTypeString; +import com.datastrato.gravitino.connector.DataTypeConverter; import com.datastrato.gravitino.rel.types.Type; import com.datastrato.gravitino.rel.types.Types; -import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.hadoop.hive.serde2.typeinfo.CharTypeInfo; import org.apache.hadoop.hive.serde2.typeinfo.DecimalTypeInfo; @@ -33,28 +44,77 @@ import org.apache.hadoop.hive.serde2.typeinfo.UnionTypeInfo; import org.apache.hadoop.hive.serde2.typeinfo.VarcharTypeInfo; -/** Converts Hive data types to corresponding Gravitino data types. */ -public class FromHiveType { +public class HiveDataTypeConverter implements DataTypeConverter { + public static final HiveDataTypeConverter CONVERTER = new HiveDataTypeConverter(); - /** - * Converts a Hive data type string to the corresponding Gravitino data type. - * - * @param hiveType The Hive data type string to convert. - * @return The equivalent Gravitino data type. - */ - public static Type convert(String hiveType) { - TypeInfo hiveTypeInfo = getTypeInfoFromTypeString(hiveType); - return toGravitinoType(hiveTypeInfo); + @Override + public TypeInfo fromGravitino(Type type) { + switch (type.name()) { + case BOOLEAN: + return getPrimitiveTypeInfo(BOOLEAN_TYPE_NAME); + case BYTE: + return getPrimitiveTypeInfo(TINYINT_TYPE_NAME); + case SHORT: + return getPrimitiveTypeInfo(SMALLINT_TYPE_NAME); + case INTEGER: + return getPrimitiveTypeInfo(INT_TYPE_NAME); + case LONG: + return getPrimitiveTypeInfo(BIGINT_TYPE_NAME); + case FLOAT: + return getPrimitiveTypeInfo(FLOAT_TYPE_NAME); + case DOUBLE: + return getPrimitiveTypeInfo(DOUBLE_TYPE_NAME); + case STRING: + return getPrimitiveTypeInfo(STRING_TYPE_NAME); + case VARCHAR: + return getVarcharTypeInfo(((Types.VarCharType) type).length()); + case FIXEDCHAR: + return getCharTypeInfo(((Types.FixedCharType) type).length()); + case DATE: + return getPrimitiveTypeInfo(DATE_TYPE_NAME); + case TIMESTAMP: + return getPrimitiveTypeInfo(TIMESTAMP_TYPE_NAME); + case DECIMAL: + Types.DecimalType decimalType = (Types.DecimalType) type; + return getDecimalTypeInfo(decimalType.precision(), decimalType.scale()); + case BINARY: + return getPrimitiveTypeInfo(BINARY_TYPE_NAME); + case INTERVAL_YEAR: + return getPrimitiveTypeInfo(INTERVAL_YEAR_MONTH_TYPE_NAME); + case INTERVAL_DAY: + return getPrimitiveTypeInfo(INTERVAL_DAY_TIME_TYPE_NAME); + case LIST: + return getListTypeInfo(fromGravitino(((Types.ListType) type).elementType())); + case MAP: + Types.MapType mapType = (Types.MapType) type; + return getMapTypeInfo(fromGravitino(mapType.keyType()), fromGravitino(mapType.valueType())); + case STRUCT: + Types.StructType structType = (Types.StructType) type; + List typeInfos = + Arrays.stream(structType.fields()) + .map(t -> fromGravitino(t.type())) + .collect(Collectors.toList()); + List names = + Arrays.stream(structType.fields()) + .map(Types.StructType.Field::name) + .collect(Collectors.toList()); + return getStructTypeInfo(names, typeInfos); + case UNION: + return getUnionTypeInfo( + Arrays.stream(((Types.UnionType) type).types()) + .map(this::fromGravitino) + .collect(Collectors.toList())); + default: + throw new UnsupportedOperationException("Unsupported conversion to Hive type: " + type); + } + } + + @Override + public Type toGravitino(String hiveType) { + return toGravitino(getTypeInfoFromTypeString(hiveType)); } - /** - * Converts a Hive TypeInfo object to the corresponding Gravitino Type. - * - * @param hiveTypeInfo The Hive TypeInfo object to convert. - * @return The equivalent Gravitino Type. - */ - @VisibleForTesting - public static Type toGravitinoType(TypeInfo hiveTypeInfo) { + private Type toGravitino(TypeInfo hiveTypeInfo) { switch (hiveTypeInfo.getCategory()) { case PRIMITIVE: switch (hiveTypeInfo.getTypeName()) { @@ -102,12 +162,12 @@ public static Type toGravitinoType(TypeInfo hiveTypeInfo) { } case LIST: return Types.ListType.nullable( - toGravitinoType(((ListTypeInfo) hiveTypeInfo).getListElementTypeInfo())); + toGravitino(((ListTypeInfo) hiveTypeInfo).getListElementTypeInfo())); case MAP: MapTypeInfo mapTypeInfo = (MapTypeInfo) hiveTypeInfo; return Types.MapType.valueNullable( - toGravitinoType(mapTypeInfo.getMapKeyTypeInfo()), - toGravitinoType(mapTypeInfo.getMapValueTypeInfo())); + toGravitino(mapTypeInfo.getMapKeyTypeInfo()), + toGravitino(mapTypeInfo.getMapValueTypeInfo())); case STRUCT: StructTypeInfo structTypeInfo = (StructTypeInfo) hiveTypeInfo; ArrayList fieldNames = structTypeInfo.getAllStructFieldNames(); @@ -117,19 +177,17 @@ public static Type toGravitinoType(TypeInfo hiveTypeInfo) { .mapToObj( i -> Types.StructType.Field.nullableField( - fieldNames.get(i), toGravitinoType(typeInfos.get(i)))) + fieldNames.get(i), toGravitino(typeInfos.get(i)))) .toArray(Types.StructType.Field[]::new); return Types.StructType.of(fields); case UNION: UnionTypeInfo unionTypeInfo = (UnionTypeInfo) hiveTypeInfo; return Types.UnionType.of( unionTypeInfo.getAllUnionObjectTypeInfos().stream() - .map(FromHiveType::toGravitinoType) + .map(this::toGravitino) .toArray(Type[]::new)); default: return Types.ExternalType.of(hiveTypeInfo.getQualifiedName()); } } - - private FromHiveType() {} } diff --git a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/ToHiveType.java b/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/ToHiveType.java deleted file mode 100644 index b29481ebc4c..00000000000 --- a/catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/ToHiveType.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2023 Datastrato Pvt Ltd. - * This software is licensed under the Apache License version 2. - */ -package com.datastrato.gravitino.catalog.hive.converter; - -import static org.apache.hadoop.hive.serde.serdeConstants.BIGINT_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.BINARY_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.BOOLEAN_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.DATE_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.DOUBLE_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.FLOAT_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.INTERVAL_DAY_TIME_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.INTERVAL_YEAR_MONTH_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.INT_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.SMALLINT_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.STRING_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.TIMESTAMP_TYPE_NAME; -import static org.apache.hadoop.hive.serde.serdeConstants.TINYINT_TYPE_NAME; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getCharTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getDecimalTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getListTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getMapTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getPrimitiveTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getStructTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getUnionTypeInfo; -import static org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory.getVarcharTypeInfo; - -import com.datastrato.gravitino.rel.types.Type; -import com.datastrato.gravitino.rel.types.Types; -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; -import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo; - -/** Converts Gravitino data types to corresponding Hive data types. */ -public class ToHiveType { - public static TypeInfo convert(Type type) { - switch (type.name()) { - case BOOLEAN: - return getPrimitiveTypeInfo(BOOLEAN_TYPE_NAME); - case BYTE: - return getPrimitiveTypeInfo(TINYINT_TYPE_NAME); - case SHORT: - return getPrimitiveTypeInfo(SMALLINT_TYPE_NAME); - case INTEGER: - return getPrimitiveTypeInfo(INT_TYPE_NAME); - case LONG: - return getPrimitiveTypeInfo(BIGINT_TYPE_NAME); - case FLOAT: - return getPrimitiveTypeInfo(FLOAT_TYPE_NAME); - case DOUBLE: - return getPrimitiveTypeInfo(DOUBLE_TYPE_NAME); - case STRING: - return getPrimitiveTypeInfo(STRING_TYPE_NAME); - case VARCHAR: - return getVarcharTypeInfo(((Types.VarCharType) type).length()); - case FIXEDCHAR: - return getCharTypeInfo(((Types.FixedCharType) type).length()); - case DATE: - return getPrimitiveTypeInfo(DATE_TYPE_NAME); - case TIMESTAMP: - return getPrimitiveTypeInfo(TIMESTAMP_TYPE_NAME); - case DECIMAL: - Types.DecimalType decimalType = (Types.DecimalType) type; - return getDecimalTypeInfo(decimalType.precision(), decimalType.scale()); - case BINARY: - return getPrimitiveTypeInfo(BINARY_TYPE_NAME); - case INTERVAL_YEAR: - return getPrimitiveTypeInfo(INTERVAL_YEAR_MONTH_TYPE_NAME); - case INTERVAL_DAY: - return getPrimitiveTypeInfo(INTERVAL_DAY_TIME_TYPE_NAME); - case LIST: - return getListTypeInfo(convert(((Types.ListType) type).elementType())); - case MAP: - Types.MapType mapType = (Types.MapType) type; - return getMapTypeInfo(convert(mapType.keyType()), convert(mapType.valueType())); - case STRUCT: - Types.StructType structType = (Types.StructType) type; - List typeInfos = - Arrays.stream(structType.fields()) - .map(t -> convert(t.type())) - .collect(Collectors.toList()); - List names = - Arrays.stream(structType.fields()) - .map(Types.StructType.Field::name) - .collect(Collectors.toList()); - return getStructTypeInfo(names, typeInfos); - case UNION: - return getUnionTypeInfo( - Arrays.stream(((Types.UnionType) type).types()) - .map(ToHiveType::convert) - .collect(Collectors.toList())); - default: - throw new UnsupportedOperationException("Unsupported conversion to Hive type: " + type); - } - } - - private ToHiveType() {} -} diff --git a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/converter/TestTypeConverter.java b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/converter/TestTypeConverter.java index 15947229893..15b68cec655 100644 --- a/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/converter/TestTypeConverter.java +++ b/catalogs/catalog-hive/src/test/java/com/datastrato/gravitino/catalog/hive/converter/TestTypeConverter.java @@ -4,6 +4,7 @@ */ package com.datastrato.gravitino.catalog.hive.converter; +import static com.datastrato.gravitino.catalog.hive.converter.HiveDataTypeConverter.CONVERTER; import static org.apache.hadoop.hive.serde.serdeConstants.BIGINT_TYPE_NAME; import static org.apache.hadoop.hive.serde.serdeConstants.BINARY_TYPE_NAME; import static org.apache.hadoop.hive.serde.serdeConstants.BOOLEAN_TYPE_NAME; @@ -29,7 +30,6 @@ import com.datastrato.gravitino.rel.types.Types; import java.util.Arrays; -import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo; import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -72,24 +72,14 @@ public void testTypeConverter() { Arrays.asList( getPrimitiveTypeInfo(STRING_TYPE_NAME), getPrimitiveTypeInfo(INT_TYPE_NAME))) .getTypeName()); - Assertions.assertEquals( - Types.ExternalType.of(USER_DEFINED_TYPE), - FromHiveType.toGravitinoType(new UserDefinedTypeInfo())); Assertions.assertThrows( UnsupportedOperationException.class, - () -> ToHiveType.convert(Types.ExternalType.of(USER_DEFINED_TYPE))); + () -> CONVERTER.fromGravitino(Types.ExternalType.of(USER_DEFINED_TYPE))); } private void testConverter(String typeName) { TypeInfo hiveType = getTypeInfoFromTypeString(typeName); - TypeInfo convertedType = ToHiveType.convert(FromHiveType.convert(hiveType.getTypeName())); + TypeInfo convertedType = CONVERTER.fromGravitino(CONVERTER.toGravitino(hiveType.getTypeName())); Assertions.assertEquals(hiveType, convertedType); } - - static class UserDefinedTypeInfo extends PrimitiveTypeInfo { - @Override - public String getTypeName() { - return USER_DEFINED_TYPE; - } - } } diff --git a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/converter/JdbcTypeConverter.java b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/converter/JdbcTypeConverter.java index e9bc3443a49..477686cbb54 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/converter/JdbcTypeConverter.java +++ b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/converter/JdbcTypeConverter.java @@ -4,11 +4,11 @@ */ package com.datastrato.gravitino.catalog.jdbc.converter; -import com.datastrato.gravitino.rel.types.Type; +import com.datastrato.gravitino.connector.DataTypeConverter; import java.util.Objects; -/** @param Implement the corresponding JDBC data type to be converted */ -public abstract class JdbcTypeConverter { +public abstract class JdbcTypeConverter + implements DataTypeConverter { public static final String DATE = "date"; public static final String TIME = "time"; @@ -16,22 +16,6 @@ public abstract class JdbcTypeConverter { public static final String VARCHAR = "varchar"; public static final String TEXT = "text"; - /** - * Convert from JDBC type to Gravitino type - * - * @param type The common jdbc type bean. - * @return Gravitino type. - */ - public abstract Type toGravitinoType(JdbcTypeBean type); - - /** - * Convert from Gravitino type to JDBC type - * - * @param type Gravitino type. - * @return Implement the corresponding JDBC data type to be converted. - */ - public abstract TO fromGravitinoType(Type type); - public static class JdbcTypeBean { /** Data type name. */ private String typeName; diff --git a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java index 126e792905d..c2fefce2068 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java +++ b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java @@ -456,7 +456,7 @@ protected JdbcColumn.Builder getBasicJdbcColumnInfo(ResultSet column) throws SQL return JdbcColumn.builder() .withName(column.getString("COLUMN_NAME")) - .withType(typeConverter.toGravitinoType(typeBean)) + .withType(typeConverter.toGravitino(typeBean)) .withComment(StringUtils.isEmpty(comment) ? null : comment) .withNullable(nullable) .withDefaultValue(defaultValue); diff --git a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/converter/SqliteTypeConverter.java b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/converter/SqliteTypeConverter.java index 9e6ec75756f..76e8882c70f 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/converter/SqliteTypeConverter.java +++ b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/converter/SqliteTypeConverter.java @@ -11,7 +11,7 @@ import java.util.Map; import org.apache.commons.lang3.StringUtils; -public class SqliteTypeConverter extends JdbcTypeConverter { +public class SqliteTypeConverter extends JdbcTypeConverter { protected static final Map GRAVITINO_TO_SQLITE_MAPPING = new HashMap<>(); @@ -23,7 +23,7 @@ public class SqliteTypeConverter extends JdbcTypeConverter { } @Override - public Type toGravitinoType(JdbcTypeBean type) { + public Type toGravitino(JdbcTypeBean type) { return GRAVITINO_TO_SQLITE_MAPPING.entrySet().stream() .filter(entry -> StringUtils.equalsIgnoreCase(type.getTypeName(), entry.getValue())) .map(Map.Entry::getKey) @@ -32,7 +32,7 @@ public Type toGravitinoType(JdbcTypeBean type) { } @Override - public String fromGravitinoType(Type type) { + public String fromGravitino(Type type) { return GRAVITINO_TO_SQLITE_MAPPING.get(type); } diff --git a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java index 66fa734ce08..65f8d705495 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java +++ b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java @@ -42,7 +42,7 @@ protected String generateCreateTableSql( sqlBuilder .append(column.name()) .append(" ") - .append(typeConverter.fromGravitinoType(column.dataType())); + .append(typeConverter.fromGravitino(column.dataType())); if (!column.nullable()) { sqlBuilder.append(" NOT NULL"); } diff --git a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java index 30e25987bc2..035182f4e97 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java @@ -29,7 +29,7 @@ public String shortName() { @Override protected CatalogOperations newOps(Map config) { - JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); + JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); return new MySQLProtocolCompatibleCatalogOperations( createExceptionConverter(), jdbcTypeConverter, diff --git a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/converter/DorisTypeConverter.java b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/converter/DorisTypeConverter.java index fa7fc1595f4..4793ade72aa 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/converter/DorisTypeConverter.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/converter/DorisTypeConverter.java @@ -9,7 +9,7 @@ import com.datastrato.gravitino.rel.types.Types; /** Type converter for Doris. */ -public class DorisTypeConverter extends JdbcTypeConverter { +public class DorisTypeConverter extends JdbcTypeConverter { static final String BOOLEAN = "boolean"; static final String TINYINT = "tinyint"; static final String SMALLINT = "smallint"; @@ -23,7 +23,7 @@ public class DorisTypeConverter extends JdbcTypeConverter { static final String STRING = "string"; @Override - public Type toGravitinoType(JdbcTypeBean typeBean) { + public Type toGravitino(JdbcTypeBean typeBean) { switch (typeBean.getTypeName().toLowerCase()) { case BOOLEAN: return Types.BooleanType.get(); @@ -59,7 +59,7 @@ public Type toGravitinoType(JdbcTypeBean typeBean) { } @Override - public String fromGravitinoType(Type type) { + public String fromGravitino(Type type) { if (type instanceof Types.BooleanType) { return BOOLEAN; } else if (type instanceof Types.ByteType) { diff --git a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java index 3a1100e576b..32b1133523f 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java @@ -523,7 +523,7 @@ private String updateColumnCommentFieldDefinition( } private String addColumnFieldDefinition(TableChange.AddColumn addColumn) { - String dataType = (String) typeConverter.fromGravitinoType(addColumn.getDataType()); + String dataType = typeConverter.fromGravitino(addColumn.getDataType()); if (addColumn.fieldName().length > 1) { throw new UnsupportedOperationException("Doris does not support nested column names."); } @@ -637,10 +637,7 @@ private String updateColumnTypeFieldDefinition( private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sqlBuilder) { // Add data type - sqlBuilder - .append(SPACE) - .append(typeConverter.fromGravitinoType(column.dataType())) - .append(SPACE); + sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); // Add NOT NULL if the column is marked as such if (column.nullable()) { diff --git a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java index 69ee53a0dd0..0ed1c3e8441 100644 --- a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java +++ b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java @@ -33,7 +33,7 @@ public String shortName() { @Override protected CatalogOperations newOps(Map config) { - JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); + JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); return new MySQLProtocolCompatibleCatalogOperations( createExceptionConverter(), jdbcTypeConverter, @@ -48,7 +48,7 @@ protected JdbcExceptionConverter createExceptionConverter() { } @Override - protected JdbcTypeConverter createJdbcTypeConverter() { + protected JdbcTypeConverter createJdbcTypeConverter() { return new MysqlTypeConverter(); } diff --git a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java index b82f4629a08..a32550bea98 100644 --- a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java +++ b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java @@ -9,7 +9,7 @@ import com.datastrato.gravitino.rel.types.Types; /** Type converter for MySQL. */ -public class MysqlTypeConverter extends JdbcTypeConverter { +public class MysqlTypeConverter extends JdbcTypeConverter { static final String TINYINT = "tinyint"; static final String SMALLINT = "smallint"; @@ -23,7 +23,7 @@ public class MysqlTypeConverter extends JdbcTypeConverter { static final String DATETIME = "datetime"; @Override - public Type toGravitinoType(JdbcTypeBean typeBean) { + public Type toGravitino(JdbcTypeBean typeBean) { switch (typeBean.getTypeName().toLowerCase()) { case TINYINT: return Types.ByteType.get(); @@ -66,7 +66,7 @@ public Type toGravitinoType(JdbcTypeBean typeBean) { } @Override - public String fromGravitinoType(Type type) { + public String fromGravitino(Type type) { if (type instanceof Types.ByteType) { return TINYINT; } else if (type instanceof Types.ShortType) { diff --git a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java index 0446713c77b..2f0573581c4 100644 --- a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java +++ b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java @@ -508,7 +508,7 @@ private String updateColumnCommentFieldDefinition( } private String addColumnFieldDefinition(TableChange.AddColumn addColumn) { - String dataType = (String) typeConverter.fromGravitinoType(addColumn.getDataType()); + String dataType = typeConverter.fromGravitino(addColumn.getDataType()); if (addColumn.fieldName().length > 1) { throw new UnsupportedOperationException(MYSQL_NOT_SUPPORT_NESTED_COLUMN_MSG); } @@ -683,10 +683,7 @@ private String updateColumnTypeFieldDefinition( private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sqlBuilder) { // Add data type - sqlBuilder - .append(SPACE) - .append(typeConverter.fromGravitinoType(column.dataType())) - .append(SPACE); + sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); // Add NOT NULL if the column is marked as such if (column.nullable()) { diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java index 62aa8c0ed44..6d31b5b8068 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java @@ -70,17 +70,17 @@ public void testFromGravitinoType() { checkGravitinoTypeToJdbcType(USER_DEFINED_TYPE, Types.ExternalType.of(USER_DEFINED_TYPE)); Assertions.assertThrows( IllegalArgumentException.class, - () -> MYSQL_TYPE_CONVERTER.fromGravitinoType(Types.UnparsedType.of(USER_DEFINED_TYPE))); + () -> MYSQL_TYPE_CONVERTER.fromGravitino(Types.UnparsedType.of(USER_DEFINED_TYPE))); } protected void checkGravitinoTypeToJdbcType(String jdbcTypeName, Type gravitinoType) { - Assertions.assertEquals(jdbcTypeName, MYSQL_TYPE_CONVERTER.fromGravitinoType(gravitinoType)); + Assertions.assertEquals(jdbcTypeName, MYSQL_TYPE_CONVERTER.fromGravitino(gravitinoType)); } protected void checkJdbcTypeToGravitinoType( Type gravitinoType, String jdbcTypeName, String columnSize, String scale) { JdbcTypeConverter.JdbcTypeBean typeBean = createTypeBean(jdbcTypeName, columnSize, scale); - Assertions.assertEquals(gravitinoType, MYSQL_TYPE_CONVERTER.toGravitinoType(typeBean)); + Assertions.assertEquals(gravitinoType, MYSQL_TYPE_CONVERTER.toGravitino(typeBean)); } protected static JdbcTypeConverter.JdbcTypeBean createTypeBean( diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java index 90d7ad1c16f..c6293500af8 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java @@ -27,7 +27,7 @@ public String shortName() { @Override protected CatalogOperations newOps(Map config) { - JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); + JdbcTypeConverter jdbcTypeConverter = createJdbcTypeConverter(); return new PostgreSQLCatalogOperations( createExceptionConverter(), jdbcTypeConverter, diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java index 77956e171e5..e936d1cbda5 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java @@ -11,7 +11,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -public class PostgreSqlTypeConverter extends JdbcTypeConverter { +public class PostgreSqlTypeConverter extends JdbcTypeConverter { static final String BOOL = "bool"; static final String INT_2 = "int2"; @@ -28,7 +28,7 @@ public class PostgreSqlTypeConverter extends JdbcTypeConverter { @VisibleForTesting static final String ARRAY_TOKEN = "[]"; @Override - public Type toGravitinoType(JdbcTypeBean typeBean) { + public Type toGravitino(JdbcTypeBean typeBean) { String typeName = typeBean.getTypeName().toLowerCase(); if (typeName.startsWith(JDBC_ARRAY_PREFIX)) { return toGravitinoArrayType(typeName); @@ -71,7 +71,7 @@ public Type toGravitinoType(JdbcTypeBean typeBean) { } @Override - public String fromGravitinoType(Type type) { + public String fromGravitino(Type type) { if (type instanceof Types.BooleanType) { return BOOL; } else if (type instanceof Types.ShortType) { @@ -130,13 +130,13 @@ private String fromGravitinoArrayType(ListType listType) { Preconditions.checkArgument( !(elementType instanceof ListType), "PostgreSQL doesn't support multidimensional list internally, please use one dimensional list"); - String elementTypeString = fromGravitinoType(elementType); + String elementTypeString = fromGravitino(elementType); return elementTypeString + ARRAY_TOKEN; } private ListType toGravitinoArrayType(String typeName) { String elementTypeName = typeName.substring(JDBC_ARRAY_PREFIX.length(), typeName.length()); JdbcTypeBean bean = new JdbcTypeBean(elementTypeName); - return ListType.of(toGravitinoType(bean), false); + return ListType.of(toGravitino(bean), false); } } diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java index 57e9006655f..e03d9b7ea77 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java @@ -210,10 +210,7 @@ private static String getIndexFieldStr(String[][] fieldNames) { private void appendColumnDefinition(JdbcColumn column, StringBuilder sqlBuilder) { // Add data type - sqlBuilder - .append(SPACE) - .append(typeConverter.fromGravitinoType(column.dataType())) - .append(SPACE); + sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); if (column.autoIncrement()) { if (!Types.allowAutoIncrement(column.dataType())) { @@ -532,7 +529,7 @@ private String updateColumnTypeFieldDefinition( .append(col) .append(PG_QUOTE) .append(" SET DATA TYPE ") - .append(typeConverter.fromGravitinoType(updateColumnType.getNewDataType())); + .append(typeConverter.fromGravitino(updateColumnType.getNewDataType())); if (!column.nullable()) { sqlBuilder .append(",\n") @@ -591,7 +588,7 @@ private List addColumnFieldDefinition( .append(col) .append(PG_QUOTE) .append(SPACE) - .append(typeConverter.fromGravitinoType(addColumn.getDataType())) + .append(typeConverter.fromGravitino(addColumn.getDataType())) .append(SPACE); if (addColumn.isAutoIncrement()) { diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java index 4710073d300..54f2ca38af0 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java @@ -94,19 +94,17 @@ public void testFromGravitinoType() { checkGravitinoTypeToJdbcType(USER_DEFINED_TYPE, Types.ExternalType.of(USER_DEFINED_TYPE)); Assertions.assertThrows( IllegalArgumentException.class, - () -> - POSTGRE_SQL_TYPE_CONVERTER.fromGravitinoType(Types.UnparsedType.of(USER_DEFINED_TYPE))); + () -> POSTGRE_SQL_TYPE_CONVERTER.fromGravitino(Types.UnparsedType.of(USER_DEFINED_TYPE))); } protected void checkGravitinoTypeToJdbcType(String jdbcTypeName, Type gravitinoType) { - Assertions.assertEquals( - jdbcTypeName, POSTGRE_SQL_TYPE_CONVERTER.fromGravitinoType(gravitinoType)); + Assertions.assertEquals(jdbcTypeName, POSTGRE_SQL_TYPE_CONVERTER.fromGravitino(gravitinoType)); } protected void checkJdbcTypeToGravitinoType( Type gravitinoType, String jdbcTypeName, String columnSize, String scale) { JdbcTypeConverter.JdbcTypeBean typeBean = createTypeBean(jdbcTypeName, columnSize, scale); - Assertions.assertEquals(gravitinoType, POSTGRE_SQL_TYPE_CONVERTER.toGravitinoType(typeBean)); + Assertions.assertEquals(gravitinoType, POSTGRE_SQL_TYPE_CONVERTER.toGravitino(typeBean)); } protected static JdbcTypeConverter.JdbcTypeBean createTypeBean( diff --git a/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/ConvertUtil.java b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/ConvertUtil.java index c50c9d7aa38..20b11bb4691 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/ConvertUtil.java +++ b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/ConvertUtil.java @@ -4,12 +4,13 @@ */ package com.datastrato.gravitino.catalog.lakehouse.iceberg.converter; +import static com.datastrato.gravitino.catalog.lakehouse.iceberg.converter.IcebergDataTypeConverter.CONVERTER; + import com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergColumn; import com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergTable; import java.util.Arrays; import org.apache.iceberg.Schema; import org.apache.iceberg.types.Type; -import org.apache.iceberg.types.TypeUtil; import org.apache.iceberg.types.Types; public class ConvertUtil { @@ -28,26 +29,6 @@ public static Schema toIcebergSchema(IcebergTable gravitinoTable) { return new Schema(converted.asNestedType().asStructType().fields()); } - /** - * Convert the Gravitino type to the Iceberg type. - * - * @param gravitinoType Gravitino type. - * @return Iceberg type. - */ - public static Type toIcebergType(com.datastrato.gravitino.rel.types.Type gravitinoType) { - return ToIcebergTypeVisitor.visit(gravitinoType, new ToIcebergType()); - } - - /** - * Convert the nested type of Iceberg to the type of gravitino. - * - * @param type Iceberg type of field. - * @return Gravitino type. - */ - public static com.datastrato.gravitino.rel.types.Type formIcebergType(Type type) { - return TypeUtil.visit(type, new FromIcebergType()); - } - /** * Convert the nested field of Iceberg to the Iceberg column. * @@ -59,7 +40,7 @@ public static IcebergColumn fromNestedField(Types.NestedField nestedField) { .withName(nestedField.name()) .withNullable(nestedField.isOptional()) .withComment(nestedField.doc()) - .withType(ConvertUtil.formIcebergType(nestedField.type())) + .withType(CONVERTER.toGravitino(nestedField.type())) .build(); } diff --git a/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/IcebergDataTypeConverter.java b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/IcebergDataTypeConverter.java new file mode 100644 index 00000000000..3522f47e668 --- /dev/null +++ b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/IcebergDataTypeConverter.java @@ -0,0 +1,23 @@ +/* + * Copyright 2024 Datastrato Pvt Ltd. + * This software is licensed under the Apache License version 2. + */ +package com.datastrato.gravitino.catalog.lakehouse.iceberg.converter; + +import com.datastrato.gravitino.connector.DataTypeConverter; +import org.apache.iceberg.types.Type; +import org.apache.iceberg.types.TypeUtil; + +public class IcebergDataTypeConverter implements DataTypeConverter { + public static final IcebergDataTypeConverter CONVERTER = new IcebergDataTypeConverter(); + + @Override + public Type fromGravitino(com.datastrato.gravitino.rel.types.Type gravitinoType) { + return ToIcebergTypeVisitor.visit(gravitinoType, new ToIcebergType()); + } + + @Override + public com.datastrato.gravitino.rel.types.Type toGravitino(Type type) { + return TypeUtil.visit(type, new FromIcebergType()); + } +} diff --git a/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/ops/IcebergTableOpsHelper.java b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/ops/IcebergTableOpsHelper.java index 6c87ee98a49..add54ce2797 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/ops/IcebergTableOpsHelper.java +++ b/catalogs/catalog-lakehouse-iceberg/src/main/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/ops/IcebergTableOpsHelper.java @@ -5,8 +5,9 @@ package com.datastrato.gravitino.catalog.lakehouse.iceberg.ops; +import static com.datastrato.gravitino.catalog.lakehouse.iceberg.converter.IcebergDataTypeConverter.CONVERTER; + import com.datastrato.gravitino.NameIdentifier; -import com.datastrato.gravitino.catalog.lakehouse.iceberg.converter.ConvertUtil; import com.datastrato.gravitino.rel.TableChange; import com.datastrato.gravitino.rel.TableChange.AddColumn; import com.datastrato.gravitino.rel.TableChange.After; @@ -150,8 +151,7 @@ private void doUpdateColumnType( fieldName); icebergTableSchema.findField(fieldName).isOptional(); - org.apache.iceberg.types.Type type = - ConvertUtil.toIcebergType(updateColumnType.getNewDataType()); + org.apache.iceberg.types.Type type = CONVERTER.fromGravitino(updateColumnType.getNewDataType()); Preconditions.checkArgument( type.isPrimitiveType(), "Cannot update %s, not a primitive type: %s", fieldName, type); icebergUpdateSchema.updateColumn(fieldName, (PrimitiveType) type); @@ -199,7 +199,7 @@ private void doAddColumn( icebergUpdateSchema.addColumn( getParentName(addColumn.fieldName()), getLeafName(addColumn.fieldName()), - ConvertUtil.toIcebergType(addColumn.getDataType()), + CONVERTER.fromGravitino(addColumn.getDataType()), addColumn.getComment()); } else { // TODO: figure out how to enable users to add required columns @@ -207,7 +207,7 @@ private void doAddColumn( icebergUpdateSchema.addRequiredColumn( getParentName(addColumn.fieldName()), getLeafName(addColumn.fieldName()), - ConvertUtil.toIcebergType(addColumn.getDataType()), + CONVERTER.fromGravitino(addColumn.getDataType()), addColumn.getComment()); } diff --git a/catalogs/catalog-lakehouse-iceberg/src/test/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/TestConvertUtil.java b/catalogs/catalog-lakehouse-iceberg/src/test/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/TestConvertUtil.java index b5784543dd7..062de4d7781 100644 --- a/catalogs/catalog-lakehouse-iceberg/src/test/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/TestConvertUtil.java +++ b/catalogs/catalog-lakehouse-iceberg/src/test/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/converter/TestConvertUtil.java @@ -4,6 +4,8 @@ */ package com.datastrato.gravitino.catalog.lakehouse.iceberg.converter; +import static com.datastrato.gravitino.catalog.lakehouse.iceberg.converter.IcebergDataTypeConverter.CONVERTER; + import com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergColumn; import com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergTable; import com.datastrato.gravitino.meta.AuditInfo; @@ -89,7 +91,7 @@ public void testToPrimitiveType() { ByteType byteType = ByteType.get(); IllegalArgumentException exception = Assertions.assertThrows( - IllegalArgumentException.class, () -> ConvertUtil.toIcebergType(byteType)); + IllegalArgumentException.class, () -> CONVERTER.fromGravitino(byteType)); Assertions.assertTrue( exception .getMessage() @@ -98,63 +100,63 @@ public void testToPrimitiveType() { ShortType shortType = ShortType.get(); exception = Assertions.assertThrows( - IllegalArgumentException.class, () -> ConvertUtil.toIcebergType(shortType)); + IllegalArgumentException.class, () -> CONVERTER.fromGravitino(shortType)); Assertions.assertTrue( exception .getMessage() .contains("Iceberg do not support Byte and Short Type, use Integer instead")); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.BooleanType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.BooleanType.get()) instanceof Types.BooleanType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.StringType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.StringType.get()) instanceof Types.StringType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.IntegerType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.IntegerType.get()) instanceof Types.IntegerType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.LongType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.LongType.get()) instanceof Types.LongType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.FloatType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.FloatType.get()) instanceof Types.FloatType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.DoubleType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.DoubleType.get()) instanceof Types.DoubleType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.DateType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.DateType.get()) instanceof Types.DateType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.TimeType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.TimeType.get()) instanceof Types.TimeType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.BinaryType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.BinaryType.get()) instanceof Types.BinaryType); Assertions.assertTrue( - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.UUIDType.get()) + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.UUIDType.get()) instanceof Types.UUIDType); Type timestampTZ = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.TimestampType.withTimeZone()); Assertions.assertTrue(timestampTZ instanceof Types.TimestampType); Assertions.assertTrue(((Types.TimestampType) timestampTZ).shouldAdjustToUTC()); Type timestamp = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.TimestampType.withoutTimeZone()); Assertions.assertTrue(timestamp instanceof Types.TimestampType); Assertions.assertFalse(((Types.TimestampType) timestamp).shouldAdjustToUTC()); Type decimalType = - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.DecimalType.of(9, 2)); + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.DecimalType.of(9, 2)); Assertions.assertTrue(decimalType instanceof Types.DecimalType); Assertions.assertEquals(9, ((Types.DecimalType) decimalType).precision()); Assertions.assertEquals(2, ((Types.DecimalType) decimalType).scale()); Type fixedCharType = - ConvertUtil.toIcebergType(com.datastrato.gravitino.rel.types.Types.FixedType.of(9)); + CONVERTER.fromGravitino(com.datastrato.gravitino.rel.types.Types.FixedType.of(9)); Assertions.assertTrue(fixedCharType instanceof Types.FixedType); Assertions.assertEquals(9, ((Types.FixedType) fixedCharType).length()); @@ -163,14 +165,14 @@ public void testToPrimitiveType() { com.datastrato.gravitino.rel.types.Types.StringType.get(), com.datastrato.gravitino.rel.types.Types.IntegerType.get(), true); - Type convertedMapType = ConvertUtil.toIcebergType(mapType); + Type convertedMapType = CONVERTER.fromGravitino(mapType); Assertions.assertTrue(convertedMapType instanceof Types.MapType); Assertions.assertTrue(((Types.MapType) convertedMapType).keyType() instanceof Types.StringType); Assertions.assertTrue( ((Types.MapType) convertedMapType).valueType() instanceof Types.IntegerType); Type listType = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.ListType.of( com.datastrato.gravitino.rel.types.Types.FloatType.get(), true)); Assertions.assertTrue(listType instanceof Types.ListType); @@ -180,7 +182,7 @@ public void testToPrimitiveType() { @Test public void testToNestedType() { Type listTypeNullable = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.ListType.of( com.datastrato.gravitino.rel.types.Types.FloatType.get(), true)); Assertions.assertTrue(listTypeNullable instanceof Types.ListType); @@ -188,7 +190,7 @@ public void testToNestedType() { Assertions.assertTrue(listTypeNullable.asListType().isElementOptional()); Type listTypeNotNull = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.ListType.of( com.datastrato.gravitino.rel.types.Types.FloatType.get(), false)); Assertions.assertTrue(listTypeNotNull instanceof Types.ListType); @@ -196,7 +198,7 @@ public void testToNestedType() { Assertions.assertTrue(listTypeNotNull.asListType().isElementRequired()); Type mapTypeNullable = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.MapType.of( com.datastrato.gravitino.rel.types.Types.StringType.get(), com.datastrato.gravitino.rel.types.Types.IntegerType.get(), @@ -207,7 +209,7 @@ public void testToNestedType() { Assertions.assertTrue(mapTypeNullable.asMapType().isValueOptional()); Type mapTypeNotNull = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.MapType.of( com.datastrato.gravitino.rel.types.Types.StringType.get(), com.datastrato.gravitino.rel.types.Types.IntegerType.get(), @@ -218,7 +220,7 @@ public void testToNestedType() { Assertions.assertTrue(mapTypeNotNull.asMapType().isValueRequired()); Type structTypeNullable = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.StructType.of( com.datastrato.gravitino.rel.types.Types.StructType.Field.nullableField( "col1", @@ -243,7 +245,7 @@ public void testToNestedType() { structTypeNullable.asStructType().fields().get(1).type().asListType().isElementOptional()); Type structTypeNotNull = - ConvertUtil.toIcebergType( + CONVERTER.fromGravitino( com.datastrato.gravitino.rel.types.Types.StructType.of( com.datastrato.gravitino.rel.types.Types.StructType.Field.notNullField( "col1", @@ -270,49 +272,49 @@ public void testToNestedType() { @Test public void testFormIcebergType() { Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.StringType.get()) + CONVERTER.toGravitino(Types.StringType.get()) instanceof com.datastrato.gravitino.rel.types.Types.StringType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.BinaryType.get()) + CONVERTER.toGravitino(Types.BinaryType.get()) instanceof com.datastrato.gravitino.rel.types.Types.BinaryType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.TimeType.get()) + CONVERTER.toGravitino(Types.TimeType.get()) instanceof com.datastrato.gravitino.rel.types.Types.TimeType); com.datastrato.gravitino.rel.types.Type TimestampTypeWithoutZone = - ConvertUtil.formIcebergType(Types.TimestampType.withoutZone()); + CONVERTER.toGravitino(Types.TimestampType.withoutZone()); Assertions.assertTrue( TimestampTypeWithoutZone instanceof com.datastrato.gravitino.rel.types.Types.TimestampType); Assertions.assertFalse( ((com.datastrato.gravitino.rel.types.Types.TimestampType) TimestampTypeWithoutZone) .hasTimeZone()); com.datastrato.gravitino.rel.types.Type TimestampTypeWithZone = - ConvertUtil.formIcebergType(Types.TimestampType.withZone()); + CONVERTER.toGravitino(Types.TimestampType.withZone()); Assertions.assertTrue( TimestampTypeWithZone instanceof com.datastrato.gravitino.rel.types.Types.TimestampType); Assertions.assertTrue( ((com.datastrato.gravitino.rel.types.Types.TimestampType) TimestampTypeWithZone) .hasTimeZone()); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.DoubleType.get()) + CONVERTER.toGravitino(Types.DoubleType.get()) instanceof com.datastrato.gravitino.rel.types.Types.DoubleType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.FloatType.get()) + CONVERTER.toGravitino(Types.FloatType.get()) instanceof com.datastrato.gravitino.rel.types.Types.FloatType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.IntegerType.get()) + CONVERTER.toGravitino(Types.IntegerType.get()) instanceof com.datastrato.gravitino.rel.types.Types.IntegerType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.LongType.get()) + CONVERTER.toGravitino(Types.LongType.get()) instanceof com.datastrato.gravitino.rel.types.Types.LongType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.UUIDType.get()) + CONVERTER.toGravitino(Types.UUIDType.get()) instanceof com.datastrato.gravitino.rel.types.Types.UUIDType); Assertions.assertTrue( - ConvertUtil.formIcebergType(Types.DateType.get()) + CONVERTER.toGravitino(Types.DateType.get()) instanceof com.datastrato.gravitino.rel.types.Types.DateType); com.datastrato.gravitino.rel.types.Type decimalType = - ConvertUtil.formIcebergType(Types.DecimalType.of(9, 2)); + CONVERTER.toGravitino(Types.DecimalType.of(9, 2)); Assertions.assertTrue( decimalType instanceof com.datastrato.gravitino.rel.types.Types.DecimalType); Assertions.assertEquals( @@ -321,14 +323,14 @@ public void testFormIcebergType() { 2, ((com.datastrato.gravitino.rel.types.Types.DecimalType) decimalType).scale()); com.datastrato.gravitino.rel.types.Type fixedType = - ConvertUtil.formIcebergType(Types.FixedType.ofLength(2)); + CONVERTER.toGravitino(Types.FixedType.ofLength(2)); Assertions.assertTrue(fixedType instanceof com.datastrato.gravitino.rel.types.Types.FixedType); Assertions.assertEquals( 2, ((com.datastrato.gravitino.rel.types.Types.FixedType) fixedType).length()); Types.MapType mapType = Types.MapType.ofOptional(1, 2, Types.StringType.get(), Types.IntegerType.get()); - com.datastrato.gravitino.rel.types.Type gravitinoMapType = ConvertUtil.formIcebergType(mapType); + com.datastrato.gravitino.rel.types.Type gravitinoMapType = CONVERTER.toGravitino(mapType); Assertions.assertTrue( gravitinoMapType instanceof com.datastrato.gravitino.rel.types.Types.MapType); Assertions.assertTrue( @@ -339,8 +341,7 @@ public void testFormIcebergType() { instanceof com.datastrato.gravitino.rel.types.Types.IntegerType); Types.ListType listType = Types.ListType.ofOptional(1, Types.StringType.get()); - com.datastrato.gravitino.rel.types.Type gravitinoListType = - ConvertUtil.formIcebergType(listType); + com.datastrato.gravitino.rel.types.Type gravitinoListType = CONVERTER.toGravitino(listType); Assertions.assertTrue( gravitinoListType instanceof com.datastrato.gravitino.rel.types.Types.ListType); Assertions.assertTrue( @@ -357,8 +358,7 @@ public void testFormIcebergType() { Types.StructType.of( Types.NestedField.optional(0, "integer_type", Types.IntegerType.get(), "integer type"), Types.NestedField.optional(1, "struct_type", structTypeInside, "struct type inside")); - com.datastrato.gravitino.rel.types.Type gravitinoStructType = - ConvertUtil.formIcebergType(structType); + com.datastrato.gravitino.rel.types.Type gravitinoStructType = CONVERTER.toGravitino(structType); // check for type Assertions.assertTrue( (gravitinoStructType) instanceof com.datastrato.gravitino.rel.types.Types.StructType); @@ -497,7 +497,7 @@ public void testFromNestedField() { } private static void checkType(Type type, com.datastrato.gravitino.rel.types.Type expected) { - com.datastrato.gravitino.rel.types.Type actual = ConvertUtil.formIcebergType(type); + com.datastrato.gravitino.rel.types.Type actual = CONVERTER.toGravitino(type); checkType(actual, expected); } diff --git a/common/src/main/resources/project.properties b/common/src/main/resources/project.properties new file mode 100644 index 00000000000..f9ef90b286f --- /dev/null +++ b/common/src/main/resources/project.properties @@ -0,0 +1,7 @@ +# +# Copyright 2023 Datastrato Pvt Ltd. +# This software is licensed under the Apache License version 2. +# +project.version=0.5.1-SNAPSHOT +compile.date=30/05/2024 20:03:36 +git.commit.id=3dc0376e5eaf2fece68b7e0dcc885f22fa03acf6 diff --git a/core/src/main/java/com/datastrato/gravitino/connector/DataTypeConverter.java b/core/src/main/java/com/datastrato/gravitino/connector/DataTypeConverter.java new file mode 100644 index 00000000000..e283e34793d --- /dev/null +++ b/core/src/main/java/com/datastrato/gravitino/connector/DataTypeConverter.java @@ -0,0 +1,33 @@ +/* + * Copyright 2024 Datastrato Pvt Ltd. + * This software is licensed under the Apache License version 2. + */ +package com.datastrato.gravitino.connector; + +import com.datastrato.gravitino.rel.types.Type; + +/** + * The interface for converting data types between Gravitino and catalogs. In most cases, the ToType + * and FromType are the same. But in some cases, such as converting between Gravitino and JDBC + * types, the ToType is String and the FromType is JdbcTypeBean. + * + * @param The Gravitino type will be converted to. + * @param The type will be converted to Gravitino type. + */ +public interface DataTypeConverter { + /** + * Convert the Gravitino type to the catalog type. + * + * @param type The Gravitino type. + * @return The catalog type. + */ + ToType fromGravitino(Type type); + + /** + * Convert the catalog type to the Gravitino type. + * + * @param type The catalog type. + * @return The Gravitino type. + */ + Type toGravitino(FromType type); +} From a4a8100a2c74c8f70c45c486277e9d5db2a602f2 Mon Sep 17 00:00:00 2001 From: mchades Date: Thu, 6 Jun 2024 13:36:35 +0800 Subject: [PATCH 4/4] split to catalog tests --- .github/workflows/backend-integration-test.yml | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/workflows/backend-integration-test.yml b/.github/workflows/backend-integration-test.yml index be5c0a620b0..49305d3fa46 100644 --- a/.github/workflows/backend-integration-test.yml +++ b/.github/workflows/backend-integration-test.yml @@ -62,6 +62,7 @@ jobs: java-version: [ 8, 11, 17 ] test-mode: [ embedded, deploy ] backend: [ jdbcBackend, kvBackend] + catalog: [ jdbc-doris, jdbc-mysql, jdbc-postgresql, lakehouse-iceberg, hadoop, hive, kafka ] env: PLATFORM: ${{ matrix.architecture }} steps: @@ -92,19 +93,16 @@ jobs: run: | dev/ci/util_free_space.sh - - name: Backend Integration Test - id: integrationTest - run: > - ./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -P${{ matrix.backend }} -PskipWebITs - -x :web:test -x :clients:client-python:test -x :flink-connector:test -x :spark-connector:test -x :spark-connector:spark-common:test - -x :spark-connector:spark-3.3:test -x :spark-connector:spark-3.4:test -x :spark-connector:spark-3.5:test - -x :spark-connector:spark-runtime-3.3:test -x :spark-connector:spark-runtime-3.4:test -x :spark-connector:spark-runtime-3.5:test + - name: catalog test + id: catalogTest + run: | + ./gradlew :catalogs:catalog-${{ matrix.catalog }}:test -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -P${{ matrix.backend }} - name: Upload integrate tests reports uses: actions/upload-artifact@v3 - if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }} + if: ${{ (failure() && steps.catalogTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }} with: - name: integrate-test-reports-${{ matrix.java-version }}-${{ matrix.test-mode }}-${{ matrix.backend }} + name: integrate-test-reports-${{ matrix.catalog }}-${{ matrix.java-version }}-${{ matrix.test-mode }}-${{ matrix.backend }} path: | build/reports integration-test/build/*.log