From 32bc3148d542478e867372876b47b9e85eac1207 Mon Sep 17 00:00:00 2001 From: Kazuyuki Tanimura Date: Wed, 22 May 2024 22:26:31 -0700 Subject: [PATCH] address review comments --- .github/workflows/pr_build.yml | 42 ++++++------------- .../apache/comet/shims/CometExprShim.scala | 4 +- .../apache/comet/shims/CometExprShim.scala | 4 +- .../apache/comet/shims/CometExprShim.scala | 4 +- .../apache/comet/shims/CometExprShim.scala | 4 +- .../comet/exec/CometExec3_4PlusSuite.scala | 2 +- 6 files changed, 21 insertions(+), 39 deletions(-) diff --git a/.github/workflows/pr_build.yml b/.github/workflows/pr_build.yml index 483e663a2..410f1e1fe 100644 --- a/.github/workflows/pr_build.yml +++ b/.github/workflows/pr_build.yml @@ -81,7 +81,7 @@ jobs: matrix: os: [ubuntu-latest] java_version: [17] - test-target: [rust, java] + test-target: [java] spark-version: ['4.0'] is_push_event: - ${{ github.event_name == 'push' }} @@ -97,22 +97,16 @@ jobs: with: rust-version: ${{env.RUST_VERSION}} jdk-version: ${{ matrix.java_version }} - - if: matrix.test-target == 'rust' - name: Rust test steps - uses: ./.github/actions/rust-test - - if: matrix.test-target == 'java' - name: Clone Spark + - name: Clone Spark uses: actions/checkout@v4 with: repository: "apache/spark" path: "apache-spark" - - if: matrix.test-target == 'java' - name: Install Spark + - name: Install Spark shell: bash working-directory: ./apache-spark run: build/mvn install -Phive -Phadoop-cloud -DskipTests - - if: matrix.test-target == 'java' - name: Java test steps + - name: Java test steps uses: ./.github/actions/java-test with: # TODO: remove -DskipTests after fixing tests @@ -218,7 +212,7 @@ jobs: matrix: os: [macos-13] java_version: [17] - test-target: [rust, java] + test-target: [java] spark-version: ['4.0'] fail-fast: false if: github.event_name == 'push' @@ -231,22 +225,16 @@ jobs: with: rust-version: ${{env.RUST_VERSION}} jdk-version: ${{ matrix.java_version }} - - if: matrix.test-target == 'rust' - name: Rust test steps - uses: ./.github/actions/rust-test - - if: matrix.test-target == 'java' - name: Clone Spark + - name: Clone Spark uses: actions/checkout@v4 with: repository: "apache/spark" path: "apache-spark" - - if: matrix.test-target == 'java' - name: Install Spark + - name: Install Spark shell: bash working-directory: ./apache-spark run: build/mvn install -Phive -Phadoop-cloud -DskipTests - - if: matrix.test-target == 'java' - name: Java test steps + - name: Java test steps uses: ./.github/actions/java-test with: # TODO: remove -DskipTests after fixing tests @@ -258,7 +246,7 @@ jobs: strategy: matrix: java_version: [17] - test-target: [rust, java] + test-target: [java] spark-version: ['4.0'] is_push_event: - ${{ github.event_name == 'push' }} @@ -277,22 +265,16 @@ jobs: jdk-version: ${{ matrix.java_version }} jdk-architecture: aarch64 protoc-architecture: aarch_64 - - if: matrix.test-target == 'rust' - name: Rust test steps - uses: ./.github/actions/rust-test - - if: matrix.test-target == 'java' - name: Clone Spark + - name: Clone Spark uses: actions/checkout@v4 with: repository: "apache/spark" path: "apache-spark" - - if: matrix.test-target == 'java' - name: Install Spark + - name: Install Spark shell: bash working-directory: ./apache-spark run: build/mvn install -Phive -Phadoop-cloud -DskipTests - - if: matrix.test-target == 'java' - name: Java test steps + - name: Java test steps uses: ./.github/actions/java-test with: # TODO: remove -DskipTests after fixing tests diff --git a/spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala index 0c45a9c2c..f5a578f82 100644 --- a/spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.2/org/apache/comet/shims/CometExprShim.scala @@ -25,8 +25,8 @@ import org.apache.spark.sql.catalyst.expressions._ */ trait CometExprShim { /** - * Returns a tuple of expressions for the `unhex` function. - */ + * Returns a tuple of expressions for the `unhex` function. + */ def unhexSerde(unhex: Unhex): (Expression, Expression) = { (unhex.child, Literal(false)) } diff --git a/spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala index 0c45a9c2c..f5a578f82 100644 --- a/spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.3/org/apache/comet/shims/CometExprShim.scala @@ -25,8 +25,8 @@ import org.apache.spark.sql.catalyst.expressions._ */ trait CometExprShim { /** - * Returns a tuple of expressions for the `unhex` function. - */ + * Returns a tuple of expressions for the `unhex` function. + */ def unhexSerde(unhex: Unhex): (Expression, Expression) = { (unhex.child, Literal(false)) } diff --git a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala index 409e1c94b..3f2301f0a 100644 --- a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala @@ -25,8 +25,8 @@ import org.apache.spark.sql.catalyst.expressions._ */ trait CometExprShim { /** - * Returns a tuple of expressions for the `unhex` function. - */ + * Returns a tuple of expressions for the `unhex` function. + */ def unhexSerde(unhex: Unhex): (Expression, Expression) = { (unhex.child, Literal(unhex.failOnError)) } diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala index 7d4ec1537..01f923206 100644 --- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala @@ -25,8 +25,8 @@ import org.apache.spark.sql.catalyst.expressions._ */ trait CometExprShim { /** - * Returns a tuple of expressions for the `unhex` function. - */ + * Returns a tuple of expressions for the `unhex` function. + */ protected def unhexSerde(unhex: Unhex): (Expression, Expression) = { (unhex.child, Literal(unhex.failOnError)) } diff --git a/spark/src/test/spark-3.4-plus/org/apache/comet/exec/CometExec3_4PlusSuite.scala b/spark/src/test/spark-3.4-plus/org/apache/comet/exec/CometExec3_4PlusSuite.scala index 8d1a68176..1ceec4b92 100644 --- a/spark/src/test/spark-3.4-plus/org/apache/comet/exec/CometExec3_4PlusSuite.scala +++ b/spark/src/test/spark-3.4-plus/org/apache/comet/exec/CometExec3_4PlusSuite.scala @@ -27,7 +27,7 @@ import org.apache.spark.sql.CometTestBase import org.apache.comet.CometConf /** - * This test suite contains tests for only Spark 3.4. + * This test suite contains tests for only Spark 3.4+. */ class CometExec3_4PlusSuite extends CometTestBase { import testImplicits._