[#209]feat(catalog): Hive table entity serde and store support #233

mchades · 2023-08-14T07:13:51Z

What changes were proposed in this pull request?

SerDe Hive table entity as Table Proto in Graviton store while operating Hive table

Why are the changes needed?

we could store the additional entity information to our own storage.

Fix: #209

Does this PR introduce any user-facing change?

no

How was this patch tested?

UTs added

github-actions · 2023-08-14T07:17:14Z

Code Coverage Report

Overall Project	59.23% `-0.81%`	🟢
Files changed	54.34%	🔴

Module	Coverage
core	66.65% `-0.97%`	🔴
catalog-hive	58.28% `-3.17%`	🟢

Files

Module	File	Coverage
core	ProtoEntitySerDe.java	65.2%	🟢
	BaseTable.java	57.67% `-6.41%`	🔴
	TableEntitySerde.java	0%	🔴
catalog-hive	HiveCatalogOperations.java	66.15% `-6.67%`	🟢

yuqi1129 · 2023-08-14T07:25:18Z

catalog-hive/src/main/java/com/datastrato/graviton/catalog/hive/HiveCatalogOperations.java

@@ -402,26 +403,34 @@ public boolean dropSchema(NameIdentifier ident, boolean cascade) throws NonEmpty
    EntityStore store = GravitonEnv.getInstance().entityStore();
    Namespace schemaNamespace =
        Namespace.of(ArrayUtils.add(ident.namespace().levels(), ident.name()));
+    List<BaseTable> tables = Lists.newArrayList();


Unnecessary initiation

use addAll in line 416

yuqi1129 · 2023-08-14T07:26:47Z

catalog-hive/src/main/java/com/datastrato/graviton/catalog/hive/HiveCatalogOperations.java

    }

    try {
+      List<BaseTable> finalTables = tables;


Why do you introduce a new local variant? if you want to handle final variant in lambda function, I advice you can use addAll in line 416

### What changes were proposed in this pull request? Add diqiu50 to maintainer ### Why are the changes needed? Fix: #755 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No

### What changes were proposed in this pull request? (Please outline the changes and how this PR fixes the issue.) ### Why are the changes needed? (Please clarify why the changes are needed. For instance, 1. If you propose a new API, clarify the use case for a new API. 2. If you fix a bug, describe the bug.) Fix: # (issue) ### Does this PR introduce _any_ user-facing change? (Please list the user-facing changes introduced by your change, including 1. Change in user-facing APIs. 2. Addition or removal of property keys.) ### How was this patch tested? (Please test your changes, and provide instructions on how to test it: 1. If you add a feature or fix a bug, add a test to cover your changes. 2. If you fix a flaky test, repeat it for many times to prove it works.)

)  ### What changes were proposed in this pull request? close #1724 ### Why are the changes needed? Add server-side REST API support for fileset Fix: #1724 ### Does this PR introduce _any_ user-facing change? (Please list the user-facing changes introduced by your change, including 1. Change in user-facing APIs. 2. Addition or removal of property keys.) ### How was this patch tested? - add unit tests

…2043)  ### What changes were proposed in this pull request? Unify the style of configuration options ### Why are the changes needed? Fix: #2042 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Just doc. Co-authored-by: Heng Qin <qqtt@123.com>

…2045)  ### What changes were proposed in this pull request? Unify the style of configuration options ### Why are the changes needed? Fix: #2042 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Just doc. Co-authored-by: qqqttt123 <148952220+qqqttt123@users.noreply.github.com> Co-authored-by: Heng Qin <qqtt@123.com>

