Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into ISSUE-42
Browse files Browse the repository at this point in the history
  • Loading branch information
Heng Qin committed Dec 18, 2023
2 parents 18faf4d + dbdc1c9 commit b17e332
Show file tree
Hide file tree
Showing 24 changed files with 387 additions and 181 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ When adding new code or fixing a bug be sure to add unit tests to provide covera
Spotless checks code formatting. If your code isn't correctly formatted, the build fails. To correctly format your code please use Spotless.

```bash
./grawdlew spotlessApply
./gradlew spotlessApply
```

All files must have a license header and the build fails if any files are missing license headers. If you are adding third-party code be sure to understand how to add the third-party license to Gravitino LICENSE and NOTICE files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@ public class PostgreSqlTableOperations extends JdbcTableOperations {
+ " pg_namespace AS n ON n.oid = c.relnamespace\n"
+ "WHERE \n"
+ " a.attnum > 0 \n"
+ " AND c.relname = ?";
+ " AND c.relname = ? AND n.nspname = ?";

private static final String SHOW_COLUMN_INFO_SQL =
"select * FROM information_schema.columns WHERE table_name = ? order by ordinal_position";
"select * FROM information_schema.columns WHERE table_name = ? AND table_schema = ? order by ordinal_position";

private static final String SHOW_TABLE_COMMENT_SQL =
"SELECT tb.table_name, d.description\n"
+ "FROM information_schema.tables tb\n"
+ " JOIN pg_class c ON c.relname = tb.table_name\n"
+ " LEFT JOIN pg_description d ON d.objoid = c.oid AND d.objsubid = '0'\n"
+ "WHERE tb.table_name = ?;";
+ "WHERE tb.table_name = ? AND table_schema = ?;";

private String database;

Expand All @@ -80,15 +80,18 @@ public void initialize(
public JdbcTable load(String schema, String tableName) throws NoSuchTableException {
try (Connection connection = getConnection(schema)) {
// The first step is to obtain the comment information of the column.
Map<String, String> columnCommentMap = selectColumnComment(tableName, connection);
Map<String, String> columnCommentMap = selectColumnComment(schema, tableName, connection);

// The second step is to obtain the column information of the table.
List<JdbcColumn> jdbcColumns =
selectColumnInfoAndExecute(
tableName, connection, (builder, s) -> builder.withComment(columnCommentMap.get(s)));
schema,
tableName,
connection,
(builder, s) -> builder.withComment(columnCommentMap.get(s)));

// The third step is to obtain the comment information of the table.
String comment = selectTableComment(tableName, connection);
String comment = selectTableComment(schema, tableName, connection);
return new JdbcTable.Builder()
.withName(tableName)
.withColumns(jdbcColumns.toArray(new JdbcColumn[0]))
Expand All @@ -102,13 +105,15 @@ public JdbcTable load(String schema, String tableName) throws NoSuchTableExcepti
}

private List<JdbcColumn> selectColumnInfoAndExecute(
String schemaName,
String tableName,
Connection connection,
BiConsumer<JdbcColumn.Builder, String> builderConsumer)
throws SQLException {
List<JdbcColumn> jdbcColumns = new ArrayList<>();
try (PreparedStatement preparedStatement = connection.prepareStatement(SHOW_COLUMN_INFO_SQL)) {
preparedStatement.setString(1, tableName);
preparedStatement.setString(2, schemaName);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
while (resultSet.next()) {
ColDataType colDataType = new ColDataType();
Expand Down Expand Up @@ -153,10 +158,12 @@ private static List<String> getArgList(ResultSet resultSet) throws SQLException
return result;
}

private String selectTableComment(String tableName, Connection connection) throws SQLException {
private String selectTableComment(String schema, String tableName, Connection connection)
throws SQLException {
try (PreparedStatement preparedStatement =
connection.prepareStatement(SHOW_TABLE_COMMENT_SQL)) {
preparedStatement.setString(1, tableName);
preparedStatement.setString(2, schema);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
if (resultSet.next()) {
return resultSet.getString("description");
Expand All @@ -170,13 +177,14 @@ private String selectTableComment(String tableName, Connection connection) throw
* @return Returns the column names and comments of the table
* @throws SQLException
*/
private Map<String, String> selectColumnComment(String tableName, Connection connection)
throws SQLException {
private Map<String, String> selectColumnComment(
String schema, String tableName, Connection connection) throws SQLException {
Map<String, String> columnCommentMap = new HashMap<>();

try (PreparedStatement preparedStatement =
connection.prepareStatement(SHOW_COLUMN_COMMENT_SQL)) {
preparedStatement.setString(1, tableName);
preparedStatement.setString(2, schema);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
while (resultSet.next()) {
String comment = resultSet.getString("comment");
Expand Down
39 changes: 37 additions & 2 deletions common/src/main/java/com/datastrato/gravitino/dto/MetalakeDTO.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
import com.datastrato.gravitino.Audit;
import com.datastrato.gravitino.Metalake;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import java.util.Map;
import javax.annotation.Nullable;
import lombok.EqualsAndHashCode;
import lombok.ToString;

/** Represents a Metalake Data Transfer Object (DTO) that implements the Metalake interface. */
@EqualsAndHashCode
@ToString
public class MetalakeDTO implements Metalake {

Expand Down Expand Up @@ -132,4 +131,40 @@ public MetalakeDTO build() {
return new MetalakeDTO(name, comment, properties, audit);
}
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
MetalakeDTO that = (MetalakeDTO) o;
return Objects.equal(name, that.name)
&& Objects.equal(comment, that.comment)
&& propertyEqual(properties, that.properties)
&& Objects.equal(audit, that.audit);
}

private boolean propertyEqual(Map<String, String> p1, Map<String, String> p2) {
if (p1 == null && p2 == null) {
return true;
}

if (p1 != null && p1.isEmpty() && p2 == null) {
return true;
}

if (p2 != null && p2.isEmpty() && p1 == null) {
return true;
}

return java.util.Objects.equals(p1, p2);
}

@Override
public int hashCode() {
return Objects.hashCode(name, comment, audit);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -704,9 +704,13 @@ public TableChange.ColumnPosition deserialize(JsonParser p, DeserializationConte
node != null && !node.isNull(),
"Cannot parse column position from invalid JSON: %s",
node);
if (node.isTextual() && node.asText().equals(POSITION_FIRST)) {
if (node.isTextual()
&& (node.asText().equals(POSITION_FIRST)
|| node.asText().equals(POSITION_FIRST.toUpperCase()))) {
return TableChange.ColumnPosition.first();
} else if (node.isTextual() && node.asText().equals(POSITION_DEFAULT)) {
} else if (node.isTextual()
&& (node.asText().equalsIgnoreCase(POSITION_DEFAULT)
|| node.asText().equalsIgnoreCase(POSITION_DEFAULT.toUpperCase()))) {
return TableChange.ColumnPosition.defaultPos();
} else if (node.isObject()) {
String afterColumn = getString(POSITION_AFTER, node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ public void testAddTableColumnRequest() throws JsonProcessingException {
+ "}";
Assertions.assertEquals(
JsonUtils.objectMapper().readTree(expected), JsonUtils.objectMapper().readTree(jsonString));
Assertions.assertTrue(
JsonUtils.objectMapper()
.readValue(
jsonString.replace("first", "FIRST"),
TableUpdateRequest.AddTableColumnRequest.class)
.getPosition()
instanceof TableChange.First);

// test default position
addTableColumnRequest =
Expand All @@ -170,5 +177,12 @@ public void testAddTableColumnRequest() throws JsonProcessingException {
+ "}";
Assertions.assertEquals(
JsonUtils.objectMapper().readTree(expected), JsonUtils.objectMapper().readTree(jsonString));
Assertions.assertTrue(
JsonUtils.objectMapper()
.readValue(
jsonString.replace("default", "DEFAULT"),
TableUpdateRequest.AddTableColumnRequest.class)
.getPosition()
instanceof TableChange.Default);
}
}
22 changes: 22 additions & 0 deletions core/src/main/java/com/datastrato/gravitino/StringIdentifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -108,6 +109,27 @@ public static Map<String, String> addToProperties(
.build();
}

/**
* Remove StringIdentifier from properties.
*
* @param properties the properties to remove the string identifier from.
* @return the properties with the string identifier removed.
*/
public static Map<String, String> removeIdFromProperties(Map<String, String> properties) {
if (properties == null) {
return null;
}

if (!properties.containsKey(ID_KEY)) {
return properties;
}

Map<String, String> copy = Maps.newHashMap(properties);
copy.remove(ID_KEY);

return ImmutableMap.<String, String>builder().putAll(copy).build();
}

public static StringIdentifier fromProperties(Map<String, String> properties) {
if (properties == null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.datastrato.gravitino.Field;
import com.datastrato.gravitino.HasIdentifier;
import com.datastrato.gravitino.Metalake;
import com.datastrato.gravitino.StringIdentifier;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -126,7 +127,7 @@ public EntityType type() {
*/
@Override
public Map<String, String> properties() {
return properties;
return StringIdentifier.removeIdFromProperties(properties);
}

/** Builder class for creating instances of {@link BaseMetalake}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ public MetalakeManager(EntityStore store, IdGenerator idGenerator) {
@Override
public BaseMetalake[] listMetalakes() {
try {
return store
.list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE)
.toArray(new BaseMetalake[0]);
return store.list(Namespace.empty(), BaseMetalake.class, EntityType.METALAKE).stream()
.toArray(BaseMetalake[]::new);
} catch (IOException ioe) {
LOG.error("Listing Metalakes failed due to storage issues.", ioe);
throw new RuntimeException(ioe);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public final class KvGarbageCollector implements Closeable {
new ScheduledThreadPoolExecutor(
2,
r -> {
Thread t = new Thread(r, "KvEntityStore-Garbage-Collector-%d");
Thread t = new Thread(r, "KvEntityStore-Garbage-Collector");
t.setDaemon(true);
return t;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,6 @@ private void testProperties(Map<String, String> expectedProps, Map<String, Strin
Assertions.assertEquals(v, testProps.get(k));
});

Assertions.assertTrue(testProps.containsKey(StringIdentifier.ID_KEY));
StringIdentifier StringId = StringIdentifier.fromString(testProps.get(StringIdentifier.ID_KEY));
Assertions.assertEquals(StringId.toString(), testProps.get(StringIdentifier.ID_KEY));
Assertions.assertFalse(testProps.containsKey(StringIdentifier.ID_KEY));
}
}
10 changes: 5 additions & 5 deletions docs/how-to-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This software is licensed under the Apache License version 2."

Gravitino has two types of tests: unit tests and integration tests. Unit tests are mainly
focused on the functionalities of the specific class, module, or component. Integration tests
are end-to-end tests that covers the whole system.
are end-to-end tests that cover the whole system.

:::note before test
* If you want to run the complete integration test suites, you need to install Docker in your
Expand All @@ -31,15 +31,15 @@ To run the unit test, you can simply run the following command:
./gradlew test -PskipITs
```

This command runs all the unit tests and skip the integration tests.
This command runs all the unit tests and skips the integration tests.

## Run the integration tests

Gravitino has two modes to run the integration tests, the `embedded` and `deploy` modes.
Gravitino has two modes to run the integration tests, the default `embedded` mode and `deploy` mode.

* With the `embedded` mode, the integration test starts an embedded `MiniGravitino` server
within the same process of the integration test to run the integration tests.
* With the `deploy` mode, the user has to deploy a Gravitino binary package beforehand, the
* With the `deploy` mode, the user has to build (`./gradlew compileDistribution`) a Gravitino binary package beforehand, the
integration test launches and connects to the local Gravitino server to run the integration
tests.

Expand Down Expand Up @@ -153,7 +153,7 @@ server code, follow these steps:

If a test fails, you can retrieve valuable information from the logs and test report. Test reports are in the `./build/reports` directory. The integration test logs are in the `./integrate-test/build` directory. In deploy mode, Gravitino server logs are in the `./distribution/package/logs/` directory. In the event of a test failure within the GitHub workflow, the system generates archived logs and test reports. To obtain the archive, follow these steps:

1. Click the `detail` link associated with the failed integrate test in the pull request. This redirects you to the job page.
1. Click the `detail` link associated with the failed integration test in the pull request. This redirects you to the job page.

![pr page Image](assets/test-fail-pr.png)

Expand Down
2 changes: 1 addition & 1 deletion docs/iceberg-rest-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ The Gravitino Iceberg REST catalog service using memory catalog for default. You
| `gravitino.auxService.iceberg-rest.uri` | The Hive metadata address, such as `thrift://127.0.0.1:9083`. | (none) | Yes | 0.2.0 |
| `gravitino.auxService.iceberg-rest.warehouse ` | The warehouse directory of the Hive catalog, such as `/user/hive/warehouse-hive/`. | (none) | Yes | 0.2.0 |

#### JDBC catalog configuration
#### Iceberg JDBC backend configuration

| Configuration item | Description | Default value | Required | Since Version |
|-----------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------|---------------|----------|---------------|
Expand Down
10 changes: 5 additions & 5 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ metadata access for data and AI assets.

## Downloading

You can get Graviton from the [GitHub release page](https://github.com/datastrato/gravitino/releases),
or you can build Gravitino from source, please see [How to build Gravitino](./how-to-build.md).
You can get Gravitino from the [GitHub release page](https://github.com/datastrato/gravitino/releases),
or you can build Gravitino from source code, please see [How to build Gravitino](./how-to-build.md).

Gravitino runs on both Linux and macOS, and requires Java 8. Gravitino trino-connector runs with
Trino, and requires Java 17. This should include JVMs on x86_64 and
Expand Down Expand Up @@ -46,7 +46,7 @@ To get started with Gravitino, please see [Getting started](./getting-started.md

## Gravitino playground

To experience Gravitino with other components simply, Gravitino provides a playground to run. It
To experience Gravitino with other components easily, Gravitino provides a playground to run. It
integrates Apache Hadoop, Apache Hive, Trino, MySQL, PostgreSQL, and Gravitino together as a
complete environment. To experience the whole features, please also see
[Getting started](./getting-started.md) and [How to use the Gravitino playground](./how-to-use-the-playground.md)
Expand All @@ -66,7 +66,7 @@ to learn how to use the playground.
* [Manage metadata using Gravitino](./manage-metadata-using-gravitino.md): provides the complete
functionalities of Gravitino metadata management. Including metalake, catalog, schema and
table management.
* [Gravitino Open API](pathname:///docs/0.3.0/api/rest/index.html): provides the complete Open API definition of
* [Gravitino Open API](./api/rest/gravitino-rest-api): provides the complete Open API definition of
Gravitino.
* [Gravitino Javadoc](pathname:///docs/0.3.0/api/java/index.html): provides the Javadoc for Gravitino API.

Expand Down Expand Up @@ -98,7 +98,7 @@ Gravitino supports different catalogs to manage the metadata in different source
Gravitino provides a Trino connector to connect to Gravitino to manage the metadata in a unified
way. to use the Trino connector, please see:

* [How to use Gravitino Trino connector](./trino-connector/index): a complete guide to use Gravitino
* [How to use Gravitino Trino connector](./trino-connector/index.md): a complete guide to use Gravitino
Trino connector.

### Development guides
Expand Down
Loading

0 comments on commit b17e332

Please sign in to comment.