[PATCH] Binding sql variables in dbpool

"Robert Galach" <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]> 
hi again,

here is a patch to dbpool[.c, .h, _p.h, _oracle.h] and dlr_oracle.c

this patch enables binding variable in sql statements
and contains implementation for oracle.
in other types of dbpool database select and update operations are not
handled,
so patch doesn't affect them.

variables are bound as strings only for now,
but it works good for current purposes and doesn't complicate dbpool
interface too much

this makes oracle sql history short and clean
and lets oracle engine cache statements and speed up execution.

as oracle documentation says:
"Statements that are the same except for literals that are used in the
"WHERE" clause could potentially be rewritten to use bind variables.
Reparsing of basically the same SQL statement wastes time and resources. "

it is also a good starting point to implement reusing prepared oci
statements
with different bound variables against preparing them with every
select/update operation

in dlr_oracle.c there is also some cosmetic changes with formatting sql,
rewriten queries to use binds
and added "where" condition to check destination field in dlr search/update
statements
(there may my few dlr rows with the same timestamp from the same smsc). I
remember that it was checked in earlier dlr driver,
but disapeared somwhere and the dst parameter left in functions only. It is
important to me to identify excactly dlr, because of different cgi variable
value in every dlrurl.
if you agree, i guess it should be changed in other dlr_[mem/mysql/sdb].c
files too, to keep semantic clear - I can add it if needed.

patch attached.

please vote & commit :)

regards
Robert
db_pool_binding_variables.patch (application/octet-stream, 19.5 KB) 
Index: gwlib/dbpool.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool.c,v
retrieving revision 1.8
diff -u -r1.8 dbpool.c
--- gwlib/dbpool.c      22 Jan 2004 14:08:25 -0000      1.8
+++ gwlib/dbpool.c      10 Mar 2004 18:37:32 -0000
@@ -61,6 +61,8 @@
  *      2003 Initial version.
  * Alexander Malysh <[email protected]>
  *      2003 Made dbpool more generic.
+ * Robert Ga³ach <[email protected]>
+ *      2004 Added support for binding variables.
  */

 #include "gwlib.h"
@@ -319,7 +321,7 @@
     return n;
 }

-int inline dbpool_conn_select(DBPoolConn *conn, const Octstr *sql, List **result)
+int inline dbpool_conn_select(DBPoolConn *conn, const Octstr *sql, List *binds, List **result)
 {
     if (sql == NULL || conn == NULL)
         return -1;
@@ -327,10 +329,10 @@
     if (conn->pool->db_ops->select == NULL)
         return -1; /* may be panic here ??? */

-    return conn->pool->db_ops->select(conn->conn, sql, result);
+    return conn->pool->db_ops->select(conn->conn, sql, binds, result);
 }

-int inline dbpool_conn_update(DBPoolConn *conn, const Octstr *sql)
+int inline dbpool_conn_update(DBPoolConn *conn, const Octstr *sql, List *binds)
 {
     if (sql == NULL || conn == NULL)
         return -1;
@@ -338,6 +340,6 @@
     if (conn->pool->db_ops->update == NULL)
         return -1; /* may be panic here ??? */

-    return conn->pool->db_ops->update(conn->conn, sql);
+    return conn->pool->db_ops->update(conn->conn, sql, binds);
 }
 #endif /* HAVE_DBPOOL */
Index: gwlib/dbpool.h
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool.h,v
retrieving revision 1.6
diff -u -r1.6 dbpool.h
--- gwlib/dbpool.h      22 Jan 2004 14:08:25 -0000      1.6
+++ gwlib/dbpool.h      10 Mar 2004 18:37:32 -0000
@@ -177,8 +177,8 @@
  */
 void dbpool_conn_produce(DBPoolConn *conn);

