[DBD::Pg 1/5] Factor placeholder key creation into function

[dbdpg-...@bucardo.org]

Committed by =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>

Subject: [DBD::Pg 1/5] Factor placeholder key creation into function

---
dbdimp.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/dbdimp.c b/dbdimp.c
index d364716..a8959fc 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -77,6 +77,7 @@ static ExecStatusType _result(pTHX_ imp_dbh_t *imp_dbh, const char *sql);
static void _fatal_sqlstate(pTHX_ imp_dbh_t *imp_dbh);
static ExecStatusType _sqlstate(pTHX_ imp_dbh_t *imp_dbh, PGresult *result);
static int pg_db_rollback_commit (pTHX_ SV *dbh, imp_dbh_t *imp_dbh, int action);
+static SV *pg_st_placeholder_key (imp_sth_t *imp_sth, ph_t *currph, int i);
static void pg_st_split_statement (pTHX_ imp_sth_t *imp_sth, int version, char *statement);
static int pg_st_prepare_statement (pTHX_ SV *sth, imp_sth_t *imp_sth);
static int is_high_bit_set(pTHX_ const unsigned char *val, STRLEN size);
@@ -998,6 +999,11 @@ int dbd_db_STORE_attrib (SV * dbh, imp_dbh_t * imp_dbh, SV * keysv, SV * valuesv

} /* end of dbd_db_STORE_attrib */

+static SV * pg_st_placeholder_key (imp_sth_t * imp_sth, ph_t * currph, int i) {
+ if (3 == imp_sth->placeholder_type)
+ return newSVpv(currph->fooname, 0);
+ return newSViv(i+1);
+}

