Merge pull request #1 from qqqttt123/ISSUE-1206

Issue 1206

.github/workflows/build.yml

@@ -51,6 +51,9 @@ jobs:
   build:
     # The type of runner that the job will run on
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        java-version: [ 8, 11, 17 ]
     timeout-minutes: 30
     needs: changes
     if: needs.changes.outputs.source_changes == 'true'
@@ -61,11 +64,11 @@ jobs:
 
       - uses: actions/setup-java@v3
         with:
-          java-version: '8'
+          java-version: ${{ matrix.java-version }}
           distribution: 'temurin'
 
       - name: Build with Gradle
-        run: ./gradlew build -PskipITs
+        run: ./gradlew build -PskipITs -PjdkVersion=${{ matrix.java-version }}
 
       - name: Upload unit tests report
         uses: actions/upload-artifact@v3

.github/workflows/integration-test.yml

@@ -55,6 +55,8 @@ jobs:
     strategy:
       matrix:
         architecture: [linux/amd64]
+        java-version: [ 8, 11, 17 ]
+        test-mode: [ embedded, deploy ]
     env:
       DOCKER_RUN_NAME: hive-amd64
       PLATFORM: ${{ matrix.architecture }}
@@ -63,16 +65,16 @@ jobs:
 
       - uses: actions/setup-java@v3
         with:
-          java-version: '8'
+          java-version: ${{ matrix.java-version }}
           distribution: 'temurin'
 
       - name: Set up QEMU
         uses: docker/setup-qemu-action@v2
 
       - name: Package Gravitino
         run: |
-          ./gradlew build -x test
-          ./gradlew compileDistribution -x test
+          ./gradlew build -x test -PjdkVersion=${{ matrix.java-version }}
+          ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }}
 
       - name: Setup debug Github Action
         if: ${{ contains(github.event.pull_request.labels.*.name, 'debug action') }}
@@ -81,14 +83,13 @@ jobs:
       - name: Integration Test
         id: integrationTest
         run: |
-          ./gradlew test --rerun-tasks -PskipTests -PtestMode=embedded
-          ./gradlew test --rerun-tasks -PskipTests -PtestMode=deploy
+          ./gradlew test --rerun-tasks -PskipTests -PtestMode=${{ matrix.test-mode }} -PjdkVersion=${{ matrix.java-version }}
 
       - name: Upload integrate tests reports
         uses: actions/upload-artifact@v3
         if: ${{ failure() && steps.integrationTest.outcome == 'failure' }}
         with:
-          name: integrate test reports 
+          name: integrate test reports
           path: |
             build/reports
             integration-test/build/integration-test.log

CONTRIBUTING.md

@@ -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.

README.md

@@ -81,7 +81,7 @@ To stop the Gravitino server, please run:
 
 ### Using Trino with Gravitino
 
-Gravitino provides a Trino connector to access the metadata in Gravitino. To use Trino with Gravitino, please follow the [trino-gravitino-connector doc](docs/trino-gravitino-connector.md).
+Gravitino provides a Trino connector to access the metadata in Gravitino. To use Trino with Gravitino, please follow the [trino-gravitino-connector doc](docs/trino-connector/index.md).
 
 ## Development guide
 

bin/common.sh

@@ -44,6 +44,7 @@ fi
 
 GRAVITINO_CLASSPATH+=":${GRAVITINO_CONF_DIR}"
 
