Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:52510
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.42 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:sender:in-reply-to:references:from:date
         :x-google-sender-auth:message-id:subject:to:cc:content-type
         :content-transfer-encoding;
        b=SXvbF2WXCRnN2kqqJQI2+7raM7tkxwodcUUMheoxIa+KJL5U+OgXcVzKF3zupDLPSP
         qrmP6H1CE2DpjZccjUE8kHkra4m7IEwWxj4LkZQoncIpCr+Wj8lhVFbYti9vwedMZvhH
         TzwxQwsUx/8IIYRkxjJojsFEjHcfZjSEGS9kY=
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <BANLkTimoP95r6PgtCrgByBvLOvriu-pdAw@mail.gmail.com>
References: <BANLkTik4GVWKdRxiZs+--tNdTsSD1QBNnA@mail.gmail.com> <BANLkTimoP95r6PgtCrgByBvLOvriu-pdAw@mail.gmail.com>
Date: Tue, 24 May 2011 13:37:00 +0900
Message-ID: <BANLkTikZF6gSWFD889rYWtzDLzuhvxnPTQ@mail.gmail.com>
To: Hannes Magnusson <hannes.magnusson@gmail.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH] pg_escape_literal/pg_escape_identifier for pgsql
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

2011/5/20 Hannes Magnusson <hannes.magnusson@gmail.com>:
> 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 pat=
ch 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.

I suppose it would be ancient PostgreSQL (6.5?) which would not have PQesca=
pe,
I'll fix it anyway.

>
> There is also very inconsistent usage of space vs tab in the patch.

For some reason, my emacs ignored the marker(?)
I'll fix it

>
> + =A0 =A0if (!tmp) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 php_error_docref(NULL TSRMLS_CC, E_WARNING,=
"Failed to escape");
> + =A0 =A0 =A0 =A0RETURN_FALSE;
> + =A0 =A0}
>
> Won't this also trigger the error if there was no from passed?
> I suppose checking from_len wouldn't be stupid.

This error occurs only if libpq could not allocate memory.
Empty string should be valid string for inserting db.

> Should we really provide fallback if PQescape(Literal|Identifier)
> isn't available? Did you copy the escaping from postgresql directly?

Wouldn't it nice to have the same escaping capability for 8.x?
It's not possible to copy the code, since it's binded to PostgreSQL's
multibyte function. It's possible to use mbstring function for thatbut it's
not default.

I also looked into PostgreSQL code to see if it's possible to bundle
libpq, but it's closely related to core and there is no easy way to
cut down.

Most code was copied from PostgreSQL and it works like original
function. As I wrote in comment, current implementation has limitation
for some multibyte encodings. However, it works for encoding that is
compatible to ISO-8859-1. Therefore, it works for almost all encodings.

Thanks for the comment.
Is there any comment?

--
Yasuo Ohgaki