Minor additional improvements for ecpglib/prepare.c.

Avoid allocating never-used entries in stmtCacheEntries[], other than the
intentionally-unused zero'th entry.  Tie the array size directly to the
bucket count and size, rather than having undocumented dependencies between
three magic constants.  Fix the hash calculation to be platform-independent
--- notably, it was sensitive to the signed'ness of "char" before, not to
mention having an unnecessary hard-wired dependency on the existence and
size of type "long long".  (The lack of complaints says it's been a long
time since anybody tried to build PG on a compiler without "long long",
and certainly with the requirement for C99 this isn't a live bug anymore.
But it's still not per project coding style.)  Fix ecpg_auto_prepare's
new-cache-entry path so that it increments the exec count for the new
cache entry not the dummy zero'th entry.

The last of those is an actual bug, though one of little consequence;
the rest is mostly future-proofing and neatnik-ism.  Doesn't seem
necessary to back-patch.

diff --git a/src/interfaces/ecpg/ecpglib/prepare.c b/src/interfaces/ecpg/ecpglib/prepare.c
index f904d326e48c293f4bd4968a619c459e1e82c888..5368886f67b47b63ecfa1a8b94cd30f54ccc93e7 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/prepare.c
+++ b/src/interfaces/ecpg/ecpglib/prepare.c
@@ -13,7 +13,17 @@
 
 #define STMTID_SIZE 32
 
-#define N_STMTCACHE_ENTRIES 16384
+/*
+ * The statement cache contains stmtCacheNBuckets hash buckets, each
+ * having stmtCacheEntPerBucket entries, which we recycle as needed,
+ * giving up the least-executed entry in the bucket.
+ * stmtCacheEntries[0] is never used, so that zero can be a "not found"
+ * indicator.
+ */
+#define stmtCacheNBuckets              2039    /* should be a prime number */
+#define stmtCacheEntPerBucket  8
+
+#define stmtCacheArraySize (stmtCacheNBuckets * stmtCacheEntPerBucket + 1)
 
 typedef struct
 {
@@ -25,8 +35,6 @@ typedef struct
 } stmtCacheEntry;
 
 static int     nextStmtID = 1;
-static const int stmtCacheNBuckets = 2039;     /* # buckets - a prime # */
-static const int stmtCacheEntPerBucket = 8; /* # entries/bucket */
 static stmtCacheEntry *stmtCacheEntries = NULL;
 
 static bool deallocate_one(int lineno, enum COMPAT_MODE c, struct connection *con,
@@ -330,7 +338,7 @@ HashStmt(const char *ecpgQuery)
                                bucketNo,
                                hashLeng,
                                stmtLeng;
-       long long       hashVal,
+       uint64          hashVal,
                                rotVal;
 
        stmtLeng = strlen(ecpgQuery);
@@ -341,16 +349,17 @@ HashStmt(const char *ecpgQuery)
        hashVal = 0;
        for (stmtIx = 0; stmtIx < hashLeng; ++stmtIx)
        {
-               hashVal = hashVal + (int) ecpgQuery[stmtIx];
+               hashVal = hashVal + (unsigned char) ecpgQuery[stmtIx];
+               /* rotate 32-bit hash value left 13 bits */
                hashVal = hashVal << 13;
-               rotVal = (hashVal & 0x1fff00000000LL) >> 32;
-               hashVal = (hashVal & 0xffffffffLL) | rotVal;
+               rotVal = (hashVal & UINT64CONST(0x1fff00000000)) >> 32;
+               hashVal = (hashVal & UINT64CONST(0xffffffff)) | rotVal;
        }
 
        bucketNo = hashVal % stmtCacheNBuckets;
-       bucketNo += 1;                          /* don't use bucket # 0 */
 
-       return (bucketNo * stmtCacheEntPerBucket);
+       /* Add 1 so that array entry 0 is never used */
+       return bucketNo * stmtCacheEntPerBucket + 1;
 }
 
 /*
@@ -451,7 +460,7 @@ AddStmtToCache(int lineno,          /* line # of statement */
        if (stmtCacheEntries == NULL)
        {
                stmtCacheEntries = (stmtCacheEntry *)
-                       ecpg_alloc(sizeof(stmtCacheEntry) * N_STMTCACHE_ENTRIES, lineno);
+                       ecpg_alloc(sizeof(stmtCacheEntry) * stmtCacheArraySize, lineno);
                if (stmtCacheEntries == NULL)
                        return -1;
        }
@@ -534,7 +543,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
 
                if (!ECPGprepare(lineno, connection_name, 0, stmtID, query))
                        return false;
-               if (AddStmtToCache(lineno, stmtID, connection_name, compat, query) < 0)
+
+               entNo = AddStmtToCache(lineno, stmtID, connection_name, compat, query);
+               if (entNo < 0)
                        return false;
 
                *name = ecpg_strdup(stmtID, lineno);

diff --git a/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr b/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr
index ea21e82ca6daa84e7ddb82a280e0d3d19b223d96..bfeea59a7535320bb2fd68d22360cbee3a22c362 100644 (file)
--- a/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr
+++ b/src/interfaces/ecpg/test/expected/preproc-autoprep.stderr
@@ -30,7 +30,7 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_process_output on line 24: OK: INSERT 0 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1640
+[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1633
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_execute on line 26: query: insert into T values ( 1 , $1  ); with 1 parameter(s) on connection ecpg1_regression
 [NO_PID]: sqlca: code: 0, state: 00000
@@ -168,7 +168,7 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_process_output on line 21: OK: CREATE TABLE
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_auto_prepare on line 23: statement found in cache; entry 15328
+[NO_PID]: ecpg_auto_prepare on line 23: statement found in cache; entry 15321
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: prepare_common on line 23: name ecpg1; query: "insert into T values ( 1 , null )"
 [NO_PID]: sqlca: code: 0, state: 00000
@@ -178,7 +178,7 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_process_output on line 23: OK: INSERT 0 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_auto_prepare on line 24: statement found in cache; entry 1640
+[NO_PID]: ecpg_auto_prepare on line 24: statement found in cache; entry 1633
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: prepare_common on line 24: name ecpg2; query: "insert into T values ( 1 , $1  )"
 [NO_PID]: sqlca: code: 0, state: 00000
@@ -190,7 +190,7 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_process_output on line 24: OK: INSERT 0 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1640
+[NO_PID]: ecpg_auto_prepare on line 26: statement found in cache; entry 1633
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_execute on line 26: query: insert into T values ( 1 , $1  ); with 1 parameter(s) on connection ecpg1_regression
 [NO_PID]: sqlca: code: 0, state: 00000
@@ -208,7 +208,7 @@
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: ecpg_process_output on line 28: OK: INSERT 0 1
 [NO_PID]: sqlca: code: 0, state: 00000
-[NO_PID]: ecpg_auto_prepare on line 30: statement found in cache; entry 13056
+[NO_PID]: ecpg_auto_prepare on line 30: statement found in cache; entry 13049
 [NO_PID]: sqlca: code: 0, state: 00000
 [NO_PID]: prepare_common on line 30: name ecpg3; query: "select Item2 from T order by Item2 nulls last"
 [NO_PID]: sqlca: code: 0, state: 00000