+JVM_VERSION=8
 function check_java_version() {
   if [[ -n "${JAVA_HOME+x}" ]]; then
     JAVA="$JAVA_HOME/bin/java"

bin/gravitino.sh

@@ -130,6 +130,30 @@ GRAVITINO_SERVER_NAME=com.datastrato.gravitino.server.GravitinoServer
 JAVA_OPTS+=" -Dfile.encoding=UTF-8"
 JAVA_OPTS+=" -Dlog4j2.configurationFile=file://${GRAVITINO_CONF_DIR}/log4j2.properties"
 JAVA_OPTS+=" -Dgravitino.log.path=${GRAVITINO_LOG_DIR} ${GRAVITINO_MEM}"
+if [ "$JVM_VERSION" -eq 17 ]; then
+  JAVA_OPTS+=" -XX:+IgnoreUnrecognizedVMOptions"
+  JAVA_OPTS+=" --add-opens java.base/java.io=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.lang.invoke=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.lang.reflect=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.lang=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.math=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.net=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.nio=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.text=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.time=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.util.concurrent.atomic=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.util.concurrent=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.util.regex=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/java.util=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/jdk.internal.ref=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/jdk.internal.reflect=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.sql/java.sql=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/sun.util.calendar=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/sun.nio.ch=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/sun.nio.cs=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/sun.security.action=ALL-UNNAMED"
+  JAVA_OPTS+=" --add-opens java.base/sun.util.calendar=ALL-UNNAMED"
+fi
 
 addJarInDir "${GRAVITINO_HOME}/libs"
 
@@ -149,4 +173,4 @@ case "${1}" in
     ;;
   *)
     echo ${USAGE}
-esac
+esac

build.gradle.kts

@@ -32,7 +32,7 @@ plugins {
     alias(libs.plugins.spotless)
   } else {
     throw GradleException(
-      "Gravitino Gradle current doesn't support " +
+      "Gravitino Gradle toolchain current doesn't support " +
         "Java version: ${JavaVersion.current()}. Please use JDK8 to 17."
     )
   }
@@ -44,6 +44,42 @@ plugins {
   id("org.cyclonedx.bom") version "1.5.0" // Newer version fail due to our setup
 }
 
+if (extra["jdkVersion"] !in listOf("8", "11", "17")) {
+  throw GradleException(
+    "Gravitino current doesn't support building with " +
+      "Java version: ${extra["jdkVersion"]}. Please use JDK8, 11 or 17."
+  )
+}
+
+project.extra["extraJvmArgs"] = if (extra["jdkVersion"] in listOf("8", "11")) {
+  listOf()
+} else {
+  listOf(
+    "-XX:+IgnoreUnrecognizedVMOptions",
+    "--add-opens", "java.base/java.io=ALL-UNNAMED",
+    "--add-opens", "java.base/java.lang.invoke=ALL-UNNAMED",
+    "--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED",
+    "--add-opens", "java.base/java.lang=ALL-UNNAMED",
+    "--add-opens", "java.base/java.math=ALL-UNNAMED",
+    "--add-opens", "java.base/java.net=ALL-UNNAMED",
+    "--add-opens", "java.base/java.nio=ALL-UNNAMED",
+    "--add-opens", "java.base/java.text=ALL-UNNAMED",
+    "--add-opens", "java.base/java.time=ALL-UNNAMED",
+    "--add-opens", "java.base/java.util.concurrent.atomic=ALL-UNNAMED",
+    "--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED",
+    "--add-opens", "java.base/java.util.regex=ALL-UNNAMED",
+    "--add-opens", "java.base/java.util=ALL-UNNAMED",
+    "--add-opens", "java.base/jdk.internal.ref=ALL-UNNAMED",
+    "--add-opens", "java.base/jdk.internal.reflect=ALL-UNNAMED",
+    "--add-opens", "java.sql/java.sql=ALL-UNNAMED",
+    "--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED",
+    "--add-opens", "java.base/sun.nio.ch=ALL-UNNAMED",
+    "--add-opens", "java.base/sun.nio.cs=ALL-UNNAMED",
+    "--add-opens", "java.base/sun.security.action=ALL-UNNAMED",
+    "--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED"
+  )
+}
+
 licenseReport {
   renderers = arrayOf<ReportRenderer>(InventoryHtmlReportRenderer("report.html", "Backend"))
   filters = arrayOf<DependencyFilter>(LicenseBundleNormalizer())
@@ -129,7 +165,9 @@ subprojects {
       if (project.name == "trino-connector") {
         languageVersion.set(JavaLanguageVersion.of(17))
       } else {
-        languageVersion.set(JavaLanguageVersion.of(8))
+        languageVersion.set(JavaLanguageVersion.of(extra["jdkVersion"].toString().toInt()))
+        sourceCompatibility = JavaVersion.VERSION_1_8
+        targetCompatibility = JavaVersion.VERSION_1_8
       }
     }
   }
