heavyai / heavyai-jdbc

A JDBC driver for connecting to an HeavyAI GPU database and running queries.
https://www.heavy.ai/
Other
9 stars 16 forks source link

Support PreparedStatement.setNull for jdbc #17

Open poroszd opened 7 years ago

poroszd commented 7 years ago

I'm using the jdbc interface to move data from Spark to MapD, but it does not work for null values, as MapDPreparedStatement.setNull() is not implemented.

Am I missing something, or it would be really this simple?

   @Override
   public void setNull(int parameterIndex, int sqlType) throws SQLException { //logger.debug("Entered");
-    throw new UnsupportedOperationException("Not supported yet," + " line:" + new Throwable().getStackTrace()[0].
-            getLineNumber() + " class:" + new Throwable().getStackTrace()[0].getClassName() + " method:" + new Throwable().
-            getStackTrace()[0].getMethodName());
+    parmRep[parameterIndex - 1] = "NULL";
+    repCount++;
   }
asuhan commented 7 years ago

@dwayneberry should we make the suggested change?

poroszd commented 7 years ago

FWIW, I compiled it myself (I had to update the thrift.version to 0.10.0, e.g. org.apache.thrift.TSerializable wasn't even present in 0.9.3) and moved the data successfully.

poroszd commented 7 years ago

Well, I was indeed missing something. This does not work for batch inserts (e.g. it will insert 'NULL' as string value for text columns). This is because the prepared statement keeps track of only the string representation of the parameters (parmRep), and the addBatch() method tries to infer whether the parameter was null using this string representation. I think you either have to handle null specially in getQuery(), or keep track of the type of the parameters.

billmaimone commented 7 years ago

@RalphLoen Any comment on this? You've been playing with Spark.

dwayneberry commented 7 years ago

@poroszd Currently this code does not differentiate between empty string and NULL. It treats empty string as NULL.

To get your fix to work

 @Override
   public void setNull(int parameterIndex, int sqlType) throws SQLException { //logger.debug("Entered");
-    throw new UnsupportedOperationException("Not supported yet," + " line:" + new Throwable().getStackTrace()[0].
-            getLineNumber() + " class:" + new Throwable().getStackTrace()[0].getClassName() + " method:" + new Throwable().
-            getStackTrace()[0].getMethodName());
+    parmRep[parameterIndex - 1] = "";
+    repCount++;
   }

Should do it.

But really the longer term solution, if you want to be able to support an empty length string (edge case this fix will not support), we will need to add an additional flag