Skip to content

Conversation

@jackieMaKing
Copy link
Contributor

In the previous method, fields.toArray will cast java.util.List[StructField] into Array[Object] which can not cast into Array[StructField], thus when invoking this method will throw "java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Lorg.apache.spark.sql.types.StructField;"
I directly cast java.util.List[StructField] into Array[StructField] in this patch.

@srowen
Copy link
Member

srowen commented Nov 12, 2015

Please fix the title of this PR. Can you also not create a zero-length array each time? keep one instance; it's immutable

@jackieMaKing jackieMaKing changed the title fixed SPARK-11679 [SPARK-11679][SQL] Invoking method " apply(fields: java.util.List[StructField])" in "StructType" gets ClassCastException Nov 13, 2015
@jackieMaKing
Copy link
Contributor Author

Call toArray(T[] a) in Java List to cast List to a specific type Array, thus I create a zero-length array. @srowen what do you mean, better to create an array with the same length of the list to pass to toArray? Like this "StructType(fields.toArray(new Array'[StructField]'(fields.size())))"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can import scala.collection.JavaConverters._ and use StructType(fields.asScala) here.

@jackieMaKing
Copy link
Contributor Author

As suggested by cloud-fan, I pushed the new patch. I use scala.collection.JavaConverters to create StructType.

@cloud-fan
Copy link
Contributor

can you also add a test for it in JavaDataFrameSuite? Thanks!

@jackieMaKing
Copy link
Contributor Author

cloud-fun, I have add a test for SPARK-11679 in JavaDataFrameSuite.

@cloud-fan
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented Nov 14, 2015

Test build #45927 has finished for PR 9649 at commit c639ab4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this test case locally, but can't reproduce the bug. However, the code snippet you provided in the JIRA do reproduce the bug. Can you investigate this and improve the test? Thanks!

@jackieMaKing
Copy link
Contributor Author

Oops!When create List using Arrays.asList, we get a java.util.Arrays$ArrayList instead of java.util.ArrayList. The previous method works fine on java.util.Arrays$ArrayList but gets exception on java.util.ArrayList. This patch works fine on both.

@SparkQA
Copy link

SparkQA commented Nov 16, 2015

Test build #45990 has finished for PR 9649 at commit 4496753.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

Choose a reason for hiding this comment

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

I generally wouldn't bother collapsing these imports. But don't change it unless you need to make other changes for this PR.

@jackieMaKing
Copy link
Contributor Author

srowen, I have restored the original import sentences, but I imported "java.util.ArrayList". cloud-fan, I have already changed f_1 and f_2 to fields1 and field2.

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46042 has finished for PR 9649 at commit 2ebeb4f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

LGTM, cc @yhuai

Copy link
Contributor

Choose a reason for hiding this comment

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

Without this change, was there any issue when we use org.apache.spark.sql.types.DataTypes.createStructType? Or, the problem only appear when we use this apply method directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

DataTypes.createStructType works well because it use fields.toArray(new StructField[0]). However, I think it's bad to create an empty StructField array every time we call that method.
You remind me that we should also update DataTypes.createStructType to use this apply method.

@marmbrus
Copy link
Contributor

Thanks, I'm going to merge this to 1.6 and master.

Copy link
Contributor

Choose a reason for hiding this comment

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

Related, we should just deprecate this. Its weird to have an apply for java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Yes, a java friendly API is preferred.

asfgit pushed a commit that referenced this pull request Nov 17, 2015
…uctField])" in "StructType" gets ClassCastException

In the previous method, fields.toArray will cast java.util.List[StructField] into Array[Object] which can not cast into Array[StructField], thus when invoking this method will throw "java.lang.ClassCastException: [Ljava.lang.Object; cannot be cast to [Lorg.apache.spark.sql.types.StructField;"
I directly cast java.util.List[StructField] into Array[StructField]  in this patch.

Author: mayuanwen <[email protected]>

Closes #9649 from jackieMaKing/Spark-11679.

(cherry picked from commit e8833dd)
Signed-off-by: Michael Armbrust <[email protected]>
@asfgit asfgit closed this in e8833dd Nov 17, 2015
@jackieMaKing jackieMaKing deleted the Spark-11679 branch November 18, 2015 01:44
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.

6 participants