@@ -224,6 +262,8 @@ subprojects {
       } else {
         useJUnitPlatform()
       }
+
+      jvmArgs(project.property("extraJvmArgs") as List<*>)
       finalizedBy(tasks.getByName("jacocoTestReport"))
     }
   }

catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynConstructors.java

@@ -23,7 +23,6 @@
 import com.google.common.collect.Maps;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
-import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Map;
@@ -245,6 +244,7 @@ public Builder hiddenImpl(String className, Class<?>... types) {
       return this;
     }
 
+    @SuppressWarnings("removal")
     public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) {
       // don't do any work if an implementation has been found
       if (ctor != null) {
@@ -253,7 +253,7 @@ public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) {
 
       try {
         Constructor<T> hidden = targetClass.getDeclaredConstructor(types);
-        AccessController.doPrivileged(new MakeAccessible(hidden));
+        java.security.AccessController.doPrivileged(new MakeAccessible(hidden));
         ctor = new Ctor<T>(hidden, targetClass);
       } catch (SecurityException e) {
         // unusable

catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynFields.java

@@ -25,7 +25,6 @@
 import com.google.common.collect.Sets;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Set;
 
@@ -302,6 +301,7 @@ public Builder hiddenImpl(String className, String fieldName) {
      * @see Class#forName(String)
      * @see Class#getField(String)
      */
+    @SuppressWarnings("removal")
     public Builder hiddenImpl(Class<?> targetClass, String fieldName) {
       // don't do any work if an implementation has been found
       if (field != null || targetClass == null) {
@@ -310,7 +310,7 @@ public Builder hiddenImpl(Class<?> targetClass, String fieldName) {
 
       try {
         Field hidden = targetClass.getDeclaredField(fieldName);
-        AccessController.doPrivileged(new MakeFieldAccessible(hidden));
+        java.security.AccessController.doPrivileged(new MakeFieldAccessible(hidden));
         this.field = new UnboundField(hidden, fieldName);
       } catch (SecurityException | NoSuchFieldException e) {
         // unusable

catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynMethods.java

@@ -23,7 +23,6 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 
@@ -393,6 +392,7 @@ public Builder hiddenImpl(String className, Class<?>... argClasses) {
      * @see Class#forName(String)
      * @see Class#getMethod(String, Class[])
      */
+    @SuppressWarnings("removal")
     public Builder hiddenImpl(Class<?> targetClass, String methodName, Class<?>... argClasses) {
       // don't do any work if an implementation has been found
       if (method != null) {
@@ -401,7 +401,7 @@ public Builder hiddenImpl(Class<?> targetClass, String methodName, Class<?>... a
 
       try {
         Method hidden = targetClass.getDeclaredMethod(methodName, argClasses);
-        AccessController.doPrivileged(new MakeAccessible(hidden));
+        java.security.AccessController.doPrivileged(new MakeAccessible(hidden));
         this.method = new UnboundMethod(hidden, name);
       } catch (SecurityException | NoSuchMethodException e) {
         // unusable or not the right implementation

catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogOperations.java

@@ -203,7 +203,7 @@ public JdbcSchema loadSchema(NameIdentifier ident) throws NoSuchSchemaException
     }
     Map<String, String> properties =
         load.properties() == null ? Maps.newHashMap() : Maps.newHashMap(load.properties());
-    StringIdentifier.addToProperties(id, properties);
+    StringIdentifier.newPropertiesWithId(id, properties);
     return new JdbcSchema.Builder()
         .withAuditInfo(load.auditInfo())
         .withName(load.name())
@@ -276,7 +276,7 @@ public Table loadTable(NameIdentifier tableIdent) throws NoSuchTableException {
     }
     Map<String, String> properties =
         load.properties() == null ? Maps.newHashMap() : Maps.newHashMap(load.properties());
-    StringIdentifier.addToProperties(id, properties);
+    properties = StringIdentifier.newPropertiesWithId(id, properties);
     return new JdbcTable.Builder()
         .withAuditInfo(load.auditInfo())
         .withName(tableName)