-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-1460] Returning SchemaRDD instead of normal RDD on Set operations... #448
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,18 +71,12 @@ class VertexRDD[@specialized VD: ClassTag]( | |
override protected def getPreferredLocations(s: Partition): Seq[String] = | ||
partitionsRDD.preferredLocations(s) | ||
|
||
override def persist(newLevel: StorageLevel): VertexRDD[VD] = { | ||
override def persist(newLevel: StorageLevel): this.type = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rxin can we just remove these functions if RDD returns this.type instead? It looks like you were only overloading them for the covariant return type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @marmbrus it actually has its own logic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right, I guess I was actually looking at the functions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right. |
||
partitionsRDD.persist(newLevel) | ||
this | ||
} | ||
|
||
/** Persist this RDD with the default storage level (`MEMORY_ONLY`). */ | ||
override def persist(): VertexRDD[VD] = persist(StorageLevel.MEMORY_ONLY) | ||
|
||
/** Persist this RDD with the default storage level (`MEMORY_ONLY`). */ | ||
override def cache(): VertexRDD[VD] = persist() | ||
|
||
override def unpersist(blocking: Boolean = true): VertexRDD[VD] = { | ||
override def unpersist(blocking: Boolean = true): this.type = { | ||
partitionsRDD.unpersist(blocking) | ||
this | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fairly ignorent of scala; I am not sure I follow, where is type coming from ? And what is it exactly ?
Also , does this change mean it is an incompat interface change ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to allow child classes to not have to override functions like
persist
andcache
that are used for chaining:http://scalada.blogspot.com/2008/02/thistype-for-chaining-method-calls.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, thanks !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess this cant be applied to checkpointRDD and randomSplit ?
What about things like filter, distinct, repartition, sample, filterWith etc ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mridulm if you look at this patch, it explicitly overrides those for
SchemaRDD
. You can't usethis.type
there because the return type is actually a new RDD class (FilteredRDD
and so on).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mridulm agree with Patrick, you have to return
this
forthis.type
return type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying, in retrospect that looks obvious !
On 07-May-2014 2:52 am, "Patrick Wendell" [email protected] wrote: