Skip to content

Commit 7213dcb

Browse files
jerryshaojerryshao
authored
Merge branch 'main' into fix-leak
2 parents 5ab931d + 97b61ca commit 7213dcb

File tree

106 files changed

+2553
-406
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

106 files changed

+2553
-406
lines changed

‎.github/workflows/access-control-integration-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ jobs:
9292
./gradlew -PtestMode=deploy -PjdbcBackend=postgresql -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :authorizations:test
9393
9494
- name: Upload integrate tests reports
95-
uses: actions/upload-artifact@v3
95+
uses: actions/upload-artifact@v4
9696
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
9797
with:
9898
name: authorizations-integrate-test-reports-${{ matrix.java-version }}

‎.github/workflows/backend-integration-test-action.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ jobs:
6464
-x :authorizations:authorization-chain:test -x :authorizations:authorization-ranger:test
6565
6666
- name: Upload integrate tests reports
67-
uses: actions/upload-artifact@v3
67+
uses: actions/upload-artifact@v4
6868
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
6969
with:
7070
name: integrate-test-reports-${{ inputs.java-version }}-${{ inputs.test-mode }}-${{ inputs.backend }}

‎.github/workflows/build.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ jobs:
9090
./gradlew :spark-connector:spark-3.5:build -PscalaVersion=2.13 -PskipITs -PskipDockerTests=false
9191
9292
- name: Upload unit tests report
93-
uses: actions/upload-artifact@v3
93+
uses: actions/upload-artifact@v4
9494
if: failure()
9595
with:
9696
name: unit test report
@@ -129,7 +129,7 @@ jobs:
129129
run: ./gradlew build -PskipITs -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false -x :clients:client-python:build
130130

131131
- name: Upload unit tests report
132-
uses: actions/upload-artifact@v3
132+
uses: actions/upload-artifact@v4
133133
if: failure()
134134
with:
135135
name: unit test report

‎.github/workflows/cron-integration-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ jobs:
8787
./gradlew test -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false
8888
8989
- name: Upload integrate tests reports
90-
uses: actions/upload-artifact@v3
90+
uses: actions/upload-artifact@v4
9191
if: ${{ failure() && steps.integrationTest.outcome == 'failure' }}
9292
with:
9393
name: integrate test reports

‎.github/workflows/docker-image.yml

+23-4
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ on:
3232
description: 'Publish Docker token'
3333
required: true
3434
type: string
35+
publish-latest-tag:
36+
description: 'Whether to update the latest tag. This operation is only applicable to official releases and should not be used for Release Candidate (RC).'
37+
required: false
38+
type: boolean
39+
default: false
3540

3641
jobs:
3742
publish-docker-image:
@@ -83,6 +88,12 @@ jobs:
8388
echo "image_type=iceberg-rest-server" >> $GITHUB_ENV
8489
echo "image_name=apache/gravitino-iceberg-rest" >> $GITHUB_ENV
8590
fi
91+
92+
if [ "${{ github.event.inputs.publish-latest-tag }}" == "true" ]; then
93+
echo "publish_latest=true" >> $GITHUB_ENV
94+
else
95+
echo "publish_latest=false" >> $GITHUB_ENV
96+
fi
8697
8798
- name: Check publish Docker token
8899
run: |
@@ -115,8 +126,16 @@ jobs:
115126
sudo rm -rf /usr/local/lib/android
116127
sudo rm -rf /opt/hostedtoolcache/CodeQL
117128
118-
if [[ "${image_type}" == "gravitino" || "${image_type}" == "iceberg-rest-server" ]]; then
119-
./dev/docker/build-docker.sh --platform all --type ${image_type} --image ${image_name} --tag ${{ github.event.inputs.version }} --latest
129+
if [[ -n "${tag_name}" ]]; then
130+
full_tag_name="${tag_name}-${{ github.event.inputs.version }}"
131+
else
132+
full_tag_name="${{ github.event.inputs.version }}"
133+
fi
134+
135+
if [[ "${publish_latest}" == "true" ]]; then
136+
echo "Publish tag ${full_tag_name}, and update latest too."
137+
./dev/docker/build-docker.sh --platform all --type ${image_type} --image ${image_name} --tag ${full_tag_name} --latest
120138
else
121-
./dev/docker/build-docker.sh --platform all --type ${image_type} --image ${image_name} --tag "${tag_name}-${{ github.event.inputs.version }}"
122-
fi
139+
echo "Publish tag ${full_tag_name}."
140+
./dev/docker/build-docker.sh --platform all --type ${image_type} --image ${image_name} --tag ${full_tag_name}
141+
fi

