You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhztheplayer (via GitHub)" <gi...@apache.org> on 2025/07/28 16:03:52 UTC
[PR] Wip 52978 [spark]
zhztheplayer opened a new pull request, #51690:
URL: https://github.com/apache/spark/pull/51690
### What changes were proposed in this pull request?
The PR proposes a change to make the V1 file write API FileFormatWriter.write customizable via a SQL configuration option.
### Why are the changes needed?
Doing this will:
1. Allow 3rd columnar plugins (by setting this option by default) to specify a columnar v1 writer during plugin initialization;
2. Allow user to specify a customized row-based writer for certain purposes, e.g., performance, specific handling of partitioning, etc.;
### Does this PR introduce _any_ user-facing change?
The user calls to `FileFormatWriter` needs to be changed to `DefaultFileFormatWriter` as the former becomes a trait.
For example, before:
```scala
FileFormatWriter.write(...)
```
After:
```scala
DefaultFileFormatWriter.write(...)
```
### How was this patch tested?
Existing tests. An individual test for the customization is also being added.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #51690:
URL: https://github.com/apache/spark/pull/51690#discussion_r2243085904
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DefaultFileFormatWriter.scala:
##########
@@ -0,0 +1,441 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources
+
+import java.util.{Date, UUID}
+
+import org.apache.hadoop.conf.Configuration
+import org.apache.hadoop.fs.Path
+import org.apache.hadoop.mapreduce._
+import org.apache.hadoop.mapreduce.lib.output.FileOutputFormat
+import org.apache.hadoop.mapreduce.task.TaskAttemptContextImpl
+
+import org.apache.spark._
+import org.apache.spark.internal.{Logging, LogKeys, MDC}
+import org.apache.spark.internal.LogKeys._
+import org.apache.spark.internal.io.{FileCommitProtocol, SparkHadoopWriterUtils}
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.catalog.BucketSpec
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.expressions.BindReferences.bindReferences
+import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap, DateTimeUtils}
+import org.apache.spark.sql.classic.SparkSession
+import org.apache.spark.sql.connector.write.WriterCommitMessage
+import org.apache.spark.sql.execution.{ProjectExec, SortExec, SparkPlan, SQLExecution}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanExec
+import org.apache.spark.util.{SerializableConfiguration, Utils}
+import org.apache.spark.util.ArrayImplicits._
+
+
+/** A helper object for writing FileFormat data out to a location. */
+object DefaultFileFormatWriter extends FileFormatWriter with Logging {
Review Comment:
To ease reviewing:
Git determines this file as a creation. However this is mostly a trivial file move, i.e., mv `FileFormatWriter.scala` `DefaultFileFormatWriter.scala`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #51690:
URL: https://github.com/apache/spark/pull/51690#issuecomment-3505468422
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #51690: [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration
URL: https://github.com/apache/spark/pull/51690
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #51690:
URL: https://github.com/apache/spark/pull/51690#issuecomment-3131523094
The CI error doesn't seem related:
```
2025-07-29T06:09:46.5179855Z [0m[[0m[0minfo[0m] [0m[0m[32mOracleIntegrationSuite:[0m[0m
2025-07-29T06:20:04.4130715Z [0m[[0m[0minfo[0m] [0m[0m[31morg.apache.spark.sql.jdbc.OracleIntegrationSuite *** ABORTED *** (10 minutes, 17 seconds)[0m[0m
2025-07-29T06:20:04.4133584Z [0m[[0m[0minfo[0m] [0m[0m[31m The code passed to eventually never returned normally. Attempted 599 times over 10.012181455083333 minutes. Last failure message: ORA-12541: Cannot connect. No listener at host 10.1.0.216 port 33347. (CONNECTION_ID=JatCXhizR4K4tbKsMUeaDQ==)[0m[0m
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #51690:
URL: https://github.com/apache/spark/pull/51690#discussion_r2243085904
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DefaultFileFormatWriter.scala:
##########
@@ -0,0 +1,441 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources
+
+import java.util.{Date, UUID}
+
+import org.apache.hadoop.conf.Configuration
+import org.apache.hadoop.fs.Path
+import org.apache.hadoop.mapreduce._
+import org.apache.hadoop.mapreduce.lib.output.FileOutputFormat
+import org.apache.hadoop.mapreduce.task.TaskAttemptContextImpl
+
+import org.apache.spark._
+import org.apache.spark.internal.{Logging, LogKeys, MDC}
+import org.apache.spark.internal.LogKeys._
+import org.apache.spark.internal.io.{FileCommitProtocol, SparkHadoopWriterUtils}
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.catalog.BucketSpec
+import org.apache.spark.sql.catalyst.expressions._
+import org.apache.spark.sql.catalyst.expressions.BindReferences.bindReferences
+import org.apache.spark.sql.catalyst.util.{CaseInsensitiveMap, DateTimeUtils}
+import org.apache.spark.sql.classic.SparkSession
+import org.apache.spark.sql.connector.write.WriterCommitMessage
+import org.apache.spark.sql.execution.{ProjectExec, SortExec, SparkPlan, SQLExecution}
+import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanExec
+import org.apache.spark.util.{SerializableConfiguration, Utils}
+import org.apache.spark.util.ArrayImplicits._
+
+
+/** A helper object for writing FileFormat data out to a location. */
+object DefaultFileFormatWriter extends FileFormatWriter with Logging {
Review Comment:
To ease reviewing:
Git determines this file as a creation. However this is mostly a trivial file move, i.e., mv `FileFormatWriter.scala` `DefaultFileFormatWriter.scala`. The content is basically identical.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-52978][SQL] Make FileFormatWriter customizable via SQL configuration [spark]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #51690:
URL: https://github.com/apache/spark/pull/51690#issuecomment-3131648956
cc @cloud-fan @yaooqinn @dongjoon-hyun Appreciate your thoughts on this.
We had some long-standing issues in Gluten that could be resolved by opening up this API. It will also be helpful for lake support in Gluten as well.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org