[aur-dev] [PATCH 1/1] use convenience wrapper for mysql_real_escape_string to aid database portability

[Lukas Fleischer]

On Tue, Apr 26, 2011 at 10:42:58AM -0500, Dan McGee wrote:
> On Tue, Apr 26, 2011 at 10:10 AM, Lukas Fleischer
> <archlinux at cryptocrack.de> wrote:
> > On Mon, Apr 25, 2011 at 10:21:55PM -0700, elij wrote:
> >> when converting to postgres, each mysql_real_escape_string instance had to be
> >> changed, which was tedious. Centralizing the escape mechanism code would allow
> >> for much easier porting, in the same way that db_query provides a lightweight
> >> query abstraction.
> >> ---
> >>  web/html/account.php              |    2 +-
> >>  web/html/addvote.php              |   10 +++++-----
> >>  web/html/logout.php               |    2 +-
> >>  web/html/passreset.php            |    4 ++--
> >>  web/html/pkgsubmit.php            |   36 ++++++++++++++++++------------------
> >>  web/html/voters.php               |    2 +-
> >>  web/lib/acctfuncs.inc             |   26 +++++++++++++-------------
> >>  web/lib/aur.inc                   |   30 ++++++++++++++++++------------
> >>  web/lib/aurjson.class.php         |    8 ++++----
> >>  web/lib/pkgfuncs.inc              |   12 ++++++------
> >>  web/lib/stats.inc                 |    2 +-
> >>  web/template/pkg_comment_form.php |    2 +-
> >>  12 files changed, 71 insertions(+), 65 deletions(-)
> >>
> >
> > Sounds like a good idea as well, but I'm not sure if this makes a lot of
> > sense if we keep any other mysql_*() invocations. I'd say we should use
> > some proper database abstraction layers if we aim at database
> > independent code...
> 
> Well, one has to start somewhere to get said abstraction layer, and
> this seems like as good as any start. It is the most-used function
> [1], with "result row processing functions" coming in next. So this
> patch gets a +1 from me, and eventually we can have a set of db_*
> functions we are able to use, can move to a db_mysql.inc file, and go
> from there.

Well, what I wanted to say is that I'd prefer to have that bundled with
another bunch of patches before merging it into master. Just as if we'd
switch from MySQL functions to MDB2 or any other existant abstraction
layer, migrating small parts only does not make any sense here. I am
pretty sure that this will be forgotten if I just apply that patch. And
having some mixture of native MySQL functions and database abstraction
isn't any better than using native MySQL functions only.

> 
> -Dan
> 
> [1] dmcgee at clifden ~/projects/aur (master)
> $ find -name '*.php' | xargs grep -R --color -o -h 'mysql_[^( ]*(' * |
> sort | uniq -c | sort -n
>       1 mysql_connect(
>       1 mysql_init(
>       1 mysql_library_end(
>       1 mysql_library_init(
>       1 mysql_real_connect(
>       1 mysql_store_result(
>       2 mysql_affected_rows(
>       2 mysql_close(
>       2 mysql_insert_id(
>       3 mysql_fetch_array(
>       3 mysql_free_result(
>       4 mysql_fetch_object(
>       5 mysql_result(
>       5 mysql_select_db(
>       6 mysql_error(
>      10 mysql_die(
>      10 mysql_query(
>      25 mysql_fetch_row(
>      36 mysql_fetch_assoc(
>      38 mysql_num_rows(
>      99 mysql_real_escape_string(

Not that it makes a huge difference but your stats are somewhat
confusing - especially since you seem to use find(1) and xargs(1)
without doing any replacement in the grep(1) invocation (note the
wildcard character used with grep(1) here).

Searching in the actual source files only seems to me to be more
appropriate:

----
$ grep -oh 'mysql_[^( ]*(' $(git ls-files) | sort | uniq -c | sort -n
      1 mysql_affected_rows(
      1 mysql_autocommit(
      1 mysql_connect(
      1 mysql_init(
      1 mysql_insert_id(
      1 mysql_library_end(
      1 mysql_library_init(
      1 mysql_real_connect(
      1 mysql_store_result(
      2 mysql_close(
      2 mysql_fetch_object(
      2 mysql_free_result(
      3 mysql_fetch_array(
      3 mysql_select_db(
      5 mysql_result(
      6 mysql_error(
     10 mysql_query(
     13 mysql_die(
     22 mysql_fetch_assoc(
     22 mysql_num_rows(
     25 mysql_fetch_row(
     66 mysql_real_escape_string(
----