Copilot commented on code in PR #3364:
URL: https://github.com/apache/fluss/pull/3364#discussion_r3287382547
##########
fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/lake/FlussLakeUtils.scala:
##########
@@ -25,17 +25,25 @@ import org.apache.fluss.utils.PropertiesUtils
import java.util
+import scala.collection.JavaConverters._
+
object FlussLakeUtils {
+ private val SPARK_CATALOG_PREFIX = "spark.catalog."
Review Comment:
`SPARK_CATALOG_PREFIX` is set to `spark.catalog.` but Spark’s catalog plugin
configuration keys are `spark.sql.catalog.*` (see e.g. FlussSparkTestBase and
website docs). With the current prefix, lake storage plugins that expect Spark
catalog configs won’t be able to discover them. Consider switching to
`spark.sql.catalog.` (and ensuring keys are constructed in the same shape Spark
expects).
##########
fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/lake/FlussLakeUtils.scala:
##########
@@ -25,17 +25,25 @@ import org.apache.fluss.utils.PropertiesUtils
import java.util
+import scala.collection.JavaConverters._
+
object FlussLakeUtils {
+ private val SPARK_CATALOG_PREFIX = "spark.catalog."
+
def createLakeSource(
+ catalogProperties: util.Map[String, String],
tableProperties: util.Map[String, String],
tablePath: TablePath): LakeSource[LakeSplit] = {
val tableConfig = Configuration.fromMap(tableProperties)
val datalakeFormat = tableConfig.get(ConfigOptions.TABLE_DATALAKE_FORMAT)
val dataLakePrefix = "table.datalake." + datalakeFormat + "."
- val catalogProperties =
PropertiesUtils.extractAndRemovePrefix(tableProperties, dataLakePrefix)
- val lakeConfig = Configuration.fromMap(catalogProperties)
+ val lakeConfig =
+
Configuration.fromMap(PropertiesUtils.extractAndRemovePrefix(tableProperties,
dataLakePrefix))
+ catalogProperties.asScala.foreach {
+ case (k, v) => lakeConfig.setString(s"$SPARK_CATALOG_PREFIX$k", v)
+ }
Review Comment:
This change forwards catalog properties into `lakeConfig` to enable
overriding table defaults, but there’s no unit/integration test asserting that
catalog-provided configs actually take precedence over
`table.datalake.<format>.*` values. Adding a Spark lake read UT that sets
conflicting values via catalog options and verifies the plugin observes the
catalog value would prevent regressions.
--
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]