…uri (#2136)  ### What changes were proposed in this pull request? Change `gravitino.url` to `gravitino.uri`. ### Why are the changes needed? The example in trino-connector/installation.md uses different keys for gravitino link. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/installation.md?plain=1#L75-L81 From source code, it should be `gravitino.uri`. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoConfig.java#L22 We also need to fix the one in trino-connector/configuration.md. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/configuration.md?plain=1#L13 Fix: #2118 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Check there is no `gravitino.url` in the document. Signed-off-by: PoAn Yang <payang@apache.org>

…uri (#2142)  ### What changes were proposed in this pull request? Change `gravitino.url` to `gravitino.uri`. ### Why are the changes needed? The example in trino-connector/installation.md uses different keys for gravitino link. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/installation.md?plain=1#L75-L81 From source code, it should be `gravitino.uri`. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/trino-connector/src/main/java/com/datastrato/gravitino/trino/connector/GravitinoConfig.java#L22 We also need to fix the one in trino-connector/configuration.md. https://github.com/datastrato/gravitino/blob/edfcdbea89dbf16d8e48833573c8917f9bcd5972/docs/trino-connector/configuration.md?plain=1#L13 Fix: #2118 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Check there is no `gravitino.url` in the document. Signed-off-by: PoAn Yang <payang@apache.org> Co-authored-by: PoAn Yang <payang@apache.org>

### What changes were proposed in this pull request? Dependencies follow alphabetical order ### Why are the changes needed? For better readability Fix: #738 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? N/A --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>

### What changes were proposed in this pull request? Replace LinkedList with ArrayList. ### Why are the changes needed? Address linter issues mentioned in #2163. Fix #2163 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Covered by existing tests.

…#2138)  ### What changes were proposed in this pull request? close #1955 ### Why are the changes needed? (Please clarify why the changes are needed. For instance, 1. If you propose a new API, clarify the use case for a new API. 2. If you fix a bug, describe the bug.) Fix: #1955 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - unit

…n atomic to keep thread safe (#2139)  ### What changes were proposed in this pull request? close #1953 #1946 #1941 #1920 #1910 ### Why are the changes needed? Fix: #1953 #1946 #1941 #1920 #1910 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - ut

### What changes were proposed in this pull request? - close #2154 ### Why are the changes needed? - Fix: #2154 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? `./gradlew build` and UTs

…2403)  ### What changes were proposed in this pull request? - close #2367 ### Why are the changes needed? - Fix: #2367 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - `./gradlew build` --------- Co-authored-by: mchades <liminghuang@datastrato.com>

…for consistency (#2437)  ### What changes were proposed in this pull request? Make all Spark SQL keywords used in spark connector integration test uppercase ### Why are the changes needed? Fix: #2424 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew build -PskipTests`

…le clean (#2531)  ### What changes were proposed in this pull request? Remove directory `spark-warehouse` when exectue gradle clean in moudle catalog-lakehouse-iceberg. Remove directory `src/main/resources` when execute gradle clean in module server, since which is generated by gradle build. ### Why are the changes needed? Fix: #2508 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? `./gradlew :catalogs:catalog-lakehouse-iceberg:clean` `./gradlew :server:clean` --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>

…rdedByInstance error-prone (#2514)  ### What changes were proposed in this pull request? `StaticGuardedByInstance` amis to avoid `public static` fields. Because this field is not thread-safe. See more details at https://errorprone.info/bugpattern/StaticGuardedByInstance. In this MR, I remove some test code in `com.datastrato.gravitino.trino.connector.GravitinoPlugin` and `com.datastrato.gravitino.trino.connector.GravitinoConnectorFactory`. At the same time, I enable `StaticGuardedByInstance` error-prone. ### Why are the changes needed? - Fix: #2167 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - original unit tests.

…ticGuardedByInstance error-prone (apache#2514)  ### What changes were proposed in this pull request? `StaticGuardedByInstance` amis to avoid `public static` fields. Because this field is not thread-safe. See more details at https://errorprone.info/bugpattern/StaticGuardedByInstance. In this MR, I remove some test code in `com.datastrato.gravitino.trino.connector.GravitinoPlugin` and `com.datastrato.gravitino.trino.connector.GravitinoConnectorFactory`. At the same time, I enable `StaticGuardedByInstance` error-prone. ### Why are the changes needed? - Fix: apache#2167 ### Does this PR introduce _any_ user-facing change? - no ### How was this patch tested? - original unit tests.

…ult (#2764)  ### What changes were proposed in this pull request? When listing metalakes they get sorted by their createdTime value ### Why are the changes needed? Its requested in the issue Fix: #2754 (issue) ### Does this PR introduce _any_ user-facing change? metalakes will be shown in ASC order ### How was this patch tested? N/A --------- Co-authored-by: CHEYNE <contact@ch3yne.com>

### What changes were proposed in this pull request? Add PoAn Yang (FrankYang0529) to Contributors list. Signed-off-by: PoAn Yang <payang@apache.org>

…ate_fileset function (#3535)  ### What changes were proposed in this pull request? * Change the suggest type of `type` from `Catalog.Type` to `Fileset.Type` ### Why are the changes needed? Fix: #3508 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew :clients:client-python:test` Co-authored-by: noidname01 <55401762+noidname01@users.noreply.github.com> Co-authored-by: TimWang <tim.wang@pranaq.com>

…tor (#2635)  ### What changes were proposed in this pull request? - support GravitinoCatalogStore to register the catalog. In the MR, we will support to create the hive catalog. ### Why are the changes needed? - Fix #3362 ### Does this PR introduce _any_ user-facing change? - support flink in gravitino ### How was this patch tested? - add UTs

… for client-python (#3568)  ### What changes were proposed in this pull request? * Replace varaible names `type` and `property`, which is Python functions and keywords, with more readable and custom variable names ### Why are the changes needed? Fix: #3561 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? `./gradlew clients:client-python:test` Co-authored-by: noidname01 <55401762+noidname01@users.noreply.github.com> Co-authored-by: TimWang <tim.wang@pranaq.com>

…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 <ericqin@tencent.com>

…c property issue (apache#3014)  ### What changes were proposed in this pull request? Load catalogs first when refresh schema or table level page <img width="1395" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/f3af84e6-d6dc-474e-9f55-e714edb9be35"> Fix kafka topic property issue ### Why are the changes needed? Fix: apache#2996, Fix: apache#3011 ### Does this PR introduce _any_ user-facing change? When schema, table or fileset page network error, refresh the page ### How was this patch tested? <img width="735" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/d69bc9f3-d4f7-4f1e-bc2c-65941821a510">

…hema level and kafka topic level (apache#2937) …a topic level  ### What changes were proposed in this pull request? Improvement of fileset and kafka test case ### Why are the changes needed? Fix: apache#2896 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? <img width="340" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/520204d9-2d5b-4050-b020-ad47275d2469">

…ve the interceptors of axios (apache#3116)  ### What changes were proposed in this pull request? 1. Support special character like '//' for database name or table name and so on. <img width="852" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/9ae20ce1-3683-453a-a651-53a2277a8e25"> 3. Improvment interceptors of axois for avoide repeat request 4. Fix some runtime errors ### Why are the changes needed? N/A Fix: apache#3062 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Manual

### What changes were proposed in this pull request? Fixed assertion Fix: apache#3251 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Ran test locally multiple times --------- Signed-off-by: Rohit Satya <work.rohitsatya@gmail.com>

…n null cause list tables error on some jdbc 8.x driver (apache#3294)  ### What changes were proposed in this pull request? replace `DatabaseMetaData#getSchema` by `DatabaseMetaData#getCatalog` and provide a more general way to get tables through jdbc driver ### Why are the changes needed? jdbc driver `DatabaseMetaData#getSchema` method return null causes list tables failure on some jdbc mysql 8.x driver (mysql-connector-java-8.0.11). Fix: apache#3269 ### Does this PR introduce _any_ user-facing change? not ### How was this patch tested? (Please test your changes, and provide instructions on how to test it: 1. If you add a feature or fix a bug, add a test to cover your changes. 2. If you fix a flaky test, repeat it for many times to prove it works.) --------- Co-authored-by: wangqi <wangqi@xinxuan.net>

… catalog (apache#3384)  ### What changes were proposed in this pull request? Add a default optional properties for fileset catalog <img width="597" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/1fe02e25-d80e-4182-bb47-6079b956fe39"> Remove fix width of version component <img width="289" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/cbc690cd-d721-4916-9d5d-1dea077499d5"> <img width="335" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/19ab6609-32cf-44e1-9bc0-e2eb12241ab0"> ### Why are the changes needed? Although the location property is optional, it is better to be there for ease of use The width of version component is should be flexible Fix: apache#3257 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual

…ache#3385)  ### What changes were proposed in this pull request? disable the metalake select with only one <img width="505" alt="image" src="https://github.com/datastrato/gravitino/assets/9210625/f91df10f-3856-4947-a8cb-6e6cdbe004f3"> ### Why are the changes needed? With only one metalake UI presents user with a choice Fix: apache#3285 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual

### What changes were proposed in this pull request? Add PoAn Yang (FrankYang0529) to Contributors list. Signed-off-by: PoAn Yang <payang@apache.org>

…connector (apache#2635)  ### What changes were proposed in this pull request? - support GravitinoCatalogStore to register the catalog. In the MR, we will support to create the hive catalog. ### Why are the changes needed? - Fix apache#3362 ### Does this PR introduce _any_ user-facing change? - support flink in gravitino ### How was this patch tested? - add UTs

…ables (apache#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: apache#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 <ericqin@tencent.com>

### What changes were proposed in this pull request? orignal -> original

### What changes were proposed in this pull request? orignal -> original

### What changes were proposed in this pull request? Fixed an incorrect Java example. ### Why are the changes needed? Fix: #4077 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No testing required Co-authored-by: rqyin <rqyin@easipass.com>

### What changes were proposed in this pull request? Fix irregular variable names: tablename -> tableName ### Why are the changes needed? Variable name `tablename` is irregular ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Modify variable name naming convention, no testing required.

…y environment variant (#4229)  ### What changes were proposed in this pull request? Read the environment variable (if exists) when -PskipDockerTests is not specified. ### Why are the changes needed? Fix: #4181 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? ``` export SKIP_DOCKER_TESTS=false ./gradlew build # check docket tests are running ```

… mutable (#4379)  ### What changes were proposed in this pull request? switch catalog properties value input disable to enable hive catalog before: <img width="623" alt="Pasted Graphic" src="https://github.com/user-attachments/assets/77472113-ddd8-4051-825a-21bbaaf07bdd"> hive catalog after: <img width="708" alt="Pasted Graphic 1" src="https://github.com/user-attachments/assets/d08634da-0044-452d-9ded-ff186aae3507"> Iceberg catalog before: <img width="623" alt="Pasted Graphic 2" src="https://github.com/user-attachments/assets/3ecca557-6b2d-42ab-8d0f-115044fe97b3"> iceberg catalog after: <img width="588" alt="Pasted Graphic 3" src="https://github.com/user-attachments/assets/5585d55c-4bec-42c3-b496-eaf75dc55e9f"> pg catalog before: <img width="580" alt="Pasted Graphic 6" src="https://github.com/user-attachments/assets/334abb54-ad33-42b5-9d96-6f3d7fdded19"> pg catalog after: <img width="594" alt="Pasted Graphic 7" src="https://github.com/user-attachments/assets/4aca0e85-8ba2-4bbe-a19b-58c52685ae42"> kafka catalog before: <img width="605" alt="Pasted Graphic 4" src="https://github.com/user-attachments/assets/37f1938d-90df-45ff-81bf-7081238921db"> kafka catalog after: <img width="592" alt="Pasted Graphic 5" src="https://github.com/user-attachments/assets/4dd7e321-dd6a-4e86-8e5d-dae6d601fbf7"> fileset catalog before: <img width="607" alt="Pasted Graphic 8" src="https://github.com/user-attachments/assets/1b430711-f494-4b87-89a2-ea9c774c381c"> fileset catalog after: <img width="618" alt="Pasted Graphic 9" src="https://github.com/user-attachments/assets/04ce17cb-5eac-4f25-b1e0-e309db2a1103"> ### Why are the changes needed? make the change by the [PR](#4262) merged Fix: #4351 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? manual Co-authored-by: Qian Xia <lauraxiaqian@gmail.com>

### What changes were proposed in this pull request? Add `**/*.out` to the list of exclusions will skip the licence check on the *.out file. ### Why are the changes needed? Fix: #4439 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? No Co-authored-by: Saker <46983675+IamSaker@users.noreply.github.com>

#4667)  ### What changes were proposed in this pull request? The documents should be updated. ### Why are the changes needed? To correct reference from MySQL to PostgreSQL in JDBC catalog documentation. Fix: #4666 ### Does this PR introduce _any_ user-facing change? No; Just the markdown documents. ### How was this patch tested? Manual review.

### What changes were proposed in this pull request? `TestHiveTableOperations` can be tested before `TestHiveTable` initialized. Make `TestHiveTableOperations::steup()` run alone. ### Why are the changes needed? When we try to just run `gradle :catalogs:catalog-hive:test --tests 'org.apache.gravitino.catalog.hive.TestHiveTableOperations'`, `TestHiveTableOperations` needs static variables which must be initialized ahead, and it can't be run alone. Fix: #4851 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Refactor of unit test setup, local unit tests have passed.

### What changes were proposed in this pull request? In `DorisTableOperations`, handled a possible Null Pointer Exception scenario ### Why are the changes needed? Fix: (#4605) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Local Test Run - Passed --------- Co-authored-by: Avinash <AMoorthy@csod.com>

Hive table entity serde and store support

c901df5

mchades requested review from jerryshao and yuqi1129 August 14, 2023 07:13

mchades self-assigned this Aug 14, 2023

yuqi1129 reviewed Aug 14, 2023

View reviewed changes

handle final variant in lambda function

84d797d

jerryshao approved these changes Aug 14, 2023

View reviewed changes

jerryshao merged commit bb29195 into apache:main Aug 14, 2023
2 checks passed

liujinhui1994 mentioned this pull request Mar 21, 2024

[#2625] fix: cannot compile under docker bash #2626

Closed

yuqi1129 mentioned this pull request Jul 8, 2024

[TEST] Test authentication #4103

Closed

yuqi1129 mentioned this pull request Aug 14, 2024

[#4445] feat(docker): Publish Docker image to Apache official DockerH… #4522

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#209]feat(catalog): Hive table entity serde and store support #233

[#209]feat(catalog): Hive table entity serde and store support #233

mchades commented Aug 14, 2023

github-actions bot commented Aug 14, 2023 •

edited

Loading

yuqi1129 Aug 14, 2023

mchades Aug 14, 2023

yuqi1129 Aug 14, 2023

mchades Aug 14, 2023

[#209]feat(catalog): Hive table entity serde and store support #233

[#209]feat(catalog): Hive table entity serde and store support #233

Conversation

mchades commented Aug 14, 2023

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

github-actions bot commented Aug 14, 2023 • edited Loading

Code Coverage Report

yuqi1129 Aug 14, 2023

Choose a reason for hiding this comment

mchades Aug 14, 2023

Choose a reason for hiding this comment

yuqi1129 Aug 14, 2023

Choose a reason for hiding this comment

mchades Aug 14, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 14, 2023 •

edited

Loading