/* ================================================================== */
SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv)
@@ -1021,7 +1027,7 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv)
int i;
for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) {
SV *key, *val;
- key = (3==imp_sth->placeholder_type ? newSVpv(currph->fooname,0) : newSViv(i+1));
+ key = pg_st_placeholder_key(imp_sth, currph, i);
val = newSViv(NULL == currph->bind_type ? 0 : 1);
if (! hv_store_ent(pvhv, key, val, 0)) {
SvREFCNT_dec(val);
@@ -1046,7 +1052,7 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv)
int i;
for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) {
SV *key, *val;
- key = (3==imp_sth->placeholder_type ? newSVpv(currph->fooname,0) : newSViv(i+1));
+ key = pg_st_placeholder_key(imp_sth, currph, i);
if (NULL == currph->bind_type) {
val = newSV(0);
if (! hv_store_ent(pvhv, key, val, 0)) {
@@ -1080,8 +1086,7 @@ SV * dbd_st_FETCH_attrib (SV * sth, imp_sth_t * imp_sth, SV * keysv)
int i;
for (i=0,currph=imp_sth->ph; NULL != currph; currph=currph->nextph,i++) {
SV *key, *val;
- key = (3==imp_sth->placeholder_type ? newSVpv(currph->fooname,0) : newSViv(i+1));
-
+ key = pg_st_placeholder_key(imp_sth, currph, i);
if (NULL == currph->value) {
val = newSV(0);
if (!hv_store_ent(pvhv, key, val, 0)) {
--
1.8.4

[dbdpg-...@bucardo.org]

Committed by =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>

Subject: [DBD::Pg 4/5] Use named enum values for pqtype

---
dbdimp.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/dbdimp.c b/dbdimp.c
index c50279e..0858028 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -67,6 +67,14 @@ typedef enum
} PGErrorVerbosity;
#endif

+typedef enum
+ {
+ PQTYPE_UNKNOWN,
+ PQTYPE_EXEC,
+ PQTYPE_PARAMS,
+ PQTYPE_PREPARED,
+ } PQExecType;
+
#define IS_DBI_HANDLE(h) \
(SvROK(h) && SvTYPE(SvRV(h)) == SVt_PVHV && \
SvRMAGICAL(SvRV(h)) && (SvMAGIC(SvRV(h)))->mg_type == 'P')
@@ -3163,7 +3171,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
char * statement = NULL;
int num_fields;
int ret = -2;
- int pqtype = 0;
+ PQExecType pqtype = PQTYPE_UNKNOWN;
long power_of_ten;

if (TSTART_slow) TRC(DBILOGFP, "%sBegin dbd_st_execute\n", THEADER_slow);
@@ -3243,10 +3251,6 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
Now, we need to build the statement to send to the backend
We are using one of PQexec, PQexecPrepared, or PQexecParams
Let's figure out which we are going to use and set pqtype
- 0= unknown
- 1= PQexec
- 2= PQexecParams
- 3= PQexecPrepared
*/

if (TRACE4_slow) TRC(DBILOGFP,
@@ -3278,12 +3282,12 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
|| !imp_sth->server_prepare
|| (2==imp_sth->server_prepare && imp_sth->numbound != imp_sth->numphs)
)
- pqtype = 1; /* PQexec */
+ pqtype = PQTYPE_EXEC;
else if (0==imp_sth->switch_prepared || imp_sth->number_iterations < imp_sth->switch_prepared) {
- pqtype = 2; /* PQexecParams */
+ pqtype = PQTYPE_PARAMS;
}
else {
- pqtype = 3; /* PQexecPrepared */
+ pqtype = PQTYPE_PREPARED;
}

/* We use the new server_side prepare style if:
@@ -3299,7 +3303,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
execsize = imp_sth->totalsize; /* Total of all segments */

/* If using plain old PQexec, we need to quote each value ourselves */
- if (1 == pqtype) {
+ if (PQTYPE_EXEC == pqtype) {
for (currph=imp_sth->ph; NULL != currph; currph=currph->nextph) {
if (currph->isdefault) {
Renew(currph->quoted, 8, char); /* freed in dbd_st_destroy */
@@ -3364,7 +3368,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
}

/* Run one of PQexec, PQexecParams, or PQexecPrepared */
- if (1 == pqtype) { /* PQexec */
+ if (PQTYPE_EXEC == pqtype) { /* PQexec */

if (TRACE5_slow) TRC(DBILOGFP, "%sPQexec\n", THEADER_slow);

@@ -3402,7 +3406,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
Safefree(statement);

}
- else if (2 == pqtype) { /* PQexecParams */
+ else if (PQTYPE_PARAMS == pqtype) { /* PQexecParams */
if (TRACE5_slow) TRC(DBILOGFP, "%sPQexecParams\n", THEADER_slow);

/* Figure out how big the statement plus placeholders will be */
@@ -3472,7 +3476,7 @@ int dbd_st_execute (SV * sth, imp_sth_t * imp_sth)
Safefree(statement);

}
- else if (3 == pqtype) { /* PQexecPrepared */
+ else if (PQTYPE_PREPARED == pqtype) { /* PQexecPrepared */

if (TRACE4_slow) TRC(DBILOGFP, "%sPQexecPrepared\n", THEADER_slow);

--
1.8.4

[dbdpg-...@bucardo.org]

Committed by =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>

Subject: [DBD::Pg 3/5] Use named enum values for placeholder_type

---
dbdimp.c | 36 ++++++++++++++++++------------------
dbdimp.h | 11 ++++++++++-
2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/dbdimp.c b/dbdimp.c
index 9e97701..c50279e 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -1000,7 +1000,7 @@ int dbd_db_STORE_attrib (SV * dbh, imp_dbh_t * imp_dbh, SV * keysv, SV * valuesv

} /* end of dbd_db_STORE_attrib */

static SV * pg_st_placeholder_key (imp_sth_t * imp_sth, ph_t * currph, int i) {

- if (3 == imp_sth->placeholder_type)
+ if (PLACEHOLDER_COLON == imp_sth->placeholder_type)
return newSVpv(currph->fooname, 0);
return newSViv(i+1);
}
@@ -1566,7 +1566,7 @@ int dbd_st_prepare_sv (SV * sth, imp_sth_t * imp_sth, SV * statement_sv, SV * at
croak ("Cannot prepare empty statement");

/* Set default values for this statement handle */
- imp_sth->placeholder_type = 0;
+ imp_sth->placeholder_type = PLACEHOLDER_NONE;
imp_sth->numsegs = 0;
imp_sth->numphs = 0;
imp_sth->numbound = 0;
@@ -1698,7 +1698,7 @@ int dbd_st_prepare_sv (SV * sth, imp_sth_t * imp_sth, SV * statement_sv, SV * at
} /* end of dbd_st_prepare */


-static const char *placeholder_string[4] = {
+static const char *placeholder_string[PLACEHOLDER_TYPE_COUNT] = {
"", "?", "$1", ":foo"
};

@@ -1722,7 +1722,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char

int topdollar; /* Used to enforce sequential $1 arguments */

- int placeholder_type; /* Which type we are in: one of 0,1,2,3 (none,?,$,:) */
+ PGPlaceholderType placeholder_type; /* Which type we are in: one of none,?,$,: */

unsigned char ch; /* The current character being checked */

@@ -2018,7 +2018,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
sectionstop=currpos-1;

/* Figure out if we have a placeholder */
- placeholder_type = 0;
+ placeholder_type = PLACEHOLDER_NONE;

/* Dollar sign placeholder style */
if ('$' == ch && isDIGIT(*statement)) {
@@ -2028,12 +2028,12 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
++statement;
++currpos;
}
- placeholder_type = 2;
+ placeholder_type = PLACEHOLDER_DOLLAR;
}
else if (! imp_sth->dollaronly) {
/* Question mark style */
if ('?' == ch) {
- placeholder_type = 1;
+ placeholder_type = PLACEHOLDER_QUESTIONMARK;
}
/* Colon style */
else if (':' == ch && ! imp_sth->nocolons) {
@@ -2061,13 +2061,13 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
++statement;
++currpos;
}
- placeholder_type = 3;
+ placeholder_type = PLACEHOLDER_COLON;
}
}
}

/* Check for conflicting placeholder types */
- if (placeholder_type!=0) {
+ if (placeholder_type != PLACEHOLDER_NONE) {
if (imp_sth->placeholder_type && placeholder_type != imp_sth->placeholder_type)
croak("Cannot mix placeholder styles \"%s\" and \"%s\"",
placeholder_string[imp_sth->placeholder_type],
@@ -2075,7 +2075,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
}

/* Move on to the next letter unless we found a placeholder, or we are at the end of the string */
- if (0==placeholder_type && ch)
+ if (PLACEHOLDER_NONE == placeholder_type && ch)
continue;

/* If we got here, we have a segment that needs to be saved */
@@ -2084,13 +2084,13 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
newseg->placeholder = 0;
newseg->ph = NULL;

- if (1==placeholder_type) {
+ if (PLACEHOLDER_QUESTIONMARK == placeholder_type) {
newseg->placeholder = ++imp_sth->numphs;
}
- else if (2==placeholder_type) {
+ else if (PLACEHOLDER_DOLLAR == placeholder_type) {
newseg->placeholder = atoi(statement-(currpos-sectionstop-1));
}
- else if (3==placeholder_type) {
+ else if (PLACEHOLDER_COLON == placeholder_type) {
sectionsize = currpos-sectionstop;
/* Have we seen this placeholder yet? */
for (xint=1,thisph=imp_sth->ph; NULL != thisph; thisph=thisph->nextph,xint++) {
@@ -2157,7 +2157,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
sectionstart = currpos;
imp_sth->numsegs++;

- if (placeholder_type > 0)
+ if (placeholder_type != PLACEHOLDER_NONE)
imp_sth->placeholder_type = placeholder_type;

/*
@@ -2171,7 +2171,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
} /* end large while(1) loop: statement parsing */

/* For dollar sign placeholders, ensure that the rules are followed */
- if (2==imp_sth->placeholder_type) {
+ if (PLACEHOLDER_DOLLAR == imp_sth->placeholder_type) {
/*
We follow the Pg rules: must start with $1, repeats are allowed,
numbers must be sequential. We change numphs if repeats found
@@ -2197,7 +2197,7 @@ static void pg_st_split_statement (pTHX_ imp_sth_t * imp_sth, int version, char
}

/* Create sequential placeholders */
- if (3 != imp_sth->placeholder_type) {
+ if (PLACEHOLDER_COLON != imp_sth->placeholder_type) {
for (xint=1; xint <= imp_sth->numphs; xint++) {
New(0, newph, 1, ph_t); /* freed in dbd_st_destroy */
newph->nextph = NULL;
@@ -2430,7 +2430,7 @@ int dbd_bind_ph (SV * sth, imp_sth_t * imp_sth, SV * ph_name, SV * newvalue, IV
(void)mg_get(ph_name);
}
name = SvPV(ph_name, name_len);
- if (3==imp_sth->placeholder_type) {
+ if (PLACEHOLDER_COLON == imp_sth->placeholder_type) {
if (':' != *name) {
croak("Placeholders must begin with ':' when using the \":foo\" style");
}
@@ -2445,7 +2445,7 @@ int dbd_bind_ph (SV * sth, imp_sth_t * imp_sth, SV * ph_name, SV * newvalue, IV

/* Find the placeholder in question */

- if (3==imp_sth->placeholder_type) {
+ if (PLACEHOLDER_COLON == imp_sth->placeholder_type) {
for (x=0,currph=imp_sth->ph; NULL != currph; currph = currph->nextph) {
if (0==strcmp(currph->fooname, name)) {
x=1;
diff --git a/dbdimp.h b/dbdimp.h
index 7cc6e3c..09d8f7c 100644
--- a/dbdimp.h
+++ b/dbdimp.h
@@ -74,6 +74,15 @@ struct ph_st {
};
typedef struct ph_st ph_t;

+typedef enum
+ {
+ PLACEHOLDER_NONE,
+ PLACEHOLDER_QUESTIONMARK,
+ PLACEHOLDER_DOLLAR,
+ PLACEHOLDER_COLON
+ } PGPlaceholderType;
+#define PLACEHOLDER_TYPE_COUNT (PLACEHOLDER_COLON + 1)
+
/* Define sth implementor data structure */
struct imp_sth_st {
dbih_stc_t com; /* MUST be first element in structure */
@@ -81,7 +90,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 placeholder_type; /* which style is being used 1=? 2=$1 3=:foo */
+ 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 */
int numbound; /* how many placeholders were explicitly bound by the client, not us */
--
1.8.4