# PATCH for bug #47199 - Thread: https://externals.io/message/46965 - Original archive: https://news-web.php.net/php.internals/46965 - Started by ewgraf@gmail.com on 2010-02-09T11:27:21+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #46965 — ewgraf@gmail.com — 2010-02-09T11:27:21+00:00 URL: https://externals.io/message/46965#46965 Index: tests/27_bug47199.phpt =================================================================== --- tests/27_bug47199.phpt (revision 0) +++ tests/27_bug47199.phpt (revision 0) @@ -0,0 +1,23 @@ +--TEST-- +Bug #47199 pg_delete fails on `NULL` +--SKIPIF-- + +--FILE-- + 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 WHERE null_field IS NULL AND not_null_field=2; +Done \ No newline at end of file Property changes on: tests/27_bug47199.phpt ___________________________________________________________________ Added: svn:executable + * Index: pgsql.c =================================================================== --- pgsql.c (revision 294790) +++ 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, ';'); --- ## #46966 — johannes@php.net — 2010-02-09T13:56:46+00:00 URL: https://externals.io/message/46965#46966 · In reply to: #46965 Евгений, thanks for the patch.I didn't fully review it but I wonder about a thing: > Index: tests/27_bug47199.phpt > =================================================================== > --- tests/27_bug47199.phpt (revision 0) > +++ tests/27_bug47199.phpt (revision 0) [...9 > +--EXPECTF-- > +DELETE FROM WHERE null_field IS NULL AND > not_null_field=2; > +Done > \ No newline at end of file did you really run this test and was it successful? - doesn't looklike for me. > Property changes on: tests/27_bug47199.phpt > ___________________________________________________________________ > Added: svn:executable > + * This isn't needed. And please also add a link to the patch to the bug report, so it won't get lost, which can happen on a mailing list too easy. johannes --- ## #46968 — Sokolov Evgeniy — 2010-02-09T15:11:14+00:00 URL: https://externals.io/message/46965#46968 · In reply to: #46966 > 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? > > […] > This isn't needed. Okey, I just use previous test in this dir as example. Remove this property > 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 2010/2/9 Johannes Schlüter : > Евгений, > > thanks for the patch.I didn't fully review it but I wonder about a > thing: > > > > […] > [...9 > > […] > > did you really run this test and was it successful? - doesn't looklike > for me. > > > […] > > This isn't needed. > > And please also add a link to the patch to the bug report, so it won't > get lost, which can happen on a mailing list too easy. > > johannes > > best regards --- ## #46969 — johannes@php.net — 2010-02-09T15:38:44+00:00 URL: https://externals.io/message/46965#46969 · In reply to: #46968 Hi, > > […] > 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! > > […] > 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 --- ## #46971 — Daniel Convissor — 2010-02-09T19:08:51+00:00 URL: https://externals.io/message/46965#46971 · In reply to: #46968 Hi Skolov: > I just run "php tests/27_bug47199.phpt" and check the result output. > Is this correct way? Use an EXPECT or EXPECTF section. See http://qa.php.net/phpt_details.php for more information. Thanks, --Dan -- T H E A N A L Y S I S A N D S O L U T I O N S C O M P A N Y data intensive web and database programming http://www.AnalysisAndSolutions.com/ 4015 7th Ave #4, Brooklyn NY 11232 v: 718-854-0335 f: 718-854-0409 --- ## #46974 — Sokolov Evgeniy — 2010-02-10T16:37:36+00:00 URL: https://externals.io/message/46965#46974 · In reply to: #46969 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-- + +--FILE-- + 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 : > Hi, > > > > […] > > 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! > > > […] > > 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 --- ## #46975 — Pierre Joye — 2010-02-10T16:43:31+00:00 URL: https://externals.io/message/46965#46975 · In reply to: #46974 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 : > 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-- > + +require_once('skipif.inc'); > +?> > +--FILE-- > + + > +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 : > > […] > > > best regards > > -- > > > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #46976 — Sokolov Evgeniy — 2010-02-10T16:55:13+00:00 URL: https://externals.io/message/46965#46976 · In reply to: #46975 No problem. I will remember for the future 2010/2/10 Pierre Joye : > 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 : > > […] > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org > -- -- С уважением, Соколов Евгений --- ## #46977 — Sokolov Evgeniy — 2010-02-10T17:12:46+00:00 URL: https://externals.io/message/46965#46977 · In reply to: #46976 Oups. Haven't noticed the attachment. I will try to attach the file again. 2010/2/10 Sokolov Evgeniy : > No problem. I will remember for the future > > 2010/2/10 Pierre Joye : > > […] best regards