-int inline dbpool_conn_select(DBPoolConn *conn, const Octstr *sql, List **result);
-int inline dbpool_conn_update(DBPoolConn *conn, const Octstr *sql);
+int inline dbpool_conn_select(DBPoolConn *conn, const Octstr *sql, List *binds, List **result);
+int inline dbpool_conn_update(DBPoolConn *conn, const Octstr *sql, List *binds);

 /*
  * Perfoms a check of all connections within the pool and tries to
Index: gwlib/dbpool_p.h
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool_p.h,v
retrieving revision 1.4
diff -u -r1.4 dbpool_p.h
--- gwlib/dbpool_p.h    22 Jan 2004 14:08:25 -0000      1.4
+++ gwlib/dbpool_p.h    10 Mar 2004 18:37:32 -0000
@@ -91,16 +91,19 @@
      *           result is the list of rows each row will be stored also as list each column is stored as Octstr.
      * If someone has better idea please tell me ...
      *
-     * @params conn - database specific connection; sql - sql statement ; result - result will be saved here
+     * @params conn - database specific connection; sql - sql statement ;
+     *         binds - list of Octstr values for binding holes in sql (NULL if no binds);
+     *         result - result will be saved here
      * @return 0 if all was fine ; -1 otherwise
      */
-    int (*select) (void *conn, const Octstr *sql, List **result);
+    int (*select) (void *conn, const Octstr *sql, List *binds, List **result);
     /*
      * Database specific update/insert/delete.
-     * @params conn - database specific connection ; sql - sql statement
+     * @params conn - database specific connection ; sql - sql statement;
+     *         binds - list of Octstr values for binding holes in sql (NULL if no binds);
      * @return #rows processed ; -1 if a error occurs
      */
-    int (*update) (void *conn, const Octstr *sql);
+    int (*update) (void *conn, const Octstr *sql, List *binds);
 };

 struct DBPool
Index: gwlib/dbpool_oracle.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool_oracle.c,v
retrieving revision 1.4
diff -u -r1.4 dbpool_oracle.c
--- gwlib/dbpool_oracle.c       22 Jan 2004 14:08:25 -0000      1.4
+++ gwlib/dbpool_oracle.c       10 Mar 2004 18:37:32 -0000
@@ -63,6 +63,8 @@
  * believe me, just check it youself with valgrind ;)
  *
  * Alexander Malysh <[email protected]>
+ * Robert Ga³ach <[email protected]>
+ *      2004 Added support for binding variables.
  */

 #ifdef HAVE_ORACLE
@@ -70,7 +72,7 @@
 #include <oci.h>

 /* forward decl. */
-static int oracle_select(void *theconn, const Octstr *sql, List **res);
+static int oracle_select(void *theconn, const Octstr *sql, List *binds, List **res);

 struct ora_conn {
     /* environment handle */
@@ -82,7 +84,7 @@
 };

 /* This function prints the error */
-void oracle_checkerr(OCIError *errhp, sword status)
+static void oracle_checkerr(OCIError *errhp, sword status)
 {
     text errbuf[512];
     sb4 errcode = 0;
@@ -247,7 +249,7 @@
     /* TODO Check for appropriate OCI function */
     sql = octstr_create("SELECT 1 FROM DUAL");

-    ret = oracle_select(conn, sql, &res);
+    ret = oracle_select(conn, sql, NULL, &res);
     if (ret != -1 && list_len(res) > 0) {
         List *row = list_extract_first(res);
         list_destroy(row, octstr_destroy_item);
@@ -274,9 +276,10 @@
 }


-static int oracle_select(void *theconn, const Octstr *sql, List **res)
+static int oracle_select(void *theconn, const Octstr *sql, List *binds, List **res)
 {
     List *row;
+    List *bind_list = list_create();
     OCIStmt *stmt;
     OCIParam *dparam;
     sword status;
@@ -290,6 +293,7 @@
     };
     struct data_s *data;
     struct ora_conn *conn = (struct ora_conn*) theconn;
+    int binds_len = (binds ? list_len(binds) : 0);

     *res = NULL;

@@ -307,6 +311,25 @@
         OCIHandleFree(stmt, OCI_HTYPE_STMT);
         return -1;
     }
+    /* prepare ocibind list */
+    for (i = 0; i < binds_len; i++) {
+        OCIBind *bndhp = NULL;
+        list_append(bind_list, bndhp);
+    }
+    /* bind variables */
+    for (i = 0; i < binds_len; i++) {
+        Octstr *bind = list_get(binds, i);
+        OCIBind *bndhp = list_get(bind_list, i);
+        status = OCIBindByPos(stmt, &bndhp,
+                              conn->errhp, (i+1), (dvoid *) octstr_get_cstr(bind),
+                              (sword) octstr_len(bind)+1, SQLT_STR, (dvoid *) 0, (ub2 *)0,
+                              (ub2 *)0, (ub4)0, (ub4 *)0, OCI_DEFAULT);
+        if (OCI_SUCCESS != status) {
+            oracle_checkerr(conn->errhp, status);
+            OCIHandleFree(stmt, OCI_HTYPE_STMT);
+            return -1;
+        }
+    }
     /* execute our statement */
     status = OCIStmtExecute(conn->svchp, stmt, conn->errhp, 0, 0, NULL, NULL,
                             OCI_DEFAULT);
@@ -433,19 +456,22 @@
         gw_free(data[i].data);

     gw_free(data);
+    list_destroy(bind_list, NULL);
     OCIHandleFree(stmt, OCI_HTYPE_STMT);

     return 0;
 }


-static int oracle_update(void *theconn, const Octstr *sql)
+static int oracle_update(void *theconn, const Octstr *sql, List *binds)
 {
     OCIStmt *stmt;
     sword status;
-    ub4 rows = 0;
+    ub4 rows = 0, i;
     struct ora_conn *conn = (struct ora_conn*) theconn;
-
+    int binds_len = (binds ? list_len(binds) : 0);
+    List *bind_list = list_create();
+
     /* allocate statement handle */
     status = OCIHandleAlloc(conn->envp, (dvoid**)&stmt, OCI_HTYPE_STMT, 0,0);
     if (OCI_SUCCESS != status) {
@@ -462,6 +488,27 @@
         return -1;
     }
     debug("dbpool.oracle",0,"OCIStmtPrepare done");
+    /* prepare ocibind list */
+    for (i = 0; i < binds_len; i++) {
+        OCIBind *bndhp = NULL;
+        list_append(bind_list, bndhp);
+    }
+
+    /* bind variables */
+    for (i = 0; i < binds_len; i++) {
+        Octstr *bind = list_get(binds, i);
+        OCIBind *bndhp = list_get(bind_list, i);
+        status = OCIBindByPos(stmt, &bndhp,
+                              conn->errhp, (i+1), (dvoid *) octstr_get_cstr(bind),
+                              (sword) octstr_len(bind)+1, SQLT_STR, (dvoid *) 0, (ub2 *)0,
+                              (ub2 *)0, (ub4)0, (ub4 *)0, OCI_DEFAULT);
+        if (OCI_SUCCESS != status) {
+            oracle_checkerr(conn->errhp, status);
+            OCIHandleFree(stmt, OCI_HTYPE_STMT);
+            return -1;
+        }
+    }
+
     /* execute our statement */
     status = OCIStmtExecute(conn->svchp, stmt, conn->errhp, 1, 0, NULL, NULL,
                             /*OCI_DEFAULT*/ OCI_COMMIT_ON_SUCCESS);
@@ -480,8 +527,9 @@
     }
     debug("dbpool.oracle",0,"rows processed = %d", rows);

+    list_destroy(bind_list, NULL);
     OCIHandleFree(stmt, OCI_HTYPE_STMT);
-
+
     return (int) rows;
 }

Index: gw/dlr_oracle.c
===================================================================
RCS file: /home/cvs/gateway/gw/dlr_oracle.c,v
retrieving revision 1.5
diff -u -r1.5 dlr_oracle.c
--- gw/dlr_oracle.c     22 Jan 2004 14:08:24 -0000      1.5
+++ gw/dlr_oracle.c     10 Mar 2004 18:37:32 -0000
@@ -58,6 +58,8 @@
  * dlr_oracle.c - Oracle dlr storage implementation.
  *
  * Author: Alexander Malysh <[email protected]>, (C) 2003
