Re: PATCH for bug #47199

Oups. Haven't noticed the attachment. I will try to attach the file again. 2010/2/10 Sokolov Evgeniy <ewgraf@gmail.com>: > No problem. I will remember for the future > > 2010/2/10 Pierre Joye <pierre.php@gmail.com>: >> Can you attach the patch instead of copying it in your post? Simply >> name it mypatch.txt (.txt is important for the list). >> >> 2010/2/10 Sokolov Evgeniy <ewgraf@gmail.com>: >>> Thank you very much. This patch is passed, also as other patches from >>> this extension. It's time for choose next bug :) >>> >>> >>> >>> Index: ext/pgsql/tests/bug47199.phpt >>> =================================================================== >>> --- ext/pgsql/tests/bug47199.phpt       (revision 0) >>> +++ ext/pgsql/tests/bug47199.phpt       (revision 0) >>> @@ -0,0 +1,23 @@ >>> +--TEST-- >>> +Bug #47199  pg_delete fails on NULL >>> +--SKIPIF-- >>> +<?php >>> +require_once('skipif.inc'); >>> +?> >>> +--FILE-- >>> +<?php >>> + >>> +require_once('config.inc'); >>> + >>> +$dbh = @pg_connect($conn_str); >>> +$tbl_name = 'test_47199'; >>> +@pg_query("DROP TABLE $tbl_name"); >>> +pg_query("CREATE TABLE $tbl_name (null_field INT, not_null_field INT)"); >>> +print(pg_delete($dbh, $tbl_name, array('null_field' => NULL, >>> 'not_null_field' => 2),PGSQL_DML_STRING)); >>> +@pg_query("DROP TABLE $tbl_name"); >>> +pg_close($dbh); >>> +echo PHP_EOL."Done".PHP_EOL; >>> +?> >>> +--EXPECTF-- >>> +DELETE FROM %s WHERE null_field IS NULL AND not_null_field=2; >>> +Done >>> Index: ext/pgsql/pgsql.c >>> =================================================================== >>> --- ext/pgsql/pgsql.c   (revision 294790) >>> +++ ext/pgsql/pgsql.c   (working copy) >>> @@ -5523,7 +5523,7 @@ >>>  } >>>  /* }}} */ >>> >>> -static inline int build_assignment_string(smart_str *querystr, >>> HashTable *ht, const char *pad, int pad_len TSRMLS_DC) >>> +static inline int build_assignment_string(smart_str *querystr, >>> HashTable *ht, int where_cond, const char *pad, int pad_len TSRMLS_DC) >>>  { >>>       HashPosition pos; >>>       uint fld_len; >>> @@ -5542,8 +5542,13 @@ >>>                       return -1; >>>               } >>>               smart_str_appendl(querystr, fld, fld_len - 1); >>> -               smart_str_appendc(querystr, '='); >>> >>> +               if (where_cond && Z_TYPE_PP(val) == IS_STRING && >>> !strcmp(Z_STRVAL_PP(val), "NULL")) { >>> +                       smart_str_appends(querystr, " IS "); >>> +               } else { >>> +                       smart_str_appendc(querystr, '='); >>> +               } >>> + >>>               switch(Z_TYPE_PP(val)) { >>>                       case IS_STRING: >>>                               smart_str_appendl(querystr, >>> Z_STRVAL_PP(val), Z_STRLEN_PP(val)); >>> @@ -5604,12 +5609,12 @@ >>>       smart_str_appends(&querystr, table); >>>       smart_str_appends(&querystr, " SET "); >>> >>> -       if (build_assignment_string(&querystr, Z_ARRVAL_P(var_array), >>> ",", 1 >>> TSRMLS_CC)) >>> +       if (build_assignment_string(&querystr, Z_ARRVAL_P(var_array), 0, >>> ",", 1 TSRMLS_CC)) >>>               goto cleanup; >>> >>>       smart_str_appends(&querystr, " WHERE "); >>> >>> -       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND >>> ", sizeof(" AND ")-1 TSRMLS_CC)) >>> +       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " >>> AND ", sizeof(" AND ")-1 TSRMLS_CC)) >>>               goto cleanup; >>> >>>       smart_str_appendc(&querystr, ';'); >>> @@ -5705,7 +5710,7 @@ >>>       smart_str_appends(&querystr, table); >>>       smart_str_appends(&querystr, " WHERE "); >>> >>> -       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND >>> ", sizeof(" AND ")-1 TSRMLS_CC)) >>> +       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " >>> AND ", sizeof(" AND ")-1 TSRMLS_CC)) >>>               goto cleanup; >>> >>>       smart_str_appendc(&querystr, ';'); >>> @@ -5841,7 +5846,7 @@ >>>       smart_str_appends(&querystr, table); >>>       smart_str_appends(&querystr, " WHERE "); >>> >>> -       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND >>> ", sizeof(" AND ")-1 TSRMLS_CC)) >>> +       if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " >>> AND ", sizeof(" AND ")-1 TSRMLS_CC)) >>>               goto cleanup; >>> >>>       smart_str_appendc(&querystr, ';'); >>> >>> >>> >>> >>> 2010/2/9 Johannes Schlüter <johannes@php.net>: >>>> Hi, >>>> >>>> On Tue, 2010-02-09 at 20:11 +0500, Sokolov Evgeniy wrote: >>>>> > did you really run this test and was it successful? - doesn't >>>>> > looklike >>>>> > for me. >>>>> I just run "php tests/27_bug47199.phpt" and check the result output. >>>>> Is this correct way? >>>> >>>> The easy way to run it using the test Framework is by executing >>>> >>>>   make test TESTS=tests/27_bug47199.phpt >>>> >>>> from your build directory which will prepare some environment things and >>>> then run run-tests.php from PHP's source directory which has the >>>> implementation of our test system, you can also run it directly to have >>>> more ocntrol. See README.TESTING in the php-src root folder and >>>> http://qa.php.net/write-test.php for more details >>>> on the test system. >>>> >>>> If you run your test it will tell you it failed as you're tests EXPECTF >>>> section has to look something like this: >>>> >>>> --EXPECTF-- >>>> DELETE FROM %s WHERE null_field IS NULL AND not_null_field=2; >>>> Done >>>> >>>> Mind the %s place holder there! >>>> >>>>> > And please also add a link to the patch to the bug report >>>>> Can it be link on pastebin.org? For example >>>>> http://pastebin.org/88880 >>>> >>>> I don't know how persistent these pastebins are, >>>> http://news.php.net/php.internals/46965 might be an >>>> alternative link >>>> though :-) >>>> >>>> johannes >>> >>> >>> best regards >>> >>> -- >>> PHP Internals - PHP Runtime Development Mailing List >>> To unsubscribe, visit: http://www.php.net/unsub.php >>> >>> >> >> >> >> -- >> Pierre >> >> @pierrejoye | http://blog.thepimp.net | >> http://www.libgd.org best regards

