[sr-dev] git:4.1: db_cassandra: bug fixes for raw queries using sqlops

Carlos Ruiz Díaz carlos.ruizdiaz at gmail.com
Wed Mar 12 15:10:22 CET 2014


You have to define the table structure with the right column types
(including the mandatory KEY field), using the auxiliary db schema file [1].

Selecting all fields is never a good idea with Cassandra since the purpose
of using SELECT in sqlops is to get the value of certain field into a
pseudovariable. If you are selecting all of the fields, how do you know
what column filled what pvar?

[1] http://kamailio.org/docs/modules/devel/modules/db_cassandra#idp2531032


On Wed, Mar 12, 2014 at 4:52 AM, jay binks <jaybinks at gmail.com> wrote:

> So after moving to GIT head ( was using a version from a few weeks ago )
> Im now getting heaps of these
>
>  0(29141) ALERT: <script>: SELECT * FROM ns_ua_blacklist WHERE key =
> 'Cisco/SPA525G-7.4.6' limit 1;
>  0(29141) ERROR: db_cassandra [dbcassa_base.cpp:649]: cql_get_columns():
> No column with name [KEY] found
>  0(29141) ERROR: db_cassandra [dbcassa_base.cpp:907]:
> db_cassa_raw_query(): Error getting column names.
>  0(29141) ERROR: avpops [avpops_db.c:333]: db_query_avp(): cannot do the
> query
>
> seemingly after this patch.
> ill trace it through and see if I can produce a fix but I thought id just
> report it at least..
>
> ALSO there are a few "LM_ERR" in there that need \n on the end..
> ( ill include this in my patch I guess )
>
>
>
> On 5 March 2014 21:39, Daniel-Constantin Mierla <miconda at gmail.com> wrote:
>
>> Module: sip-router
>> Branch: 4.1
>> Commit: 520276f856474521cb419c11fa185ce9d6a8a285
>> URL:
>> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=520276f856474521cb419c11fa185ce9d6a8a285
>>
>> Author: Carlos Ruiz Diaz <carlos.ruizdiaz at gmail.com>
>> Committer: Daniel-Constantin Mierla <miconda at gmail.com>
>> Date:   Tue Jan 14 10:04:53 2014 -0300
>>
>> db_cassandra: bug fixes for raw queries using sqlops
>>
>> (cherry picked from commit 2f9e8d2e122edc5b7950ee6ab0991851c9cd7bbd)
>>
>> ---
>>
>>  modules/db_cassandra/dbcassa_base.cpp |  210
>> +++++++++++++++++++++++++++------
>>  1 files changed, 175 insertions(+), 35 deletions(-)
>>
>> diff --git a/modules/db_cassandra/dbcassa_base.cpp
>> b/modules/db_cassandra/dbcassa_base.cpp
>> index e9d3a32..ffa9e3b 100644
>> --- a/modules/db_cassandra/dbcassa_base.cpp
>> +++ b/modules/db_cassandra/dbcassa_base.cpp
>> @@ -252,6 +252,7 @@ static int cassa_convert_result(db_key_t qcol,
>> std::vector<oac::ColumnOrSuperCol
>>         res_col = result[idx_rescol].column;
>>
>>         col_val.s = (char*)res_col.value.c_str();
>> +
>>         if(!col_val.s) {
>>                 LM_DBG("Column not found in result %.*s- NULL\n",
>> qcol->len, qcol->s);
>>                 sr_cell->nul  = 1;
>> @@ -585,7 +586,7 @@ ColumnVecPtr cassa_translate_query(const db1_con_t*
>> _h, const db_key_t* _k,
>>   * \param _r result set for storage
>>   * \return zero on success, negative value on failure
>>   */
>> -int cql_get_columns(oac::CqlResult& _cql_res, db1_res_t* _r)
>> +int cql_get_columns(oac::CqlResult& _cql_res, db1_res_t* _r,
>> dbcassa_table_p tbc)
>>  {
>>         std::vector<oac::CqlRow>  res_cql_rows = _cql_res.rows;
>>         int rows_no = res_cql_rows.size();
>> @@ -630,14 +631,86 @@ int cql_get_columns(oac::CqlResult& _cql_res,
>> db1_res_t* _r)
>>                 /* The pointer that is here returned is part of the
>> result structure. */
>>                 RES_NAMES(_r)[col]->s = (char*)
>> res_cql_rows[0].columns[col].name.c_str();
>>                 RES_NAMES(_r)[col]->len = strlen(RES_NAMES(_r)[col]->s);
>> -               RES_TYPES(_r)[col] = DB1_STR;
>>
>> +               /* search the column in table schema to get the type */
>> +               dbcassa_column_p colp = cassa_search_col(tbc, (db_key_t)
>> RES_NAMES(_r)[col]);
>> +               if(!colp) {
>> +                       LM_ERR("No column with name [%.*s] found\n",
>> RES_NAMES(_r)[col]->len, RES_NAMES(_r)[col]->s);
>> +                       RES_COL_N(_r) = col;
>> +                       db_free_columns(_r);
>> +                       return -4;
>> +               }
>> +
>> +               RES_TYPES(_r)[col] = colp->type;
>> +
>> +               LM_DBG("Column with name [%.*s] found: %d\n",
>> RES_NAMES(_r)[col]->len, RES_NAMES(_r)[col]->s, colp->type);
>>                 LM_DBG("RES_NAMES(%p)[%d]=[%.*s]\n", RES_NAMES(_r)[col],
>> col,
>>                         RES_NAMES(_r)[col]->len, RES_NAMES(_r)[col]->s);
>>         }
>>         return 0;
>>  }
>>
>> +static int cassa_convert_result_raw(db_val_t* sr_cell, str *col_val) {
>> +
>> +       if(!col_val->s) {
>> +               LM_DBG("Column not found in result - NULL\n");
>> +               sr_cell->nul  = 1;
>> +               return 0;
>> +       }
>> +       col_val->len = strlen(col_val->s);
>> +
>> +       sr_cell->nul  = 0;
>> +       sr_cell->free  = 0;
>> +
>> +       switch (sr_cell->type) {
>> +               case DB1_INT:
>> +                       if(str2int(col_val, (unsigned
>> int*)&sr_cell->val.int_val) < 0) {
>> +                               LM_ERR("Wrong value [%s] - len=%d,
>> expected integer\n", col_val->s, col_val->len);
>> +                               return -1;
>> +                       }
>> +                       break;
>> +               case DB1_BIGINT:
>> +                       if(sscanf(col_val->s, "%lld",
>> &sr_cell->val.ll_val) < 0) {
>> +                               LM_ERR("Wrong value [%s], expected
>> integer\n", col_val->s);
>> +                               return -1;
>> +                       }
>> +                       break;
>> +               case DB1_DOUBLE:
>> +                       if(sscanf(col_val->s, "%lf",
>> &sr_cell->val.double_val) < 0) {
>> +                               LM_ERR("Wrong value [%s], expected
>> integer\n", col_val->s);
>> +                               return -1;
>> +                       }
>> +                       break;
>> +               case DB1_STR:
>> +                       pkg_str_dup(&sr_cell->val.str_val, col_val);
>> +                       sr_cell->free  = 1;
>> +                       break;
>> +               case DB1_STRING:
>> +                       col_val->len++;
>> +                       pkg_str_dup(&sr_cell->val.str_val, col_val);
>> +                       sr_cell->val.str_val.len--;
>> +                       sr_cell->val.str_val.s[col_val->len-1]='\0';
>> +                       sr_cell->free  = 1;
>> +                       break;
>> +               case DB1_BLOB:
>> +                       pkg_str_dup(&sr_cell->val.blob_val, col_val);
>> +                       sr_cell->free  = 1;
>> +                       break;
>> +               case DB1_BITMAP:
>> +                       if(str2int(col_val, &sr_cell->val.bitmap_val) <
>> 0) {
>> +                               LM_ERR("Wrong value [%s], expected
>> integer\n", col_val->s);
>> +                               return -1;
>> +                       }
>> +                       break;
>> +               case DB1_DATETIME:
>> +                       if(sscanf(col_val->s, "%ld", (long
>> int*)&sr_cell->val.time_val) < 0) {
>> +                               LM_ERR("Wrong value [%s], expected
>> integer\n", col_val->s);
>> +                               return -1;
>> +                       }
>> +                       break;
>> +       }
>> +       return 0;
>> +}
>>
>>
>>  /**
>> @@ -649,35 +722,33 @@ int cql_get_columns(oac::CqlResult& _cql_res,
>> db1_res_t* _r)
>>   * \param _r result set for storage
>>   * \return zero on success, negative value on failure
>>   */
>> +
>>  int cql_convert_row(oac::CqlResult& _cql_res, db1_res_t* _r)
>>  {
>>         std::vector<oac::CqlRow>  res_cql_rows = _cql_res.rows;
>> -       int rows_no = res_cql_rows.size();
>> -       int cols_no = res_cql_rows[0].columns.size();
>> -       str col_val;
>> -
>> -       RES_ROW_N(_r) = rows_no;
>> -
>> -       if (db_allocate_rows(_r) < 0) {
>> -               LM_ERR("Could not allocate rows.\n");
>> -               return -1;
>> -       }
>> -
>> -       for(int ri=0; ri < rows_no; ri++) {
>> -               if (db_allocate_row(_r, &(RES_ROWS(_r)[ri])) != 0) {
>> -                       LM_ERR("Could not allocate row.\n");
>> -                       return -2;
>> -               }
>> -
>> -               /* complete the row with the columns */
>> +        int rows_no = res_cql_rows.size();
>> +        int cols_no = res_cql_rows[0].columns.size();
>> +        str col_val;
>> +        RES_ROW_N(_r) = rows_no;
>> +
>> +        if (db_allocate_rows(_r) < 0) {
>> +                LM_ERR("Could not allocate rows.\n");
>> +                return -1;
>> +        }
>> +
>> +        for(int ri=0; ri < rows_no; ri++) {
>> +                if (db_allocate_row(_r, &(RES_ROWS(_r)[ri])) != 0) {
>> +                        LM_ERR("Could not allocate row.\n");
>> +                        return -2;
>> +                }
>> +
>> +               // complete the row with the columns
>>                 for(int col = 0; col< cols_no; col++) {
>> -                       RES_ROWS(_r)[ri].values[col].type = DB1_STR;
>> -
>>                         col_val.s =
>> (char*)res_cql_rows[ri].columns[col].value.c_str();
>> -                       col_val.len = strlen(col_val.s);
>> -
>> pkg_str_dup(&RES_ROWS(_r)[ri].values[col].val.str_val, &col_val);
>> -                       RES_ROWS(_r)[ri].values[col].free  = 1;
>> -                       RES_ROWS(_r)[ri].values[col].nul  = 0;
>> +                        col_val.len = strlen(col_val.s);
>> +
>> +                       RES_ROWS(_r)[ri].values[col].type =
>> RES_TYPES(_r)[col];
>> +
>> cassa_convert_result_raw(&RES_ROWS(_r)[ri].values[col], &col_val);
>>
>>                         LM_DBG("Field index %d. %s = %s.\n", col,
>>
>> res_cql_rows[ri].columns[col].name.c_str(),
>> @@ -685,13 +756,62 @@ int cql_convert_row(oac::CqlResult& _cql_res,
>> db1_res_t* _r)
>>                 }
>>         }
>>         return 0;
>> -}
>> -
>> +}
>>
>>  /*
>>   *     The functions for the DB Operations: query, delete, update.
>>   * */
>>
>> +/*
>> + * Extracts table name from DML query being used
>> + *
>> + * */
>> +static int get_table_from_query(const str *cql, str *table) {
>> +
>> +       char *ptr = cql->s,
>> +               *begin = NULL;
>> +
>> +        if (cql->s[0] == 's' || cql->s[0] == 'S') {
>> +                ptr = strcasestr(cql->s, "from");
>> +                ptr += sizeof(char) * 4;
>> +        }
>> +        else if (cql->s[0] == 'u' || cql->s[0] == 'U') {
>> +                ptr = cql->s + sizeof("update") - 1;
>> +        }
>> +        else if (cql->s[0] == 'd' || cql->s[0] == 'D') {
>> +                ptr = strcasestr(cql->s, "from");
>> +                ptr += sizeof(char) * 4;
>> +        }
>> +        else if (cql->s[0] == 'i' || cql->s[0] == 'I') {
>> +                ptr = strcasestr(cql->s, "into");
>> +                ptr += sizeof(char) * 4;
>> +        }
>> +       else
>> +               goto error;
>> +
>> +        while (*ptr == ' ' && (ptr - cql->s) <= cql->len) {
>> +                ptr++;
>> +        }
>> +
>> +        begin = ptr;
>> +        ptr   = strchr(begin, ' ');
>> +
>> +        if (ptr == NULL)
>> +               ptr = cql->s + cql->len;
>> +
>> +       if (ptr - begin <= 0)
>> +               goto error;
>> +
>> +       table->s = begin;
>> +       table->len = ptr - begin;
>> +
>> +       return 0;
>> +
>> +error:
>> +       LM_ERR("Unable to determine operation in cql [%*s]\n", cql->len,
>> cql->s);
>> +       return -1;
>> +}
>> +
>>  /**
>>   * Execute a raw SQL query.
>>   * \param _h handle for the database
>> @@ -702,14 +822,29 @@ int cql_convert_row(oac::CqlResult& _cql_res,
>> db1_res_t* _r)
>>  int db_cassa_raw_query(const db1_con_t* _h, const str* _s, db1_res_t**
>> _r)
>>  {
>>         db1_res_t* db_res = 0;
>> +       str table_name;
>> +       dbcassa_table_p tbc;
>> +       std::vector<oac::CqlRow>  res_cql_rows;
>>
>> -       if (!_h || !CON_TABLE(_h) || !_r) {
>> +       if (!_h || !_r) {
>>                 LM_ERR("Invalid parameter value\n");
>>                 return -1;
>>         }
>> -       LM_DBG("query table=%s\n", _h->table->s);
>> +
>> +       if (get_table_from_query(_s, &table_name) < 0) {
>> +               LM_ERR("Error parsing table name in CQL string");
>> +               return -1;
>> +       }
>> +
>> +       LM_DBG("query table=%.*s\n", table_name.len, table_name.s);
>>         LM_DBG("CQL=%s\n", _s->s);
>>
>> +       tbc = dbcassa_db_get_table(&CON_CASSA(_h)->db_name, &table_name);
>> +        if(!tbc) {
>> +                LM_ERR("table %.*s does not exist!\n", table_name.len,
>> table_name.s);
>> +                return -1;
>> +        }
>> +
>>         std::string cql_query(_s->s);
>>
>>         oac::CqlResult cassa_cql_res;
>> @@ -728,9 +863,11 @@ int db_cassa_raw_query(const db1_con_t* _h, const
>> str* _s, db1_res_t** _r)
>>
>>         if (!cassa_cql_res.__isset.rows) {
>>                 LM_ERR("The resultype rows was not set, no point trying
>> to parse result.\n");
>> -               return -1;
>> +               goto error;
>>         }
>>
>> +       res_cql_rows = cassa_cql_res.rows;
>> +
>>         /* TODO Handle the other types */
>>         switch(cassa_cql_res.type) {
>>                 case 1:  LM_DBG("Result set is an ROW Type.\n");
>> @@ -741,8 +878,6 @@ int db_cassa_raw_query(const db1_con_t* _h, const
>> str* _s, db1_res_t** _r)
>>                         break;
>>         }
>>
>> -       std::vector<oac::CqlRow>  res_cql_rows = cassa_cql_res.rows;
>> -
>>         db_res = db_new_result();
>>         if (!db_res) {
>>                 LM_ERR("no memory left\n");
>> @@ -754,10 +889,10 @@ int db_cassa_raw_query(const db1_con_t* _h, const
>> str* _s, db1_res_t** _r)
>>                 RES_ROW_N(db_res) = 0;
>>                 RES_COL_N(db_res)= 0;
>>                 *_r = db_res;
>> -               return 0;
>> +               goto done;
>>         }
>>
>> -       if (cql_get_columns(cassa_cql_res, db_res) < 0) {
>> +       if (cql_get_columns(cassa_cql_res, db_res, tbc) < 0) {
>>                 LM_ERR("Error getting column names.");
>>                 goto error;
>>         }
>> @@ -768,12 +903,17 @@ int db_cassa_raw_query(const db1_con_t* _h, const
>> str* _s, db1_res_t** _r)
>>         }
>>
>>         *_r = db_res;
>> +done:
>> +       dbcassa_lock_release(tbc);
>> +
>>         LM_DBG("Exited with success\n");
>>         return 0;
>>
>>  error:
>>         if(db_res)
>>                 db_free_result(db_res);
>> +
>> +       dbcassa_lock_release(tbc);
>>         return -1;
>>  }
>>
>>
>>
>> _______________________________________________
>> sr-dev mailing list
>> sr-dev at lists.sip-router.org
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>
>
>
> --
> Sincerely
>
> Jay
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
>


-- 
Carlos
http://caruizdiaz.com
http://ngvoice.com
+595981146623
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20140312/ef335e9f/attachment-0001.html>


More information about the sr-dev mailing list