-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-10049][SPARKR] Support collecting data of ArraryType in DataFrame. #8458
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 1 commit
1f7ab95
e3af422
9b5bd05
02c64eb
2bc97ad
a1f4fcb
1e223e0
adde91f
a7aa017
afad2c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,7 +125,7 @@ private[r] class RBackendHandler(server: RBackend) | |
| val methods = cls.getMethods | ||
| val selectedMethods = methods.filter(m => m.getName == methodName) | ||
| if (selectedMethods.length > 0) { | ||
| val (index, convertedArgs) = matchMethod( | ||
| val index = matchMethod( | ||
| selectedMethods.map(_.getParameterTypes), | ||
| args) | ||
|
|
||
|
|
@@ -138,15 +138,15 @@ private[r] class RBackendHandler(server: RBackend) | |
| throw new Exception(s"No matched method found for $cls.$methodName") | ||
| } | ||
|
|
||
| val ret = selectedMethods(index.get).invoke(obj, convertedArgs : _*) | ||
| val ret = selectedMethods(index.get).invoke(obj, args : _*) | ||
|
|
||
| // Write status bit | ||
| writeInt(dos, 0) | ||
| writeObject(dos, ret.asInstanceOf[AnyRef]) | ||
| } else if (methodName == "<init>") { | ||
| // methodName should be "<init>" for constructor | ||
| val ctors = cls.getConstructors | ||
| val (index, convertedArgs) = matchMethod( | ||
| val index = matchMethod( | ||
| ctors.map(_.getParameterTypes), | ||
| args) | ||
|
|
||
|
|
@@ -159,7 +159,7 @@ private[r] class RBackendHandler(server: RBackend) | |
| throw new Exception(s"No matched constructor found for $cls") | ||
| } | ||
|
|
||
| val obj = ctors(index.get).newInstance(convertedArgs : _*) | ||
| val obj = ctors(index.get).newInstance(args : _*) | ||
|
|
||
| writeInt(dos, 0) | ||
| writeObject(dos, obj.asInstanceOf[AnyRef]) | ||
|
|
@@ -178,7 +178,7 @@ private[r] class RBackendHandler(server: RBackend) | |
|
|
||
| // Read a number of arguments from the data input stream | ||
| def readArgs(numArgs: Int, dis: DataInputStream): Array[java.lang.Object] = { | ||
| (0 until numArgs).map { arg => | ||
| (0 until numArgs).map { _ => | ||
| readObject(dis) | ||
| }.toArray | ||
| } | ||
|
|
@@ -187,54 +187,66 @@ private[r] class RBackendHandler(server: RBackend) | |
| // according to the passed arguments. Arguments may be converted in | ||
| // order to match a method. | ||
| // | ||
| // Returns a two-element tuple. The first element is the index of | ||
| // the matched method in the list of the methods of the same name. | ||
| // The second element is the list of converted arguments. | ||
| // Returns an Option[Int] which is the index of the matched method in | ||
| // the list of the methods of the same name. | ||
| def matchMethod( | ||
|
Contributor
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. Could you also document what the returned objects are for this method ?
Contributor
Author
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. sure. will add. |
||
| parameterTypesOfMethods: Array[Array[Class[_]]], | ||
| args: Array[Object]): (Option[Int], Array[Object]) = { | ||
| args: Array[Object]): Option[Int] = { | ||
| val numArgs = args.length | ||
|
|
||
| for (index <- 0 to parameterTypesOfMethods.length - 1) { | ||
|
Contributor
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. 0 until xxx
Contributor
Author
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. fixed |
||
| val parameterTypes = parameterTypesOfMethods(index) | ||
|
|
||
| if (parameterTypes.length == numArgs) { | ||
| val convertedArgs = new Array[Object](numArgs) | ||
| Array.copy(args, 0, convertedArgs, 0, numArgs) | ||
|
|
||
| var argMatched = true | ||
| var i = 0 | ||
| while (i < numArgs && argMatched) { | ||
| val parameterType = parameterTypes(i) | ||
| var parameterWrapperType = parameterType | ||
|
|
||
| // Convert native parameters to Object types as args is Array[Object] here | ||
| if (parameterType.isPrimitive) { | ||
| parameterWrapperType = parameterType match { | ||
| case java.lang.Integer.TYPE => classOf[java.lang.Integer] | ||
| case java.lang.Long.TYPE => classOf[java.lang.Integer] | ||
| case java.lang.Double.TYPE => classOf[java.lang.Double] | ||
| case java.lang.Boolean.TYPE => classOf[java.lang.Boolean] | ||
| case _ => parameterType | ||
|
|
||
| if (parameterType == classOf[Seq[Any]] && args(i).getClass.isArray) { | ||
| // The case that the parameter type is a Scala Seq and the argument | ||
| // is a Java array is considered matching. The array will be converted | ||
| // to a Seq later if this method is matched. | ||
| } else { | ||
| var parameterWrapperType = parameterType | ||
|
|
||
| // Convert native parameters to Object types as args is Array[Object] here | ||
| if (parameterType.isPrimitive) { | ||
| parameterWrapperType = parameterType match { | ||
| case java.lang.Integer.TYPE => classOf[java.lang.Integer] | ||
| case java.lang.Long.TYPE => classOf[java.lang.Integer] | ||
| case java.lang.Double.TYPE => classOf[java.lang.Double] | ||
| case java.lang.Boolean.TYPE => classOf[java.lang.Boolean] | ||
| case _ => parameterType | ||
| } | ||
| } | ||
| if (!parameterWrapperType.isInstance(args(i))) { | ||
| argMatched = false | ||
| } | ||
| } else if (parameterType == classOf[Seq[Any]] && args(i).getClass.isArray) { | ||
| // Convert a Java array to scala Seq | ||
| convertedArgs(i) = args(i).asInstanceOf[Array[_]].toSeq | ||
| } | ||
| if (!parameterWrapperType.isInstance(convertedArgs(i))) { | ||
| argMatched = false | ||
| } | ||
|
|
||
| i = i + 1 | ||
| } | ||
|
|
||
| if (argMatched) { | ||
| // For now, we return the first matching method. | ||
| // TODO: find best method in matching methods. | ||
| return (Some(index), convertedArgs) | ||
|
|
||
| // Convert args if needed | ||
| val parameterTypes = parameterTypesOfMethods(index) | ||
|
|
||
| (0 until numArgs).map { i => | ||
| if (parameterTypes(i) == classOf[Seq[Any]] && args(i).getClass.isArray) { | ||
| // Convert a Java array to scala Seq | ||
| args(i) = args(i).asInstanceOf[Array[_]].toSeq | ||
| } | ||
| } | ||
|
|
||
| return Some(index) | ||
| } | ||
| } | ||
| } | ||
| (None, args) | ||
| None | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
Why does not return the found method directly, instead of index? Since we change the semantics of this function, we should also rename it, for example, findMethodByArgs ?
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.
In java reflection, constructors and normal methods are of different classes, and share no parent class that provides methods for reflection uses. I can't find a unified way to handle them in this function, so I have to pass in parameterTypes: Array[Array[Class[_]]] and return an index.
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 see. Maybe
findMatchedSignature?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.
done