Skip to content

[SPARK-11753][SQL][test-hadoop2.2] Make allowNonNumericNumbers option work #9759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
viirya wants to merge 12 commits into from
Closed

[SPARK-11753][SQL][test-hadoop2.2] Make allowNonNumericNumbers option work #9759

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2777677
Make allowNonNumericNumbers option work.
viirya Nov 17, 2015
b2a835d
Not to quote non numeric numbers when enabling allowNonNumericNumbers.
viirya Nov 17, 2015
186fa5e
Deal with quoted non-numeric number.
viirya Nov 20, 2015
dc9abc3
Merge remote-tracking branch 'upstream/master' into fix-json-nonnumric
viirya Nov 27, 2015
6d90b24
For comments.
viirya Nov 27, 2015
c74d715
Merge remote-tracking branch 'upstream/master' into fix-json-nonnumric
viirya May 16, 2016
6f668c3
Update doc.
viirya May 16, 2016
1cfd1dc
Fix dependency test.
viirya May 16, 2016
4fca52c
Address comment. Revert change of InferSchema.
viirya May 18, 2016
0e473fc
Support +INF, -INF.
viirya May 18, 2016
025edea
Manage paranamer up to 2.8.
viirya May 22, 2016
af1e3a1
Support INF and +Infinity when allowNonNumericNumbers is off.
viirya May 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address comment. Revert change of InferSchema.
  • Loading branch information
@viirya
viirya committed May 18, 2016
commit 4fca52c68ae15eb792dab08cea0dc725c3437397
11 changes: 1 addition & 10 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/json/InferSchema.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,7 @@ private[sql] object InferSchema {
// record fields' types have been combined.
NullType

case VALUE_STRING =>
// If there is only one row, the following non-numeric numbers will be incorrectly
// recognized as StringType.
val value = parser.getText
if (value.equals("NaN") ||
value.equals("Infinity") ||
value.equals("-Infinity")) {
return DoubleType
}
return StringType
case VALUE_STRING => StringType
case START_OBJECT =>
val builder = Array.newBuilder[StructField]
while (nextUntil(parser, END_OBJECT)) {
Expand Down
8 changes: 5 additions & 3 deletions .../test/scala/org/apache/spark/sql/execution/datasources/json/JsonParsingOptionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package org.apache.spark.sql.execution.datasources.json

import org.apache.spark.sql.QueryTest
import org.apache.spark.sql.test.SharedSQLContext
import org.apache.spark.sql.types.{DoubleType, StructField, StructType}

/**
* Test cases for various [[JSONOptions]].
Expand Down Expand Up @@ -107,10 +108,11 @@ class JsonParsingOptionsSuite extends QueryTest with SharedSQLContext {
// quoted non-numeric numbers should still work even allowNonNumericNumbers is off.
testCases = Seq("""{"age": "NaN"}""", """{"age": "Infinity"}""", """{"age": "-Infinity"}""")
val tests: Seq[Double => Boolean] = Seq(_.isNaN, _.isPosInfinity, _.isNegInfinity)
val schema = StructType(StructField("age", DoubleType, true) :: Nil)

testCases.zipWithIndex.foreach { case (str, idx) =>
val rdd = spark.sparkContext.parallelize(Seq(str))
val df = spark.read.option("allowNonNumericNumbers", "false").json(rdd)
val df = spark.read.option("allowNonNumericNumbers", "false").schema(schema).json(rdd)

assert(df.schema.head.name == "age")
assert(tests(idx)(df.first().getDouble(0)))
Copy link
Contributor

@cloud-fan cloud-fan May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it's double type? Shouldn't it be string if allowNonNumericNumbers is off?

Copy link
Member Author

@viirya viirya May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from @rxin comment that we want to support quoted non-numeric numbers when allowNonNumericNumbers is off.

Copy link
Contributor

@cloud-fan cloud-fan May 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make sense to me. What if users really want to use "NaN" as string?
cc @rxin

Copy link
Member Author

@viirya viirya May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I shouldn't change InferSchem. Tests here are also need to add few doubles. I will update it.

Copy link
Member Author

@viirya viirya May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, it should be number when the field is inferred as double/float type.

Copy link
Member Author

@viirya viirya May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan I updated it.

Non-quoted non-numeric numbers are parsed as double when the corresponding field is double/float type. This behavior is as same as before this patch.

Copy link
Contributor

@rxin rxin May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I say that anywhere?

Copy link
Contributor

@rxin rxin May 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I don't think that's what I meant there.

Expand All @@ -124,10 +126,10 @@ class JsonParsingOptionsSuite extends QueryTest with SharedSQLContext {
val tests: Seq[Double => Boolean] = Seq(_.isNaN, _.isPosInfinity, _.isNegInfinity,
Copy link
Member Author

@viirya viirya Nov 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides, I found that "Inf", "-Inf" seems not working even JsonGenerator.Feature.QUOTE_NON_NUMERIC_NUMBERS is enabled.

Copy link
Member Author

@viirya viirya May 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to upgrade jackson library version in order to support "INF" and "-INF" (case-sensitive).

_.isPosInfinity, _.isNegInfinity, _.isNaN, _.isPosInfinity, _.isNegInfinity,
_.isPosInfinity, _.isNegInfinity)

val schema = StructType(StructField("age", DoubleType, true) :: Nil)
testCases.zipWithIndex.foreach { case (str, idx) =>
val rdd = spark.sparkContext.parallelize(Seq(str))
val df = spark.read.option("allowNonNumericNumbers", "true").json(rdd)
val df = spark.read.option("allowNonNumericNumbers", "true").schema(schema).json(rdd)

assert(df.schema.head.name == "age")
assert(tests(idx)(df.first().getDouble(0)))
Expand Down