Skip to content

Conversation

@mmolimar
Copy link
Contributor

What changes were proposed in this pull request?

Previous Spark versions didn't require Java 8 and an Optional Spark Java API had to be implemented to support optional values.

Since Spark 2.4 uses Java 8, the Optional Spark Java API should be removed so that Spark uses the original Java API.

How was this patch tested?

OptionalSuite class was removed to test Spark Java API Optional class (this class as well).
Notice that the get method in the Spark Java API Optional class throws a NullPointerException when the value is not set whereas the native Java API java.util.Optional throws a NoSuchElementException.

@vanzin
Copy link
Contributor

vanzin commented Sep 10, 2018

We can't do this until we decide whether the next version will be a major version or not. This breaks backwards compatibility. (See the bug I duplicated your bug to.)

@srowen
Copy link
Member

srowen commented Sep 10, 2018

Yeah this is the right kind of change but needs to wait for now. Can you update the title?

@mmolimar mmolimar changed the title [SPARK-25395][JavaAPI] Removing Optional Spark Java API [SPARK-25395][JavaAPI] Replace Spark Optional class with Java Optional Sep 11, 2018
@mmolimar
Copy link
Contributor Author

Done @srowen

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

@mmolimar we can move forward with this now. Can you put SPARK-25395 in the title instead? that was the original JIRA.

new Tuple2<>(4, 'w')
));
List<Tuple2<Integer,Tuple2<Integer,Optional<Character>>>> joined =
List<Tuple2<Integer,Tuple2<Integer, Optional<Character>>>> joined =
Copy link
Member

Choose a reason for hiding this comment

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

@mmolimar Could you revert these whitespace changes?

@mmolimar
Copy link
Contributor Author

Updated @srowen
The PR title already contains SPARK-25395, is that what you're expecting or another PR?

@srowen
Copy link
Member

srowen commented Oct 11, 2018

Oops, I mean SPARK-25362. SPARK-25395 was a duplicate.

@mmolimar mmolimar changed the title [SPARK-25395][JavaAPI] Replace Spark Optional class with Java Optional [SPARK-25362][JavaAPI] Replace Spark Optional class with Java Optional Oct 11, 2018
@mmolimar
Copy link
Contributor Author

No problem. Done ;-)

@SparkQA
Copy link

SparkQA commented Oct 11, 2018

Test build #4368 has finished for PR 22383 at commit 054c79a.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Oct 11, 2018

Yeah, you'll have to add this to the 3.0 excludes section of project/MimaExcludes:

ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.api.java.Optional")

@SparkQA
Copy link

SparkQA commented Oct 13, 2018

Test build #4376 has finished for PR 22383 at commit 6eaa3f1.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 16, 2018

Test build #4379 has finished for PR 22383 at commit f188243.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Oct 16, 2018

Oh, hm:

Serialization stack:
	- object not serializable (class: java.util.Optional, value: Optional[x])
	- field (class: scala.Tuple2, name: _2, type: class java.lang.Object)
	- object (class scala.Tuple2, (1,Optional[x]))
	- field (class: scala.Tuple2, name: _2, type: class java.lang.Object)
	- object (class scala.Tuple2, (1,(1,Optional[x])))
	- element of array (index: 0)
	- array (class [Lscala.Tuple2;, size 5)

So java.util.Optional isn't Serializable. Well, that may scuttle this whole idea. I think we're going to find a number of instances where Spark or user apps need to collect() Optional objects.

Unless someone has a bright idea I think we can't do this. Same reason I was unable to change Spark to use java.util.function interfaces -- lambdas aren't otherwise Serializable in Java!

@mmolimar
Copy link
Contributor Author

I agree @srowen.
What do you think about reusing the current implementation we already have, for example, in the guava lib instead of having that class in Spark?

@vanzin
Copy link
Contributor

vanzin commented Oct 18, 2018

Spark's class only reason to exist is so we do not use the Guava class, since everyone and their pet want to use a different version of Guava (and Spark shades it for that reason).

@srowen
Copy link
Member

srowen commented Oct 18, 2018

Yeah, I think we just can't do this unfortunately. It was worth looking into.

@srowen srowen mentioned this pull request Oct 24, 2018
@asfgit asfgit closed this in 65c653f Oct 25, 2018
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
Closes apache#22567
Closes apache#18457
Closes apache#21517
Closes apache#21858
Closes apache#22383
Closes apache#19219
Closes apache#22401
Closes apache#22811
Closes apache#20405
Closes apache#21933

Closes apache#22819 from srowen/ClosePRs.

Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
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.

4 participants