You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srowen (via GitHub)" <gi...@apache.org> on 2023/07/09 15:55:59 UTC

[GitHub] [spark] srowen commented on a diff in pull request #41904: [SPARK-43389] [PySpark, SQL] Added a null check for lineSep option

srowen commented on code in PR #41904:
URL: https://github.com/apache/spark/pull/41904#discussion_r1257505488


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/CSVOptions.scala:
##########
@@ -253,16 +253,18 @@ class CSVOptions(
   /**
    * A string between two consecutive JSON records.
    */
-  val lineSeparator: Option[String] = parameters.get(LINE_SEP).map { sep =>
-    require(sep.nonEmpty, "'lineSep' cannot be an empty string.")
-    // Intentionally allow it up to 2 for Window's CRLF although multiple
-    // characters have an issue with quotes. This is intentionally undocumented.
-    require(sep.length <= 2, "'lineSep' can contain only 1 character.")
-    if (sep.length == 2) logWarning("It is not recommended to set 'lineSep' " +
-      "with 2 characters due to the limitation of supporting multi-char 'lineSep' within quotes.")
-    sep
+  val lineSeparator: Option[String] = parameters.get(LINE_SEP) match {
+    case Some(sep) if sep != null =>

Review Comment:
   Is a null line separator even valid? I'd imagine that should be an error, if an empty one is



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org