+ * Robert Ga³ach <[email protected]>
+ *      2004 Rewrited sql queries to use binding variables.
  *
  * Copyright: See COPYING file that comes with this distribution
  */
@@ -91,12 +93,12 @@
     if (conn == NULL)
         return -1;

-    sql = octstr_format("SELECT count(*) FROM %s", octstr_get_cstr(fields->table));
+    sql = octstr_format("SELECT count(*) FROM %S", fields->table);
 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif

-    if (dbpool_conn_select(conn, sql, &result) != 0) {
+    if (dbpool_conn_select(conn, sql, NULL, &result) != 0) {
         octstr_destroy(sql);
         dbpool_conn_produce(conn);
         return -1;
@@ -122,9 +124,9 @@

 static void dlr_add_oracle(struct dlr_entry *entry)
 {
-    Octstr *sql;
+    Octstr *sql, *os_mask;
     DBPoolConn *pconn;
-
+    List *binds = list_create();
     debug("dlr.oracle", 0, "adding DLR entry into database");

     pconn = dbpool_conn_consume(pool);
@@ -134,26 +136,32 @@
         return;
     }

-    sql = octstr_format("INSERT INTO %s (%s, %s, %s, %s, %s, %s, %s, %s, %s) VALUES "
-                        "('%s', '%s', '%s', '%s', '%s', '%s', '%d', '%s', '%d')",
-                        octstr_get_cstr(fields->table), octstr_get_cstr(fields->field_smsc),
-                        octstr_get_cstr(fields->field_ts),
-                        octstr_get_cstr(fields->field_src), octstr_get_cstr(fields->field_dst),
-                        octstr_get_cstr(fields->field_serv), octstr_get_cstr(fields->field_url),
-                        octstr_get_cstr(fields->field_mask), octstr_get_cstr(fields->field_boxc),
-                        octstr_get_cstr(fields->field_status),
-                        octstr_get_cstr(entry->smsc), octstr_get_cstr(entry->timestamp), octstr_get_cstr(entry->source),
-                        octstr_get_cstr(entry->destination), octstr_get_cstr(entry->service), octstr_get_cstr(entry->url),
-                        entry->mask, octstr_get_cstr(entry->boxc_id), 0);
-
+    sql = octstr_format("INSERT INTO %S (%S, %S, %S, %S, %S, %S, %S, %S, %S) VALUES "
+                        "(:1, :2, :3, :4, :5, :6, :7, :8, 0)",
+                        fields->table, fields->field_smsc, fields->field_ts,
+                        fields->field_src, fields->field_dst, fields->field_serv,
+                        fields->field_url, fields->field_mask, fields->field_boxc,
+                        fields->field_status);
+    os_mask = octstr_format("%d", entry->mask);
+
+    list_append(binds, entry->smsc);         /* :1 */
+    list_append(binds, entry->timestamp);    /* :2 */
+    list_append(binds, entry->source);       /* :3 */
+    list_append(binds, entry->destination);  /* :4 */
+    list_append(binds, entry->service);      /* :5 */
+    list_append(binds, entry->url);          /* :6 */
+    list_append(binds, os_mask);             /* :7 */
+    list_append(binds, entry->boxc_id);      /* :8 */
 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql) == -1)
+    if (dbpool_conn_update(pconn, sql, binds) == -1)
         error(0, "DLR: ORACLE: Error while adding dlr entry for DST<%s>", octstr_get_cstr(entry->destination));

     dbpool_conn_produce(pconn);
     octstr_destroy(sql);
+    list_destroy(binds, NULL);
+    octstr_destroy(os_mask);
     dlr_entry_destroy(entry);
 }

@@ -161,6 +169,7 @@
 {
     Octstr *sql;
     DBPoolConn *pconn;
+    List *binds = list_create();

     debug("dlr.oracle", 0, "removing DLR from database");

@@ -169,18 +178,23 @@
     if (pconn == NULL)
         return;

-    sql = octstr_format("DELETE FROM %s WHERE %s='%s' AND %s='%s' AND ROWNUM < 2",
-                        octstr_get_cstr(fields->table), octstr_get_cstr(fields->field_smsc),
-                        octstr_get_cstr(smsc), octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+    sql = octstr_format("DELETE FROM %S WHERE %S=:1 AND %S=:2 AND %S=:3 AND ROWNUM < 2",
+                        fields->table, fields->field_smsc,
+                        fields->field_ts, fields->field_dst);
+
+    list_append(binds, (Octstr *)smsc);      /* :1 */
+    list_append(binds, (Octstr *)ts);        /* :2 */
+    list_append(binds, (Octstr *)dst);       /* :3 */

 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif

-    if (dbpool_conn_update(pconn, sql) == -1)
+    if (dbpool_conn_update(pconn, sql, binds) == -1)
         error(0, "DLR: ORACLE: Error while removing dlr entry for DST<%s>", octstr_get_cstr(dst));

     dbpool_conn_produce(pconn);
+    list_destroy(binds, NULL);
     octstr_destroy(sql);
 }

@@ -190,27 +204,33 @@
     DBPoolConn *pconn;
     List *result = NULL, *row;
     struct dlr_entry *res = NULL;
+    List *binds = list_create();

     pconn = dbpool_conn_consume(pool);
     if (pconn == NULL) /* should not happens, but sure is sure */
         return NULL;

-    sql = octstr_format("SELECT %s, %s, %s, %s, %s, %s FROM %s WHERE %s='%s' AND %s='%s' AND ROWNUM < 2",
-                        octstr_get_cstr(fields->field_mask), octstr_get_cstr(fields->field_serv),
-                        octstr_get_cstr(fields->field_url), octstr_get_cstr(fields->field_src),
-                        octstr_get_cstr(fields->field_dst), octstr_get_cstr(fields->field_boxc),
-                        octstr_get_cstr(fields->table), octstr_get_cstr(fields->field_smsc),
-                        octstr_get_cstr(smsc), octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
+    sql = octstr_format("SELECT %S, %S, %S, %S, %S, %S FROM %S WHERE %S=:1 AND %S=:2 AND %S=:3 AND ROWNUM < 2",
+                        fields->field_mask, fields->field_serv,
+                        fields->field_url, fields->field_src,
+                        fields->field_dst, fields->field_boxc,
+                        fields->table, fields->field_smsc,
+                        fields->field_ts, fields->field_dst);
+
+    list_append(binds, (Octstr *)smsc);      /* :1 */
+    list_append(binds, (Octstr *)ts);        /* :2 */
+    list_append(binds, (Octstr *)dst);       /* :3 */

 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_select(pconn, sql, &result) != 0) {
+    if (dbpool_conn_select(pconn, sql, binds, &result) != 0) {
         octstr_destroy(sql);
         dbpool_conn_produce(pconn);
         return NULL;
     }
     octstr_destroy(sql);
+    list_destroy(binds, NULL);
     dbpool_conn_produce(pconn);

 #define LO2CSTR(r, i) octstr_get_cstr(list_get(r, i))
@@ -237,8 +257,9 @@

 static void dlr_update_oracle(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int status)
 {
-    Octstr *sql;
+    Octstr *sql, *os_status;
     DBPoolConn *pconn;
+    List *binds = list_create();

     debug("dlr.oracle", 0, "updating DLR status in database");

@@ -247,19 +268,48 @@
     if (pconn == NULL)
         return;

-    sql = octstr_format("UPDATE %s SET %s=%d WHERE %s='%s' AND %s='%s' AND ROWNUM < 2",
-                        octstr_get_cstr(fields->table),
-                        octstr_get_cstr(fields->field_status), status,
-                        octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
-                        octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
-
+    sql = octstr_format("UPDATE %S SET %S=:1 WHERE %S=:2 AND %S=:3 AND %S=:4 AND ROWNUM < 2",
+                        fields->table, fields->field_status,
+                        fields->field_smsc, fields->field_ts, fields->field_dst);
+
+    os_status = octstr_format("%d", status);
+    list_append(binds, (Octstr *)os_status); /* :1 */
+    list_append(binds, (Octstr *)smsc);      /* :2 */
+    list_append(binds, (Octstr *)ts);        /* :3 */
+    list_append(binds, (Octstr *)dst);       /* :4 */
 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql) == -1)
+    if (dbpool_conn_update(pconn, sql, binds) == -1)
         error(0, "DLR: ORACLE: Error while updating dlr entry for DST<%s>", octstr_get_cstr(dst));

     dbpool_conn_produce(pconn);
+    list_destroy(binds, NULL);
+    octstr_destroy(os_status);
+    octstr_destroy(sql);
+}
+
+static void dlr_flush_oracle (void)
+{
+    Octstr *sql;
+    DBPoolConn *pconn;
+    int rows;
+
+    pconn = dbpool_conn_consume(pool);
+    /* just for sure */
+    if (pconn == NULL)
+        return;
+
+    sql = octstr_format("DELETE FROM DLR");
+#if defined(DLR_TRACE)
+    debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
+#endif
+    rows = dbpool_conn_update(pconn, sql, NULL);
+    if (rows == -1)
+        error(0, "DLR: ORACLE: Error while flushing dlr entries from database");
+    else
+        debug("dlr.oracle", 0, "Flushing %d DLR entries from database", rows);
+    dbpool_conn_produce(pconn);
     octstr_destroy(sql);
 }

@@ -270,7 +320,8 @@
     .dlr_add = dlr_add_oracle,
     .dlr_get = dlr_get_oracle,
     .dlr_remove = dlr_remove_oracle,
-    .dlr_update = dlr_update_oracle
+    .dlr_update = dlr_update_oracle,
+    .dlr_flush = dlr_flush_oracle
 };

 struct dlr_storage *dlr_init_oracle(Cfg *cfg)
