[SPARK-15491][SQL]fix assertion failure for JDBC DataFrame to JSON

[huaxingao]

What changes were proposed in this pull request?

in TreeNode.scala parseToJson, it has

case p: Product => try {
      val fieldNames = getConstructorParameterNames(p.getClass)
      val fieldValues = p.productIterator.toSeq
      assert(fieldNames.length == fieldValues.length)
      ("product-class" -> JString(p.getClass.getName)) :: fieldNames.zip(fieldValues).map {
        case (name, value) => name -> parseToJson(value)
      }.toList

For a class that has two level of constructor parameters, for example,

private[sql] case class JDBCRelation(
    url: String,
    table: String,
    parts: Array[Partition],
    properties: Properties = new Properties())(@transient val sparkSession: SparkSession)

val fieldValues = p.productIterator.toSeq will only get the values for the first level of constructor parameters, in this case, 4 parameters, but
val fieldNames = getConstructorParameterNames(p.getClass)
will get all the 5 parameters, so assert(fieldNames.length == fieldValues.length) will fail.

I will change code to make fieldValues have all the parameters.

How was this patch tested?

Added a unit test.

[AmplabJenkins]

Can one of the admins verify this patch?

[rxin]

Thanks for the pull request. Can you format the description better?

[huaxingao]

@rxin Sorry, I didn't notice your comment until this morning. Reformatted the description. Thanks!

[rxin]

Can we create a unit test for TreeNode, rather than using JDBC?

[huaxingao]

@rxin Thanks for your comment.
When I wrote the test, I found there is one more place I need to change. So there are two places that have problems:

1. TreeNode that has two layers of constructor parameters.
2. TreeNode has a constructor parameter that contains two layers of constructor parameters.
   I have both cases in my test. Hopefully it is what you want. Thanks!

[huaxingao]

@rxin gentle ping

[gatorsmile]

Is it still an issue?

[huaxingao]

@gatorsmile
Sorry for the late response. I just came back from China.

For a class that has two level of constructor parameters, e.g.
private[sql] case class JDBCRelation(
url: String,
table: String,
parts: Array[Partition],
properties: Properties = new Properties())(@transient val sparkSession: SparkSession)
toJson will have problem. The current code doesn't have problem any more because in the new method TreeNode.shouldConvertToJson, JDBCRelation returns false and is not allowed to convert to JSON. However, it seems to me that the root problem is not fixed. In the following cases:

1. TreeNode that has two layers of constructor parameters.
2. TreeNode has a constructor parameter that contains two layers of constructor parameters.
   And if the TreeNode can be converted to JSON, the Exception will still occur.

[gatorsmile]

Could you check the master branch? We limit toJson to limited node types after #14990. shouldConvertToJson. It sounds like the class you mentioned above is not part of the support types

[huaxingao]

Thank you very much for your reply.
Shall we also allow the JDBCRelation to be converted to JSON like the following?

     val jdbcDF = sqlContext.read.format("jdbc").options(
       Map("url" -> "jdbc:h2:mem:testdb0;user=testUser;password=testPass",
         "dbtable" -> "people")).load().queryExecution.logical.toJSON

[gatorsmile]

Maybe not needed now unless it is required.

[huaxingao]

Thanks. I will close this PR.