‎.github/workflows/flink-integration-test-action.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
./gradlew -PskipTests -PtestMode=deploy -PjdkVersion=${{ inputs.java-version }} -PskipDockerTests=false :flink-connector:flink:test --tests "org.apache.gravitino.flink.connector.integration.test.**"
5353
5454
- name: Upload integrate tests reports
55-
uses: actions/upload-artifact@v3
55+
uses: actions/upload-artifact@v4
5656
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
5757
with:
5858
name: flink-connector-integrate-test-reports-${{ inputs.java-version }}

‎.github/workflows/frontend-integration-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ jobs:
8888
./gradlew -PskipTests -PtestMode=deploy -PjdkVersion=${{ matrix.java-version }} -PskipDockerTests=false :web:integration-test:test
8989
9090
- name: Upload integrate tests reports
91-
uses: actions/upload-artifact@v3
91+
uses: actions/upload-artifact@v4
9292
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
9393
with:
9494
name: integrate-test-reports-${{ matrix.java-version }}

‎.github/workflows/gvfs-fuse-build-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ jobs:
8888
dev/ci/util_free_space.sh
8989
9090
- name: Upload tests reports
91-
uses: actions/upload-artifact@v3
91+
uses: actions/upload-artifact@v4
9292
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
9393
with:
9494
name: Gvfs-fuse integrate-test-reports-${{ matrix.java-version }}

‎.github/workflows/python-integration-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ jobs:
8282
done
8383
8484
- name: Upload integrate tests reports
85-
uses: actions/upload-artifact@v3
85+
uses: actions/upload-artifact@v4
8686
if: ${{ failure() && steps.integrationTest.outcome == 'failure' }}
8787
with:
8888
name: integrate test reports

‎.github/workflows/spark-integration-test-action.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ jobs:
6363
./gradlew -PskipTests -PtestMode=${{ inputs.test-mode }} -PjdkVersion=${{ inputs.java-version }} -PscalaVersion=${{ inputs.scala-version }} -PskipDockerTests=false :spark-connector:spark-3.5:test --tests "org.apache.gravitino.spark.connector.integration.test.**"
6464
6565
- name: Upload integrate tests reports
66-
uses: actions/upload-artifact@v3
66+
uses: actions/upload-artifact@v4
6767
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
6868
with:
6969
name: spark-connector-integrate-test-reports-${{ inputs.java-version }}-${{ inputs.test-mode }}

‎.github/workflows/trino-integration-test.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ jobs:
9090
trino-connector/integration-test/trino-test-tools/run_test.sh
9191
9292
- name: Upload integrate tests reports
93-
uses: actions/upload-artifact@v3
93+
uses: actions/upload-artifact@v4
9494
if: ${{ (failure() && steps.integrationTest.outcome == 'failure') || contains(github.event.pull_request.labels.*.name, 'upload log') }}
9595
with:
9696
name: trino-connector-integrate-test-reports-${{ matrix.java-version }}

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ replay_pid*
2727
**/.gradle
2828
**/.idea
2929
!/.idea/icon.svg
30+
!.idea/vcs.xml
3031
**/build
3132
gen
3233
**/.DS_Store

‎authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/PathBasedMetadataObject.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,13 @@ public int hashCode() {
122122

123123
@Override
124124
public String toString() {
125-
return "MetadataObject: [fullName=" + fullName() + "], [path=" + path == null
126-
? "null"
127-
: path + "], [type=" + type + "]";
125+
String strPath = path == null ? "null" : path;
126+
return "MetadataObject: [fullName="
127+
+ fullName()
128+
+ "], [path="
129+
+ strPath
130+
+ "], [type="
131+
+ type
132+
+ "]";
128133
}
129134
}

‎authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,13 @@ public Boolean onMetadataUpdated(MetadataObjectChange... changes) throws Runtime
110110
@Override
111111
public Boolean onRoleCreated(Role role) throws AuthorizationPluginException {
112112
List<String> sqls = getCreateRoleSQL(role.name());
113+
boolean createdNewly = false;
113114
for (String sql : sqls) {
114-
executeUpdateSQL(sql, "already exists");
115+
createdNewly = executeUpdateSQL(sql, "already exists");
116+
}
117+
118+
if (!createdNewly) {
119+
return true;
115120
}
116121

117122
if (role.securableObjects() != null) {
@@ -140,7 +145,6 @@ public Boolean onRoleDeleted(Role role) throws AuthorizationPluginException {
140145
@Override
141146
public Boolean onRoleUpdated(Role role, RoleChange... changes)
142147
throws AuthorizationPluginException {
143-
onRoleCreated(role);
144148
for (RoleChange change : changes) {
145149
if (change instanceof RoleChange.AddSecurableObject) {
146150
SecurableObject object = ((RoleChange.AddSecurableObject) change).getSecurableObject();
@@ -381,14 +385,15 @@ protected AuthorizationPluginException toAuthorizationPluginException(SQLExcepti
381385
"JDBC authorization plugin fail to execute SQL, error code: %d", se.getErrorCode());
382386
}
383387

384-
public void executeUpdateSQL(String sql, String ignoreErrorMsg) {
388+
public boolean executeUpdateSQL(String sql, String ignoreErrorMsg) {
385389
try (final Connection connection = getConnection()) {
386390
try (final Statement statement = connection.createStatement()) {
387391
statement.executeUpdate(sql);
392+
return true;
388393
}
389394
} catch (SQLException se) {
390395
if (ignoreErrorMsg != null && se.getMessage().contains(ignoreErrorMsg)) {
391-
return;
396+
return false;
392397
}
393398
LOG.error("JDBC authorization plugin exception: ", se);
394399
throw toAuthorizationPluginException(se);

‎authorizations/authorization-common/src/test/java/org/apache/gravitino/authorization/common/TestPathBasedMetadataObject.java

+27
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,31 @@ public void PathBasedMetadataObjectNotEquals() {
4747

4848
Assertions.assertNotEquals(pathBasedMetadataObject1, pathBasedMetadataObject2);
4949
}
50+
51+
@Test
52+
void testToString() {
53+
PathBasedMetadataObject pathBasedMetadataObject1 =
54+
new PathBasedMetadataObject("parent", "name", "path", PathBasedMetadataObject.Type.PATH);
55+
Assertions.assertEquals(
56+
"MetadataObject: [fullName=parent.name], [path=path], [type=PATH]",
57+
pathBasedMetadataObject1.toString());
58+
59+
PathBasedMetadataObject pathBasedMetadataObject2 =
60+
new PathBasedMetadataObject("parent", "name", null, PathBasedMetadataObject.Type.PATH);
61+
Assertions.assertEquals(
62+
"MetadataObject: [fullName=parent.name], [path=null], [type=PATH]",
63+
pathBasedMetadataObject2.toString());
64+
65+
PathBasedMetadataObject pathBasedMetadataObject3 =
66+
new PathBasedMetadataObject(null, "name", null, PathBasedMetadataObject.Type.PATH);
67+
Assertions.assertEquals(
68+
"MetadataObject: [fullName=name], [path=null], [type=PATH]",
69+
pathBasedMetadataObject3.toString());
70+
71+
PathBasedMetadataObject pathBasedMetadataObject4 =
72+
new PathBasedMetadataObject(null, "name", "path", PathBasedMetadataObject.Type.PATH);
73+
Assertions.assertEquals(
74+
"MetadataObject: [fullName=name], [path=path], [type=PATH]",
75+
pathBasedMetadataObject4.toString());
76+
}
5077
}

‎authorizations/authorization-common/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java

+19-11
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,10 @@ public List<String> getSetOwnerSQL(
7474
return Collections.emptyList();
7575
}
7676

77-
public void executeUpdateSQL(String sql, String ignoreErrorMsg) {
77+
public boolean executeUpdateSQL(String sql, String ignoreErrorMsg) {
7878
Assertions.assertEquals(expectSQLs.get(currentSQLIndex), sql);
7979
currentSQLIndex++;
80+
return true;
8081
}
8182
};
8283

@@ -148,23 +149,21 @@ public void testPermissionManagement() {
148149

149150
// Test metalake object and different role change
150151
resetSQLIndex();
151-
expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp");
152+
expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE *.* TO ROLE tmp");
152153
SecurableObject metalakeObject =
153154
SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.SelectTable.allow()));
154155
RoleChange roleChange = RoleChange.addSecurableObject("tmp", metalakeObject);
155156
plugin.onRoleUpdated(role, roleChange);
156157

157158
resetSQLIndex();
158-
expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "REVOKE SELECT ON TABLE *.* FROM ROLE tmp");
159+
expectSQLs = Lists.newArrayList("REVOKE SELECT ON TABLE *.* FROM ROLE tmp");
159160
roleChange = RoleChange.removeSecurableObject("tmp", metalakeObject);
160161
plugin.onRoleUpdated(role, roleChange);
161162

162163
resetSQLIndex();
163164
expectSQLs =
164165
Lists.newArrayList(
165-
"CREATE ROLE tmp",
166-
"REVOKE SELECT ON TABLE *.* FROM ROLE tmp",
167-
"GRANT CREATE ON TABLE *.* TO ROLE tmp");
166+
"REVOKE SELECT ON TABLE *.* FROM ROLE tmp", "GRANT CREATE ON TABLE *.* TO ROLE tmp");
168167
SecurableObject newMetalakeObject =
169168
SecurableObjects.ofMetalake("metalake", Lists.newArrayList(Privileges.CreateTable.allow()));
170169
roleChange = RoleChange.updateSecurableObject("tmp", metalakeObject, newMetalakeObject);
@@ -175,7 +174,7 @@ public void testPermissionManagement() {
175174
SecurableObject catalogObject =
176175
SecurableObjects.ofCatalog("catalog", Lists.newArrayList(Privileges.SelectTable.allow()));
177176
roleChange = RoleChange.addSecurableObject("tmp", catalogObject);
178-
expectSQLs = Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE *.* TO ROLE tmp");
177+
expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE *.* TO ROLE tmp");
179178
plugin.onRoleUpdated(role, roleChange);
180179

181180
// Test schema object
@@ -184,8 +183,7 @@ public void testPermissionManagement() {
184183
SecurableObjects.ofSchema(
185184
catalogObject, "schema", Lists.newArrayList(Privileges.SelectTable.allow()));
186185
roleChange = RoleChange.addSecurableObject("tmp", schemaObject);
187-
expectSQLs =
188-
Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.* TO ROLE tmp");
186+
expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE schema.* TO ROLE tmp");
189187
plugin.onRoleUpdated(role, roleChange);
190188

191189
// Test table object
@@ -194,8 +192,18 @@ public void testPermissionManagement() {
194192
SecurableObjects.ofTable(
195193
schemaObject, "table", Lists.newArrayList(Privileges.SelectTable.allow()));
196194
roleChange = RoleChange.addSecurableObject("tmp", tableObject);
197-
expectSQLs =
198-
Lists.newArrayList("CREATE ROLE tmp", "GRANT SELECT ON TABLE schema.table TO ROLE tmp");
195+
expectSQLs = Lists.newArrayList("GRANT SELECT ON TABLE schema.table TO ROLE tmp");
196+
plugin.onRoleUpdated(role, roleChange);
197+
198+
// Test the role with objects
199+
resetSQLIndex();
200+
role =
201+
RoleEntity.builder()
202+
.withId(-1L)
203+
.withName("tmp")
204+
.withSecurableObjects(Lists.newArrayList(tableObject))
205+
.withAuditInfo(AuditInfo.EMPTY)
206+
.build();
199207
plugin.onRoleUpdated(role, roleChange);
200208
}
201209

‎catalogs/catalog-hive/build.gradle.kts

+2
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ tasks {
158158
exclude("guava-*.jar")
159159
exclude("log4j-*.jar")
160160
exclude("slf4j-*.jar")
161+
// Exclude the following jars to avoid conflict with the jars in authorization-gcp
162+
exclude("protobuf-java-*.jar")
161163
}
162164
into("$rootDir/distribution/package/catalogs/hive/libs")
163165
}

‎catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java

+18
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.apache.gravitino.rel.TableCatalog;
5959
import org.apache.gravitino.rel.TableChange;
6060
import org.apache.gravitino.rel.expressions.distributions.Distribution;
61+
import org.apache.gravitino.rel.expressions.distributions.Distributions;
6162
import org.apache.gravitino.rel.expressions.sorts.SortOrder;
6263
import org.apache.gravitino.rel.expressions.transforms.Transform;
6364
import org.apache.gravitino.rel.indexes.Index;
@@ -513,6 +514,13 @@ public Table createTable(
513514
.build())
514515
.toArray(IcebergColumn[]::new);
515516

517+
// Gravitino NONE distribution means the client side doesn't specify distribution, which is
518+
// not the same as none distribution in Iceberg.
519+
if (Distributions.NONE.equals(distribution)) {
520+
distribution =
521+
getIcebergDefaultDistribution(sortOrders.length > 0, partitioning.length > 0);
522+
}
523+
516524
IcebergTable createdTable =
517525
IcebergTable.builder()
518526
.withName(tableIdent.name())
@@ -588,6 +596,16 @@ public void testConnection(
588596
}
589597
}
590598

599+
private static Distribution getIcebergDefaultDistribution(
600+
boolean isSorted, boolean isPartitioned) {
601+
if (isSorted) {
602+
return Distributions.RANGE;
603+
} else if (isPartitioned) {
604+
return Distributions.HASH;
605+
}
606+
return Distributions.NONE;
607+
}
608+
591609
private static String currentUser() {
592610
return PrincipalUtils.getCurrentUserName();
593611
}

0 commit comments

Comments
 (0)