kubo / ruby-oci8

Ruby-oci8 - Oracle interface for ruby
Other
169 stars 75 forks source link

Problem binding numbered placeholders #70

Closed ghost closed 9 years ago

ghost commented 9 years ago

I think that the way that numbered SQL placeholders are bound, should be changed. The problem is that:

conn.exec('SELECT :2, :1 FROM DUAL', 'first', 'second') { |row| p row }
=> ["first", "second"]

when you would expect ["second", "first"]. This results from the use of OCIBindByPos() in stmt.c when the vplaceholder has type FIXNUM, because OCIBindByPos ignores the placeholder names. This can cause problems with duplicated placeholders, as well. For example,

conn.exec("SELECT :1, :2 FROM DUAL WHERE :1 = 'first'", 'first', 'second') { |row| p row }
=> ["first", "second"]

but

conn.exec("SELECT :2, :1 FROM DUAL WHERE :1 = 'first'", 'first', 'second') { |row| p row }

returns no rows.

It is possible to work around this by using the bind_param() method and converting keys to strings. This causes OCIBindByName() to be used, producing the result that I would expect:

cursor = conn.parse("SELECT :2, :1 FROM DUAL");

cursor.bind_param(1, "one")
cursor.bind_param(2, "two")
cursor.exec()
p cursor.fetch()
=> ["one", "two"]

cursor.bind_param("1", "one")
cursor.bind_param("2", "two")
cursor.exec()
p cursor.fetch()
=> ["two", "one"]

The following change to stmt.c fixes the problem, but only for eight-bit character encodings. I do not know how to do a FIXNUM to C-string conversion that would also work for wide characters.

*** stmt.c.orig Wed Jan 28 15:05:44 2015
--- stmt.c      Sat Jan 31 16:19:01 2015
***************
*** 161,164 ****
--- 161,167 ----
      } else if (FIXNUM_P(vplaceholder)) {
          position = NUM2INT(vplaceholder);
+         placeholder_len = 16;
+         placeholder_ptr = ALLOCA_N(char, placeholder_len);
+         placeholder_len = snprintf(placeholder_ptr, placeholder_len, "%u", position);
      } else {
          OCI8StringValue(vplaceholder);
kubo commented 9 years ago

Though it may be confusable, it isn't a bug. If you bind placeholders by number, they are bound by their positions, not by their names even thought they are numbered.

First, I don't accept your patch because the following code doesn't work if it is applied.

conn.exec('select * from emp where ename = :ename', 'Smith')

Second, I won't introduce such incompatibility to ruby-oci8 even if special handling is added for numbered plalceholders by checking their names. Old ruby code using ruby-oci8 may be broken by the incompatibility.

ghost commented 9 years ago

Agreed, that it would be an incompatible change. Thanks for your quick response.