You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gengliangwang <gi...@git.apache.org> on 2018/10/24 11:54:38 UTC
[GitHub] spark pull request #22814: [WIP][SPARK-25819][SQL] Support parse mode option...
GitHub user gengliangwang opened a pull request:
https://github.com/apache/spark/pull/22814
[WIP][SPARK-25819][SQL] Support parse mode option for the function `from_avro`
## What changes were proposed in this pull request?
Current the function `from_avro` throws exception on reading corrupt records.
To follow the behavior of `from_csv` and `from_json`, let's support parse mode "PERMISSIVE" and "FAILFAST" in `from_avro`.
## How was this patch tested?
Unit test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gengliangwang/spark improve_from_avro
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22814.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22814
----
commit ee85b8c7186d78d9370df9d096fe4d0b407b5503
Author: Gengliang Wang <ge...@...>
Date: 2018-10-24T11:49:59Z
improve from_avro
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4485/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97973/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228380639
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroFunctionsSuite.scala ---
@@ -61,6 +59,24 @@ class AvroFunctionsSuite extends QueryTest with SharedSQLContext {
checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), df)
}
+ test("handle invalid input in from_avro") {
+ val count = 10
+ val df = spark.range(count).select(struct('id, 'id.as("id2")).as("struct"))
+ val avroStructDF = df.select(to_avro('struct).as("avro"))
+ val avroTypeStruct = s"""
+ |{
+ | "type": "record",
+ | "name": "struct",
+ | "fields": [
+ | {"name": "col1", "type": "long"},
+ | {"name": "col2", "type": "double"}
+ | ]
+ |}
+ """.stripMargin
+ val expected = (0 until count).map(_ => Row(Row(null, null)))
+ checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), expected)
--- End diff --
Thanks. Makes sense to me
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228388259
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/package.scala ---
@@ -31,10 +32,32 @@ package object avro {
* @since 2.4.0
*/
@Experimental
- def from_avro(data: Column, jsonFormatSchema: String): Column = {
- new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema))
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String): Column = {
+ new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema, Map.empty))
+ }
+
+ /**
+ * Converts a binary column of avro format into its corresponding catalyst value. The specified
+ * schema must match the read data, otherwise the behavior is undefined: it may fail or return
+ * arbitrary result.
+ *
+ * @param data the binary column.
+ * @param jsonFormatSchema the avro schema in JSON string format.
+ * @param options options to control how the Avro record is parsed.
+ *
+ * @since 3.0.0
+ */
+ @Experimental
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String,
+ options: Map[String, String]): Column = {
--- End diff --
ah that's a good point, as that's what we do for other APIs as well.
We can think for about this later, and change all the APIs together, if other people complains.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228380951
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/package.scala ---
@@ -31,10 +32,32 @@ package object avro {
* @since 2.4.0
*/
@Experimental
- def from_avro(data: Column, jsonFormatSchema: String): Column = {
- new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema))
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String): Column = {
+ new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema, Map.empty))
+ }
+
+ /**
+ * Converts a binary column of avro format into its corresponding catalyst value. The specified
+ * schema must match the read data, otherwise the behavior is undefined: it may fail or return
+ * arbitrary result.
+ *
+ * @param data the binary column.
+ * @param jsonFormatSchema the avro schema in JSON string format.
+ * @param options options to control how the Avro record is parsed.
+ *
+ * @since 3.0.0
+ */
+ @Experimental
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String,
+ options: Map[String, String]): Column = {
--- End diff --
One thing I am less sure is tho, what do you think about we just expose one Java specific one? This will only be usable in Scala side.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98015/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98061/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98011/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98015/testReport)** for PR 22814 at commit [`3eff2e2`](https://github.com/apache/spark/commit/3eff2e2c0f0ccc9b2de70ce789dfe78c36dc54f8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22814
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228086740
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala ---
@@ -21,16 +21,31 @@ import org.apache.avro.Schema
import org.apache.avro.generic.GenericDatumReader
import org.apache.avro.io.{BinaryDecoder, DecoderFactory}
-import org.apache.spark.sql.catalyst.expressions.{ExpectsInputTypes, Expression, UnaryExpression}
+import org.apache.spark.SparkException
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.expressions.{ExpectsInputTypes, Expression, SpecificInternalRow, UnaryExpression}
import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, CodeGenerator, ExprCode}
-import org.apache.spark.sql.types.{AbstractDataType, BinaryType, DataType}
+import org.apache.spark.sql.catalyst.util.{FailFastMode, ParseMode, PermissiveMode}
+import org.apache.spark.sql.types._
-case class AvroDataToCatalyst(child: Expression, jsonFormatSchema: String)
+case class AvroDataToCatalyst(
+ child: Expression,
+ jsonFormatSchema: String,
+ options: Map[String, String])
extends UnaryExpression with ExpectsInputTypes {
override def inputTypes: Seq[AbstractDataType] = Seq(BinaryType)
- override lazy val dataType: DataType = SchemaConverters.toSqlType(avroSchema).dataType
+ override lazy val dataType: DataType = {
+ val dt = SchemaConverters.toSqlType(avroSchema).dataType
+ parseMode match {
+ // With PermissiveMode, the output Catalyst row might contain columns of null values for
+ // corrupt records, even if some of the columns are not nullable in the user-provided schema.
+ // Therefore we force the schema to be all nullable here.
+ case PermissiveMode => dt.asNullable
--- End diff --
I see. Updated the guide in latest commit.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98004/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228061155
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala ---
@@ -44,24 +59,74 @@ case class AvroDataToCatalyst(child: Expression, jsonFormatSchema: String)
@transient private var result: Any = _
+ @transient private lazy val parseMode: ParseMode = {
+ val mode = AvroOptions(options).parseMode
+ if (mode != PermissiveMode && mode != FailFastMode) {
+ throw new AnalysisException(unacceptableModeMessage(mode.name))
+ }
+ mode
+ }
+
+ private def unacceptableModeMessage(name: String): String = {
+ s"from_avro() doesn't support the $name mode. " +
+ s"Acceptable modes are ${PermissiveMode.name} and ${FailFastMode.name}."
+ }
+
+ @transient private lazy val nullResultRow: Any = dataType match {
+ case st: StructType =>
+ val resultRow = new SpecificInternalRow(st.map(_.dataType))
+ for(i <- 0 until st.length) {
+ resultRow.setNullAt(i)
+ }
+ resultRow
+
+ case _ =>
+ null
+ }
+
+
override def nullSafeEval(input: Any): Any = {
val binary = input.asInstanceOf[Array[Byte]]
- decoder = DecoderFactory.get().binaryDecoder(binary, 0, binary.length, decoder)
- result = reader.read(result, decoder)
- deserializer.deserialize(result)
+ try {
+ decoder = DecoderFactory.get().binaryDecoder(binary, 0, binary.length, decoder)
+ result = reader.read(result, decoder)
+ deserializer.deserialize(result)
+ } catch {
+ // There could be multiple possible exceptions here, e.g. java.io.IOException,
+ // AvroRuntimeException, ArrayIndexOutOfBoundsException, etc.
+ // To make it simple, catch all the exceptions here.
+ case e: Exception => parseMode match {
--- End diff --
we should catch `NonFatal` to be safer
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4518/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98023/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4473/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98028/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228148950
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroFunctionsSuite.scala ---
@@ -61,6 +59,24 @@ class AvroFunctionsSuite extends QueryTest with SharedSQLContext {
checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), df)
}
+ test("handle invalid input in from_avro") {
+ val count = 10
+ val df = spark.range(count).select(struct('id, 'id.as("id2")).as("struct"))
+ val avroStructDF = df.select(to_avro('struct).as("avro"))
+ val avroTypeStruct = s"""
+ |{
+ | "type": "record",
+ | "name": "struct",
+ | "fields": [
+ | {"name": "col1", "type": "long"},
+ | {"name": "col2", "type": "double"}
+ | ]
+ |}
+ """.stripMargin
+ val expected = (0 until count).map(_ => Row(Row(null, null)))
+ checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), expected)
--- End diff --
In practice, there could be various reasons of data corruption. It would be good to support PERMISSIVE mode and allow the function `from_avro` to process all the input file/streaming, which is consistent with `from_json` and `from_csv`.
I don't see much down side of this.
But I do think about making the default mode as FAILFAST, to expose the problem of input data to users, instead of hiding it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228064742
--- Diff: docs/sql-data-sources-avro.md ---
@@ -177,6 +180,18 @@ Data source options of Avro can be set using the `.option` method on `DataFrameR
Currently supported codecs are <code>uncompressed</code>, <code>snappy</code>, <code>deflate</code>, <code>bzip2</code> and <code>xz</code>.<br> If the option is not set, the configuration <code>spark.sql.avro.compression.codec</code> config is taken into account.</td>
<td>write</td>
</tr>
+ <tr>
+ <td><code>mode</code></td>
+ <td>PERMISSIVE</td>
+ <td>The <code>mode</code> option allows to specify parse mode for function <code>from_avro</code>.<br>
+ Currently supported modes are:
+ <ul>
+ <li><code>PERMISSIVE</code>: Corrupt records are processed as null result.</li>
--- End diff --
If the input schema is not `record` type in Avro, e.g. a `int`, then it is simply `null`. Just try using the wording "null result" to include both case.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98009 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98009/testReport)** for PR 22814 at commit [`53f3a64`](https://github.com/apache/spark/commit/53f3a64f7ed28ecabff15f2bf18f20eac84dbfc1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228381742
--- Diff: docs/sql-data-sources-avro.md ---
@@ -177,6 +180,19 @@ Data source options of Avro can be set using the `.option` method on `DataFrameR
Currently supported codecs are <code>uncompressed</code>, <code>snappy</code>, <code>deflate</code>, <code>bzip2</code> and <code>xz</code>.<br> If the option is not set, the configuration <code>spark.sql.avro.compression.codec</code> config is taken into account.</td>
<td>write</td>
</tr>
+ <tr>
+ <td><code>mode</code></td>
+ <td>FAILFAST</td>
+ <td>The <code>mode</code> option allows to specify parse mode for function <code>from_avro</code>.<br>
+ Currently supported modes are:
+ <ul>
+ <li><code>FAILFAST</code>: Throws an exception on processing corrupted record.</li>
+ <li><code>PERMISSIVE</code>: Corrupt records are processed as null result. To implement this, the
--- End diff --
not a big deal but ..`To implement this` sounds a little bit awkward .. I think you can just say ... `Corrupt records are processed as null; therefore, the given schema is forced to be fully nullable.` (feel free to fix words as you think righter).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228115771
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroFunctionsSuite.scala ---
@@ -61,6 +59,24 @@ class AvroFunctionsSuite extends QueryTest with SharedSQLContext {
checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), df)
}
+ test("handle invalid input in from_avro") {
+ val count = 10
+ val df = spark.range(count).select(struct('id, 'id.as("id2")).as("struct"))
+ val avroStructDF = df.select(to_avro('struct).as("avro"))
+ val avroTypeStruct = s"""
+ |{
+ | "type": "record",
+ | "name": "struct",
+ | "fields": [
+ | {"name": "col1", "type": "long"},
+ | {"name": "col2", "type": "double"}
+ | ]
+ |}
+ """.stripMargin
+ val expected = (0 until count).map(_ => Row(Row(null, null)))
+ checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), expected)
--- End diff --
BTW, when would there be malformed records - is this usually only when the schema is different? Main purpose for parse modes at CSV and JSON is to cover the limitation from semi structured input data. Was wondering how useful it is in Avro if the most of cases are only when the input schema is different.
Also, one good thing about PERMISSIVE mode is that we allow to fill invalid records at `columnNameOfCorruptRecord`. Here it looks it doesn't quite make sense to add this functionality at Avro.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4484/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228160490
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroFunctionsSuite.scala ---
@@ -61,6 +59,24 @@ class AvroFunctionsSuite extends QueryTest with SharedSQLContext {
checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), df)
}
+ test("handle invalid input in from_avro") {
+ val count = 10
+ val df = spark.range(count).select(struct('id, 'id.as("id2")).as("struct"))
+ val avroStructDF = df.select(to_avro('struct).as("avro"))
+ val avroTypeStruct = s"""
+ |{
+ | "type": "record",
+ | "name": "struct",
+ | "fields": [
+ | {"name": "col1", "type": "long"},
+ | {"name": "col2", "type": "double"}
+ | ]
+ |}
+ """.stripMargin
+ val expected = (0 until count).map(_ => Row(Row(null, null)))
+ checkAnswer(avroStructDF.select(from_avro('avro, avroTypeStruct)), expected)
--- End diff --
> Also, one good thing about PERMISSIVE mode is that we allow to fill invalid records at columnNameOfCorruptRecord
yea Avro can't do it. But returning null instead of failing should still be a good thing for `from_avro`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98004 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98004/testReport)** for PR 22814 at commit [`4f9e033`](https://github.com/apache/spark/commit/4f9e03360642b5f34c190c43387158e4e3abd4e2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/22814
The major code is ready.
I will add a new section in doc soon.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22814
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98063 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98063/testReport)** for PR 22814 at commit [`b33a5ad`](https://github.com/apache/spark/commit/b33a5ade4b3f091d5e67d3f3bdc47e87f9b37eee).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4474/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22814
LGTM too
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228113346
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -10,6 +10,9 @@ displayTitle: Spark SQL Upgrading Guide
## Upgrading From Spark SQL 2.4 to 3.0
- In PySpark, when creating a `SparkSession` with `SparkSession.builder.getOrCreate()`, if there is an existing `SparkContext`, the builder was trying to update the `SparkConf` of the existing `SparkContext` with configurations specified to the builder, but the `SparkContext` is shared by all `SparkSession`s, so we should not update them. Since 3.0, the builder comes to not update the configurations. This is the same behavior as Java/Scala API in 2.3 and above. If you want to update them, you need to update them prior to creating a `SparkSession`.
+ - In Avro data source, the function `from_avro` supports following parse modes:
--- End diff --
nit: let's add a space to make it more readable (I added each space between each item).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97973 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97973/testReport)** for PR 22814 at commit [`ee85b8c`](https://github.com/apache/spark/commit/ee85b8c7186d78d9370df9d096fe4d0b407b5503).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class AvroDataToCatalyst(`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98004 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98004/testReport)** for PR 22814 at commit [`4f9e033`](https://github.com/apache/spark/commit/4f9e03360642b5f34c190c43387158e4e3abd4e2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228164087
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -11,6 +11,10 @@ displayTitle: Spark SQL Upgrading Guide
- In PySpark, when creating a `SparkSession` with `SparkSession.builder.getOrCreate()`, if there is an existing `SparkContext`, the builder was trying to update the `SparkConf` of the existing `SparkContext` with configurations specified to the builder, but the `SparkContext` is shared by all `SparkSession`s, so we should not update them. Since 3.0, the builder comes to not update the configurations. This is the same behavior as Java/Scala API in 2.3 and above. If you want to update them, you need to update them prior to creating a `SparkSession`.
+ - In Avro data source, the function `from_avro` supports following parse modes:
+ * `PERMISSIVE`: Corrupt records are processed as null result. To implement this, the data schema is forced to be fully nullable, which might be different from the one user provided. This is the default mode.
+ * `FAILFAST`: Throws an exception on processing corrupted record.
--- End diff --
Let's explain what changes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97978 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97978/testReport)** for PR 22814 at commit [`ef4dd4a`](https://github.com/apache/spark/commit/ef4dd4a1c3a304831f4ab393b1d0ff2a09a0604f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/22814
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98016 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98016/testReport)** for PR 22814 at commit [`53513cf`](https://github.com/apache/spark/commit/53513cff32e371df59bdb5c68a745bb20e52e8ac).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98009/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4446/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98011 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98011/testReport)** for PR 22814 at commit [`3c1f956`](https://github.com/apache/spark/commit/3c1f95627cedc3e6ac51f25627c28d7acde03a68).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98061 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98061/testReport)** for PR 22814 at commit [`4bf622a`](https://github.com/apache/spark/commit/4bf622a3be19664996515baa35743010527a6630).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228392145
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/package.scala ---
@@ -31,10 +32,32 @@ package object avro {
* @since 2.4.0
*/
@Experimental
- def from_avro(data: Column, jsonFormatSchema: String): Column = {
- new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema))
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String): Column = {
+ new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema, Map.empty))
+ }
+
+ /**
+ * Converts a binary column of avro format into its corresponding catalyst value. The specified
+ * schema must match the read data, otherwise the behavior is undefined: it may fail or return
+ * arbitrary result.
+ *
+ * @param data the binary column.
+ * @param jsonFormatSchema the avro schema in JSON string format.
+ * @param options options to control how the Avro record is parsed.
+ *
+ * @since 3.0.0
+ */
+ @Experimental
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String,
+ options: Map[String, String]): Column = {
--- End diff --
+1 with @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97982 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97982/testReport)** for PR 22814 at commit [`6e0354b`](https://github.com/apache/spark/commit/6e0354b8be1cf99db95d5462f17683b01a25eb4d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97982 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97982/testReport)** for PR 22814 at commit [`6e0354b`](https://github.com/apache/spark/commit/6e0354b8be1cf99db95d5462f17683b01a25eb4d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98063/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228164819
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroCatalystDataConversionSuite.scala ---
@@ -167,4 +184,26 @@ class AvroCatalystDataConversionSuite extends SparkFunSuite with ExpressionEvalH
// avro reader reads the first 4 bytes of a double as a float, the result is totally undefined.
checkResult(data, avroTypeJson, 5.848603E35f)
}
+
+ test("Handle unsupported input of record type") {
+ val actualSchema = StructType(Seq(
+ StructField("col_0", StringType, false),
+ StructField("col_1", ShortType, false),
+ StructField("col_2", DecimalType(8, 4), false),
+ StructField("col_3", BooleanType, true),
+ StructField("col_4", DecimalType(38, 38), false)))
+
+ val expectedSchema = StructType(Seq(
+ StructField("col_0", BinaryType, false),
+ StructField("col_1", DoubleType, false),
+ StructField("col_2", DecimalType(18, 4), false),
+ StructField("col_3", StringType, true),
+ StructField("col_4", DecimalType(38, 38), false)))
+
+ val data = RandomDataGenerator.randomRow(new scala.util.Random, actualSchema)
--- End diff --
let's include the seed with `withClue`, so that people can reproduce test failures
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98063 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98063/testReport)** for PR 22814 at commit [`b33a5ad`](https://github.com/apache/spark/commit/b33a5ade4b3f091d5e67d3f3bdc47e87f9b37eee).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228059940
--- Diff: docs/sql-data-sources-avro.md ---
@@ -177,6 +180,18 @@ Data source options of Avro can be set using the `.option` method on `DataFrameR
Currently supported codecs are <code>uncompressed</code>, <code>snappy</code>, <code>deflate</code>, <code>bzip2</code> and <code>xz</code>.<br> If the option is not set, the configuration <code>spark.sql.avro.compression.codec</code> config is taken into account.</td>
<td>write</td>
</tr>
+ <tr>
+ <td><code>mode</code></td>
+ <td>PERMISSIVE</td>
+ <td>The <code>mode</code> option allows to specify parse mode for function <code>from_avro</code>.<br>
+ Currently supported modes are:
+ <ul>
+ <li><code>PERMISSIVE</code>: Corrupt records are processed as null result.</li>
--- End diff --
not null result, but a row with all columns null.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98028 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98028/testReport)** for PR 22814 at commit [`ae6f746`](https://github.com/apache/spark/commit/ae6f746733e3d0016a6a751e05f48f9f416a2e12).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98014 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98014/testReport)** for PR 22814 at commit [`4ce9734`](https://github.com/apache/spark/commit/4ce97344b06c5a5c8ee99975f45f6d5e46389042).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98014/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4442/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228388968
--- Diff: docs/sql-data-sources-avro.md ---
@@ -177,6 +180,19 @@ Data source options of Avro can be set using the `.option` method on `DataFrameR
Currently supported codecs are <code>uncompressed</code>, <code>snappy</code>, <code>deflate</code>, <code>bzip2</code> and <code>xz</code>.<br> If the option is not set, the configuration <code>spark.sql.avro.compression.codec</code> config is taken into account.</td>
<td>write</td>
</tr>
+ <tr>
+ <td><code>mode</code></td>
+ <td>FAILFAST</td>
+ <td>The <code>mode</code> option allows to specify parse mode for function <code>from_avro</code>.<br>
+ Currently supported modes are:
+ <ul>
+ <li><code>FAILFAST</code>: Throws an exception on processing corrupted record.</li>
+ <li><code>PERMISSIVE</code>: Corrupt records are processed as null result. To implement this, the
--- End diff --
I see. Let me update it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22814
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98015/testReport)** for PR 22814 at commit [`3eff2e2`](https://github.com/apache/spark/commit/3eff2e2c0f0ccc9b2de70ce789dfe78c36dc54f8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228062545
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala ---
@@ -44,24 +59,74 @@ case class AvroDataToCatalyst(child: Expression, jsonFormatSchema: String)
@transient private var result: Any = _
+ @transient private lazy val parseMode: ParseMode = {
+ val mode = AvroOptions(options).parseMode
+ if (mode != PermissiveMode && mode != FailFastMode) {
+ throw new AnalysisException(unacceptableModeMessage(mode.name))
+ }
+ mode
+ }
+
+ private def unacceptableModeMessage(name: String): String = {
+ s"from_avro() doesn't support the $name mode. " +
+ s"Acceptable modes are ${PermissiveMode.name} and ${FailFastMode.name}."
+ }
+
+ @transient private lazy val nullResultRow: Any = dataType match {
+ case st: StructType =>
+ val resultRow = new SpecificInternalRow(st.map(_.dataType))
+ for(i <- 0 until st.length) {
+ resultRow.setNullAt(i)
+ }
+ resultRow
+
+ case _ =>
+ null
+ }
+
+
override def nullSafeEval(input: Any): Any = {
val binary = input.asInstanceOf[Array[Byte]]
- decoder = DecoderFactory.get().binaryDecoder(binary, 0, binary.length, decoder)
- result = reader.read(result, decoder)
- deserializer.deserialize(result)
+ try {
+ decoder = DecoderFactory.get().binaryDecoder(binary, 0, binary.length, decoder)
+ result = reader.read(result, decoder)
+ deserializer.deserialize(result)
+ } catch {
+ // There could be multiple possible exceptions here, e.g. java.io.IOException,
+ // AvroRuntimeException, ArrayIndexOutOfBoundsException, etc.
+ // To make it simple, catch all the exceptions here.
+ case e: Exception => parseMode match {
+ case PermissiveMode => nullResultRow
+ case FailFastMode =>
+ throw new SparkException("Malformed records are detected in record parsing. " +
+ s"Parse Mode: ${FailFastMode.name}.", e.getCause)
+ case _ =>
+ throw new AnalysisException(unacceptableModeMessage(parseMode.name))
+ }
+ }
}
override def simpleString: String = {
- s"from_avro(${child.sql}, ${dataType.simpleString})"
+ s"from_avro(${child.sql}, ${dataType.simpleString}, ${options.toString()})"
}
override def sql: String = {
- s"from_avro(${child.sql}, ${dataType.catalogString})"
+ s"from_avro(${child.sql}, ${dataType.catalogString}, ${options.toString()})"
}
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val expr = ctx.addReferenceObj("this", this)
- defineCodeGen(ctx, ev, input =>
- s"(${CodeGenerator.boxedType(dataType)})$expr.nullSafeEval($input)")
+ nullSafeCodeGen(ctx, ev, eval => {
+ val result = ctx.freshName("tempResult")
+ s"""
+ ${CodeGenerator.boxedType(dataType)} $result =
--- End diff --
nit: maybe
```
val dt = CodeGenerator.boxedType(dataType)
....
dt $result = ($dt) ...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228162464
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -11,6 +11,10 @@ displayTitle: Spark SQL Upgrading Guide
- In PySpark, when creating a `SparkSession` with `SparkSession.builder.getOrCreate()`, if there is an existing `SparkContext`, the builder was trying to update the `SparkConf` of the existing `SparkContext` with configurations specified to the builder, but the `SparkContext` is shared by all `SparkSession`s, so we should not update them. Since 3.0, the builder comes to not update the configurations. This is the same behavior as Java/Scala API in 2.3 and above. If you want to update them, you need to update them prior to creating a `SparkSession`.
+ - In Avro data source, the function `from_avro` supports following parse modes:
+ * `PERMISSIVE`: Corrupt records are processed as null result. To implement this, the data schema is forced to be fully nullable, which might be different from the one user provided. This is the default mode.
+ * `FAILFAST`: Throws an exception on processing corrupted record.
--- End diff --
We don't change existing APIs but add a new `from_avro` method to take an extra `option` parameter. Users won't hit any problem when upgrading Spark, and ideally they should read release notes and use this new feature if they need. I don't think we need to put it in migration guide.
Let's not abuse the migration guide.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98023/testReport)** for PR 22814 at commit [`ae6f746`](https://github.com/apache/spark/commit/ae6f746733e3d0016a6a751e05f48f9f416a2e12).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98028 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98028/testReport)** for PR 22814 at commit [`ae6f746`](https://github.com/apache/spark/commit/ae6f746733e3d0016a6a751e05f48f9f416a2e12).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98023 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98023/testReport)** for PR 22814 at commit [`ae6f746`](https://github.com/apache/spark/commit/ae6f746733e3d0016a6a751e05f48f9f416a2e12).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228063256
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroOptions.scala ---
@@ -79,4 +80,23 @@ class AvroOptions(
val compression: String = {
parameters.get("compression").getOrElse(SQLConf.get.avroCompressionCodec)
}
+
+ @transient private val acceptableParseMode = Seq(PermissiveMode, FailFastMode)
--- End diff --
do we need to check mode here? The `AvroOptions` can be used with reading avro files, and DROP MALFORMED can be supported.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4470/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4440/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4479/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98009 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98009/testReport)** for PR 22814 at commit [`53f3a64`](https://github.com/apache/spark/commit/53f3a64f7ed28ecabff15f2bf18f20eac84dbfc1).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4516/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4475/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97978/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228065259
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala ---
@@ -21,16 +21,31 @@ import org.apache.avro.Schema
import org.apache.avro.generic.GenericDatumReader
import org.apache.avro.io.{BinaryDecoder, DecoderFactory}
-import org.apache.spark.sql.catalyst.expressions.{ExpectsInputTypes, Expression, UnaryExpression}
+import org.apache.spark.SparkException
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.expressions.{ExpectsInputTypes, Expression, SpecificInternalRow, UnaryExpression}
import org.apache.spark.sql.catalyst.expressions.codegen.{CodegenContext, CodeGenerator, ExprCode}
-import org.apache.spark.sql.types.{AbstractDataType, BinaryType, DataType}
+import org.apache.spark.sql.catalyst.util.{FailFastMode, ParseMode, PermissiveMode}
+import org.apache.spark.sql.types._
-case class AvroDataToCatalyst(child: Expression, jsonFormatSchema: String)
+case class AvroDataToCatalyst(
+ child: Expression,
+ jsonFormatSchema: String,
+ options: Map[String, String])
extends UnaryExpression with ExpectsInputTypes {
override def inputTypes: Seq[AbstractDataType] = Seq(BinaryType)
- override lazy val dataType: DataType = SchemaConverters.toSqlType(avroSchema).dataType
+ override lazy val dataType: DataType = {
+ val dt = SchemaConverters.toSqlType(avroSchema).dataType
+ parseMode match {
+ // With PermissiveMode, the output Catalyst row might contain columns of null values for
+ // corrupt records, even if some of the columns are not nullable in the user-provided schema.
+ // Therefore we force the schema to be all nullable here.
+ case PermissiveMode => dt.asNullable
--- End diff --
This looks going to be an external behaviour change to users from 2.4.0. Please update migration guide as well.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97982/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228063724
--- Diff: external/avro/src/test/scala/org/apache/spark/sql/avro/AvroCatalystDataConversionSuite.scala ---
@@ -17,30 +17,49 @@
package org.apache.spark.sql.avro
+import org.apache.avro.Schema
+
import org.apache.spark.SparkFunSuite
-import org.apache.spark.sql.RandomDataGenerator
+import org.apache.spark.sql.{RandomDataGenerator, Row}
import org.apache.spark.sql.catalyst.{CatalystTypeConverters, InternalRow}
import org.apache.spark.sql.catalyst.expressions.{ExpressionEvalHelper, GenericInternalRow, Literal}
import org.apache.spark.sql.catalyst.util.{ArrayBasedMapData, GenericArrayData, MapData}
+import org.apache.spark.sql.test.SharedSQLContext
import org.apache.spark.sql.types._
-class AvroCatalystDataConversionSuite extends SparkFunSuite with ExpressionEvalHelper {
+class AvroCatalystDataConversionSuite extends SparkFunSuite
+ with SharedSQLContext
+ with ExpressionEvalHelper {
private def roundTripTest(data: Literal): Unit = {
val avroType = SchemaConverters.toAvroType(data.dataType, data.nullable)
checkResult(data, avroType.toString, data.eval())
}
private def checkResult(data: Literal, schema: String, expected: Any): Unit = {
- checkEvaluation(
- AvroDataToCatalyst(CatalystDataToAvro(data), schema),
- prepareExpectedResult(expected))
+ Seq("FAILFAST", "PERMISSIVE").foreach { mode =>
--- End diff --
why do we test 2 modes here? The mode is for when failure happens, but `checkResult` is for when no failures.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97978 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97978/testReport)** for PR 22814 at commit [`ef4dd4a`](https://github.com/apache/spark/commit/ef4dd4a1c3a304831f4ab393b1d0ff2a09a0604f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #97973 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97973/testReport)** for PR 22814 at commit [`ee85b8c`](https://github.com/apache/spark/commit/ee85b8c7186d78d9370df9d096fe4d0b407b5503).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98014 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98014/testReport)** for PR 22814 at commit [`4ce9734`](https://github.com/apache/spark/commit/4ce97344b06c5a5c8ee99975f45f6d5e46389042).
* This patch **fails Spark unit tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [SPARK-25819][SQL] Support parse mode option for ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r228062884
--- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/package.scala ---
@@ -31,10 +32,32 @@ package object avro {
* @since 2.4.0
*/
@Experimental
- def from_avro(data: Column, jsonFormatSchema: String): Column = {
- new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema))
+ def from_avro(
+ data: Column,
+ jsonFormatSchema: String): Column = {
+ new Column(AvroDataToCatalyst(data.expr, jsonFormatSchema, Map.empty))
}
+ /**
+ * Converts a binary column of avro format into its corresponding catalyst value. The specified
+ * schema must match the read data, otherwise the behavior is undefined: it may fail or return
+ * arbitrary result.
+ *
+ * @param data the binary column.
+ * @param jsonFormatSchema the avro schema in JSON string format.
+ * @param options options to control how the Avro record is parsed.
+ *
+ * @since 2.4.0
--- End diff --
3.0.0
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98016 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98016/testReport)** for PR 22814 at commit [`53513cf`](https://github.com/apache/spark/commit/53513cff32e371df59bdb5c68a745bb20e52e8ac).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98061 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98061/testReport)** for PR 22814 at commit [`4bf622a`](https://github.com/apache/spark/commit/4bf622a3be19664996515baa35743010527a6630).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22814: [WIP][SPARK-25819][SQL] Support parse mode option...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/22814#discussion_r227755767
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala ---
@@ -60,9 +60,11 @@ trait ExpressionEvalHelper extends GeneratorDrivenPropertyChecks with PlanTestBa
def expr = prepareEvaluation(expression)
val catalystValue = CatalystTypeConverters.convertToCatalyst(expected)
checkEvaluationWithoutCodegen(expr, catalystValue, inputRow)
- checkEvaluationWithMutableProjection(expr, catalystValue, inputRow)
- if (GenerateUnsafeProjection.canSupport(expr.dataType)) {
- checkEvaluationWithUnsafeProjection(expr, catalystValue, inputRow)
+ if (expected != null) {
--- End diff --
Need to figure out why the unsafe project failed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/22814
@cloud-fan @HyukjinKwon I have changed the default value to "FAILFAST" and remove the migration guide.
See the PR description for details.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98016/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22814
**[Test build #98011 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98011/testReport)** for PR 22814 at commit [`3c1f956`](https://github.com/apache/spark/commit/3c1f95627cedc3e6ac51f25627c28d7acde03a68).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4472/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [WIP][SPARK-25819][SQL] Support parse mode option for th...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4466/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22814: [SPARK-25819][SQL] Support parse mode option for the fun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22814
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org