Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-09-07 Thread Tom Lane
Alvaro Herrera from 2ndQuadrant  writes:
> Marked ready for committer.

Thanks for reviewing.  I adopted your doc change suggestions
and pushed it.

regards, tom lane




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-09-06 Thread Alvaro Herrera from 2ndQuadrant
On 2019-May-23, Tom Lane wrote:

> +   
> +Another nonstandard extension is that following the escape character
> +with a letter or digit provides access to the same escape sequences
> +defined for POSIX regular expressions, below (see
> +,
> +, and
> +).
> 

I think the word "same" in this para is more confusing than helpful;
also the tables are an integral part of this rather than just an
illustration, so they should not be in parenthesis but after only a
semicolon or such.  So:

> +Another nonstandard extension is that following the escape character
> +with a letter or digit provides access to the escape sequences
> +defined for POSIX regular expressions; see
> +,
> +, and
> + below.

I think it would be useful to provide a trivial example that illustrates
this in the  below; say '\mabc\M' not matching "zabc".

All in all, these are pretty trivial points and I would certainly not be
mad if it's committed without these changes.

Marked ready for committer.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-09-06 Thread Andrew Gierth
> "Tom" == Tom Lane  writes:

 > Alvaro Herrera from 2ndQuadrant  writes:
 >> This discussion seems to have died down.  Apparently we have three
 >> directions here, from three different people.  Are we doing anything?

 Tom> I don't really want to do anything beyond the patch I submitted in
 Tom> this thread (at <32617.1558649...@sss.pgh.pa.us>).  That's what the
 Tom> CF entry is for, IMO.

I have no issues with this approach.

 Tom> I'm not excited about the change-of-keywords business, but if
 Tom> someone else is, they should start a new CF entry about that.

It's enough of a can of worms that I don't feel inclined to mess with it
absent some good reason (the spec probably isn't a good enough reason).
If postfix operators should happen to go away at some point then this
can be revisited.

-- 
Andrew (irc:RhodiumToad)




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-09-06 Thread Tom Lane
Alvaro Herrera from 2ndQuadrant  writes:
> This discussion seems to have died down.  Apparently we have three
> directions here, from three different people.  Are we doing anything?

I don't really want to do anything beyond the patch I submitted in
this thread (at <32617.1558649...@sss.pgh.pa.us>).  That's what the
CF entry is for, IMO.  I'm not excited about the change-of-keywords
business, but if someone else is, they should start a new CF entry
about that.

regards, tom lane




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-09-06 Thread Alvaro Herrera from 2ndQuadrant
This discussion seems to have died down.  Apparently we have three
directions here, from three different people.  Are we doing anything?

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-23 Thread Tom Lane
I wrote:
> I propose therefore that we leave similar_escape in place with its
> current behavior, as a compatibility measure for cases like this.
> Intead, invent two new strict functions, say
>   similar_to_escape(pattern)
>   similar_to_escape(pattern, escape)
> and change the parser and the implementation of SUBSTRING() to
> rely on these going forward.

> The net effect will be to make explicit "ESCAPE NULL" spec-compliant,
> and to get rid of the performance problem from inlining failure for
> substring().  All else is just doc clarifications.

Here's a proposed patch for that.  I think it's a bit too late to be
messing with this kind of thing for v12, so I'll add this to the
upcoming CF.

regards, tom lane

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a79e7c0..7d4472a 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -4146,6 +4146,14 @@ cast(-44 as bit(12))   11010100

 

+According to the SQL standard, omitting ESCAPE
+means there is no escape character (rather than defaulting to a
+backslash), and a zero-length ESCAPE value is
+disallowed.  PostgreSQL's behavior in
+this regard is therefore slightly nonstandard.
+   
+
+   
 The key word ILIKE can be used instead of
 LIKE to make the match case-insensitive according
 to the active locale.  This is not in the SQL standard but is a
@@ -4280,9 +4288,27 @@ cast(-44 as bit(12))   11010100

 

-As with LIKE, a backslash disables the special meaning
-of any of these metacharacters; or a different escape character can
-be specified with ESCAPE.
+As with LIKE, a backslash disables the special
+meaning of any of these metacharacters.  A different escape character
+can be specified with ESCAPE, or the escape
+capability can be disabled by writing ESCAPE ''.
+   
+
+   
+According to the SQL standard, omitting ESCAPE
+means there is no escape character (rather than defaulting to a
+backslash), and a zero-length ESCAPE value is
+disallowed.  PostgreSQL's behavior in
+this regard is therefore slightly nonstandard.
+   
+
+   
+Another nonstandard extension is that following the escape character
+with a letter or digit provides access to the same escape sequences
+defined for POSIX regular expressions, below (see
+,
+, and
+).

 

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 8311b1d..6462e13 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -13073,15 +13073,15 @@ a_expr:		c_expr	{ $$ = $1; }
 
 			| a_expr SIMILAR TO a_expr			%prec SIMILAR
 {
-	FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),
-			   list_make2($4, makeNullAConst(-1)),
+	FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
+			   list_make1($4),
 			   @2);
 	$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~",
    $1, (Node *) n, @2);
 }
 			| a_expr SIMILAR TO a_expr ESCAPE a_expr			%prec SIMILAR
 {
-	FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),
+	FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
 			   list_make2($4, $6),
 			   @2);
 	$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~",
@@ -13089,15 +13089,15 @@ a_expr:		c_expr	{ $$ = $1; }
 }
 			| a_expr NOT_LA SIMILAR TO a_expr	%prec NOT_LA
 {
-	FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),
-			   list_make2($5, makeNullAConst(-1)),
+	FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
+			   list_make1($5),
 			   @2);
 	$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~",
    $1, (Node *) n, @2);
 }
 			| a_expr NOT_LA SIMILAR TO a_expr ESCAPE a_expr		%prec NOT_LA
 {
-	FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),
+	FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
 			   list_make2($5, $7),
 			   @2);
 	$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~",
@@ -14323,9 +14323,9 @@ subquery_Op:
 			| NOT_LA ILIKE
 	{ $$ = list_make1(makeString("!~~*")); }
 /* cannot put SIMILAR TO here, because SIMILAR TO is a hack.
- * the regular expression is preprocessed by a function (similar_escape),
+ * the regular expression is preprocessed by a function (similar_to_escape),
  * and the ~ operator for posix regular expressions is used.
- *x SIMILAR TO y ->x ~ similar_escape(y)
+ *x SIMILAR TO y ->x ~ similar_to_escape(y)
  * this transformation is made on the fly by the parser upwards.
  * however the SubLink structure which handles any/some/all stuff
  * is not ready for such a thing.
diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.c
index 90a9197..3d38aef 100644
--- a/src/backend/utils/adt/regexp.c
+++ b/src/back

Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-21 Thread Andrew Gierth
> "Robert" == Robert Haas  writes:

 Robert> But the number of people using out-of-core postfix operators
 Robert> has got to be really tiny -- unless, maybe, there's some really
 Robert> popular extension like PostGIS that uses them.

If there's any extension that uses them I've so far failed to find it.

For the record, the result of my Twitter poll was 29:2 in favour of
removing them, for what little that's worth.

-- 
Andrew (irc:RhodiumToad)




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-17 Thread Tom Lane
Robert Haas  writes:
> I think it's pretty clear that the theoretical beauty of being able to
> handle postfix operators is not worth the tangible cost they impose on
> our parser.  We're losing more users as a result of SQL that other
> systems can accept and we cannot than we are gaining by being able to
> support user-defined postfix operators.

I suppose it's possible to make such an argument, but you haven't
actually made one --- just asserted something without providing
evidence.

If we can lay out some concrete gains that justify zapping postfix
operators, I'd be willing to do it.  I agree that it would likely
hurt few users ... but we need to be able to explain to those few
why we broke it.  And show that the benefits outweigh the cost.

regards, tom lane




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-17 Thread Robert Haas
On Mon, May 13, 2019 at 2:39 PM Andrew Gierth
 wrote:
> Or we could kill off postfix operators...

/me helps Andrew hijack the thread.

We wouldn't even have to go that far.  We could just restrict it to a
specific list of operators that are hard-coded into the lexer and
parser, like say only '!'.

Even if we killed postfix operators completely, the number of users
who would be affected would probably be minimal, because the only
postfix operator we ship is for factorial, and realistically, that's
not exactly a critical thing for most users, especially considering
that our implementation is pretty slow.  But the number of people
using out-of-core postfix operators has got to be really tiny --
unless, maybe, there's some really popular extension like PostGIS that
uses them.

I think it's pretty clear that the theoretical beauty of being able to
handle postfix operators is not worth the tangible cost they impose on
our parser.  We're losing more users as a result of SQL that other
systems can accept and we cannot than we are gaining by being able to
support user-defined postfix operators.  The latter is not exactly a
mainstream need.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-14 Thread Tom Lane
[ backing up to a different sub-discussion ]

Andrew Gierth  writes:
> "Tom" == Tom Lane  writes:
>  Tom> To support the first usage, similar_escape is non-strict, and it
>  Tom> takes a NULL second argument to mean '\'. This is already a SQL
>  Tom> spec violation, because as far as I can tell from the spec, if you
>  Tom> don't write an ESCAPE clause then there is *no* escape character;
>  Tom> there certainly is not a default of '\'. However, we document this
>  Tom> behavior, so I don't know if we want to change it.

> This is the same spec violation that we also have for LIKE, which also
> is supposed to have no escape character in the absense of an explicit
> ESCAPE clause.

Right.  After further thought, I propose that what we ought to do is
unify LIKE, SIMILAR TO, and 3-arg SUBSTRING on a single set of behaviors
for the ESCAPE argument:

1. They are strict, ie a NULL value for the escape string produces a
NULL result.  This is per spec, and we don't document anything different,
and nobody would really expect something different.  (But see below
about keeping similar_escape() as a legacy compatibility function.)

2. Omitting the ESCAPE option (not possible for SUBSTRING) results in a
default of '\'.  This is not per spec, but we've long documented it this
way, and frankly I'd say that it's a far more useful default than the
spec's behavior of "there is no escape character".  I propose that we
just document that this is not-per-spec and move on.

3. Interpret an empty ESCAPE string as meaning "there is no escape
character".  This is not per spec either (the spec would have us
throw an error) but it's our historical behavior, and it seems like
a saner approach than the way the spec wants to do it --- in particular,
there's no way to get that behavior in 3-arg SUBSTRING if we don't allow
this.

So only point 1 represents an actual behavioral change from what we've
been doing; the other two just require doc clarifications.

Now, I don't have any problem with changing what happens when somebody
actually writes "a LIKE b ESCAPE NULL"; it seems fairly unlikely that
anyone would expect that to yield a non-null result.  However, we do
have a problem with the fact that the implementation is partially
exposed:

regression=# create view v1 as select f1 similar to 'x*' from text_tbl;
CREATE VIEW
regression=# \d+ v1
...
View definition:
 SELECT text_tbl.f1 ~ similar_escape('x*'::text, NULL::text)
   FROM text_tbl;

If we just change similar_escape() to be strict, then this view will
stop working, which is a bit hard on users who did not write anything
non-spec-compliant.

I propose therefore that we leave similar_escape in place with its
current behavior, as a compatibility measure for cases like this.
Intead, invent two new strict functions, say
similar_to_escape(pattern)
similar_to_escape(pattern, escape)
and change the parser and the implementation of SUBSTRING() to
rely on these going forward.

The net effect will be to make explicit "ESCAPE NULL" spec-compliant,
and to get rid of the performance problem from inlining failure for
substring().  All else is just doc clarifications.

Comments?

regards, tom lane




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-13 Thread Andrew Gierth
> "Tom" == Tom Lane  writes:

 Tom> Hmm.  Oddly, you can't fix it by adding parens:

 Tom> regression=# select 'foo' similar to ('f' || escape) escape escape from 
(values ('oo')) v(escape);
 Tom> psql: ERROR:  syntax error at or near "escape"
 Tom> LINE 1: select 'foo' similar to ('f' || escape) escape escape from (...
 Tom> ^

 Tom> Since "escape" is an unreserved word, I'd have expected that to
 Tom> work. Odd.

Simpler cases fail too:

select 'f' || escape from (values ('o')) v(escape);
psql: ERROR:  syntax error at or near "escape"

select 1 + escape from (values (1)) v(escape);  -- works
select 1 & escape from (values (1)) v(escape);  -- fails

in short ESCAPE can't follow any generic operator, because its lower
precedence forces the operator to be reduced as a postfix op instead.

 Tom> The big picture here is that fixing grammar ambiguities by adding
 Tom> precedence is a dangerous business :-(

Yeah. But the alternative is usually reserving words more strictly,
which has its own issues :-(

Or we could kill off postfix operators...

-- 
Andrew (irc:RhodiumToad)




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-13 Thread Tom Lane
Andrew Gierth  writes:
> "Andrew" == Andrew Gierth  writes:
>  Andrew> The ESCAPE part could in theory be ambiguous if the SIMILAR
>  Andrew> expression ends in a ... SIMILAR TO xxx operator, since then we
>  Andrew> wouldn't know whether to attach the ESCAPE to that or keep it
>  Andrew> as part of the function syntax. But I think this is probably a
>  Andrew> non-issue. More significant is that ... COLNAME ! ESCAPE ...
>  Andrew> again has postfix- vs. infix-operator ambiguities.

> And this ambiguity shows up already in other contexts:

> select 'foo' similar to 'f' || escape escape escape from (values ('oo')) 
> v(escape);
> psql: ERROR:  syntax error at or near "escape"
> LINE 1: select 'foo' similar to 'f' || escape escape escape from (va...


Hmm.  Oddly, you can't fix it by adding parens:

regression=# select 'foo' similar to ('f' || escape) escape escape from (values 
('oo')) v(escape);
psql: ERROR:  syntax error at or near "escape"
LINE 1: select 'foo' similar to ('f' || escape) escape escape from (...
^

Since "escape" is an unreserved word, I'd have expected that to work.
Odd.

The big picture here is that fixing grammar ambiguities by adding
precedence is a dangerous business :-(

regards, tom lane




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-13 Thread Andrew Gierth
> "Andrew" == Andrew Gierth  writes:

 Andrew> The ESCAPE part could in theory be ambiguous if the SIMILAR
 Andrew> expression ends in a ... SIMILAR TO xxx operator, since then we
 Andrew> wouldn't know whether to attach the ESCAPE to that or keep it
 Andrew> as part of the function syntax. But I think this is probably a
 Andrew> non-issue. More significant is that ... COLNAME ! ESCAPE ...
 Andrew> again has postfix- vs. infix-operator ambiguities.

And this ambiguity shows up already in other contexts:

select 'foo' similar to 'f' || escape escape escape from (values ('oo')) 
v(escape);
psql: ERROR:  syntax error at or near "escape"
LINE 1: select 'foo' similar to 'f' || escape escape escape from (va...

select 'foo' similar to 'f' || escape escape from (values ('oo')) v(escape);
psql: ERROR:  operator does not exist: unknown ||
LINE 1: select 'foo' similar to 'f' || escape escape from (values ('...

I guess this happens because ESCAPE has precedence below POSTFIXOP, so
the ('f' ||) gets reduced in preference to shifting in the first ESCAPE
token.

-- 
Andrew (irc:RhodiumToad)




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-13 Thread Andrew Gierth
> "Andrew" == Andrew Gierth  writes:

 Tom> I am, frankly, inclined to ignore this as a bad idea. We do have
 Tom> SIMILAR and ESCAPE as keywords already, but they're
 Tom> type_func_name_keyword and unreserved_keyword respectively. To
 Tom> support this syntax, I'm pretty sure we'd have to make them both
 Tom> fully reserved.

 Andrew> I only did a quick trial but it doesn't seem to require
 Andrew> reserving them more strictly - just adding the obvious
 Andrew> productions to the grammar doesn't introduce any conflicts.

Digging deeper, that's because both SIMILAR and ESCAPE have been
assigned precedence. Ambiguities that exist include:

   ... COLNAME ! SIMILAR ( ...

which could be COLNAME postfix-op SIMILAR a_expr, or COLNAME infix-op
function-call. Postfix operators strike again... we really should kill
those off.

The ESCAPE part could in theory be ambiguous if the SIMILAR expression
ends in a ... SIMILAR TO xxx operator, since then we wouldn't know
whether to attach the ESCAPE to that or keep it as part of the function
syntax. But I think this is probably a non-issue. More significant is
that ... COLNAME ! ESCAPE ... again has postfix- vs. infix-operator
ambiguities.

-- 
Andrew (irc:RhodiumToad)




Re: SQL-spec incompatibilities in similar_escape() and related stuff

2019-05-12 Thread Andrew Gierth
> "Tom" == Tom Lane  writes:

 Tom> but in recent versions it's

 Tom>  ::=
 Tom>   SUBSTRING  
 Tom>  SIMILAR 
 Tom>  ESCAPE  

 Tom> I am, frankly, inclined to ignore this as a bad idea. We do have
 Tom> SIMILAR and ESCAPE as keywords already, but they're
 Tom> type_func_name_keyword and unreserved_keyword respectively. To
 Tom> support this syntax, I'm pretty sure we'd have to make them both
 Tom> fully reserved.

I only did a quick trial but it doesn't seem to require reserving them
more strictly - just adding the obvious productions to the grammar
doesn't introduce any conflicts.

 Tom> * Our function similar_escape() is not documented, but it
 Tom> underlies three things in the grammar:

 Tom>   a SIMILAR TO b
 Tom>   Translated as "a ~ similar_escape(b, null)"

 Tom>   a SIMILAR TO b ESCAPE e
 Tom>   Translated as "a ~ similar_escape(b, e)"

 Tom>   substring(a, b, e)
 Tom>   This is a SQL function expanding to
 Tom>   select pg_catalog.substring($1, pg_catalog.similar_escape($2, $3))

 Tom> To support the first usage, similar_escape is non-strict, and it
 Tom> takes a NULL second argument to mean '\'. This is already a SQL
 Tom> spec violation, because as far as I can tell from the spec, if you
 Tom> don't write an ESCAPE clause then there is *no* escape character;
 Tom> there certainly is not a default of '\'. However, we document this
 Tom> behavior, so I don't know if we want to change it.

This is the same spec violation that we also have for LIKE, which also
is supposed to have no escape character in the absense of an explicit
ESCAPE clause.

-- 
Andrew (irc:RhodiumToad)