From baabd1192973156ac79c35f6d1b0dced4af8e8fb Mon Sep 17 00:00:00 2001 From: Matteo Beccati Date: Tue, 4 Jun 2013 17:20:20 +0200 Subject: [PATCH] Refactored custom PDO_pgsql methods to trigger errors/exceptions BC Break: the custom methods were previously just return false on failure. Now they throw an exception with a proper error message. An hopefully welcome improvement, but some application might be depending on the old behaviour. FWIW the PDO::pgsqlCopy* methods are not documented, even though they are available since 5.3.x. --- ext/pdo_pgsql/pgsql_driver.c | 83 ++++++++++++++++++++---------- ext/pdo_pgsql/php_pdo_pgsql_int.h | 8 +-- ext/pdo_pgsql/tests/copy_from.phpt | 31 +++++++---- ext/pdo_pgsql/tests/copy_to.phpt | 30 +++++++---- 4 files changed, 103 insertions(+), 49 deletions(-) diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index 252bfff25b..a83fb4c2da 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -29,6 +29,7 @@ #include "ext/standard/info.h" #include "pdo/php_pdo.h" #include "pdo/php_pdo_driver.h" +#include "pdo/php_pdo_error.h" #include "ext/standard/file.h" #undef PACKAGE_BUGREPORT @@ -60,7 +61,7 @@ static char * _pdo_pgsql_trim_message(const char *message, int persistent) return tmp; } -int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char *sqlstate, const char *file, int line TSRMLS_DC) /* {{{ */ +int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char *sqlstate, const char *msg, const char *file, int line TSRMLS_DC) /* {{{ */ { pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data; pdo_error_type *pdo_err = stmt ? &stmt->error_code : &dbh->error_code; @@ -83,7 +84,10 @@ int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char * strcpy(*pdo_err, sqlstate); } - if (errmsg) { + if (msg) { + einfo->errmsg = estrdup(msg); + } + else if (errmsg) { einfo->errmsg = _pdo_pgsql_trim_message(errmsg, dbh->is_persistent); } @@ -91,7 +95,7 @@ int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char * zend_throw_exception_ex(php_pdo_get_exception(), einfo->errcode TSRMLS_CC, "SQLSTATE[%s] [%d] %s", *pdo_err, einfo->errcode, einfo->errmsg); } - + return errcode; } /* }}} */ @@ -535,6 +539,7 @@ static PHP_METHOD(PDO, pgsqlCopyFromArray) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); if (pg_fields) { spprintf(&query, 0, "COPY %s (%s) FROM STDIN DELIMITERS E'%c' WITH NULL AS E'%s'", table_name, pg_fields, (pg_delim_len ? *pg_delim : '\t'), (pg_null_as_len ? pg_null_as : "\\\\N")); @@ -583,7 +588,8 @@ static PHP_METHOD(PDO, pgsqlCopyFromArray) query[query_len] = '\0'; if (PQputCopyData(H->server, query, query_len) != 1) { efree(query); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "copy failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } zend_hash_move_forward_ex(Z_ARRVAL_P(pg_rows), &pos); @@ -593,22 +599,25 @@ static PHP_METHOD(PDO, pgsqlCopyFromArray) } if (PQputCopyEnd(H->server, NULL) != 1) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "putcopyend failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } while ((pgsql_result = PQgetResult(H->server))) { if (PGRES_COMMAND_OK != PQresultStatus(pgsql_result)) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); command_failed = 1; } PQclear(pgsql_result); } + PDO_HANDLE_DBH_ERR(); RETURN_BOOL(!command_failed); } else { + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); PQclear(pgsql_result); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -637,10 +646,12 @@ static PHP_METHOD(PDO, pgsqlCopyFromFile) /* Obtain db Handler */ dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); - stream = php_stream_open_wrapper_ex(filename, "rb", ENFORCE_SAFE_MODE | REPORT_ERRORS, NULL, FG(default_context)); + stream = php_stream_open_wrapper_ex(filename, "rb", ENFORCE_SAFE_MODE, NULL, FG(default_context)); if (!stream) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Unable to open the file"); + pdo_pgsql_error_msg(dbh, PGRES_FATAL_ERROR, "Unable to open the file"); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } @@ -674,8 +685,9 @@ static PHP_METHOD(PDO, pgsqlCopyFromFile) while ((buf = php_stream_get_line(stream, NULL, 0, &line_len)) != NULL) { if (PQputCopyData(H->server, buf, line_len) != 1) { efree(buf); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "copy failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); php_stream_close(stream); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } efree(buf); @@ -683,23 +695,26 @@ static PHP_METHOD(PDO, pgsqlCopyFromFile) php_stream_close(stream); if (PQputCopyEnd(H->server, NULL) != 1) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "putcopyend failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } while ((pgsql_result = PQgetResult(H->server))) { if (PGRES_COMMAND_OK != PQresultStatus(pgsql_result)) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); command_failed = 1; } PQclear(pgsql_result); } + PDO_HANDLE_DBH_ERR(); RETURN_BOOL(!command_failed); } else { - PQclear(pgsql_result); php_stream_close(stream); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); + PQclear(pgsql_result); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -730,12 +745,14 @@ static PHP_METHOD(PDO, pgsqlCopyToFile) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); H = (pdo_pgsql_db_handle *)dbh->driver_data; - stream = php_stream_open_wrapper_ex(filename, "wb", ENFORCE_SAFE_MODE | REPORT_ERRORS, NULL, FG(default_context)); + stream = php_stream_open_wrapper_ex(filename, "wb", ENFORCE_SAFE_MODE, NULL, FG(default_context)); if (!stream) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Unable to open the file for writing"); + pdo_pgsql_error_msg(dbh, PGRES_FATAL_ERROR, "Unable to open the file for writing"); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } @@ -767,16 +784,18 @@ static PHP_METHOD(PDO, pgsqlCopyToFile) break; /* done */ } else if (ret > 0) { if (php_stream_write(stream, csv, ret) != ret) { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Unable to write to file"); + pdo_pgsql_error_msg(dbh, PGRES_FATAL_ERROR, "Unable to write to file"); PQfreemem(csv); php_stream_close(stream); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } else { PQfreemem(csv); } } else { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed: getline failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); php_stream_close(stream); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -788,8 +807,9 @@ static PHP_METHOD(PDO, pgsqlCopyToFile) RETURN_TRUE; } else { php_stream_close(stream); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); PQclear(pgsql_result); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -817,6 +837,7 @@ static PHP_METHOD(PDO, pgsqlCopyToArray) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); H = (pdo_pgsql_db_handle *)dbh->driver_data; @@ -851,7 +872,8 @@ static PHP_METHOD(PDO, pgsqlCopyToArray) add_next_index_stringl(return_value, csv, ret, 1); PQfreemem(csv); } else { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed: getline failed"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -860,8 +882,9 @@ static PHP_METHOD(PDO, pgsqlCopyToArray) PQclear(pgsql_result); } } else { + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, pdo_pgsql_sqlstate(pgsql_result)); PQclear(pgsql_result); - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "Copy command failed"); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } } @@ -878,6 +901,7 @@ static PHP_METHOD(PDO, pgsqlLOBCreate) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); H = (pdo_pgsql_db_handle *)dbh->driver_data; lfd = lo_creat(H->server, INV_READ|INV_WRITE); @@ -887,8 +911,9 @@ static PHP_METHOD(PDO, pgsqlLOBCreate) spprintf(&buf, 0, "%lu", (long) lfd); RETURN_STRING(buf, 0); } - - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "HY000"); + + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } /* }}} */ @@ -924,6 +949,7 @@ static PHP_METHOD(PDO, pgsqlLOBOpen) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); H = (pdo_pgsql_db_handle *)dbh->driver_data; @@ -936,8 +962,10 @@ static PHP_METHOD(PDO, pgsqlLOBOpen) return; } } else { - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "HY000"); + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); } + + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } /* }}} */ @@ -964,13 +992,16 @@ static PHP_METHOD(PDO, pgsqlLOBUnlink) dbh = zend_object_store_get_object(getThis() TSRMLS_CC); PDO_CONSTRUCT_CHECK; + PDO_DBH_CLEAR_ERR(); H = (pdo_pgsql_db_handle *)dbh->driver_data; - + if (1 == lo_unlink(H->server, oid)) { RETURN_TRUE; } - pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, "HY000"); + + pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); + PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } /* }}} */ diff --git a/ext/pdo_pgsql/php_pdo_pgsql_int.h b/ext/pdo_pgsql/php_pdo_pgsql_int.h index 02a6717760..5600a92541 100644 --- a/ext/pdo_pgsql/php_pdo_pgsql_int.h +++ b/ext/pdo_pgsql/php_pdo_pgsql_int.h @@ -83,9 +83,11 @@ typedef struct { extern pdo_driver_t pdo_pgsql_driver; -extern int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char *sqlstate, const char *file, int line TSRMLS_DC); -#define pdo_pgsql_error(d,e,z) _pdo_pgsql_error(d, NULL, e, z, __FILE__, __LINE__ TSRMLS_CC) -#define pdo_pgsql_error_stmt(s,e,z) _pdo_pgsql_error(s->dbh, s, e, z, __FILE__, __LINE__ TSRMLS_CC) +extern int _pdo_pgsql_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, int errcode, const char *sqlstate, const char *msg, const char *file, int line TSRMLS_DC); +#define pdo_pgsql_error(d,e,z) _pdo_pgsql_error(d, NULL, e, z, NULL, __FILE__, __LINE__ TSRMLS_CC) +#define pdo_pgsql_error_msg(d,e,m) _pdo_pgsql_error(d, NULL, e, NULL, m, __FILE__, __LINE__ TSRMLS_CC) +#define pdo_pgsql_error_stmt(s,e,z) _pdo_pgsql_error(s->dbh, s, e, z, NULL, __FILE__, __LINE__ TSRMLS_CC) +#define pdo_pgsql_error_stmt_msg(s,e,m) _pdo_pgsql_error(s->dbh, s, e, NULL, m, __FILE__, __LINE__ TSRMLS_CC) extern struct pdo_stmt_methods pgsql_stmt_methods; diff --git a/ext/pdo_pgsql/tests/copy_from.phpt b/ext/pdo_pgsql/tests/copy_from.phpt index 10967b0fe9..de1140dfea 100644 --- a/ext/pdo_pgsql/tests/copy_from.phpt +++ b/ext/pdo_pgsql/tests/copy_from.phpt @@ -16,8 +16,6 @@ $db->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false); $db->exec('CREATE TABLE test (a integer not null primary key, b text, c integer)'); -try { - echo "Preparing test file and array for CopyFrom tests\n"; $tableRows = array(); @@ -68,10 +66,13 @@ $db->rollback(); echo "Testing pgsqlCopyFromArray() with error\n"; $db->beginTransaction(); -var_dump($db->pgsqlCopyFromArray('test_error',$tableRowsWithDifferentNullValuesAndSelectedFields,";","NULL",'a,c')); +try { + var_dump($db->pgsqlCopyFromArray('test_error',$tableRowsWithDifferentNullValuesAndSelectedFields,";","NULL",'a,c')); +} catch (Exception $e) { + echo "Exception: {$e->getMessage()}\n"; +} $db->rollback(); - echo "Testing pgsqlCopyFromFile() with default parameters\n"; $db->beginTransaction(); var_dump($db->pgsqlCopyFromFile('test',$filename)); @@ -102,14 +103,21 @@ $db->rollback(); echo "Testing pgsqlCopyFromFile() with error\n"; $db->beginTransaction(); -var_dump($db->pgsqlCopyFromFile('test_error',$filenameWithDifferentNullValuesAndSelectedFields,";","NULL",'a,c')); +try { + var_dump($db->pgsqlCopyFromFile('test_error',$filenameWithDifferentNullValuesAndSelectedFields,";","NULL",'a,c')); +} catch (Exception $e) { + echo "Exception: {$e->getMessage()}\n"; +} $db->rollback(); +echo "Testing pgsqlCopyFromFile() with non existing file\n"; +$db->beginTransaction(); +try { + var_dump($db->pgsqlCopyFromFile('test',"nonexisting/foo.csv",";","NULL",'a,c')); } catch (Exception $e) { - /* catch exceptions so that we can show the relative error */ - echo "Exception! at line ", $e->getLine(), "\n"; - var_dump($e->getMessage()); + echo "Exception: {$e->getMessage()}\n"; } +$db->rollback(); // Clean up foreach (array($filename, $filenameWithDifferentNullValues, $filenameWithDifferentNullValuesAndSelectedFields) as $f) { @@ -251,7 +259,7 @@ array(6) { NULL } Testing pgsqlCopyFromArray() with error -bool(false) +Exception: SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "test_error" does not exist Testing pgsqlCopyFromFile() with default parameters bool(true) array(6) { @@ -385,4 +393,7 @@ array(6) { NULL } Testing pgsqlCopyFromFile() with error -bool(false) +Exception: SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "test_error" does not exist +Testing pgsqlCopyFromFile() with non existing file +Exception: SQLSTATE[HY000]: General error: 7 Unable to open the file + diff --git a/ext/pdo_pgsql/tests/copy_to.phpt b/ext/pdo_pgsql/tests/copy_to.phpt index 1dc7d1de33..7bc46c6e0b 100644 --- a/ext/pdo_pgsql/tests/copy_to.phpt +++ b/ext/pdo_pgsql/tests/copy_to.phpt @@ -17,7 +17,6 @@ $db->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false); $db->exec('CREATE TABLE test (a integer not null primary key, b text, c integer)'); $db->beginTransaction(); -try { echo "Preparing test table for CopyTo tests\n"; $stmt = $db->prepare("INSERT INTO test (a, b, c) values (?, ?, ?)"); @@ -42,8 +41,11 @@ echo "Testing pgsqlCopyToArray() with only selected fields\n"; var_dump($db->pgsqlCopyToArray('test',";","NULL",'a,c')); echo "Testing pgsqlCopyToArray() with error\n"; -var_dump($db->pgsqlCopyToArray('test_error')); - +try { + var_dump($db->pgsqlCopyToArray('test_error')); +} catch (Exception $e) { + echo "Exception: {$e->getMessage()}\n"; +} echo "Testing pgsqlCopyToFile() with default parameters\n"; @@ -58,14 +60,19 @@ var_dump($db->pgsqlCopyToFile('test',$filename,";","NULL",'a,c')); echo file_get_contents($filename); echo "Testing pgsqlCopyToFile() with error\n"; -var_dump($db->pgsqlCopyToFile('test_error',$filename)); - +try { + var_dump($db->pgsqlCopyToFile('test_error',$filename)); +} catch (Exception $e) { + echo "Exception: {$e->getMessage()}\n"; +} +echo "Testing pgsqlCopyToFile() to unwritable file\n"; +try { + var_dump($db->pgsqlCopyToFile('test', 'nonexistent/foo.csv')); } catch (Exception $e) { - /* catch exceptions so that we can show the relative error */ - echo "Exception! at line ", $e->getLine(), "\n"; - var_dump($e->getMessage()); + echo "Exception: {$e->getMessage()}\n"; } + if(isset($filename)) { @unlink($filename); } @@ -109,7 +116,7 @@ array(3) { " } Testing pgsqlCopyToArray() with error -bool(false) +Exception: SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "test_error" does not exist Testing pgsqlCopyToFile() with default parameters bool(true) 0 test insert 0 \N @@ -126,4 +133,7 @@ bool(true) 1;NULL 2;NULL Testing pgsqlCopyToFile() with error -bool(false) \ No newline at end of file +Exception: SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "test_error" does not exist +Testing pgsqlCopyToFile() to unwritable file +Exception: SQLSTATE[HY000]: General error: 7 Unable to open the file for writing + -- 2.49.0