From 075feae8f02c602dd0c6f4eefb51c906b1420a74 Mon Sep 17 00:00:00 2001 From: Joe Bylund Date: Tue, 22 Sep 2015 13:11:20 -0400 Subject: [PATCH 1/4] Don't roundtrip to the database to get the column type if you already know it --- ext/pdo_pgsql/pgsql_statement.c | 81 +++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 25 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 1e211733e7a2e..a66b8efcd883e 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -36,13 +36,27 @@ #endif /* from postgresql/src/include/catalog/pg_type.h */ +#define BOOLLABEL "bool" #define BOOLOID 16 +#define BYTEALABEL "bytea" #define BYTEAOID 17 -#define INT8OID 20 +#define DATELABEL "date" +#define DATEOID 1082 +#define INT2LABEL "int2" #define INT2OID 21 +#define INT4LABEL "int4" #define INT4OID 23 -#define TEXTOID 25 +#define INT8LABEL "int8" +#define INT8OID 20 #define OIDOID 26 +#define TEXTLABEL "text" +#define TEXTOID 25 +#define TIMESTAMPLABEL "timestamp" +#define TIMESTAMPOID 1114 +#define VARCHARLABEL "varchar" +#define VARCHAROID 1043 + + static int pgsql_stmt_dtor(pdo_stmt_t *stmt) { @@ -591,29 +605,46 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r array_init(return_value); add_assoc_long(return_value, "pgsql:oid", S->cols[colno].pgsql_type); - /* Fetch metadata from Postgres system catalogue */ - spprintf(&q, 0, "SELECT TYPNAME FROM PG_TYPE WHERE OID=%u", S->cols[colno].pgsql_type); - res = PQexec(S->H->server, q); - efree(q); - - status = PQresultStatus(res); - - if (status != PGRES_TUPLES_OK) { - /* Failed to get system catalogue, but return success - * with the data we have collected so far - */ - goto done; - } - - /* We want exactly one row returned */ - if (1 != PQntuples(res)) { - goto done; - } - - add_assoc_string(return_value, "native_type", PQgetvalue(res, 0, 0)); -done: - PQclear(res); - return 1; + switch (S->cols[colno].pgsql_type) { + case BOOLOID: + add_assoc_string(return_value, "native_type", BOOLLABEL, 1); + break; + case BYTEAOID: + add_assoc_string(return_value, "native_type", BYTEALABEL, 1); + break; + case INT8OID: + add_assoc_string(return_value, "native_type", INT8LABEL, 1); + break; + case INT2OID: + add_assoc_string(return_value, "native_type", INT2LABEL, 1); + break; + case INT4OID: + add_assoc_string(return_value, "native_type", INT4LABEL, 1); + break; + case TEXTOID: + add_assoc_string(return_value, "native_type", TEXTLABEL, 1); + break; + case VARCHAROID: + add_assoc_string(return_value, "native_type", VARCHARLABEL, 1); + break; + case DATEOID: + add_assoc_string(return_value, "native_type", DATELABEL, 1); + break; + case TIMESTAMPOID: + add_assoc_string(return_value, "native_type", TIMESTAMPLABEL, 1); + break; + default: + /* Fetch metadata from Postgres system catalogue */ + spprintf(&q, 0, "SELECT TYPNAME FROM PG_TYPE WHERE OID=%u", S->cols[colno].pgsql_type); + res = PQexec(S->H->server, q); + efree(q); + status = PQresultStatus(res); + if (status == PGRES_TUPLES_OK && 1 == PQntuples(res)) { + add_assoc_string(return_value, "native_type", PQgetvalue(res, 0, 0), 1); + PQclear(res); + } + } + return 1; } static int pdo_pgsql_stmt_cursor_closer(pdo_stmt_t *stmt) From e48ecd5f8f1db9effb6b8449dc04a48c95165675 Mon Sep 17 00:00:00 2001 From: Joe Bylund Date: Tue, 22 Sep 2015 14:06:11 -0400 Subject: [PATCH 2/4] add_assoc_string now takes just three args --- ext/pdo_pgsql/pgsql_statement.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index a66b8efcd883e..9e87305d442e2 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -607,31 +607,31 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r switch (S->cols[colno].pgsql_type) { case BOOLOID: - add_assoc_string(return_value, "native_type", BOOLLABEL, 1); + add_assoc_string(return_value, "native_type", BOOLLABEL); break; case BYTEAOID: - add_assoc_string(return_value, "native_type", BYTEALABEL, 1); + add_assoc_string(return_value, "native_type", BYTEALABEL); break; case INT8OID: - add_assoc_string(return_value, "native_type", INT8LABEL, 1); + add_assoc_string(return_value, "native_type", INT8LABEL); break; case INT2OID: - add_assoc_string(return_value, "native_type", INT2LABEL, 1); + add_assoc_string(return_value, "native_type", INT2LABEL); break; case INT4OID: - add_assoc_string(return_value, "native_type", INT4LABEL, 1); + add_assoc_string(return_value, "native_type", INT4LABEL); break; case TEXTOID: - add_assoc_string(return_value, "native_type", TEXTLABEL, 1); + add_assoc_string(return_value, "native_type", TEXTLABEL); break; case VARCHAROID: - add_assoc_string(return_value, "native_type", VARCHARLABEL, 1); + add_assoc_string(return_value, "native_type", VARCHARLABEL); break; case DATEOID: - add_assoc_string(return_value, "native_type", DATELABEL, 1); + add_assoc_string(return_value, "native_type", DATELABEL); break; case TIMESTAMPOID: - add_assoc_string(return_value, "native_type", TIMESTAMPLABEL, 1); + add_assoc_string(return_value, "native_type", TIMESTAMPLABEL); break; default: /* Fetch metadata from Postgres system catalogue */ @@ -640,7 +640,7 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r efree(q); status = PQresultStatus(res); if (status == PGRES_TUPLES_OK && 1 == PQntuples(res)) { - add_assoc_string(return_value, "native_type", PQgetvalue(res, 0, 0), 1); + add_assoc_string(return_value, "native_type", PQgetvalue(res, 0, 0)); PQclear(res); } } From b0cb74cbb1cc1f6aef69e0a277b13b1280331dec Mon Sep 17 00:00:00 2001 From: Joe Bylund Date: Wed, 23 Sep 2015 14:40:35 -0400 Subject: [PATCH 3/4] nikic's fix to move pqclear outside the if, would be a possible memory leak inside the if --- ext/pdo_pgsql/pgsql_statement.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 9e87305d442e2..2e93e01432f4d 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/ext/pdo_pgsql/pgsql_statement.c @@ -641,8 +641,8 @@ static int pgsql_stmt_get_column_meta(pdo_stmt_t *stmt, zend_long colno, zval *r status = PQresultStatus(res); if (status == PGRES_TUPLES_OK && 1 == PQntuples(res)) { add_assoc_string(return_value, "native_type", PQgetvalue(res, 0, 0)); - PQclear(res); } + PQclear(res); } return 1; } From 9ff52d1f142dba22ca115cee8623d423078ff38d Mon Sep 17 00:00:00 2001 From: Joe Bylund Date: Tue, 15 Dec 2015 11:57:10 -0500 Subject: [PATCH 4/4] (joe) check behavior of getColumnMeta --- ext/pdo_pgsql/tests/bug62498.phpt | 103 ++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 ext/pdo_pgsql/tests/bug62498.phpt diff --git a/ext/pdo_pgsql/tests/bug62498.phpt b/ext/pdo_pgsql/tests/bug62498.phpt new file mode 100644 index 0000000000000..cefcd1f3f180a --- /dev/null +++ b/ext/pdo_pgsql/tests/bug62498.phpt @@ -0,0 +1,103 @@ +--TEST-- +PDO PgSQL Bug #62498 (pdo_pgsql inefficient when getColumnMeta() is used) +--SKIPIF-- + +--FILE-- +setAttribute (\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + +// create the table +$db->exec("CREATE TEMPORARY TABLE bugtest_62498 (intcol INTEGER, stringcol VARCHAR(255), boolcol BOOLEAN, datecol DATE)"); + +// insert some data +$statement = $db->prepare("INSERT INTO bugtest_62498 (intcol, stringcol, boolcol, datecol) VALUES (:intval, :stringval, :boolval, :dateval)"); +$statement->execute(array( + "intval" => "42", + "stringval" => "The Answer", + "boolval" => true, + "dateval" => '2015-12-14', +)); + +$select = $db->query('SELECT intcol, stringcol, boolcol, datecol FROM bugtest_62498'); +$meta = []; +for ($i=0; $i < 4; $i++) { + $meta[] = $select->getColumnMeta(0); +} +var_dump($meta); + +?> +Done +--EXPECT-- +Begin test... +array(4) { + [0]=> + array(6) { + ["pgsql:oid"]=> + int(23) + ["native_type"]=> + string(4) "int4" + ["name"]=> + string(6) "intcol" + ["len"]=> + int(4) + ["precision"]=> + int(-1) + ["pdo_type"]=> + int(1) + } + [1]=> + array(6) { + ["pgsql:oid"]=> + int(23) + ["native_type"]=> + string(4) "int4" + ["name"]=> + string(6) "intcol" + ["len"]=> + int(4) + ["precision"]=> + int(-1) + ["pdo_type"]=> + int(1) + } + [2]=> + array(6) { + ["pgsql:oid"]=> + int(23) + ["native_type"]=> + string(4) "int4" + ["name"]=> + string(6) "intcol" + ["len"]=> + int(4) + ["precision"]=> + int(-1) + ["pdo_type"]=> + int(1) + } + [3]=> + array(6) { + ["pgsql:oid"]=> + int(23) + ["native_type"]=> + string(4) "int4" + ["name"]=> + string(6) "intcol" + ["len"]=> + int(4) + ["precision"]=> + int(-1) + ["pdo_type"]=> + int(1) + } +} +Done