Index: ext/pgsql/tests/bug47199.phpt =================================================================== --- ext/pgsql/tests/bug47199.phpt (revision 0) +++ ext/pgsql/tests/bug47199.phpt (revision 0) @@ -0,0 +1,23 @@ +--TEST-- +Bug #47199 pg_delete fails on NULL +--SKIPIF-- +<?php +require_once('skipif.inc'); +?> +--FILE-- +<?php + +require_once('config.inc'); + +$dbh = @pg_connect($conn_str); +$tbl_name = 'test_47199'; +@pg_query("DROP TABLE $tbl_name"); +pg_query("CREATE TABLE $tbl_name (null_field INT, not_null_field INT)"); +print(pg_delete($dbh, $tbl_name, array('null_field' => NULL, 'not_null_field' => 2),PGSQL_DML_STRING)); +@pg_query("DROP TABLE $tbl_name"); +pg_close($dbh); +echo PHP_EOL."Done".PHP_EOL; +?> +--EXPECTF-- +DELETE FROM %s WHERE null_field IS NULL AND not_null_field=2; +Done Index: ext/pgsql/pgsql.c =================================================================== --- ext/pgsql/pgsql.c (revision 294790) +++ ext/pgsql/pgsql.c (working copy) @@ -5523,7 +5523,7 @@ } /* }}} */ -static inline int build_assignment_string(smart_str *querystr, HashTable *ht, const char *pad, int pad_len TSRMLS_DC) +static inline int build_assignment_string(smart_str *querystr, HashTable *ht, int where_cond, const char *pad, int pad_len TSRMLS_DC) { HashPosition pos; uint fld_len; @@ -5542,8 +5542,13 @@ return -1; } smart_str_appendl(querystr, fld, fld_len - 1); - smart_str_appendc(querystr, '='); + if (where_cond && Z_TYPE_PP(val) == IS_STRING && !strcmp(Z_STRVAL_PP(val), "NULL")) { + smart_str_appends(querystr, " IS "); + } else { + smart_str_appendc(querystr, '='); + } + switch(Z_TYPE_PP(val)) { case IS_STRING: smart_str_appendl(querystr, Z_STRVAL_PP(val), Z_STRLEN_PP(val)); @@ -5604,12 +5609,12 @@ smart_str_appends(&querystr, table); smart_str_appends(&querystr, " SET "); - if (build_assignment_string(&querystr, Z_ARRVAL_P(var_array), ",", 1 TSRMLS_CC)) + if (build_assignment_string(&querystr, Z_ARRVAL_P(var_array), 0, ",", 1 TSRMLS_CC)) goto cleanup; smart_str_appends(&querystr, " WHERE "); - if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND ", sizeof(" AND ")-1 TSRMLS_CC)) + if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1 TSRMLS_CC)) goto cleanup; smart_str_appendc(&querystr, ';'); @@ -5705,7 +5710,7 @@ smart_str_appends(&querystr, table); smart_str_appends(&querystr, " WHERE "); - if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND ", sizeof(" AND ")-1 TSRMLS_CC)) + if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1 TSRMLS_CC)) goto cleanup; smart_str_appendc(&querystr, ';'); @@ -5841,7 +5846,7 @@ smart_str_appends(&querystr, table); smart_str_appends(&querystr, " WHERE "); - if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), " AND ", sizeof(" AND ")-1 TSRMLS_CC)) + if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1 TSRMLS_CC)) goto cleanup; smart_str_appendc(&querystr, ';');