From 98a6abe75bc3afbc5155db3ff295f5906e884627 Mon Sep 17 00:00:00 2001 From: Matt Tyson Date: Sun, 9 Oct 2016 13:59:30 +1100 Subject: [PATCH 1/2] DBD::Pg Array and string optimization patch Replace string operations with an efficient string concat library Replace all linked lists with arrays Use arrays to remove quadratic complexity algorithm. Fix CPAN RT #105492 --- MANIFEST | 2 + Makefile.PL | 2 +- dbdimp.c | 536 +++++++++++++++++++++++++++------------------------- dbdimp.h | 45 +++-- sstring.c | 119 ++++++++++++ sstring.h | 19 ++ 6 files changed, 451 insertions(+), 272 deletions(-) create mode 100644 sstring.c create mode 100644 sstring.h diff --git a/MANIFEST b/MANIFEST index 265f790c..d5a40fba 100644 --- a/MANIFEST +++ b/MANIFEST @@ -24,6 +24,8 @@ types.c types.h quote.c quote.h +sstring.h +sstring.c .perlcriticrc t/dbdpg_test_setup.pl diff --git a/Makefile.PL b/Makefile.PL index 31eed053..ab030975 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -214,7 +214,7 @@ my %opts = NAME => 'DBD::Pg', VERSION_FROM => 'Pg.pm', INC => "-I$POSTGRES_INCLUDE -I$dbi_arch_dir", - OBJECT => "Pg\$(OBJ_EXT) dbdimp\$(OBJ_EXT) quote\$(OBJ_EXT) types\$(OBJ_EXT)", + OBJECT => "Pg\$(OBJ_EXT) dbdimp\$(OBJ_EXT) quote\$(OBJ_EXT) types\$(OBJ_EXT) sstring\$(OBJ_EXT)", LIBS => ["-L$POSTGRES_LIB -lpq -lm"], AUTHOR => 'Greg Sabino Mullane', ABSTRACT => 'PostgreSQL database driver for the DBI module', diff --git a/dbdimp.c b/dbdimp.c index 41badf87..a7fb0621 100644 --- a/dbdimp.c +++ b/dbdimp.c @@ -12,6 +12,7 @@ #include "Pg.h" +#include "sstring.h" #if defined (_WIN32) && !defined (atoll) #define atoll(X) _atoi64(X) @@ -95,6 +96,104 @@ static int pg_db_start_txn (pTHX_ SV *dbh, imp_dbh_t *imp_dbh); static int handle_old_async(pTHX_ SV * handle, imp_dbh_t * imp_dbh, const int asyncflag); static void pg_db_detect_client_encoding_utf8(pTHX_ imp_dbh_t *imp_dbh); + +/* NOTE: data is *copied*. This function takes ownership of all pointers */ +static void ph_array_append(imp_sth_t *imp_sth, ph_t *data) +{ + if (imp_sth->ph_ary.length == imp_sth->ph_ary.elements) { + /* The array is full, realloc the array to make it bigger */ + int new_length = imp_sth->ph_ary.length * 2; + + Renew(imp_sth->ph_ary.array, new_length, ph_t); + + imp_sth->ph_ary.length = new_length; + } + + imp_sth->ph_ary.array[imp_sth->ph_ary.elements++] = *data; +} + +/* NOTE: data is *copied*. This function takes ownership of all pointers */ +static void seg_array_append(imp_sth_t *imp_sth, seg_t *data) +{ + if (imp_sth->seg_ary.length == imp_sth->seg_ary.elements) { + /* The array is full, realloc the array to make it bigger */ + int new_length = imp_sth->seg_ary.length * 2; + + Renew(imp_sth->seg_ary.array, new_length, seg_t); + + imp_sth->seg_ary.length = new_length; + } + + imp_sth->seg_ary.array[imp_sth->seg_ary.elements++] = *data; +} + +static void ph_array_destroy(imp_sth_t *imp_sth) +{ + int i; + for(i = 0; i < imp_sth->ph_ary.elements; i++) { + ph_t *elem = &(imp_sth->ph_ary.array[i]); + + Safefree(elem->fooname); + Safefree(elem->value); + Safefree(elem->quoted); + elem->bind_type = NULL; + } + + Safefree(imp_sth->ph_ary.array); + imp_sth->ph_ary.length = 0; + imp_sth->ph_ary.elements = 0; + imp_sth->ph_ary.array = NULL; +} + +static void seg_array_destroy(imp_sth_t *imp_sth) +{ + int i; + for(i = 0; i < imp_sth->seg_ary.elements; i++) { + seg_t *elem = &(imp_sth->seg_ary.array[i]); + + Safefree(elem->segment); + } + + Safefree(imp_sth->seg_ary.array); + imp_sth->seg_ary.length = 0; + imp_sth->seg_ary.elements = 0; + imp_sth->seg_ary.array = NULL; +} + +static void ph_array_init(imp_sth_t *imp_sth) +{ + imp_sth->ph_ary.length = 15; + imp_sth->ph_ary.elements = 0; + Newz(0, imp_sth->ph_ary.array, imp_sth->ph_ary.length, ph_t); +} + +static void seg_array_init(imp_sth_t *imp_sth) +{ + imp_sth->seg_ary.length = 15; + imp_sth->seg_ary.elements = 0; + Newz(0, imp_sth->seg_ary.array, imp_sth->seg_ary.length, seg_t); +} + +static ph_t* ph_array_element(imp_sth_t *imp_sth, int idx) +{ + return &(imp_sth->ph_ary.array[idx]); +} + +static seg_t* seg_array_element(imp_sth_t *imp_sth, int idx) +{ + return &(imp_sth->seg_ary.array[idx]); +} + +static int ph_array_count(imp_sth_t *imp_sth) +{ + return imp_sth->ph_ary.elements; +} + +static int seg_array_count(imp_sth_t *imp_sth) +{ + return imp_sth->seg_ary.elements; +} + /* ================================================================== */ void dbd_init (dbistate_t *dbistate) { @@ -1032,12 +1131,11 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv) if (strEQ("pg_bound", key)) { HV *pvhv = newHV(); - ph_t *currph; int i; - for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) { - SV *key, *val; - key = pg_st_placeholder_key(imp_sth, currph, i); - val = newSViv(NULL == currph->bind_type ? 0 : 1); + for (i = 0; i < ph_array_count(imp_sth); i++) { + ph_t *currph = ph_array_element(imp_sth, i); + SV *key = pg_st_placeholder_key(imp_sth, currph, i); + SV *val = newSViv(NULL == currph->bind_type ? 0 : 1); if (! hv_store_ent(pvhv, key, val, 0)) { SvREFCNT_dec(val); } @@ -1057,11 +1155,11 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv) if (strEQ("ParamTypes", key)) { HV *pvhv = newHV(); - ph_t *currph; int i; - for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) { - SV *key, *val; - key = pg_st_placeholder_key(imp_sth, currph, i); + for (i = 0; i < ph_array_count(imp_sth); i++) { + ph_t *currph = ph_array_element(imp_sth, i); + SV *key = pg_st_placeholder_key(imp_sth, currph, i); + SV *val = newSViv(NULL == currph->bind_type ? 0 : 1); if (NULL == currph->bind_type) { val = newSV(0); if (! hv_store_ent(pvhv, key, val, 0)) { @@ -1091,11 +1189,11 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv) if (strEQ("ParamValues", key)) { HV *pvhv = newHV(); - ph_t *currph; int i; - for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) { - SV *key, *val; - key = pg_st_placeholder_key(imp_sth, currph, i); + for (i = 0; i < ph_array_count(imp_sth); i++) { + ph_t *currph = ph_array_element(imp_sth, i); + SV *key = pg_st_placeholder_key(imp_sth, currph, i); + SV *val; if (NULL == currph->value) { val = newSV(0); if (!hv_store_ent(pvhv, key, val, 0)) { @@ -1114,17 +1212,17 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv) } else if (strEQ("pg_segments", key)) { AV *arr = newAV(); - seg_t *currseg; int i; - for (i=0,currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg,i++) { + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); av_push(arr, newSVpv(currseg->segment ? currseg->segment : "NULL",0)); } retsv = newRV_noinc((SV*)arr); } else if (strEQ("pg_numbound", key)) { - ph_t *currph; - int i = 0; - for (currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { + int i; + for (i = 0; i < ph_array_count(imp_sth); i++) { + ph_t *currph = ph_array_element(imp_sth, i); i += NULL == currph->bind_type ? 0 : 1; } retsv = newSViv(i); @@ -1589,8 +1687,6 @@ int dbd_st_prepare_sv (SV * sth, imp_sth_t * imp_sth, SV * statement_sv, SV * at imp_sth->firstword = NULL; imp_sth->result = NULL; imp_sth->type_info = NULL; - imp_sth->seg = NULL; - imp_sth->ph = NULL; imp_sth->PQvals = NULL; imp_sth->PQlens = NULL; imp_sth->PQfmts = NULL; @@ -1606,6 +1702,11 @@ int dbd_st_prepare_sv (SV * sth, imp_sth_t * imp_sth, SV * statement_sv, SV * at imp_sth->all_bound = DBDPG_FALSE; /* Have all placeholders been bound? */ imp_sth->number_iterations = 0; + /* Create the array of placeholders */ + ph_array_init(imp_sth); + /* Create the array of segments */ + seg_array_init(imp_sth); + /* We inherit some preferences from the database handle */ imp_sth->server_prepare = imp_dbh->server_prepare; imp_sth->switch_prepared = imp_dbh->switch_prepared; @@ -1752,9 +1853,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char int xint; - seg_t *newseg, *currseg = NULL; /* Segment structures to help build linked lists */ - - ph_t *newph, *thisph, *currph = NULL; /* Placeholder structures to help build ll */ + seg_t newseg, *currseg = NULL; /* Segment structures to help build the placeholder array */ if (TSTART_slow) TRC(DBILOGFP, "%sBegin pg_st_split_statement\n", THEADER_slow); if (TRACE6_slow) TRC(DBILOGFP, "%spg_st_split_statement: (%s)\n", THEADER_slow, statement); @@ -1764,6 +1863,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char but simply put everything verbatim into a single segment and return. */ if (imp_sth->direct || '\0' == *statement) { + seg_t seg; if (TRACE4_slow) { TRC(DBILOGFP, "%snot splitting due to %s\n", THEADER_slow, imp_sth->direct ? "pg_direct" : "empty string"); @@ -1772,20 +1872,18 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char imp_sth->numphs = 0; imp_sth->totalsize = strlen(statement); - New(0, imp_sth->seg, 1, seg_t); /* freed in dbd_st_destroy */ - imp_sth->seg->placeholder = 0; - imp_sth->seg->nextseg = NULL; - imp_sth->seg->ph = NULL; + seg.placeholder = 0; if (imp_sth->totalsize > 0) { - New(0, imp_sth->seg->segment, imp_sth->totalsize+1, char); /* freed in dbd_st_destroy */ - Copy(statement, imp_sth->seg->segment, imp_sth->totalsize+1, char); + New(0, seg.segment, imp_sth->totalsize+1, char); /* freed in dbd_st_destroy */ + Copy(statement, seg.segment, imp_sth->totalsize+1, char); } else { - imp_sth->seg->segment = NULL; + seg.segment = NULL; } + seg_array_append(imp_sth, &seg); if (TRACE6_slow) TRC(DBILOGFP, "%sdirect split = (%s) length=(%d)\n", - THEADER_slow, imp_sth->seg->segment, (int)imp_sth->totalsize); + THEADER_slow, seg.segment, (int)imp_sth->totalsize); if (TEND_slow) TRC(DBILOGFP, "%sEnd pg_st_split_statement (direct)\n", THEADER_slow); return; } @@ -2089,21 +2187,20 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char continue; /* If we got here, we have a segment that needs to be saved */ - New(0, newseg, 1, seg_t); /* freed in dbd_st_destroy */ - newseg->nextseg = NULL; - newseg->placeholder = 0; - newseg->ph = NULL; + newseg.placeholder = 0; if (PLACEHOLDER_QUESTIONMARK == placeholder_type) { - newseg->placeholder = ++imp_sth->numphs; + newseg.placeholder = ++imp_sth->numphs; } else if (PLACEHOLDER_DOLLAR == placeholder_type) { - newseg->placeholder = atoi(statement-(currpos-sectionstop-1)); + newseg.placeholder = atoi(statement-(currpos-sectionstop-1)); } else if (PLACEHOLDER_COLON == placeholder_type) { + int i = 0; sectionsize = currpos-sectionstop; /* Have we seen this placeholder yet? */ - for (xint=1,thisph=imp_sth->ph; NULL != thisph; thisph=thisph->nextph,xint++) { + for (xint=1, i=0; i < ph_array_count(imp_sth); i++, xint++) { + ph_t *thisph = ph_array_element(imp_sth, i); /* Because we need to make sure :foobar does not match as a previous hit when seeing :foobar2, we always use the greater of the two lengths: @@ -2111,59 +2208,47 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char */ if (0==strncmp(thisph->fooname, statement-sectionsize, strlen(thisph->fooname) > sectionsize ? strlen(thisph->fooname) : sectionsize)) { - newseg->placeholder = xint; - newseg->ph = thisph; + newseg.placeholder = xint; break; } } - if (0==newseg->placeholder) { + if (0==newseg.placeholder) { + ph_t newph; + imp_sth->numphs++; - newseg->placeholder = imp_sth->numphs; - New(0, newph, 1, ph_t); /* freed in dbd_st_destroy */ - newseg->ph = newph; - newph->nextph = NULL; - newph->bind_type = NULL; - newph->value = NULL; - newph->quoted = NULL; - newph->referenced = DBDPG_FALSE; - newph->defaultval = DBDPG_TRUE; - newph->isdefault = DBDPG_FALSE; - newph->iscurrent = DBDPG_FALSE; - newph->isinout = DBDPG_FALSE; - New(0, newph->fooname, sectionsize+1, char); /* freed in dbd_st_destroy */ - Copy(statement-sectionsize, newph->fooname, sectionsize, char); - newph->fooname[sectionsize] = '\0'; - if (NULL==currph) { - imp_sth->ph = newph; - } - else { - currph->nextph = newph; - } - currph = newph; + newseg.placeholder = imp_sth->numphs; + + newph.bind_type = NULL; + newph.value = NULL; + newph.quoted = NULL; + newph.referenced = DBDPG_FALSE; + newph.defaultval = DBDPG_TRUE; + newph.isdefault = DBDPG_FALSE; + newph.iscurrent = DBDPG_FALSE; + newph.isinout = DBDPG_FALSE; + New(0, newph.fooname, sectionsize+1, char); /* freed in ph_array_destroy */ + Copy(statement-sectionsize, newph.fooname, sectionsize, char); + newph.fooname[sectionsize] = '\0'; + + ph_array_append(imp_sth, &newph); } } /* end if placeholder_type */ sectionsize = sectionstop-sectionstart; /* 4-0 for "ABCD" */ if (sectionsize>0) { - New(0, newseg->segment, sectionsize+1, char); /* freed in dbd_st_destroy */ - Copy(statement-(currpos-sectionstart), newseg->segment, sectionsize, char); - newseg->segment[sectionsize] = '\0'; + New(0, newseg.segment, sectionsize+1, char); /* freed in seg_array_destroy */ + Copy(statement-(currpos-sectionstart), newseg.segment, sectionsize, char); + newseg.segment[sectionsize] = '\0'; imp_sth->totalsize += sectionsize; } else { - newseg->segment = NULL; + newseg.segment = NULL; } if (TRACE6_slow) - TRC(DBILOGFP, "%sCreated segment (%s)\n", THEADER_slow, newseg->segment); + TRC(DBILOGFP, "%sCreated segment (%s)\n", THEADER_slow, newseg.segment); - /* Tie it in to the previous one */ - if (NULL==currseg) { - imp_sth->seg = newseg; - } - else { - currseg->nextseg = newseg; - } - currseg = newseg; + /* Add to the list of segments */ + seg_array_append(imp_sth, &newseg); sectionstart = currpos; imp_sth->numsegs++; @@ -2187,14 +2272,18 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char numbers must be sequential. We change numphs if repeats found */ topdollar=0; - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + int i; + for (i = 0; i < seg_array_count(imp_sth); i++) { + currseg = seg_array_element(imp_sth, i); if (currseg->placeholder > topdollar) topdollar = currseg->placeholder; } /* Make sure every placeholder from 1 to topdollar is used at least once */ for (xint=1; xint <= topdollar; xint++) { - for (found=0, currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { - if (currseg->placeholder==xint) { + found = 0; + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *foundseg = seg_array_element(imp_sth, i); + if (foundseg->placeholder==xint) { found = DBDPG_TRUE; break; } @@ -2208,46 +2297,41 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char /* Create sequential placeholders */ if (PLACEHOLDER_COLON != imp_sth->placeholder_type) { + ph_t newph; + + newph.bind_type = NULL; + newph.value = NULL; + newph.quoted = NULL; + newph.fooname = NULL; + newph.inout = NULL; + newph.referenced = DBDPG_FALSE; + newph.defaultval = DBDPG_TRUE; + newph.isdefault = DBDPG_FALSE; + newph.iscurrent = DBDPG_FALSE; + newph.isinout = DBDPG_FALSE; + newph.valuelen = 0; + newph.quotedlen = 0; + for (xint=1; xint <= imp_sth->numphs; xint++) { - New(0, newph, 1, ph_t); /* freed in dbd_st_destroy */ - newph->nextph = NULL; - newph->bind_type = NULL; - newph->value = NULL; - newph->quoted = NULL; - newph->fooname = NULL; - newph->referenced = DBDPG_FALSE; - newph->defaultval = DBDPG_TRUE; - newph->isdefault = DBDPG_FALSE; - newph->iscurrent = DBDPG_FALSE; - newph->isinout = DBDPG_FALSE; - /* Let the correct segment(s) point to it */ - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { - if (currseg->placeholder==xint) { - currseg->ph = newph; - } - } - if (NULL==currph) { - imp_sth->ph = newph; - } - else { - currph->nextph = newph; - } - currph = newph; + ph_array_append(imp_sth, &newph); } } if (TRACE7_slow) { + int i; TRC(DBILOGFP, "%sPlaceholder type: %d numsegs: %d numphs: %d\n", THEADER_slow, imp_sth->placeholder_type, imp_sth->numsegs, imp_sth->numphs); TRC(DBILOGFP, "%sPlaceholder numbers and segments:\n", THEADER_slow); - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *foundseg = seg_array_element(imp_sth, i); TRC(DBILOGFP, "%sPH: (%d) SEG: (%s)\n", - THEADER_slow, currseg->placeholder, currseg->segment); + THEADER_slow, foundseg->placeholder, foundseg->segment); } if (imp_sth->numphs) { TRC(DBILOGFP, "%sPlaceholder number, fooname, id:\n", THEADER_slow); - for (xlen=1,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,xlen++) { + for (xlen=1; xlen-1 < ph_array_count(imp_sth); xlen++) { + ph_t *currph = ph_array_element(imp_sth, xlen-1); TRC(DBILOGFP, "%s#%d FOONAME: (%s)\n", THEADER_slow, (int)xlen, currph->fooname); } @@ -2267,16 +2351,13 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char static int pg_st_prepare_statement (pTHX_ SV * sth, imp_sth_t * imp_sth) { D_imp_dbh_from_sth; - char * statement; - unsigned int placeholder_digits; + string_t * statement; int x; + int i; STRLEN execsize; PGresult * result; int status = -1; - seg_t * currseg; bool oldprepare = DBDPG_TRUE; - ph_t * currph; - long power_of_ten; if (TSTART_slow) TRC(DBILOGFP, "%sBegin pg_st_prepare_statement\n", THEADER_slow); @@ -2302,70 +2383,46 @@ static int pg_st_prepare_statement (pTHX_ SV * sth, imp_sth_t * imp_sth) if (oldprepare) execsize += strlen("PREPARE AS ") + strlen(imp_sth->prepare_name); /* Two spaces! */ - if (imp_sth->numphs!=0) { - if (oldprepare) { - execsize += strlen("()"); - execsize += imp_sth->numphs-1; /* for the commas */ - } - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { - if (0==currseg->placeholder) - continue; - /* The parameter itself: dollar sign plus digit(s) */ - power_of_ten = 10; - for (placeholder_digits=1; placeholder_digits<7; placeholder_digits++, power_of_ten *= 10) { - if (currseg->placeholder < power_of_ten) - break; - } - if (placeholder_digits >= 7) - croak("Too many placeholders!"); - execsize += placeholder_digits+1; - if (oldprepare) { - /* The parameter type, only once per number please */ - if (!currseg->ph->referenced) - execsize += strlen(currseg->ph->bind_type->type_name); - currseg->ph->referenced = DBDPG_TRUE; - } - } - } - - New(0, statement, execsize+1, char); /* freed below */ + statement = string_create(1024); if (oldprepare) { - sprintf(statement, "PREPARE %s", imp_sth->prepare_name); + string_append_chr(statement, "PREPARE "); + string_append_chr(statement, imp_sth->prepare_name); if (imp_sth->numphs!=0) { - strcat(statement, "("); - for (x=0, currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { - if (currseg->placeholder && currseg->ph->referenced) { + int j = 0; + x = 0; + string_append_byte(statement, '('); + for (j=0; j < seg_array_count(imp_sth); j++) { + seg_t *currseg = seg_array_element(imp_sth, j); + ph_t *currph = ph_array_element(imp_sth, currseg->placeholder - 1); + + if (currseg->placeholder && currph->referenced) { if (x!=0) - strcat(statement, ","); - strcat(statement, currseg->ph->bind_type->type_name); + string_append_byte(statement, ','); + string_append_chr(statement, currph->bind_type->type_name); x=1; - currseg->ph->referenced = DBDPG_FALSE; + currph->referenced = DBDPG_FALSE; } } - strcat(statement, ")"); + string_append_byte(statement, ')'); } - strcat(statement, " AS "); - } - else { - statement[0] = '\0'; + string_append_chr(statement, " AS "); } + /* Construct the statement, with proper placeholders */ - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); if (currseg->segment != NULL) - strcat(statement, currseg->segment); - if (currseg->placeholder) { - sprintf(strchr(statement, '\0'), "$%d", currseg->placeholder); - } + string_append_chr(statement, currseg->segment); + if (currseg->placeholder) + string_append_dint(statement, currseg->placeholder); } - statement[execsize] = '\0'; - if (TRACE6_slow) - TRC(DBILOGFP, "%sPrepared statement (%s)\n", THEADER_slow, statement); + TRC(DBILOGFP, "%sPrepared statement (%s)\n", THEADER_slow, string_get(statement)); if (oldprepare) { - status = _result(aTHX_ imp_dbh, statement); + status = _result(aTHX_ imp_dbh, string_get(statement)); } else { int params = 0; @@ -2374,24 +2431,25 @@ static int pg_st_prepare_statement (pTHX_ SV * sth, imp_sth_t * imp_sth) if (NULL == imp_sth->PQoids) { Newz(0, imp_sth->PQoids, (unsigned int)imp_sth->numphs, Oid); } - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { - imp_sth->PQoids[x++] = (currph->defaultval) ? 0 : (Oid)currph->bind_type->type_id; + for (x=0; x < ph_array_count(imp_sth); x++) { + ph_t *currph = ph_array_element(imp_sth, x) ; + imp_sth->PQoids[x] = (currph->defaultval) ? 0 : (Oid)currph->bind_type->type_id; } } if (TSQL) - TRC(DBILOGFP, "PREPARE %s AS %s;\n\n", imp_sth->prepare_name, statement); + TRC(DBILOGFP, "PREPARE %s AS %s;\n\n", imp_sth->prepare_name, string_get(statement)); TRACE_PQPREPARE; - result = PQprepare(imp_dbh->conn, imp_sth->prepare_name, statement, params, imp_sth->PQoids); + result = PQprepare(imp_dbh->conn, imp_sth->prepare_name, string_get(statement), params, imp_sth->PQoids); status = _sqlstate(aTHX_ imp_dbh, result); if (result) { TRACE_PQCLEAR; PQclear(result); } if (TRACE6_slow) - TRC(DBILOGFP, "%sUsing PQprepare: %s\n", THEADER_slow, statement); + TRC(DBILOGFP, "%sUsing PQprepare: %s\n", THEADER_slow, string_get(statement)); } - Safefree(statement); + string_destroy(statement); if (PGRES_COMMAND_OK != status) { TRACE_PQERRORMESSAGE; Safefree(imp_sth->prepare_name); @@ -2456,7 +2514,8 @@ int dbd_bind_ph (SV * sth, imp_sth_t * imp_sth, SV * ph_name, SV * newvalue, IV /* Find the placeholder in question */ if (PLACEHOLDER_COLON == imp_sth->placeholder_type) { - for (x=0,currph=imp_sth->ph; NULL != currph; currph = currph->nextph) { + for (x=0; x < ph_array_count(imp_sth); x++) { + currph = ph_array_element(imp_sth, x); if (0==strcmp(currph->fooname, name)) { x=1; break; @@ -2471,10 +2530,7 @@ int dbd_bind_ph (SV * sth, imp_sth_t * imp_sth, SV * ph_name, SV * newvalue, IV phnum = atoi(name); if (phnum < 1 || phnum > imp_sth->numphs) croak("Cannot bind unknown placeholder %d (%s)", phnum, neatsvpv(ph_name,0)); - for (x=1,currph=imp_sth->ph; NULL != currph; currph = currph->nextph,x++) { - if (x==phnum) - break; - } + currph = ph_array_element(imp_sth, phnum - 1); } /* Check the value */ @@ -3165,16 +3221,12 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) { dTHX; D_imp_dbh_from_sth; - ph_t * currph; int status = -1; STRLEN execsize, x; - unsigned int placeholder_digits; - seg_t * currseg; - char * statement = NULL; + string_t * statement = NULL; int num_fields; int ret = -2; PQExecType pqtype = PQTYPE_UNKNOWN; - long power_of_ten; if (TSTART_slow) TRC(DBILOGFP, "%sBegin dbd_st_execute\n", THEADER_slow); @@ -3189,7 +3241,9 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) /* Ensure that all the placeholders have been bound */ if (!imp_sth->all_bound && imp_sth->numphs!=0) { - for (currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { + int i; + for (i = 0; i < ph_array_count(imp_sth); i++){ + ph_t *currph = ph_array_element(imp_sth, i); if (NULL == currph->bind_type) { pg_error(aTHX_ sth, PGRES_FATAL_ERROR, "execute called with an unbound placeholder"); if (TEND_slow) TRC(DBILOGFP, "%sEnd dbd_st_execute (error: unbound placeholder)\n", THEADER_slow); @@ -3306,7 +3360,9 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) /* If using plain old PQexec, we need to quote each value ourselves */ if (PQTYPE_EXEC == pqtype) { - for (currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { + int i; + for (i = 0; i < ph_array_count(imp_sth); i++) { + ph_t *currph = ph_array_element(imp_sth, i); if (currph->isdefault) { Renew(currph->quoted, 8, char); /* freed in dbd_st_destroy */ strncpy(currph->quoted, "DEFAULT", 8); @@ -3335,9 +3391,10 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) } } /* Set the size of each actual in-place placeholder */ - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); if (currseg->placeholder!=0) - execsize += currseg->ph->quotedlen; + execsize += ph_array_element(imp_sth, currseg->placeholder - 1)->quotedlen; } } else { /* We are using a server that can handle PQexecParams/PQexecPrepared */ @@ -3345,8 +3402,9 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) if (NULL == imp_sth->PQvals) { Newz(0, imp_sth->PQvals, (unsigned int)imp_sth->numphs, const char *); /* freed in dbd_st_destroy */ } - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { - imp_sth->PQvals[x++] = currph->value; + for (x=0; x < ph_array_count(imp_sth); x++){ + ph_t *currph = ph_array_element(imp_sth, x); + imp_sth->PQvals[x] = currph->value; } /* Binary or regular? */ @@ -3356,7 +3414,8 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) Newz(0, imp_sth->PQlens, (unsigned int)imp_sth->numphs, int); /* freed in dbd_st_destroy */ Newz(0, imp_sth->PQfmts, (unsigned int)imp_sth->numphs, int); /* freed below */ } - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,x++) { + for (x=0; x < ph_array_count(imp_sth); x++){ + ph_t *currph = ph_array_element(imp_sth,x); if (PG_BYTEA==currph->bind_type->type_id) { imp_sth->PQlens[x] = (int)currph->valuelen; imp_sth->PQfmts[x] = 1; @@ -3371,82 +3430,70 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) /* Run one of PQexec, PQexecParams, or PQexecPrepared */ if (PQTYPE_EXEC == pqtype) { /* PQexec */ + int i; if (TRACE5_slow) TRC(DBILOGFP, "%sPQexec\n", THEADER_slow); - /* Go through and quote each value, then turn into a giant statement */ - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); if (currseg->placeholder!=0) - execsize += currseg->ph->quotedlen; + execsize += ph_array_element(imp_sth, currseg->placeholder - 1)->quotedlen; } - New(0, statement, execsize+1, char); /* freed below */ - statement[0] = '\0'; - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + statement = string_create(execsize + 1); + + for (i = 0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); if (currseg->segment != NULL) - strcat(statement, currseg->segment); + string_append_chr(statement, currseg->segment); if (currseg->placeholder!=0) - strcat(statement, currseg->ph->quoted); + string_append_chr(statement, ph_array_element(imp_sth, currseg->placeholder - 1)->quoted); } - statement[execsize] = '\0'; if (TRACE5_slow) TRC(DBILOGFP, "%sRunning %s with (%s)\n", - THEADER_slow, imp_sth->async_flag & PG_ASYNC ? "PQsendQuery" : "PQexec", statement); + THEADER_slow, imp_sth->async_flag & PG_ASYNC ? "PQsendQuery" : "PQexec", string_get(statement)); if (TSQL) - TRC(DBILOGFP, "%s;\n\n", statement); + TRC(DBILOGFP, "%s;\n\n", string_get(statement)); if (imp_sth->async_flag & PG_ASYNC) { TRACE_PQSENDQUERY; - ret = PQsendQuery(imp_dbh->conn, statement); + ret = PQsendQuery(imp_dbh->conn, string_get(statement)); } else { TRACE_PQEXEC; - imp_sth->result = PQexec(imp_dbh->conn, statement); + imp_sth->result = PQexec(imp_dbh->conn, string_get(statement)); } - Safefree(statement); + string_destroy(statement); } else if (PQTYPE_PARAMS == pqtype) { /* PQexecParams */ + int i; if (TRACE5_slow) TRC(DBILOGFP, "%sPQexecParams\n", THEADER_slow); - /* Figure out how big the statement plus placeholders will be */ - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { - if (0==currseg->placeholder) - continue; - /* The parameter itself: dollar sign plus digit(s) */ - power_of_ten = 10; - for (placeholder_digits=1; placeholder_digits<7; placeholder_digits++, power_of_ten *= 10) { - if (currseg->placeholder < power_of_ten) - break; - } - if (placeholder_digits >= 7) - croak("Too many placeholders!"); - execsize += placeholder_digits+1; - } - /* Create the statement */ - New(0, statement, execsize+1, char); /* freed below */ - statement[0] = '\0'; - for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) { + statement = string_create(1024); + + for (i=0; i < seg_array_count(imp_sth); i++) { + seg_t *currseg = seg_array_element(imp_sth, i); if (currseg->segment != NULL) - strcat(statement, currseg->segment); + string_append_chr(statement, currseg->segment); if (currseg->placeholder!=0) - sprintf(strchr(statement, '\0'), "$%d", currseg->placeholder); + string_append_dint(statement, currseg->placeholder); } - statement[execsize] = '\0'; /* Populate PQoids */ if (NULL == imp_sth->PQoids) { Newz(0, imp_sth->PQoids, (unsigned int)imp_sth->numphs, Oid); } - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph) { + for (x=0; x < ph_array_count(imp_sth); x++) { + ph_t *currph = ph_array_element(imp_sth, x); imp_sth->PQoids[x++] = (currph->defaultval) ? 0 : (Oid)currph->bind_type->type_id; } if (TRACE7_slow) { - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,x++) { + for (x=0; x < ph_array_count(imp_sth); x++) { TRC(DBILOGFP, "%sPQexecParams item #%d\n", THEADER_slow, (int)x); TRC(DBILOGFP, "%s-> Type: (%d)\n", THEADER_slow, imp_sth->PQoids[x]); TRC(DBILOGFP, "%s-> Value: (%s)\n", THEADER_slow, imp_sth->PQvals[x]); @@ -3456,27 +3503,26 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) } if (TSQL) { - TRC(DBILOGFP, "EXECUTE %s (\n", statement); - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,x++) { + TRC(DBILOGFP, "EXECUTE %s (\n", string_get(statement)); + for (x=0; x < ph_array_count(imp_sth); x++) { TRC(DBILOGFP, "$%d: %s\n", (int)x+1, imp_sth->PQvals[x]); } TRC(DBILOGFP, ");\n\n"); } - if (TRACE5_slow) TRC(DBILOGFP, "%sRunning PQexecParams with (%s)\n", THEADER_slow, statement); + if (TRACE5_slow) TRC(DBILOGFP, "%sRunning PQexecParams with (%s)\n", THEADER_slow, string_get(statement)); if (imp_sth->async_flag & PG_ASYNC) { TRACE_PQSENDQUERYPARAMS; ret = PQsendQueryParams - (imp_dbh->conn, statement, imp_sth->numphs, imp_sth->PQoids, imp_sth->PQvals, imp_sth->PQlens, imp_sth->PQfmts, 0); + (imp_dbh->conn, string_get(statement), imp_sth->numphs, imp_sth->PQoids, imp_sth->PQvals, imp_sth->PQlens, imp_sth->PQfmts, 0); } else { TRACE_PQEXECPARAMS; imp_sth->result = PQexecParams - (imp_dbh->conn, statement, imp_sth->numphs, imp_sth->PQoids, imp_sth->PQvals, imp_sth->PQlens, imp_sth->PQfmts, 0); + (imp_dbh->conn, string_get(statement), imp_sth->numphs, imp_sth->PQoids, imp_sth->PQvals, imp_sth->PQlens, imp_sth->PQfmts, 0); } - Safefree(statement); - + string_destroy(statement); } else if (PQTYPE_PREPARED == pqtype) { /* PQexecPrepared */ @@ -3498,7 +3544,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) } if (TRACE7_slow) { - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,x++) { + for (x=0; x < ph_array_count(imp_sth); x++) { TRC(DBILOGFP, "%sPQexecPrepared item #%d\n", THEADER_slow, (int)x); TRC(DBILOGFP, "%s-> Value: (%s)\n", THEADER_slow, (imp_sth->PQfmts && imp_sth->PQfmts[x]==1) ? "(binary, not shown)" @@ -3512,7 +3558,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth) THEADER_slow, imp_sth->prepare_name); if (TSQL) { TRC(DBILOGFP, "EXECUTE %s (\n", imp_sth->prepare_name); - for (x=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,x++) { + for (x=0; x < ph_array_count(imp_sth); x++) { TRC(DBILOGFP, "$%d: %s\n", (int)x+1, imp_sth->PQvals[x]); } TRC(DBILOGFP, ");\n\n"); @@ -3778,8 +3824,8 @@ AV * dbd_st_fetch (SV * sth, imp_sth_t * imp_sth) /* Experimental inout support */ if (imp_sth->use_inout) { - ph_t *currph; - for (i=0,currph=imp_sth->ph; NULL != currph && i < num_fields; currph=currph->nextph,i++) { + for (i=0; i < ph_array_count(imp_sth) && i < num_fields; i++) { + ph_t *currph = ph_array_element(imp_sth, i); if (currph->isinout) sv_copypv(currph->inout, AvARRAY(av)[i]); } @@ -3946,15 +3992,13 @@ void dbd_st_destroy (SV * sth, imp_sth_t * imp_sth) { dTHX; D_imp_dbh_from_sth; - seg_t * currseg; - seg_t * nextseg; - ph_t * currph; - ph_t * nextph; if (TSTART_slow) TRC(DBILOGFP, "%sBegin dbd_st_destroy\n", THEADER_slow); - if (NULL == imp_sth->seg) /* Already been destroyed! */ + if (seg_array_count(imp_sth) == 0) { + /* Already been destroyed! */ croak("dbd_st_destroy called twice!"); + } /* If the AutoInactiveDestroy flag has been set, we go no further */ if ((DBIc_AIADESTROY(imp_dbh)) && ((U32)PerlProc_getpid() != imp_dbh->pid_number)) { @@ -4004,28 +4048,10 @@ void dbd_st_destroy (SV * sth, imp_sth_t * imp_sth) } /* Free all the segments */ - currseg = imp_sth->seg; - while (NULL != currseg) { - Safefree(currseg->segment); - currseg->ph = NULL; - nextseg = currseg->nextseg; - Safefree(currseg); - currseg = nextseg; - } - imp_sth->seg = NULL; + seg_array_destroy(imp_sth); /* Free all the placeholders */ - currph = imp_sth->ph; - while (NULL != currph) { - Safefree(currph->fooname); - Safefree(currph->value); - Safefree(currph->quoted); - currph->bind_type = NULL; - nextph = currph->nextph; - Safefree(currph); - currph = nextph; - } - imp_sth->ph = NULL; + ph_array_destroy(imp_sth); if (imp_dbh->async_sth) imp_dbh->async_sth = NULL; diff --git a/dbdimp.h b/dbdimp.h index 248271d8..2f45d278 100644 --- a/dbdimp.h +++ b/dbdimp.h @@ -26,7 +26,7 @@ struct imp_dbh_st { int switch_prepared; /* how many executes until we switch to PQexecPrepared */ int async_status; /* 0=no async 1=async started -1=async has been cancelled */ - imp_sth_t *async_sth; /* current async statement handle */ + imp_sth_t *async_sth; /* current async statement handle */ AV *savepoints; /* list of savepoints */ PGconn *conn; /* connection structure */ char *sqlstate; /* from the last result */ @@ -43,20 +43,11 @@ struct imp_dbh_st { int pg_enable_utf8; /* legacy utf8 flag: force utf8 flag on or off, regardless of client_encoding */ bool pg_utf8_flag; /* are we currently flipping the utf8 flag on? */ - bool client_encoding_utf8; /* is the client_encoding utf8 last we checked? */ + bool client_encoding_utf8; /* is the client_encoding utf8 last we checked? */ }; -/* Each statement is broken up into segments */ -struct seg_st { - char *segment; /* non-placeholder string segment */ - int placeholder; /* which placeholder this points to, 0=none */ - struct ph_st *ph; /* points to the relevant ph structure */ - struct seg_st *nextseg; /* linked lists are fun */ -}; -typedef struct seg_st seg_t; - -/* The placeholders are also a linked list */ +/* The placeholder structure. Used as array elements in the ph_array_t structure */ struct ph_st { char *fooname; /* name if using :foo style */ char *value; /* the literal passed-in value, may be binary */ @@ -70,10 +61,32 @@ struct ph_st { bool isinout; /* is this a bind_param_inout value? */ SV *inout; /* what variable we are updating via inout magic */ sql_type_info_t* bind_type; /* type information for this placeholder */ - struct ph_st *nextph; /* more linked list goodness */ }; typedef struct ph_st ph_t; +/* The array container for the above segment structure */ +struct ph_array_st { + int length; /* length of the array */ + int elements; /* num of elements in the array */ + ph_t *array; /* the array of placeholders */ +}; +typedef struct ph_array_st ph_array_t; + + +/* Each statement is broken up into segments */ +struct seg_st { + char *segment; /* non-placeholder string segment */ + int placeholder; /* which placeholder this points to, 0=none */ +}; +typedef struct seg_st seg_t; + +struct seg_array_st { + int length; + int elements; + seg_t *array; +}; +typedef struct seg_array_st seg_array_t; + typedef enum { PLACEHOLDER_NONE, @@ -89,7 +102,7 @@ struct imp_sth_st { int server_prepare; /* inherited from dbh. 3 states: 0=no 1=yes 2=smart */ int switch_prepared; /* inherited from dbh */ - int number_iterations; /* how many times has the statement been executed? Used by switch_prepared */ + int number_iterations; /* how many times has the statement been executed? Used by switch_prepared */ PGPlaceholderType placeholder_type; /* which style is being used 1=? 2=$1 3=:foo */ int numsegs; /* how many segments this statement has */ int numphs; /* how many placeholders this statement has */ @@ -111,8 +124,8 @@ struct imp_sth_st { PGresult *result; /* result structure from the executed query */ sql_type_info_t **type_info; /* type of each column in result */ - seg_t *seg; /* linked list of segments */ - ph_t *ph; /* linked list of placeholders */ + seg_array_t seg_ary; /* array of segments */ + ph_array_t ph_ary; /* array of placeholders */ bool prepare_now; /* prepare this statement right away, even if it has placeholders */ bool prepared_by_us; /* false if {prepare_name} set directly */ diff --git a/sstring.c b/sstring.c new file mode 100644 index 00000000..8487d51e --- /dev/null +++ b/sstring.c @@ -0,0 +1,119 @@ +#include +#include "Pg.h" + +/* + * A Simple String library designed to be efficent for concat operations + * and to grow memory as needed. + * */ + +struct string_s { + size_t length; /* amount of data we have stored in the string, points to null byte */ + size_t memory; /* amount of memory we have allocated */ + char *string; +}; + +typedef struct string_s string_t; + +/* Returns a pointer to the null byte */ +static char* string_end(string_t *str) +{ + return &(str->string[ str->length ]); +} + +/* Returns the number of bytes free at the end of the string */ +static size_t string_free(string_t *str) +{ + return str->memory - str->length; +} + +/* Set the length of the string to 'bytes' */ +static void string_realloc(string_t *str, size_t bytes) +{ + Renew(str->string, bytes, char); + + str->memory = bytes; +} + +/* The number of characters in the string, not incl the null byte */ +size_t string_len(const string_t *str) +{ + return str->length; +} + +/* return a pointer to the string itself */ +char* string_get(string_t *str) +{ + return str->string; +} + +string_t* string_create(size_t size) +{ + string_t *str; + + if (size == 0) { + size = 1; + } + + New(0, str, 1, string_t); + New(0, str->string, (sizeof(char) * size) , char); + + str->length = 0; + str->memory = size; + str->string[0] = '\0'; + + return str; +} + +void string_destroy(string_t *str) +{ + Safefree(str->string); + Safefree(str); +} + +void string_append_chr(string_t *str, const char *text) +{ + const size_t textlen = strlen(text); + + if(str->length + textlen + 1 > str->memory){ + string_realloc(str, (str->length + textlen + 1) * 2); + } + + Copy(text, string_end(str), textlen, char); + + str->length = str->length + textlen; + + str->string[str->length] = '\0'; +} + +void string_append_byte(string_t *str, char byte) +{ + if(str->length + 1 > str->memory){ + string_realloc(str, (str->memory * 2)); + } + + *(string_end(str)) = byte; + str->length++; + str->string[str->length] = '\0'; +} + +/* + * sprintf a number in a string, prefixed by a dollar character + * */ +void string_append_dint(string_t *str, int num) +{ + const size_t bytes_free = string_free(str); + + const int written = snprintf(string_end(str), bytes_free, "$%d", num); + if(written < 0){ + /* This shouldn't ever happen */ + return; + } + + if((size_t)written >= bytes_free) { + string_realloc(str, str->memory * 2); + string_append_dint(str, num); + return; + } + + str->length += written; +} diff --git a/sstring.h b/sstring.h new file mode 100644 index 00000000..fcb5dd96 --- /dev/null +++ b/sstring.h @@ -0,0 +1,19 @@ +#ifndef SSTRING_H +#define SSTRING_H + +typedef void string_t; + +/* The length of the string */ +size_t string_len(const string_t *str); + +string_t* string_create(size_t characters); +void string_destroy(string_t *string); + +/* Returns a pointer to the internal char*. Do not free */ +char* string_get(string_t *str); + +void string_append_chr(string_t *str, const char *text); +void string_append_dint(string_t *str, int num); +void string_append_byte(string_t *str, char byte); + +#endif From d8e0ef1380c3855da84a3821555f496d4589e987 Mon Sep 17 00:00:00 2001 From: Matt Tyson Date: Tue, 11 Oct 2016 07:56:04 +1100 Subject: [PATCH 2/2] DBD::Pg Array and string optimization patch Fix test error in t/03smethod.t --- dbdimp.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dbdimp.c b/dbdimp.c index a7fb0621..f7fe53e1 100644 --- a/dbdimp.c +++ b/dbdimp.c @@ -1220,9 +1220,10 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv) retsv = newRV_noinc((SV*)arr); } else if (strEQ("pg_numbound", key)) { - int i; - for (i = 0; i < ph_array_count(imp_sth); i++) { - ph_t *currph = ph_array_element(imp_sth, i); + int i = 0; + int j; + for(j=0; j < ph_array_count(imp_sth); j++) { + ph_t *currph = ph_array_element(imp_sth, j); i += NULL == currph->bind_type ? 0 : 1; } retsv = newSViv(i);