Skip to content
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.

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

[SPARK-21015] Check field name is not null and empty in GenericRowWit… #18236

Closed
wants to merge 1 commit into from

Conversation

darionyaphet
Copy link

What changes were proposed in this pull request?

When we get field index from row with schema , we shoule make sure the field name is not null and empty .

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@rxin
Copy link
Contributor

rxin commented Jun 10, 2017

Why do we want this check? If the user passes in null value, it is ok if it is not found, isn't it?

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Jun 10, 2017

It looks we already throw the similar exception with what this PR proposes (not NPE) as below:

import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema
import org.apache.spark.sql.types._

val schema = StructType(
  StructField("col1", StringType) ::
  StructField("col2", StringType) ::
  StructField("col3", IntegerType) :: Nil)
val values = Array("value1", "value2", 1)

val sampleRow = new GenericRowWithSchema(values, schema)
sampleRow.fieldIndex("")
java.lang.IllegalArgumentException: Field "" does not exist.
  at org.apache.spark.sql.types.StructType$$anonfun$fieldIndex$1.apply(StructType.scala:292)
  at org.apache.spark.sql.types.StructType$$anonfun$fieldIndex$1.apply(StructType.scala:292)
  at scala.collection.MapLike$class.getOrElse(MapLike.scala:128)
  at scala.collection.AbstractMap.getOrElse(Map.scala:59)
  ...
sampleRow.fieldIndex(null)
java.lang.IllegalArgumentException: Field "null" does not exist.
  at org.apache.spark.sql.types.StructType$$anonfun$fieldIndex$1.apply(StructType.scala:292)
  at org.apache.spark.sql.types.StructType$$anonfun$fieldIndex$1.apply(StructType.scala:292)
  at scala.collection.MapLike$class.getOrElse(MapLike.scala:128)
  at scala.collection.AbstractMap.getOrElse(Map.scala:59)
  ...

I think we should close this.

@darionyaphet
Copy link
Author

@rxin @HyukjinKwon Thank you looking into this PR :D

Maybe I don't make it clearly : GenericRowWithSchema fetch field with field name and the field name shouldn't null or empty . So I add a check before schema.fieldIndex.

@gatorsmile
Copy link
Member

How about just updating the comment of this function to explain the behavior we have now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants