public inbox for [email protected]
help / color / mirror / Atom feedFrom: Gurjeet Singh <[email protected]>
To: Vik Fearing <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Isaac Morland <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Postgres Hackers <[email protected]>
Subject: Re: Named Operators
Date: Sat, 14 Jan 2023 06:14:02 -0800
Message-ID: <CABwTF4VEGbnZAiC8LX_MTc0Yw+co0g-T+KNt0SwdMtPkiHsJLA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CABwTF4VVuacLV+coERacdDxUXg5tDMHZxvgH5+ek0UZvoq386g@mail.gmail.com>
<CAEze2WjeYjcf-bm_4gggK7PQ+9fQQ6=hzGeYxTRY9r+D-BPt8g@mail.gmail.com>
<CABwTF4Uh3FUK6_9mj_aszciLt-CU0eYKiCdaL1nXShjxaHhbqQ@mail.gmail.com>
<CAMsGm5d3mBF=PQiHKV0ptXNiMW34aZ3u=44ams7ObWbZYfsRLQ@mail.gmail.com>
<[email protected]>
<[email protected]>
On Thu, Jan 12, 2023 at 5:55 AM Matthias van de Meent
<[email protected]> wrote:
> On Thu, 12 Jan 2023 at 11:59, Gurjeet Singh <[email protected]> wrote:
> > ... defining an operator
> > gives you many additional features that the planner can use to
> > optimize your query differently, which it can't do with functions. See
> > the COMMUTATOR, HASHES, etc. clause in the CREATE OPERATOR command.
>
> I see. Wouldn't it be better then to instead make it possible for the
> planner to detect the use of the functions used in operators and treat
> them as aliases of the operator? Or am I missing something w.r.t.
> differences between operator and function invocation?
>
> E.g. indexes on `int8pl(my_bigint, 1)` does not match queries for
> `my_bigint + 1` (and vice versa), while they should be able to support
> that, as OPERATOR(pg_catalog.+(int8, int8)) 's function is int8pl.
Such a feature would be immensely useful in its own right. But it's
also going to be at least 2 orders of magnitude (or more) effort to
implement, and to get accepted in the community. I'm thinking of
changes in planner, catalogs, etc.
On Thu, Jan 12, 2023 at 7:21 AM Tom Lane <[email protected]> wrote:
> Matthias van de Meent <[email protected]> writes:
> > I'm -1 on the chosen syntax; :name: shadows common variable
> > substitution patterns including those of psql.
>
> Yeah, this syntax is DOA because of that. I think almost
> anything you might invent is going to have conflict risks.
I remember discussing this in a meeting with Joe Conway a few weeks
ago, when this was just a proposal in my head and I was just bouncing
it off him. And I remember pointing out that colons would be a bad
choice because of their use in psql; but for life of me I can't think
of a reason (except temporary memory loss) why I failed to consider
the psql conflict when implementing the feature. If only some test in
`make check` would have pointed out the mistake, I wouldn't have made
this obvious mistake.
> We could probably make it work by allowing the existing OPERATOR
> syntax to take things that look like names as well as operators,
> like
>
> expr3 OPERATOR(contains_all) expr4
>
> But that's bulky enough that nobody will care to use it.
+1. Although that'd be better for readers than the all-special-char
names, this format is bulky enough that you won't be able to convince
the query writers to bother using it. But if all other efforts fail,
I'll take this format over the cryptic ones any day.
> On the whole I don't see this proposal going anywhere.
> There's too much investment in the existing operator names,
> and too much risk of conflicts if you try to shorten the
> syntax.
I wouldn't give up on the idea, yet :-) See new proposal below.
On Thu, Jan 12, 2023 at 9:14 AM Tom Lane <[email protected]> wrote:
> Isaac Morland <[email protected]> writes:
> > What about backticks (`)?
>
> Since they're already allowed as operator characters, you can't
> use them for this purpose without breaking existing use-cases.
>
> Even if they were completely unused, I'd be pretty hesitant to
> adopt them for this purpose because of the potential confusion
> for users coming from mysql.
Since when have we started caring for the convenience of users of
other databases?!! /s
> Pretty much the only available syntax space is curly braces,
> and I don't really want to give those up for this either.
> (One has to assume that the SQL committee has their eyes
> on those too.)
On Thu, Jan 12, 2023 at 9:45 AM Vik Fearing <[email protected]> wrote:
> They are used in row pattern recognition.
I was very hopeful of using { }, and hoping that we'd beat the SQL
committee to it, so that they have to choose something else, if we
release this into the wild before them. But it seems that they beat us
to it long ago. (tangent: Reading some blog posts, I have to say I
loved the Row Pattern Recognition feature!)
Considering that there are almost no printable characters left in
1-255 ASCII range for us to choose from, I had to get creative; and I
believe I have found a way to make it work.
Unless the SQL committee has their eyes on a freestanding backslash \
character for something, I believe we can use it as a prefix for Named
Operators. Since the most common use of backslash is for escaping
characters, I believe it would feel natural for the users to use it as
described below.
New scheme for the named operators: \#foo That is, an identifier
prefixed with \# would serve as an operator name. psql considers \ to
be the start of its commands, but it wasn't hard to convince psql to
ignore \# and let it pass through to server.
I agree that an identifier _surrounded_ by the same token (e.g. #foo#)
or the pairing token (e.g. {foo}) looks better aesthetically, so I am
okay with any of the following variations of the scheme, as well:
\#foo\# (tested; works)
\#foo# (not tested; reduces ident length by 1)
We can choose a different character, instead of #. Perhaps \{foo} !
Attached is the v2 patch that supports \#foo style Named Operators.
Following is the SQL snippet to see what the usage looks like.
create operator \#add_point
(function = box_add, leftarg = box, rightarg = point);
create table test(a box);
insert into test values('((0,0),(1,1))'), ('((0,0),(2,1))');
select a as original, a \#add_point '(1,1)' as modified from test;
drop operator \#add_point(box, point);
Although we have never done it before, but by using backslash we
might be able to define new custom token types as well, if needed.
For those interested, I have couple of different branches with
named_operators* prefix in my Git fork [1] where I'm trying different
combinations.
[1]: https://github.com/gurjeet/postgres/branches
Best regards,
Gurjeet
http://Gurje.et
Attachments:
[application/octet-stream] named_operators_v2.patch (4.3K, ../CABwTF4VEGbnZAiC8LX_MTc0Yw+co0g-T+KNt0SwdMtPkiHsJLA@mail.gmail.com/2-named_operators_v2.patch)
download | inline diff:
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 1017f2eed1..c5b8562cb5 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -31,6 +31,7 @@
#include "catalog/pg_type.h"
#include "miscadmin.h"
#include "parser/parse_oper.h"
+#include "parser/scansup.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
@@ -79,6 +80,10 @@ validOperatorName(const char *name)
if (len == 0 || len >= NAMEDATALEN)
return false;
+ /* Is this a Named Operator? */
+ if (validNamedOperator(name))
+ return true;
+
/* Can't contain any invalid characters */
/* Test string here should match op_chars in scan.l */
if (strspn(name, "~!@#^&|`?+-*/%<>=") != len)
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index db8b0fe8eb..fa282dde4d 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -379,6 +379,15 @@ self [,()\[\].;\:\+\-\*\/\%\^\<\>\=]
op_chars [\~\!\@\#\^\&\|\`\?\+\-\*\/\%\<\>\=]
operator {op_chars}+
+/*
+ * Named Operators, e.g. \#foo
+ *
+ * {namedopfailed} is an error rule to avoid scanner backup when
+ * {namedop} fails to match its trailing {identifier}.
+ */
+namedop \\\#{identifier}
+namedopfailed \\\#
+
/*
* Numbers
*
@@ -768,6 +777,18 @@ other .
}
<xdolq><<EOF>> { yyerror("unterminated dollar-quoted string"); }
+{namedop} {
+ SET_YYLLOC();
+ if (yyleng >= NAMEDATALEN)
+ yyerror("operator name too long");
+ yylval->str = downcase_identifier(yytext, yyleng, false, false);
+ return Op;
+ }
+
+{namedopfailed} {
+ yyerror("unexpected token");
+ }
+
{xdstart} {
SET_YYLLOC();
BEGIN(xd);
diff --git a/src/backend/parser/scansup.c b/src/backend/parser/scansup.c
index 602108a40f..db94a927f7 100644
--- a/src/backend/parser/scansup.c
+++ b/src/backend/parser/scansup.c
@@ -125,3 +125,65 @@ scanner_isspace(char ch)
return true;
return false;
}
+
+/*
+ * validNamedOperator() -- return true if name adheres to the scanner rule
+ * {namedop}
+ */
+bool
+validNamedOperator(const char *name)
+{
+ size_t len = strlen(name);
+ bool valid_identifier;
+
+ if (len < 3 || len >= NAMEDATALEN)
+ return false;
+
+ if (name[0] != '\\' || name[1] != '#')
+ return false;
+
+ // Disregard the delimiters
+ valid_identifier = validIdentifier(name + 2);
+
+ return valid_identifier;
+}
+
+/*
+ * validIdentifier() -- return true if name adheres to the scanner rule
+ * {identifier}
+ *
+ * Note: this function does not check if the identifier length
+ * is less than NAMEDATALEN.
+ */
+bool
+validIdentifier(const char *name)
+{
+ uint8 c;
+ size_t i, len = strlen(name);
+
+ // Reject if first character is not part of ident_start
+ c = name[0];
+ if ( !(c == '_'
+ || (c >='A' && c <= 'Z')
+ || (c >='a' && c <= 'z')
+ || (c >= 0200 && c <= 0377)))
+ {
+ return false;
+ }
+
+ // Reject if other characters are not part of ident_cont
+ for (i = 1; i < len; ++i)
+ {
+ c = name[i];
+ if ( !(c == '_' || c == '$'
+ || (c >='A' && c <= 'Z')
+ || (c >='a' && c <= 'z')
+ || (c >='0' && c <= '9')
+ || (c >= 0200 && c <= 0377)))
+ {
+ return false;
+ }
+ }
+
+ return true;
+}
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index ae531ec240..8ec9572d5e 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -317,6 +317,15 @@ self [,()\[\].;\:\+\-\*\/\%\^\<\>\=]
op_chars [\~\!\@\#\^\&\|\`\?\+\-\*\/\%\<\>\=]
operator {op_chars}+
+/*
+ * Named Operators, e.g. \#foo
+ *
+ * {namedopfailed} is an error rule to avoid scanner backup when
+ * {namedop} fails to match its trailing {identifier}.
+ */
+namedop \\\#{identifier}
+namedopfailed \\\#
+
/*
* Numbers
*
@@ -570,6 +579,14 @@ other .
ECHO;
}
+{namedop} {
+ ECHO;
+ }
+
+{namedopfailed} {
+ ECHO;
+ }
+
{xdstart} {
BEGIN(xd);
ECHO;
diff --git a/src/include/parser/scansup.h b/src/include/parser/scansup.h
index ff65224bf6..0f6aff8b44 100644
--- a/src/include/parser/scansup.h
+++ b/src/include/parser/scansup.h
@@ -24,4 +24,7 @@ extern void truncate_identifier(char *ident, int len, bool warn);
extern bool scanner_isspace(char ch);
+extern bool validNamedOperator(const char *name);
+extern bool validIdentifier(const char *name);
+
#endif /* SCANSUP_H */
view thread (14+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Named Operators
In-Reply-To: <CABwTF4VEGbnZAiC8LX_MTc0Yw+co0g-T+KNt0SwdMtPkiHsJLA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox