offthewall123 commented on a change in pull request #7815:
URL: https://github.com/apache/arrow/pull/7815#discussion_r459825776



##########
File path: 
java/plasma/src/main/java/org/apache/arrow/plasma/exceptions/PlasmaOutOfMemoryException.java
##########
@@ -22,11 +22,11 @@
  */
 public class PlasmaOutOfMemoryException extends RuntimeException {
 
-  public PlasmaOutOfMemoryException() {
+  public PlasmaOutOfMemoryException(String message) {

Review comment:
       Thanks for reply. @rymurr 
   Yes, we can add a new constructor like `public 
PlasmaOutOfMemoryException(String message) ` and keep the origin two.
   But In plasma/lib/java/org_apache_arrow_plasma_PlasmaClientJNI.cc
   line 123 `env->ThrowNew(exceptionClass, "");` here just passed an empty 
String, and i searched seems no overloaded function like 
`env->ThrowNew(exceptionClass)` which not to pass a String arg. 
   
   If we replace `String message` to the internal message -> It's an empty 
String, i think we should keep the internal message `super("The plasma store 
ran out of memory.");`
   
   If Add new constructors -> The origin two constructors are of no use.
   
   So `String message` added here is just to adapt 
`env->ThrowNew(exceptionClass, "");`
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to