[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/22895 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user gengliangwang commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229564121 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- Agree, When I post the output, I have the same thought..Haha. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229563551 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- How about we just remove `msg` change? If I am not mistaken, usually it's just added at the cause alone. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user gengliangwang commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229563086 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- I see. It would be like ``` org.apache.spark.SparkException: com.fasterxml.jackson.core.JsonParseException: Unexpected character ('1' (code 49)): was expecting a colon to separate field name and value at [Source: (InputStreamReader); line: 1, column: 7] Malformed records are detected in record parsing. Parse Mode: FAILFAST. To process malformed records as null result, try setting the option 'mode' as 'PERMISSIVE'. at org.apache.spark.sql.catalyst.util.FailureSafeParser.parse(FailureSafeParser.scala:80) at org.apache.spark.sql.catalyst.expressions.JsonToStructs.nullSafeEval(jsonExpressions.scala:594) ... Caused by: org.apache.spark.sql.catalyst.util.BadRecordException: com.fasterxml.jackson.core.JsonParseException: Unexpected character ('1' (code 49)): was expecting a colon to separate field name and value at [Source: (InputStreamReader); line: 1, column: 7] ... ``` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229562284 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- Adding causes looks fine. I was actually wondering adding `msg` is necessary. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user gengliangwang commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229553580 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } --- End diff -- Yes, but as there is such code `e.getMessage + "\n`, personally I prefer not to inline it. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user gengliangwang commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229553381 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- After the change, the backtrace will contain: ``` Caused by: org.apache.spark.sql.catalyst.util.BadRecordException: com.fasterxml.jackson.core.JsonParseException: Unexpected character ('1' (code 49)): was expecting a colon to separate field name and value at [Source: (InputStreamReader); line: 1, column: 7] at org.apache.spark.sql.catalyst.json.JacksonParser.parse(JacksonParser.scala:414) at org.apache.spark.sql.catalyst.expressions.JsonToStructs$$anonfun$parser$1.apply(jsonExpressions.scala:581) at org.apache.spark.sql.catalyst.expressions.JsonToStructs$$anonfun$parser$1.apply(jsonExpressions.scala:581) at org.apache.spark.sql.catalyst.util.FailureSafeParser.parse(FailureSafeParser.scala:66) ... 20 more ``` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229539289 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } --- End diff -- I think it's okay to make this if-else inlined --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22895: [SPARK-25886][SQL][Minor] Improve error message o...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/22895#discussion_r229539055 --- Diff: external/avro/src/main/scala/org/apache/spark/sql/avro/AvroDataToCatalyst.scala --- @@ -100,9 +100,14 @@ case class AvroDataToCatalyst( case NonFatal(e) => parseMode match { case PermissiveMode => nullResultRow case FailFastMode => - throw new SparkException("Malformed records are detected in record parsing. " + + val msg = if (e.getMessage != null) { +e.getMessage + "\n" + } else { +"" + } + throw new SparkException(msg + "Malformed records are detected in record parsing. " + s"Current parse Mode: ${FailFastMode.name}. To process malformed records as null " + -"result, try setting the option 'mode' as 'PERMISSIVE'.", e.getCause) +"result, try setting the option 'mode' as 'PERMISSIVE'.", e) --- End diff -- @gengliangwang, looks okay. How does the error message look like before/after btw? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org