Copilot commented on code in PR #7552:
URL: https://github.com/apache/kyuubi/pull/7552#discussion_r3557016764


##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/PaimonCatalogRangerSparkExtensionSuite.scala:
##########
@@ -556,7 +556,7 @@ class PaimonCatalogRangerSparkExtensionSuite extends 
RangerSparkExtensionSuite {
       val changingColumnTypeSql =
         s"""
            |ALTER TABLE $catalogV2.$namespace1.$table1
-           |ALTER COLUMN id TYPE DOUBLE
+           |ALTER COLUMN name TYPE STRING
            |""".stripMargin

Review Comment:
   This test now issues `ALTER COLUMN name TYPE STRING`, but `name` is already 
declared as `string` in `createTableSql`. If Spark/Paimon treat same-type 
changes as a no-op (or rewrite the plan differently), the test may no longer 
meaningfully exercise the ALTER COLUMN privilege extraction/authorization path 
it is meant to cover. Consider changing to a real type change on a non-PK 
column that is supported by Paimon (e.g., STRING -> VARCHAR/CHAR) or creating a 
dedicated table schema for this test.



##########
pom.xml:
##########
@@ -2101,14 +2098,13 @@
                 <spark.version>4.1.2</spark.version>
                 <spark.binary.version>4.1</spark.binary.version>
                 <antlr4.version>4.13.1</antlr4.version>
-                <delta.version>4.0.0</delta.version>
-                
<delta.artifact>delta-spark_${scala.binary.version}</delta.artifact>
-                <!-- TODO: update once Hudi support Spark 4.1 -->
-                
<hudi.artifact>hudi-spark3.5-bundle_${scala.binary.version}</hudi.artifact>
-                <!-- TODO: update once Paimon support Spark 4.1.
-                           paimon-spark-3.5 contains Scala 2.12 classes cause 
conflicts with Scala 2.13 -->
-                <paimon.artifact>paimon-common</paimon.artifact>
-                
<maven.plugin.scalatest.exclude.tags>org.scalatest.tags.Slow,org.apache.kyuubi.tags.DeltaTest,org.apache.kyuubi.tags.IcebergTest,org.apache.kyuubi.tags.PaimonTest,org.apache.kyuubi.tags.HudiTest</maven.plugin.scalatest.exclude.tags>
+                <delta.version>4.3.1</delta.version>
+                
<delta.artifact>delta-spark_${spark.binary.version}_${scala.binary.version}</delta.artifact>
+                <hudi.version>1.2.0</hudi.version>
+                <!-- TODO: update once Paimon support Spark 4.1. -->
+                <paimon.version>1.4.2</paimon.version>
+                
<paimon.artifact>paimon-spark-4.0_${scala.binary.version}</paimon.artifact>
+                
<maven.plugin.scalatest.exclude.tags>org.scalatest.tags.Slow,org.apache.kyuubi.tags.PaimonTest</maven.plugin.scalatest.exclude.tags>

Review Comment:
   The `spark-4.1` Maven profile still excludes 
`org.apache.kyuubi.tags.PaimonTest`, but the PR description states Paimon tests 
are enabled for Spark 4.0/4.1. Also, the profile pins `paimon.artifact` to a 
Spark 4.0 artifact while `spark.binary.version` is 4.1, which suggests Spark 
4.1 support may still be incomplete. Please align the profile configuration 
with the intended support level (either enable the tests and use a compatible 
artifact, or update the PR description / keep Paimon disabled intentionally).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to