Index: test/test_dbpool.c
===================================================================
RCS file: /home/cvs/gateway/test/test_dbpool.c,v
retrieving revision 1.6
diff -u -r1.6 test_dbpool.c
--- test/test_dbpool.c  22 Jan 2004 14:08:25 -0000      1.6
+++ test/test_dbpool.c  10 Mar 2004 18:37:32 -0000
@@ -186,11 +186,13 @@
 {
     DBPool *pool = arg;
     DBPoolConn *pconn = NULL;
+    /*
     struct ora_conn *conn;
-    int i;
     OCIStmt *stmt;
     sword status;
     ub2 stmt_type;
+    */
+    int i;
     List *result;

     for (i = 1; i <= queries; i++) {
@@ -199,19 +201,19 @@
         if (pconn == NULL)
             continue;
 #if 1 /* selects */
-        if (dbpool_conn_select(pconn, sql, &result) == 0) {
+        if (dbpool_conn_select(pconn, sql, NULL, &result) == 0) {
             long i,j;
             for (i=0; i < list_len(result); i++) {
                 List *row = list_get(result, i);
                 for (j=0; j < list_len(row); j++)
-                    debug("", 0, "col = %d   value = '%s'", j, octstr_get_cstr(list_get(row,j)));
+                    debug("", 0, "col = %ld   value = '%s'", j, octstr_get_cstr(list_get(row,j)));
                 list_destroy(row, octstr_destroy_item);
             }
         }
         list_destroy(result, NULL);
         dbpool_conn_produce(pconn);
 #else /* only updates */
-        debug("", 0, "rows processed = %d ", dbpool_conn_update(pconn, sql));
+        debug("", 0, "rows processed = %d ", dbpool_conn_update(pconn, sql, NULL));
         dbpool_conn_produce(pconn);
 #endif
     }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.