Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:52474
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.54 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :cc:content-type;
        b=s/EmIalYP+mFXCRFya4tT0eGNcM/jBlaaglFzqyvFDuwd17WjKMZBeqoG+4Zq25bt6
         L6pNIfyJ9ZH7a9bBEFvx8UY6ImJTQj75i1lFaUhseE5cjBTmFOgF9UzlyWKPrYNOoPzR
         ka+NfjUyQqrWqbqvOI7rISJxmDaVdwvVggjhQ=
MIME-Version: 1.0
In-Reply-To: <BANLkTik4GVWKdRxiZs+--tNdTsSD1QBNnA@mail.gmail.com>
References: <BANLkTik4GVWKdRxiZs+--tNdTsSD1QBNnA@mail.gmail.com>
Date: Fri, 20 May 2011 12:35:56 +0200
Message-ID: <BANLkTimoP95r6PgtCrgByBvLOvriu-pdAw@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [PATCH] pg_escape_literal/pg_escape_identifier for pgsql
From: hannes.magnusson@gmail.com (Hannes Magnusson)

On Fri, May 20, 2011 at 03:24, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi all
>
> Current PostgreSQL has new escape functions for literals and identifiers.
> Since there is no function for that, I made patch for that. Attached patch is
> for trunk and tested with PostgreSQL 8.4 and 9.0.
>
> This patch would also applied to 5.4 branch but how about 5.3?
> It's pain not to have escape functions.
>
> Any comments?

This won't compile if HAVE_PQESCAPE isn't declared since you only
register the arginfo when it is, but use it even if it wasn't.

There is also very inconsistent usage of space vs tab in the patch.

+    if (!tmp) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING,"Failed to escape");
+        RETURN_FALSE;
+    }

Won't this also trigger the error if there was no from passed?
I suppose checking from_len wouldn't be stupid.

Should we really provide fallback if PQescape(Literal|Identifier)
isn't available? Did you copy the escaping from postgresql directly?

-Hannes