You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "sandip-db (via GitHub)" <gi...@apache.org> on 2023/07/03 17:25:51 UTC
[GitHub] [spark] sandip-db opened a new pull request, #41832: [SPARK-44265][SQL] Built-in XML data source support
sandip-db opened a new pull request, #41832:
URL: https://github.com/apache/spark/pull/41832
### What changes were proposed in this pull request?
XML is a widely used data format. An external spark-xml package (https://github.com/databricks/spark-xml) is available to read and write XML data in spark. Making spark-xml built-in will provide a better user experience for Spark SQL and structured streaming. The proposal is to inline code from spark-xml package.
The PR has 4 main commits:
i) The first commit is just a vanilla copy of spark-xml src files to spark/connector
ii) The second commit fixes scala format issues and update/add ASF license to relevant files.
iii) Add mvn dependencies, etc.
iv) Use SharedSparkSession and testFiles to access resource files in the XML unit tests under all environment (sbt, mvn, IntelliJ).
### Why are the changes needed?
Built-in support for XML data source would provide better user experience than having to import an external package.
### Does this PR introduce _any_ user-facing change?
Yes, Add built-in support for XML data source.
### How was this patch tested?
Tested the new unit-tests that came with the imported spark-xml package.
Also ran ./dev/run-test
--
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
[GitHub] [spark] sandip-db commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1670480878
> @sandip-db would you mind adding some comments about what's new codes (e.g., pom.xml)? Then I think it'd be much easier to merge since most of the codes are already exiting one.
>
> I took a cursory look, and it seems pretty fine to me.
Hi @HyukjinKwon and reviewers,
As indicated in the description, there is very little new code in this PR. Most of it is picked up from spark-xml package.
The first commit is the vanilla copy of spark-xml with license and scala style fixes.
Reviewers should focus only on the new code in other commits, which doesn't change any functionality. These changes addresses the dependencies and test issues.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1672442580
Merged to master.
--
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
[GitHub] [spark] sandip-db commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1289506574
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlOptions.scala:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.xml
+
+import java.nio.charset.StandardCharsets
+
+import org.apache.spark.sql.catalyst.util.{ParseMode, PermissiveMode}
+
+/**
+ * Options for the XML data source.
+ */
+private[xml] class XmlOptions(
+ @transient private val parameters: Map[String, String])
+ extends Serializable {
+
+ def this() = this(Map.empty)
+
+ val charset = parameters.getOrElse("charset", XmlOptions.DEFAULT_CHARSET)
Review Comment:
Created a sub-task for documentation: https://issues.apache.org/jira/browse/SPARK-44752
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1663243018
Seems the test failure is flakiness. Mind retriggering https://github.com/sandip-db/spark/actions/runs/5745270542/job/15573043078 please @sandip-db ?
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1287807929
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlReader.scala:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.xml
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.{DataFrame, Dataset, SparkSession, SQLContext}
+import org.apache.spark.sql.catalyst.util.FailFastMode
+import org.apache.spark.sql.execution.datasources.xml.util.XmlFile
+import org.apache.spark.sql.types.StructType
+
+/**
+ * A collection of static functions for working with XML files in Spark SQL
+ */
+class XmlReader(private var schema: StructType,
Review Comment:
Actually I think we can remove the whole file but can be done separately in a followup.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/functions.scala:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.xml
+
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types.DataType
+
+/**
+ * Support functions for working with XML columns directly.
+ */
+// scalastyle:off: object.name
+object functions {
Review Comment:
As a followup, we should move this function to `org.apache.spark.sql.functions`
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/util/InferSchema.scala:
##########
@@ -0,0 +1,336 @@
+/*
Review Comment:
Would have to be placed together with CSVInferSchema at catalyst.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/util/InferSchema.scala:
##########
@@ -0,0 +1,336 @@
+/*
Review Comment:
the parsers too
##########
sql/core/src/test/resources/test-data/xml-resources/ages-with-spaces.xml:
##########
@@ -0,0 +1,20 @@
+<people>
+ <person>
+ <age born=" 1990-02-24 "> 25 </age>
Review Comment:
I was so young :-)
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/util/XmlFile.scala:
##########
@@ -0,0 +1,163 @@
+/*
+ * 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.xml.util
+
+import java.io.CharArrayWriter
+import java.nio.charset.Charset
+import javax.xml.stream.XMLOutputFactory
+
+import scala.collection.Map
+
+import com.sun.xml.txw2.output.IndentingXMLStreamWriter
+import org.apache.hadoop.conf.Configuration
+import org.apache.hadoop.io.{LongWritable, Text}
+import org.apache.hadoop.io.compress.CompressionCodec
+
+import org.apache.spark.SparkContext
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.catalyst.util.CompressionCodecs
+import org.apache.spark.sql.execution.datasources.xml.{XmlInputFormat, XmlOptions}
+import org.apache.spark.sql.execution.datasources.xml.parsers.StaxXmlGenerator
+
+private[xml] object XmlFile {
Review Comment:
we won't likely need this file too
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlDataToCatalyst.scala:
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.xml
+
+import org.apache.spark.sql.catalyst.CatalystTypeConverters
+import org.apache.spark.sql.catalyst.expressions.{ExpectsInputTypes, Expression, UnaryExpression}
+import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback
+import org.apache.spark.sql.catalyst.util.GenericArrayData
+import org.apache.spark.sql.execution.datasources.xml.parsers.StaxXmlParser
+import org.apache.spark.sql.types._
+import org.apache.spark.unsafe.types.UTF8String
+
+case class XmlDataToCatalyst(
Review Comment:
Would need to register this to SQL expression. Can be done in a followup.
##########
sql/core/src/test/java/test/org/apache/spark/sql/execution/datasources/xml/JavaXmlSuite.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * 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 test.org.apache.spark.sql.execution.datasources.xml;
+
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.SparkSession;
+import org.apache.spark.sql.execution.datasources.xml.XmlOptions;
+import org.apache.spark.sql.execution.datasources.xml.XmlReader;
+
+public final class JavaXmlSuite {
+
+ private static final int numBooks = 12;
+ private static final String booksFile = "src/test/resources/test-data/xml-resources/books.xml";
Review Comment:
I wonder if we should just name it `resources` instead of `xml-resources`. Or create a sub directory?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/package.scala:
##########
@@ -0,0 +1,162 @@
+/*
+ * 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 org.apache.hadoop.io.compress.CompressionCodec
+
+import org.apache.spark.sql.{DataFrame, DataFrameReader, DataFrameWriter, Dataset}
+import org.apache.spark.sql.{Encoders, Row, SparkSession, SQLContext}
+import org.apache.spark.sql.execution.datasources.xml.parsers.StaxXmlParser
+import org.apache.spark.sql.execution.datasources.xml.util.{InferSchema, XmlFile}
+import org.apache.spark.sql.types.{ArrayType, StructType}
+
+package object xml {
Review Comment:
Would have to remove this file too, and move those utils such as `schema_of_xml` to somewhere else.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/util/PartialResultException.scala:
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.xml.util
Review Comment:
We actually have it in the codebase already :-).
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlReader.scala:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.xml
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.{DataFrame, Dataset, SparkSession, SQLContext}
+import org.apache.spark.sql.catalyst.util.FailFastMode
+import org.apache.spark.sql.execution.datasources.xml.util.XmlFile
+import org.apache.spark.sql.types.StructType
+
+/**
+ * A collection of static functions for working with XML files in Spark SQL
+ */
+class XmlReader(private var schema: StructType,
Review Comment:
Should probably add a couple of methods in DataFrameReader/DataStreamReader
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlRelation.scala:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.xml
+
+import java.io.IOException
+
+import org.apache.hadoop.fs.Path
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql._
+import org.apache.spark.sql.execution.datasources.xml.parsers.StaxXmlParser
+import org.apache.spark.sql.execution.datasources.xml.util.{InferSchema, XmlFile}
+import org.apache.spark.sql.sources.{BaseRelation, InsertableRelation, PrunedScan}
+import org.apache.spark.sql.types._
+
+case class XmlRelation protected[spark] (
+ baseRDD: () => RDD[String],
+ location: Option[String],
+ parameters: Map[String, String],
+ userSchema: StructType = null)(@transient val sqlContext: SQLContext)
+ extends BaseRelation
+ with InsertableRelation
+ with PrunedScan {
+
+ // Hacky: ensure RDD's underlying data actually already exists early on
+ baseRDD().partitions
+
+ private val options = XmlOptions(parameters)
+
+ override val schema: StructType = {
+ Option(userSchema).getOrElse {
+ InferSchema.infer(
+ baseRDD(),
+ options)
+ }
+ }
+
+ override def buildScan(requiredColumns: Array[String]): RDD[Row] = {
+ val requiredFields = requiredColumns.map(schema(_))
+ val requestedSchema = StructType(requiredFields)
+ StaxXmlParser.parse(
+ baseRDD(),
+ requestedSchema,
+ options)
+ }
+
+ // The function below was borrowed from JSONRelation
+ override def insert(data: DataFrame, overwrite: Boolean): Unit = {
+ val filesystemPath = location match {
+ case Some(p) => new Path(p)
+ case None =>
+ throw new IOException(s"Cannot INSERT into table with no path defined")
+ }
+
+ val fs = filesystemPath.getFileSystem(sqlContext.sparkContext.hadoopConfiguration)
+
+ if (overwrite) {
+ try {
+ fs.delete(filesystemPath, true)
+ } catch {
+ case e: IOException =>
+ throw new IOException(
Review Comment:
As a followup, we should leverage error framework we added (see `QueryExecutionErrors`)
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1287845390
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/functions.scala:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.xml
+
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types.DataType
+
+/**
+ * Support functions for working with XML columns directly.
+ */
+// scalastyle:off: object.name
+object functions {
+// scalastyle:on: object.name
+ /**
+ * Parses a column containing a XML string into a `StructType` with the specified schema.
+ *
+ * @param e a string column containing XML data
+ * @param schema the schema to use when parsing the XML string. Must be a StructType if
+ * column is string-valued, or ArrayType[StructType] if column is an array of strings
+ * @param options key-value pairs that correspond to those supported by [[XmlOptions]]
+ */
+ def from_xml(e: Column, schema: DataType, options: Map[String, String] = Map.empty): Column = {
Review Comment:
should add Python and sparkR binding including Spark Connect
--
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
[GitHub] [spark] sandip-db commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1289506661
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/functions.scala:
##########
@@ -0,0 +1,42 @@
+/*
+ * 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.xml
+
+import org.apache.spark.sql.Column
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.types.DataType
+
+/**
+ * Support functions for working with XML columns directly.
+ */
+// scalastyle:off: object.name
+object functions {
+// scalastyle:on: object.name
+ /**
+ * Parses a column containing a XML string into a `StructType` with the specified schema.
+ *
+ * @param e a string column containing XML data
+ * @param schema the schema to use when parsing the XML string. Must be a StructType if
+ * column is string-valued, or ArrayType[StructType] if column is an array of strings
+ * @param options key-value pairs that correspond to those supported by [[XmlOptions]]
+ */
+ def from_xml(e: Column, schema: DataType, options: Map[String, String] = Map.empty): Column = {
Review Comment:
Added a sub-task: https://issues.apache.org/jira/browse/SPARK-44753
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1619260660
This would need an SPIP.
--
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
[GitHub] [spark] sandip-db commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1619205931
@HyukjinKwon
--
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
[GitHub] [spark] sandip-db commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1289506925
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/util/XmlFile.scala:
##########
@@ -0,0 +1,163 @@
+/*
+ * 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.xml.util
+
+import java.io.CharArrayWriter
+import java.nio.charset.Charset
+import javax.xml.stream.XMLOutputFactory
+
+import scala.collection.Map
+
+import com.sun.xml.txw2.output.IndentingXMLStreamWriter
+import org.apache.hadoop.conf.Configuration
+import org.apache.hadoop.io.{LongWritable, Text}
+import org.apache.hadoop.io.compress.CompressionCodec
+
+import org.apache.spark.SparkContext
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.DataFrame
+import org.apache.spark.sql.catalyst.util.CompressionCodecs
+import org.apache.spark.sql.execution.datasources.xml.{XmlInputFormat, XmlOptions}
+import org.apache.spark.sql.execution.datasources.xml.parsers.StaxXmlGenerator
+
+private[xml] object XmlFile {
Review Comment:
All of the above comments will be addressed in this sub-task:
https://issues.apache.org/jira/browse/SPARK-44751
--
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
[GitHub] [spark] gengliangwang commented on a diff in pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1282605700
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlInputFormat.scala:
##########
@@ -0,0 +1,341 @@
+/*
+ * 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.xml
+
+import java.io.{InputStream, InputStreamReader, IOException, Reader}
+import java.nio.ByteBuffer
+import java.nio.charset.Charset
+
+import org.apache.commons.io.input.CountingInputStream
+import org.apache.hadoop.fs.Seekable
+import org.apache.hadoop.io.{LongWritable, Text}
+import org.apache.hadoop.io.compress._
+import org.apache.hadoop.mapreduce.{InputSplit, RecordReader, TaskAttemptContext}
+import org.apache.hadoop.mapreduce.lib.input.{FileSplit, TextInputFormat}
+
+/**
+ * Reads records that are delimited by a specific start/end tag.
+ */
+class XmlInputFormat extends TextInputFormat {
+
+ override def createRecordReader(
+ split: InputSplit,
+ context: TaskAttemptContext): RecordReader[LongWritable, Text] = {
+ new XmlRecordReader
+ }
+}
+
+object XmlInputFormat {
+ /** configuration key for start tag */
+ val START_TAG_KEY: String = "xmlinput.start"
+ /** configuration key for end tag */
+ val END_TAG_KEY: String = "xmlinput.end"
+ /** configuration key for encoding type */
+ val ENCODING_KEY: String = "xmlinput.encoding"
+}
+
+/**
+ * XMLRecordReader class to read through a given xml document to output xml blocks as records
+ * as specified by the start tag and end tag.
+ *
+ * This implementation is ultimately loosely based on LineRecordReader in Hadoop.
+ */
+private[xml] class XmlRecordReader extends RecordReader[LongWritable, Text] {
+
+ private var startTag: String = _
+ private var currentStartTag: String = _
+ private var endTag: String = _
+ private var currentKey: LongWritable = _
+ private var currentValue: Text = _
+ private var start: Long = _
+ private var end: Long = _
+ private var reader: Reader = _
+ private var filePosition: Seekable = _
+ private var countingIn: CountingInputStream = _
+ private var readerLeftoverCharFn: () => Boolean = _
+ private var readerByteBuffer: ByteBuffer = _
+ private var decompressor: Decompressor = _
+ private var buffer = new StringBuilder()
+
+ override def initialize(split: InputSplit, context: TaskAttemptContext): Unit = {
+ val fileSplit = split.asInstanceOf[FileSplit]
+ val conf = context.getConfiguration
+ val charset =
+ Charset.forName(conf.get(XmlInputFormat.ENCODING_KEY, XmlOptions.DEFAULT_CHARSET))
+ startTag = conf.get(XmlInputFormat.START_TAG_KEY)
+ endTag = conf.get(XmlInputFormat.END_TAG_KEY)
+ start = fileSplit.getStart
+ end = start + fileSplit.getLength
+
+ // open the file and seek to the start of the split
+ val path = fileSplit.getPath
+ val fs = path.getFileSystem(conf)
+ val fsin = fs.open(fileSplit.getPath)
+
+ var in: InputStream = null
+ val codec = new CompressionCodecFactory(conf).getCodec(path)
+ if (null != codec) {
+ decompressor = CodecPool.getDecompressor(codec)
+ codec match {
+ case sc: SplittableCompressionCodec =>
+ val cIn = sc.createInputStream(
+ fsin,
+ decompressor,
+ start,
+ end,
+ SplittableCompressionCodec.READ_MODE.BYBLOCK)
+ start = cIn.getAdjustedStart
+ end = cIn.getAdjustedEnd
+ in = cIn
+ filePosition = cIn
+ case c: CompressionCodec =>
+ if (start != 0) {
+ // So we have a split that is only part of a file stored using
+ // a Compression codec that cannot be split.
+ throw new IOException("Cannot seek in " +
+ codec.getClass.getSimpleName + " compressed stream")
+ }
+ val cIn = c.createInputStream(fsin, decompressor)
+ in = cIn
+ filePosition = fsin
+ }
+ } else {
+ fsin.seek(start)
+ countingIn = new CountingInputStream(fsin)
+ in = countingIn
+ // don't use filePosition in this case. We have to count bytes read manually
+ }
+
+ reader = new InputStreamReader(in, charset)
+
+ if (codec == null) {
+ // Hack: in the uncompressed case (see more below), we must know how much the
+ // InputStreamReader has buffered but not processed
+ // to accurately assess how many bytes have been processed
+ val sdField = reader.getClass.getDeclaredField("sd")
+ sdField.setAccessible(true)
+ val sd = sdField.get(reader)
+ val readerLeftoverCharField = sd.getClass.getDeclaredField("haveLeftoverChar")
+ readerLeftoverCharField.setAccessible(true)
+ readerLeftoverCharFn = () => { readerLeftoverCharField.get(sd).asInstanceOf[Boolean] }
+ val bbField = sd.getClass.getDeclaredField("bb")
+ bbField.setAccessible(true)
+ readerByteBuffer = bbField.get(sd).asInstanceOf[ByteBuffer]
+ }
+ }
+
+ /**
+ * Tries to determine how many bytes of the underlying split have been read. There are two
+ * distinct cases.
+ *
+ * For compressed input, it attempts to read the current position read in the compressed input
+ * stream. This logic is copied from LineRecordReader, essentially.
+ *
+ * For uncompressed input, it counts the number of bytes read directly from the split. It
+ * further compensates for the fact that the intervening InputStreamReader buffers input and
+ * accounts for data it has read but not yet returned.
+ */
+ private def getFilePosition(): Long = {
+ // filePosition != null when input is compressed
+ if (filePosition != null) {
+ filePosition.getPos
+ } else {
+ start + countingIn.getByteCount -
+ readerByteBuffer.remaining() -
+ (if (readerLeftoverCharFn()) 1 else 0)
+ }
+ }
+
+ override def nextKeyValue: Boolean = {
+ currentKey = new LongWritable
+ currentValue = new Text
+ next(currentKey, currentValue)
+ }
+
+ /**
+ * Finds the start of the next record.
+ * It treats data from `startTag` and `endTag` as a record.
+ *
+ * @param key the current key that will be written
+ * @param value the object that will be written
+ * @return whether it reads successfully
+ */
+ private def next(key: LongWritable, value: Text): Boolean = {
+ if (readUntilStartElement()) {
+ try {
+ buffer.append(currentStartTag)
+ // Don't check whether the end element was found. Even if not, return everything
+ // that was read, which will invariably cause a parse error later
+ readUntilEndElement(currentStartTag.endsWith(">"))
+ key.set(getFilePosition())
+ value.set(buffer.toString())
+ return true
+ } finally {
+ buffer = new StringBuilder()
+ }
+ }
+ false
+ }
+
+ private def readUntilStartElement(): Boolean = {
+ currentStartTag = startTag
Review Comment:
QQ: is there a existing lib we can use for the parsing?
--
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
[GitHub] [spark] sandip-db commented on a diff in pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1282619785
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlInputFormat.scala:
##########
@@ -0,0 +1,341 @@
+/*
+ * 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.xml
+
+import java.io.{InputStream, InputStreamReader, IOException, Reader}
+import java.nio.ByteBuffer
+import java.nio.charset.Charset
+
+import org.apache.commons.io.input.CountingInputStream
+import org.apache.hadoop.fs.Seekable
+import org.apache.hadoop.io.{LongWritable, Text}
+import org.apache.hadoop.io.compress._
+import org.apache.hadoop.mapreduce.{InputSplit, RecordReader, TaskAttemptContext}
+import org.apache.hadoop.mapreduce.lib.input.{FileSplit, TextInputFormat}
+
+/**
+ * Reads records that are delimited by a specific start/end tag.
+ */
+class XmlInputFormat extends TextInputFormat {
+
+ override def createRecordReader(
+ split: InputSplit,
+ context: TaskAttemptContext): RecordReader[LongWritable, Text] = {
+ new XmlRecordReader
+ }
+}
+
+object XmlInputFormat {
+ /** configuration key for start tag */
+ val START_TAG_KEY: String = "xmlinput.start"
+ /** configuration key for end tag */
+ val END_TAG_KEY: String = "xmlinput.end"
+ /** configuration key for encoding type */
+ val ENCODING_KEY: String = "xmlinput.encoding"
+}
+
+/**
+ * XMLRecordReader class to read through a given xml document to output xml blocks as records
+ * as specified by the start tag and end tag.
+ *
+ * This implementation is ultimately loosely based on LineRecordReader in Hadoop.
+ */
+private[xml] class XmlRecordReader extends RecordReader[LongWritable, Text] {
+
+ private var startTag: String = _
+ private var currentStartTag: String = _
+ private var endTag: String = _
+ private var currentKey: LongWritable = _
+ private var currentValue: Text = _
+ private var start: Long = _
+ private var end: Long = _
+ private var reader: Reader = _
+ private var filePosition: Seekable = _
+ private var countingIn: CountingInputStream = _
+ private var readerLeftoverCharFn: () => Boolean = _
+ private var readerByteBuffer: ByteBuffer = _
+ private var decompressor: Decompressor = _
+ private var buffer = new StringBuilder()
+
+ override def initialize(split: InputSplit, context: TaskAttemptContext): Unit = {
+ val fileSplit = split.asInstanceOf[FileSplit]
+ val conf = context.getConfiguration
+ val charset =
+ Charset.forName(conf.get(XmlInputFormat.ENCODING_KEY, XmlOptions.DEFAULT_CHARSET))
+ startTag = conf.get(XmlInputFormat.START_TAG_KEY)
+ endTag = conf.get(XmlInputFormat.END_TAG_KEY)
+ start = fileSplit.getStart
+ end = start + fileSplit.getLength
+
+ // open the file and seek to the start of the split
+ val path = fileSplit.getPath
+ val fs = path.getFileSystem(conf)
+ val fsin = fs.open(fileSplit.getPath)
+
+ var in: InputStream = null
+ val codec = new CompressionCodecFactory(conf).getCodec(path)
+ if (null != codec) {
+ decompressor = CodecPool.getDecompressor(codec)
+ codec match {
+ case sc: SplittableCompressionCodec =>
+ val cIn = sc.createInputStream(
+ fsin,
+ decompressor,
+ start,
+ end,
+ SplittableCompressionCodec.READ_MODE.BYBLOCK)
+ start = cIn.getAdjustedStart
+ end = cIn.getAdjustedEnd
+ in = cIn
+ filePosition = cIn
+ case c: CompressionCodec =>
+ if (start != 0) {
+ // So we have a split that is only part of a file stored using
+ // a Compression codec that cannot be split.
+ throw new IOException("Cannot seek in " +
+ codec.getClass.getSimpleName + " compressed stream")
+ }
+ val cIn = c.createInputStream(fsin, decompressor)
+ in = cIn
+ filePosition = fsin
+ }
+ } else {
+ fsin.seek(start)
+ countingIn = new CountingInputStream(fsin)
+ in = countingIn
+ // don't use filePosition in this case. We have to count bytes read manually
+ }
+
+ reader = new InputStreamReader(in, charset)
+
+ if (codec == null) {
+ // Hack: in the uncompressed case (see more below), we must know how much the
+ // InputStreamReader has buffered but not processed
+ // to accurately assess how many bytes have been processed
+ val sdField = reader.getClass.getDeclaredField("sd")
+ sdField.setAccessible(true)
+ val sd = sdField.get(reader)
+ val readerLeftoverCharField = sd.getClass.getDeclaredField("haveLeftoverChar")
+ readerLeftoverCharField.setAccessible(true)
+ readerLeftoverCharFn = () => { readerLeftoverCharField.get(sd).asInstanceOf[Boolean] }
+ val bbField = sd.getClass.getDeclaredField("bb")
+ bbField.setAccessible(true)
+ readerByteBuffer = bbField.get(sd).asInstanceOf[ByteBuffer]
+ }
+ }
+
+ /**
+ * Tries to determine how many bytes of the underlying split have been read. There are two
+ * distinct cases.
+ *
+ * For compressed input, it attempts to read the current position read in the compressed input
+ * stream. This logic is copied from LineRecordReader, essentially.
+ *
+ * For uncompressed input, it counts the number of bytes read directly from the split. It
+ * further compensates for the fact that the intervening InputStreamReader buffers input and
+ * accounts for data it has read but not yet returned.
+ */
+ private def getFilePosition(): Long = {
+ // filePosition != null when input is compressed
+ if (filePosition != null) {
+ filePosition.getPos
+ } else {
+ start + countingIn.getByteCount -
+ readerByteBuffer.remaining() -
+ (if (readerLeftoverCharFn()) 1 else 0)
+ }
+ }
+
+ override def nextKeyValue: Boolean = {
+ currentKey = new LongWritable
+ currentValue = new Text
+ next(currentKey, currentValue)
+ }
+
+ /**
+ * Finds the start of the next record.
+ * It treats data from `startTag` and `endTag` as a record.
+ *
+ * @param key the current key that will be written
+ * @param value the object that will be written
+ * @return whether it reads successfully
+ */
+ private def next(key: LongWritable, value: Text): Boolean = {
+ if (readUntilStartElement()) {
+ try {
+ buffer.append(currentStartTag)
+ // Don't check whether the end element was found. Even if not, return everything
+ // that was read, which will invariably cause a parse error later
+ readUntilEndElement(currentStartTag.endsWith(">"))
+ key.set(getFilePosition())
+ value.set(buffer.toString())
+ return true
+ } finally {
+ buffer = new StringBuilder()
+ }
+ }
+ false
+ }
+
+ private def readUntilStartElement(): Boolean = {
+ currentStartTag = startTag
Review Comment:
XMLEventReader is being used to parse fields from XML records. It may be possible to use XMLEventReader to extract records as well. The current scope of the PR is to inline the spark-xml as is.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1670477246
@sandip-db would you mind adding some comments about what's new codes (e.g., pom.xml)? Then I think it'd be much easier to merge since most of the codes are already exiting one.
I took a cursory look, and it seems pretty fine to me.
--
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
[GitHub] [spark] sandip-db commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1663142408
> This would need an SPIP.
[SPIP link](https://docs.google.com/document/d/1ZaOBT4-YFtN58UCx2cdFhlsKbie1ugAn-Fgz_Dddz-Q/edit?usp=sharing)
[JIRA](https://issues.apache.org/jira/browse/SPARK-44265)
[Discussion Thread](https://lists.apache.org/thread/q32hxgsp738wom03mgpg9ykj9nr2n1fh)
[Vote thread](https://lists.apache.org/thread/vmcl0tyhlbf4b6njb4no2ztjxmjh1b24)
--
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
[GitHub] [spark] gengliangwang commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1663248713
@sandip-db Thanks for the work!
All the new Spark connectors are under the folder https://github.com/apache/spark/tree/master/connector. I notice that there are over 100 files in this PR. Putting 100 files under the `connector` directory will definitely make our project structure look better.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1672442840
@sandip-db I would appreciate if you create sub-tasks JIRAs based on my comments when you find some time.
--
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
[GitHub] [spark] HyukjinKwon closed pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
URL: https://github.com/apache/spark/pull/41832
--
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
[GitHub] [spark] sandip-db commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1664407023
> Seems the test failure is flakiness. Mind retriggering https://github.com/sandip-db/spark/actions/runs/5745270542/job/15573043078 please @sandip-db ?
The following test (unrelated to this PR) is failing consistently. It was supposedly fixed by the [Jira](https://issues.apache.org/jira/browse/SPARK-44074)/[PR](https://github.com/apache/spark/pull/41663). But its continuing to fail even after the fix. How do we get past this? @HyukjinKwon
2023-08-03T17:34:49.5298626Z [info] *** 1 TEST FAILED ***
2023-08-03T17:34:49.5412181Z [error] Failed: Total 9521, Failed 1, Errors 0, Passed 9520, Ignored 27
2023-08-03T17:34:49.5674077Z [error] Failed tests:
2023-08-03T17:34:49.5674974Z [error] org.apache.spark.sql.execution.QueryExecutionSuite
StackTrace:
2023-08-03T17:07:15.5739831Z [info] - Logging plan changes for execution *** FAILED *** (11 milliseconds)
2023-08-03T17:07:15.5740917Z [info] testAppender.loggingEvents.exists(((x$10: org.apache.logging.log4j.core.LogEvent) => x$10.getMessage().getFormattedMessage().contains(expectedMsg))) was false (QueryExecutionSuite.scala:233)
2023-08-03T17:07:15.5741729Z [info] org.scalatest.exceptions.TestFailedException:
2023-08-03T17:07:15.5742406Z [info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
2023-08-03T17:07:15.5743140Z [info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
2023-08-03T17:07:15.5743968Z [info] at org.scalatest.Assertions$.newAssertionFailedException(Assertions.scala:1231)
2023-08-03T17:07:15.5744873Z [info] at org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:1295)
2023-08-03T17:07:15.5745601Z [info] at org.apache.spark.sql.execution.QueryExecutionSuite.$anonfun$new$34(QueryExecutionSuite.scala:233)
2023-08-03T17:07:15.5746281Z [info] at scala.collection.immutable.List.foreach(List.scala:431)
2023-08-03T17:07:15.5746979Z [info] at org.apache.spark.sql.execution.QueryExecutionSuite.$anonfun$new$31(QueryExecutionSuite.scala:232)
--
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
[GitHub] [spark] sandip-db commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "sandip-db (via GitHub)" <gi...@apache.org>.
sandip-db commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1287818150
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlReader.scala:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.xml
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.{DataFrame, Dataset, SparkSession, SQLContext}
+import org.apache.spark.sql.catalyst.util.FailFastMode
+import org.apache.spark.sql.execution.datasources.xml.util.XmlFile
+import org.apache.spark.sql.types.StructType
+
+/**
+ * A collection of static functions for working with XML files in Spark SQL
+ */
+class XmlReader(private var schema: StructType,
Review Comment:
I have a PR ready to be sent out with these and other changes you have suggested. Just have to get this merged first :-)
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlReader.scala:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.xml
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.{DataFrame, Dataset, SparkSession, SQLContext}
+import org.apache.spark.sql.catalyst.util.FailFastMode
+import org.apache.spark.sql.execution.datasources.xml.util.XmlFile
+import org.apache.spark.sql.types.StructType
+
+/**
+ * A collection of static functions for working with XML files in Spark SQL
+ */
+class XmlReader(private var schema: StructType,
Review Comment:
I have a PR ready to be sent out with these and other changes you have suggested. Just have to get this merged first :-)
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #41832: [SPARK-44265][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41832:
URL: https://github.com/apache/spark/pull/41832#issuecomment-1670488692
I think I'd prefer to merge this first, and address them as a followup so other people can work together. I will review one more time tmr but would be great if others can review this too.
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #41832: [SPARK-44732][SQL] Built-in XML data source support
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #41832:
URL: https://github.com/apache/spark/pull/41832#discussion_r1287845139
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/xml/XmlOptions.scala:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.xml
+
+import java.nio.charset.StandardCharsets
+
+import org.apache.spark.sql.catalyst.util.{ParseMode, PermissiveMode}
+
+/**
+ * Options for the XML data source.
+ */
+private[xml] class XmlOptions(
+ @transient private val parameters: Map[String, String])
+ extends Serializable {
+
+ def this() = this(Map.empty)
+
+ val charset = parameters.getOrElse("charset", XmlOptions.DEFAULT_CHARSET)
Review Comment:
should document it in https://spark.apache.org/docs/latest/sql-data-sources.html
--
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