[R-sig-DB] Fixes for two bugs in ROracle string handling

[David Hinds]

I've sent these to David James, who is nominally the maintainer for ROracle,
but he hasn't responded, so I thought I'd at least post these here in case
anyone else runs into them:

Bug 1: the code for trimming trailing blanks and null terminating strings in
RS-DBI.c has a bad boundary condition.  If a string pulled from the database is
composed entirely of blanks, then the code places the terminal null one byte
before the start of the buffer.  So the string isn't actually null terminated at
all, and bad things can happen.  Here is a test case that shows the problem:

> dbClearResult(dbSendQuery(db, 'create table x(v varchar2(20))'))
[1] TRUE
> d <- data.frame(v=c('string1',' ','string2'), stringsAsFactors=FALSE)
> ps <- dbPrepareStatement(db, 'insert into x values(:1)', d)
> dbGetRowsAffected(dbExecStatement(ps, d))
[1] 3
> dbClearResult(ps)
> r <- dbSendQuery(db, 'select * from x')
> str(fetch(r))
'data.frame':   3 obs. of  1 variable:
 $ V: chr  "string1" "                    " "string2"

In this case the bogus second string came through as all blank, but it can also
run on into la-la land.

Bug 2: the code for sizing the buffers used for sending character string data to
Oracle mishandles NA values.  The code determines a maximum string length for
each bind variable but does not check for NA strings.  NA strings are internally
represented as 'NA' and hence have an apparent length of 2.  If you try to store
an NA into a column defined as char(1), Oracle complains:

> dbClearResult(dbSendQuery(db, 'create table x (v char(1))'))
> d <- data.frame(v=c('x',NA,'y'), stringsAsFactors=FALSE)
> ps <- dbPrepareStatement(db, 'insert into x values(:1)', d)
> dbExecStatement(ps, d)
Error in oraExecStatement(ps, data, ...) :
  RS-DBI driver: (ORA-12899: value too large for column "OPS$DHINDS"."X"."V"
(actual: 2, maximum: 1)
)

And here is a patch that fixes both bugs.  I happened to be using version 0.5-8
but the patch applies cleanly to 0.5-9 as well.  I hope that Outlook doesn't
munge it up...

-- David Hinds



--- ROracle_0.5-8/src/RS-DBI.c  2006-09-18 15:20:17.000000000 -0700
+++ ROracle/src/RS-DBI.c        2009-06-15 13:18:27.451250000 -0700
@@ -630,7 +630,8 @@
   /* null terminate string whether we delete trailing blanks or not*/
   if(del_blanks){
     for(end = str_buffer+len-1; end>=str_buffer; end--)
-      if(*end != ' ') { end++; break; }
+      if(*end != ' ') { break; }
+    end++;
     *end = '\0';
   }
   else {
--- ROracle_0.5-8/src/RS-Oracle.pc      2006-09-18 15:20:17.000000000 -0700
+++ ROracle/src/RS-Oracle.pc    2009-06-25 14:50:24.696063000 -0700
@@ -1285,8 +1285,14 @@
       if(Sclass[j]==CHARACTER_TYPE){
          n = 0;
          for(i=0; i<n_rows; i++){
-            nc = strlen(LST_CHR_EL(input, df_field, from+i));
-            n = RS_Ora_Max(n, nc);
+#ifdef USING_R
+            if(STRING_ELT(LST_EL(input,df_field),from+i)!=NA_STRING) {
+#else
+            if(!IS_NA(schars, CHARACTER_TYPE)) {  /* S-Plus specific */
+#endif
+               nc = strlen(LST_CHR_EL(input, df_field, from+i));
+               n = RS_Ora_Max(n, nc);
+            }
          }
          bind_dp->V[j] = (char *) calloc(bufferSize, n+1);  /* plus '\0' */
          bind_dp->L[j] = fld_len[j] = n+1;