Lists: | pgsql-hackerspgsql-patches |
---|
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | teodor(at)sigaev(dot)ru |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Use of backslash in tsearch2 |
Date: | 2006-08-22 00:04:01 |
Message-ID: | 200608220004.k7M041804370@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Bruce Momjian wrote:
>
> I backed out the patch, attached, and it has fixed the regression
> problem. What has me confused is that is looks like it is checking for
> ', then putting \, which doesn't make a lot of sense, but the regression
> output is corrected, so I just don't get it. Here is an example:
>
> test=> SELECT E'''1 \\''2''';
> ?column?
> ----------
> '1 \'2'
>
> My only guess is that the output is somehow a single-quoted string
> itself, and in fact \' should become ''. Is that right? Basically they
> are doing \' in their output, and it should be doing '', but then the
> query above would be wrong and shouldn't be using \'.
As part of the move to support standard-conforming strings and treat
backslash literally, I reviewed the tsearch2 code and found two place
that seemed to use \' rather than '', and generated the attached patch.
('' is standards conforming.) However, when I fixed the code, the
regression tests failed.
Teodor, are the new attached regression results correct? If so, I will
apply the patch and update the expected file.
--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/bjm/diff | text/x-diff | 1.3 KB |
/rtmp/regression.diffs | text/x-diff | 1.9 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | teodor(at)sigaev(dot)ru, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: Use of backslash in tsearch2 |
Date: | 2006-08-22 00:15:28 |
Message-ID: | 26847.1156205728@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Bruce Momjian <bruce(at)momjian(dot)us> writes:
> As part of the move to support standard-conforming strings and treat
> backslash literally, I reviewed the tsearch2 code and found two place
> that seemed to use \' rather than '', and generated the attached patch.
I thought we had decided that that code should not be changed. It has
nothing to do with SQL literals, and changing it will create unnecessary
backwards-compatibility problems.
regards, tom lane
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org, teodor(at)sigaev(dot)ru, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: Use of backslash in tsearch2 |
Date: | 2006-08-22 00:28:27 |
Message-ID: | 200608220028.k7M0SRu07606@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > As part of the move to support standard-conforming strings and treat
> > backslash literally, I reviewed the tsearch2 code and found two place
> > that seemed to use \' rather than '', and generated the attached patch.
>
> I thought we had decided that that code should not be changed. It has
> nothing to do with SQL literals, and changing it will create unnecessary
> backwards-compatibility problems.
I don't remember any comment regarding that. I think it does relate to
SQL literals because it is creating a literal inside a literal. Also, at
the time this was a core-only discussion and I am hoping from a comment
from the tsearch2 folks.
--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | teodor(at)sigaev(dot)ru, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: Use of backslash in tsearch2 |
Date: | 2006-08-22 00:48:06 |
Message-ID: | 200608220048.k7M0m6x10224@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Bruce Momjian wrote:
> Bruce Momjian wrote:
> >
> > I backed out the patch, attached, and it has fixed the regression
> > problem. What has me confused is that is looks like it is checking for
> > ', then putting \, which doesn't make a lot of sense, but the regression
> > output is corrected, so I just don't get it. Here is an example:
> >
> > test=> SELECT E'''1 \\''2''';
> > ?column?
> > ----------
> > '1 \'2'
> >
> > My only guess is that the output is somehow a single-quoted string
> > itself, and in fact \' should become ''. Is that right? Basically they
> > are doing \' in their output, and it should be doing '', but then the
> > query above would be wrong and shouldn't be using \'.
>
> As part of the move to support standard-conforming strings and treat
> backslash literally, I reviewed the tsearch2 code and found two place
> that seemed to use \' rather than '', and generated the attached patch.
> ('' is standards conforming.) However, when I fixed the code, the
> regression tests failed.
>
> Teodor, are the new attached regression results correct? If so, I will
> apply the patch and update the expected file.
Updated patch attached. The previous one was reversed.
--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/pgpatches/tsearch2 | text/x-diff | 1.3 KB |
From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Use of backslash in tsearch2 |
Date: | 2006-08-22 07:50:14 |
Message-ID: | 44EAB736.8020405@sigaev.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
>> Teodor, are the new attached regression results correct? If so, I will
>> apply the patch and update the expected file.
Patch isn't full, simple test (values are took from regression.diffs):
# create table tt (tv tsvector, tq tsquery);
# insert into tt values (E'''1 \\''2''', NULL);
# insert into tt values (E'''1 \\''2''3', NULL);
# insert into tt values ( E'''1 \\''2'' 3', NULL);
# insert into tt values ( E'''1 \\''2'' '' 3'' 4 ', NULL);
# insert into tt values ( NULL, E'''1 \\''2''');
# insert into tt values ( NULL, E'''1 \\''2''');
# insert into tt values ( NULL, E'1&(''2''&('' 4''&(\\|5 | ''6 \\'' !|&'')))');
# insert into tt values ( NULL, E'1&(''2''&('' 4''&(\\|5 | ''6 \\'' !|&'')))');
and try dump table and restore:
ERROR: syntax error
CONTEXT: COPY tt, line 5, column tq: "'1 ''2'"
PS I'm not subscribed to -patches, so I post to -hackers
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
---|---|
To: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Use of backslash in tsearch2 |
Date: | 2006-08-23 12:08:12 |
Message-ID: | 44EC452C.7080205@sigaev.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
> Patch isn't full, simple test (values are took from regression.diffs):
> and try dump table and restore:
> ERROR: syntax error
> CONTEXT: COPY tt, line 5, column tq: "'1 ''2'"
>
Attached cumulative patch fixes problem, but I have some doubts, is it really
needed?
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
Attachment | Content-Type | Size |
---|---|---|
ttt.gz | application/x-tar | 1.4 KB |
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Use of backslash in tsearch2 |
Date: | 2006-09-02 22:04:50 |
Message-ID: | 200609022204.k82M4o712559@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers pgsql-patches |
Thanks. Yes, it is need for two reasons. In 8.2 you can set
standard_conforming_strings to on, meaning \' is really treated as \ and
', and because some encodings now can't support \' for security reasons,
though I don't think tsearch2 supports those multibyte encodings.
Anyway, applied to 8.2 only, not backpatched. Thanks.
---------------------------------------------------------------------------
Teodor Sigaev wrote:
> > Patch isn't full, simple test (values are took from regression.diffs):
> > and try dump table and restore:
> > ERROR: syntax error
> > CONTEXT: COPY tt, line 5, column tq: "'1 ''2'"
> >
>
> Attached cumulative patch fixes problem, but I have some doubts, is it really
> needed?
>
>
> --
> Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
> WWW: http://www.sigaev.ru/
[ application/x-tar is not supported, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +