Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Tuesday 14 July 2015 18:22:26 you wrote: I think you had copy-pasted from one of the generic ALTER variants, like AlterOwnerStmt, which was overly generic for ALTER OPERATOR. You right. Thanks, committed after some editing. Thanks you. And it BIG editing. ;) After that, can you view my next patch?^_^ https://commitfest.postgresql.org/5/253/ -- Uriy Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On 07/13/2015 03:43 PM, Uriy Zhuravlev wrote: Hello hackers. Attached is a new version of patch: * port syntax from NULL to truth NONE * fix documentation (thanks Heikki) * rebase to master Thanks, committed after some editing. I put all the logic into AlterOperator function, in operatorcmds.c, rather than pg_operator.c. I think that's how we've lately organized commands. I also simplified the code quite a bit - I think you had copy-pasted from one of the generic ALTER variants, like AlterOwnerStmt, which was overly generic for ALTER OPERATOR. No need to look up the owner/name attributes dynamically, etc. There was one genuine bug that I fixed: the ALTER OPERATOR command didn't check all the same conditions that CREATE OPERATOR did, namely that only binary operators can have join selectivity, and that only boolean operators can have restriction/join selectivity. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Tue, Jul 14, 2015 at 8:22 AM, Heikki Linnakangas hlinn...@iki.fi wrote: On 07/13/2015 03:43 PM, Uriy Zhuravlev wrote: Hello hackers. Attached is a new version of patch: * port syntax from NULL to truth NONE * fix documentation (thanks Heikki) * rebase to master Thanks, committed after some editing. I put all the logic into AlterOperator function, in operatorcmds.c, rather than pg_operator.c. I think that's how we've lately organized commands. I also simplified the code quite a bit - I think you had copy-pasted from one of the generic ALTER variants, like AlterOwnerStmt, which was overly generic for ALTER OPERATOR. No need to look up the owner/name attributes dynamically, etc. There was one genuine bug that I fixed: the ALTER OPERATOR command didn't check all the same conditions that CREATE OPERATOR did, namely that only binary operators can have join selectivity, and that only boolean operators can have restriction/join selectivity. I'm getting some compiler warnings now: operatorcmds.c: In function 'AlterOperator': operatorcmds.c:504: warning: 'address.objectSubId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.objectSubId' was declared here operatorcmds.c:504: warning: 'address.objectId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.objectId' was declared here operatorcmds.c:504: warning: 'address.classId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.classId' was declared here gcc version 4.4.7 20120313 (Red Hat 4.4.7-11) (GCC) Thanks, Jeff
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On 07/14/2015 07:28 PM, Jeff Janes wrote: I'm getting some compiler warnings now: operatorcmds.c: In function 'AlterOperator': operatorcmds.c:504: warning: 'address.objectSubId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.objectSubId' was declared here operatorcmds.c:504: warning: 'address.objectId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.objectId' was declared here operatorcmds.c:504: warning: 'address.classId' may be used uninitialized in this function operatorcmds.c:365: note: 'address.classId' was declared here gcc version 4.4.7 20120313 (Red Hat 4.4.7-11) (GCC) Ah, thanks, fixed. I was apparently compiling with -O0. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hello hackers.
Attached is a new version of patch:
* port syntax from NULL to truth NONE
* fix documentation (thanks Heikki)
* rebase to master
Thanks.
--
Uriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/doc/src/sgml/ref/alter_operator.sgml b/doc/src/sgml/ref/alter_operator.sgml
index bdb2d02..237e4f1 100644
--- a/doc/src/sgml/ref/alter_operator.sgml
+++ b/doc/src/sgml/ref/alter_operator.sgml
@@ -26,6 +26,11 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } )
SET SCHEMA replaceablenew_schema/replaceable
+
+ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } )
+SET ( { RESTRICT = { replaceable class=parameterres_proc/replaceable | NONE }
+ | JOIN = { replaceable class=parameterjoin_proc/replaceable | NONE }
+ } [, ... ] )
/synopsis
/refsynopsisdiv
@@ -34,8 +39,7 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
para
commandALTER OPERATOR/command changes the definition of
- an operator. The only currently available functionality is to change the
- owner of the operator.
+ an operator.
/para
para
@@ -98,6 +102,25 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
/para
/listitem
/varlistentry
+
+ varlistentry
+ termreplaceable class=parameterres_proc/replaceable/term
+ listitem
+ para
+ The restriction selectivity estimator function for this operator; write NONE to remove existing selectivity estimator.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
+ termreplaceable class=parameterjoin_proc/replaceable/term
+ listitem
+ para
+ The join selectivity estimator function for this operator; write NONE to remove existing selectivity estimator.
+ /para
+ /listitem
+ /varlistentry
+
/variablelist
/refsect1
@@ -109,6 +132,13 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
programlisting
ALTER OPERATOR @@ (text, text) OWNER TO joe;
/programlisting/para
+
+ para
+Change the restriction and join selectivity estimator function of a custom operator literala b/literal for type typeint[]/type:
+programlisting
+ALTER OPERATOR (_int4, _int4) SET (RESTRICT = _int_contsel, JOIN = _int_contjoinsel);
+/programlisting/para
+
/refsect1
refsect1
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530..4c5c9c6 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -28,8 +28,10 @@
#include catalog/pg_operator.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+#include commands/defrem.h
#include miscadmin.h
#include parser/parse_oper.h
+#include parser/parse_func.h
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
@@ -53,7 +55,7 @@ static Oid OperatorShellMake(const char *operatorName,
Oid leftTypeId,
Oid rightTypeId);
-static void OperatorUpd(Oid baseId, Oid commId, Oid negId);
+static void ShellOperatorUpd(Oid baseId, Oid commId, Oid negId);
static Oid get_other_operator(List *otherOp,
Oid otherLeftTypeId, Oid otherRightTypeId,
@@ -563,7 +565,7 @@ OperatorCreate(const char *operatorName,
commutatorId = operatorObjectId;
if (OidIsValid(commutatorId) || OidIsValid(negatorId))
- OperatorUpd(operatorObjectId, commutatorId, negatorId);
+ ShellOperatorUpd(operatorObjectId, commutatorId, negatorId);
return address;
}
@@ -633,7 +635,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
}
/*
- * OperatorUpd
+ * ShellOperatorUpd
*
* For a given operator, look up its negator and commutator operators.
* If they are defined, but their negator and commutator fields
@@ -642,7 +644,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
* which are the negator or commutator of each other.
*/
static void
-OperatorUpd(Oid baseId, Oid commId, Oid negId)
+ShellOperatorUpd(Oid baseId, Oid commId, Oid negId)
{
int i;
Relation pg_operator_desc;
@@ -864,3 +866,154 @@ makeOperatorDependencies(HeapTuple tuple)
return myself;
}
+
+/*
+ * Operator update aka ALTER OPERATOR for RESTRICT, JOIN
+ */
+void OperatorUpd(Oid classId,
+ Oid baseId,
+ List *operator_params)
+{
+ int i;
+ ListCell *pl;
+ Relation catalog;
+ HeapTuple tup;
+ Oid operator_param_id = 0;
+ bool nulls[Natts_pg_operator];
+ bool replaces[Natts_pg_operator];
+ Datum values[Natts_pg_operator];
+
+ for (i = 0; i Natts_pg_operator; ++i)
+ {
+ values[i] = (Datum) 0;
+ replaces[i] = false;
+ nulls[i] = false;
+ }
+
+ catalog = heap_open(classId, RowExclusiveLock);
+ tup =
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On 07/06/2015 07:21 PM, Uriy Zhuravlev wrote:
Hello hackers.
This is the fifth version of the patch (the fourth was unsuccessful :)).
I added documentation and was held a small refactoring.
I edited the formatting of the syntax page a bit, and came up with this:
ALTER OPERATOR name ( { left_type | NONE } , { right_type | NONE } )
SET ( { RESTRICT = { res_proc | NULL }
| JOIN = { join_proc | NULL }
} [, ... ] )
A couple of minor issues with the syntax itself:
* I think it'd be better to use NONE instead of NULL above, to remove
the estimator. It seems inconsistent when we've used NONE to mean
missing earlier in the same statement.
* The way you check for the NULL in OperatorUpd is wrong. It cannot
distinguish between a quoted null, meaning a function called null,
and a NULL, meaning no function. (You can argue that you're just asking
for trouble if you name any function null, but we're careful to deal
with that correctly everywhere else.) You don't have the information
about quoting once you leave the parser, so you'll have to distinguish
those in the grammar.
Attached is a new version of your patch, rebased over current master,
and the docs formatting fixes.
- Heikki
diff --git a/doc/src/sgml/ref/alter_operator.sgml b/doc/src/sgml/ref/alter_operator.sgml
index 8a7af50..b8c353f 100644
--- a/doc/src/sgml/ref/alter_operator.sgml
+++ b/doc/src/sgml/ref/alter_operator.sgml
@@ -26,6 +26,11 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } )
SET SCHEMA replaceablenew_schema/replaceable
+
+ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } )
+SET ( { RESTRICT = { replaceable class=parameterres_proc/replaceable | NONE }
+ | JOIN = { replaceable class=parameterjoin_proc/replaceable | NONE }
+ } [, ... ] )
/synopsis
/refsynopsisdiv
@@ -34,8 +39,7 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
para
commandALTER OPERATOR/command changes the definition of
- an operator. The only currently available functionality is to change the
- owner of the operator.
+ an operator.
/para
para
@@ -98,6 +102,25 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
/para
/listitem
/varlistentry
+
+ varlistentry
+ termreplaceable class=parameterres_proc/replaceable/term
+ listitem
+ para
+ The restriction selectivity estimator function for this operator; write NONE to remove existing selectivity estimator.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
+ termreplaceable class=parameterjoin_proc/replaceable/term
+ listitem
+ para
+ The join selectivity estimator function for this operator; write NONE to remove existing selectivity estimator.
+ /para
+ /listitem
+ /varlistentry
+
/variablelist
/refsect1
@@ -109,6 +132,13 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
programlisting
ALTER OPERATOR @@ (text, text) OWNER TO joe;
/programlisting/para
+
+ para
+Change the restriction and join selectivity estimator function of a custom operator literala b/literal for type typeint[]/type:
+programlisting
+ALTER OPERATOR (_int4, _int4) SET (RESTRICT = _int_contsel, JOIN = _int_contjoinsel);
+/programlisting/para
+
/refsect1
refsect1
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530..b981a44 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -28,8 +28,10 @@
#include catalog/pg_operator.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+#include commands/defrem.h
#include miscadmin.h
#include parser/parse_oper.h
+#include parser/parse_func.h
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
@@ -53,7 +55,7 @@ static Oid OperatorShellMake(const char *operatorName,
Oid leftTypeId,
Oid rightTypeId);
-static void OperatorUpd(Oid baseId, Oid commId, Oid negId);
+static void ShellOperatorUpd(Oid baseId, Oid commId, Oid negId);
static Oid get_other_operator(List *otherOp,
Oid otherLeftTypeId, Oid otherRightTypeId,
@@ -563,7 +565,7 @@ OperatorCreate(const char *operatorName,
commutatorId = operatorObjectId;
if (OidIsValid(commutatorId) || OidIsValid(negatorId))
- OperatorUpd(operatorObjectId, commutatorId, negatorId);
+ ShellOperatorUpd(operatorObjectId, commutatorId, negatorId);
return address;
}
@@ -633,7 +635,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
}
/*
- * OperatorUpd
+ * ShellOperatorUpd
*
* For a given operator, look up its negator and commutator operators.
* If they are defined, but their
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hello. On Friday 10 July 2015 15:46:35 you wrote: * I think it'd be better to use NONE instead of NULL above, to remove the estimator. It seems inconsistent when we've used NONE to mean missing earlier in the same statement. Ok, you right. * The way you check for the NULL in OperatorUpd is wrong. It cannot distinguish between a quoted null, meaning a function called null, and a NULL, meaning no function. With none has a similar problem. I need to correct the grammar. Attached is a new version of your patch, rebased over current master, and the docs formatting fixes. Thanks. I hope to send the new patch on Monday. -- Uriy Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hello hackers.
This is the fifth version of the patch (the fourth was unsuccessful :)).
I added documentation and was held a small refactoring.
Thanks.
--
Uriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/doc/src/sgml/ref/alter_operator.sgml b/doc/src/sgml/ref/alter_operator.sgml
index bdb2d02..4aaeed0 100644
--- a/doc/src/sgml/ref/alter_operator.sgml
+++ b/doc/src/sgml/ref/alter_operator.sgml
@@ -26,6 +26,11 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } )
SET SCHEMA replaceablenew_schema/replaceable
+
+ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/replaceable | NONE } , { replaceableright_type/replaceable | NONE } ) SET (
+[, RESTRICT = { replaceable class=parameterres_proc/replaceable | NULL } ]
+[, JOIN = { replaceable class=parameterjoin_proc/replaceable | NULL } ]
+)
/synopsis
/refsynopsisdiv
@@ -34,8 +39,7 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
para
commandALTER OPERATOR/command changes the definition of
- an operator. The only currently available functionality is to change the
- owner of the operator.
+ an operator.
/para
para
@@ -98,6 +102,25 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
/para
/listitem
/varlistentry
+
+ varlistentry
+ termreplaceable class=parameterres_proc/replaceable/term
+ listitem
+ para
+ The restriction selectivity estimator function for this operator.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
+ termreplaceable class=parameterjoin_proc/replaceable/term
+ listitem
+ para
+ The join selectivity estimator function for this operator.
+ /para
+ /listitem
+ /varlistentry
+
/variablelist
/refsect1
@@ -109,6 +132,13 @@ ALTER OPERATOR replaceablename/replaceable ( { replaceableleft_type/repla
programlisting
ALTER OPERATOR @@ (text, text) OWNER TO joe;
/programlisting/para
+
+ para
+Change the restriction and join selectivity estimator function of a custom operator literala b/literal for type typeint[]/type:
+programlisting
+ALTER OPERATOR (_int4, _int4) SET (RESTRICT = _int_contsel, JOIN = _int_contjoinsel);
+/programlisting/para
+
/refsect1
refsect1
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530..f5ff381 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -28,8 +28,10 @@
#include catalog/pg_operator.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+#include commands/defrem.h
#include miscadmin.h
#include parser/parse_oper.h
+#include parser/parse_func.h
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
@@ -53,7 +55,7 @@ static Oid OperatorShellMake(const char *operatorName,
Oid leftTypeId,
Oid rightTypeId);
-static void OperatorUpd(Oid baseId, Oid commId, Oid negId);
+static void ShellOperatorUpd(Oid baseId, Oid commId, Oid negId);
static Oid get_other_operator(List *otherOp,
Oid otherLeftTypeId, Oid otherRightTypeId,
@@ -563,7 +565,7 @@ OperatorCreate(const char *operatorName,
commutatorId = operatorObjectId;
if (OidIsValid(commutatorId) || OidIsValid(negatorId))
- OperatorUpd(operatorObjectId, commutatorId, negatorId);
+ ShellOperatorUpd(operatorObjectId, commutatorId, negatorId);
return address;
}
@@ -633,7 +635,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
}
/*
- * OperatorUpd
+ * ShellOperatorUpd
*
* For a given operator, look up its negator and commutator operators.
* If they are defined, but their negator and commutator fields
@@ -642,7 +644,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
* which are the negator or commutator of each other.
*/
static void
-OperatorUpd(Oid baseId, Oid commId, Oid negId)
+ShellOperatorUpd(Oid baseId, Oid commId, Oid negId)
{
int i;
Relation pg_operator_desc;
@@ -864,3 +866,150 @@ makeOperatorDependencies(HeapTuple tuple)
return myself;
}
+
+/*
+ * Operator update aka ALTER OPERATOR for RESTRICT, JOIN
+ */
+void OperatorUpd(Oid classId,
+ Oid baseId,
+ List *operator_params)
+{
+ int i;
+ ListCell *pl;
+ Relation catalog;
+ HeapTuple tup;
+ Oid operator_param_id = 0;
+ bool nulls[Natts_pg_operator];
+ bool replaces[Natts_pg_operator];
+ Datum values[Natts_pg_operator];
+
+ for (i = 0; i Natts_pg_operator; ++i)
+ {
+ values[i] = (Datum) 0;
+ replaces[i] = false;
+ nulls[i] = false;
+ }
+
+ catalog = heap_open(classId, RowExclusiveLock);
+ tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(baseId));
+ if (HeapTupleIsValid(tup))
+ {
+ /*
+ * loop over the definition list and
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Wed, Jun 24, 2015 at 7:30 AM, Uriy Zhuravlev u.zhurav...@postgrespro.ru wrote: Because change the commutator and negator raised questions I suggest 3 version of the patch without them. In addition, for us now is much more important restrict and join (for Selectivity estimation for intarray patch). Seems broadly sensible. You will need to write docs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hello Hackers.
Because change the commutator and negator raised questions I suggest 3 version
of the patch without them. In addition, for us now is much more important
restrict and join (for Selectivity estimation for intarray patch).
What do you think?
Thanks!
--
Uriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530..f7770fd 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -28,8 +28,10 @@
#include catalog/pg_operator.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+#include commands/defrem.h
#include miscadmin.h
#include parser/parse_oper.h
+#include parser/parse_func.h
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
@@ -53,7 +55,7 @@ static Oid OperatorShellMake(const char *operatorName,
Oid leftTypeId,
Oid rightTypeId);
-static void OperatorUpd(Oid baseId, Oid commId, Oid negId);
+static void ShellOperatorUpd(Oid baseId, Oid commId, Oid negId);
static Oid get_other_operator(List *otherOp,
Oid otherLeftTypeId, Oid otherRightTypeId,
@@ -563,7 +565,7 @@ OperatorCreate(const char *operatorName,
commutatorId = operatorObjectId;
if (OidIsValid(commutatorId) || OidIsValid(negatorId))
- OperatorUpd(operatorObjectId, commutatorId, negatorId);
+ ShellOperatorUpd(operatorObjectId, commutatorId, negatorId);
return address;
}
@@ -633,7 +635,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
}
/*
- * OperatorUpd
+ * ShellOperatorUpd
*
* For a given operator, look up its negator and commutator operators.
* If they are defined, but their negator and commutator fields
@@ -642,7 +644,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
* which are the negator or commutator of each other.
*/
static void
-OperatorUpd(Oid baseId, Oid commId, Oid negId)
+ShellOperatorUpd(Oid baseId, Oid commId, Oid negId)
{
int i;
Relation pg_operator_desc;
@@ -864,3 +866,158 @@ makeOperatorDependencies(HeapTuple tuple)
return myself;
}
+
+/*
+ * Operator update aka ALTER OPERATOR for RESTRICT, JOIN
+ */
+void OperatorUpd(Oid classId,
+ Oid baseId,
+ List *operator_params)
+{
+ int i;
+ ListCell *pl;
+ Relation catalog;
+ HeapTuple tup;
+ Oid operator_param_id = 0;
+ bool nulls[Natts_pg_operator];
+ bool replaces[Natts_pg_operator];
+ Datum values[Natts_pg_operator];
+
+ for (i = 0; i Natts_pg_operator; ++i)
+ {
+ values[i] = (Datum) 0;
+ replaces[i] = false;
+ nulls[i] = false;
+ }
+
+ catalog = heap_open(classId, RowExclusiveLock);
+ tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(baseId));
+ if (HeapTupleIsValid(tup))
+ {
+ /*
+ * loop over the definition list and extract the information we need.
+ */
+ foreach(pl, operator_params)
+ {
+ DefElem*defel = (DefElem *) lfirst(pl);
+ List *param = defGetQualifiedName(defel);
+ int param_type;
+
+ if (pg_strcasecmp(defel-defname, restrict) == 0)
+param_type = Anum_pg_operator_oprrest;
+ else if (pg_strcasecmp(defel-defname, join) == 0)
+param_type = Anum_pg_operator_oprjoin;
+ else
+ {
+ereport(WARNING,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(operator attribute \%s\ not recognized,
+defel-defname)));
+continue;
+ }
+
+ /* Resets if written NULL */
+ if (pg_strcasecmp(NameListToString(param), null) == 0)
+ {
+values[param_type - 1] = ObjectIdGetDatum(InvalidOid);
+replaces[param_type - 1] = true;
+continue;
+ }
+
+ if (param_type == Anum_pg_operator_oprrest)
+ {
+if (PointerIsValid(param))
+{
+ Oid typeId[5];
+ AclResult aclresult;
+ typeId[0] = INTERNALOID; /* PlannerInfo */
+ typeId[1] = OIDOID; /* operator OID */
+ typeId[2] = INTERNALOID; /* args list */
+ typeId[3] = INT4OID; /* varRelid */
+
+ operator_param_id = LookupFuncName(param, 4, typeId, false);
+
+ /* estimators must return float8 */
+ if (get_func_rettype(operator_param_id) != FLOAT8OID)
+ ereport(ERROR,
+(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg(restriction estimator function %s must return type \float8\,
+ NameListToString(param;
+
+ /* Require EXECUTE rights for the estimator */
+ aclresult = pg_proc_aclcheck(operator_param_id, GetUserId(), ACL_EXECUTE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_PROC,
+ NameListToString(param));
+}
+else
+ operator_param_id = 0;
+ }
+ else if (param_type == Anum_pg_operator_oprjoin)
+ {
+if (PointerIsValid(param))
+{
+ Oid typeId[5];
+ AclResult aclresult;
+ typeId[0] = INTERNALOID; /* PlannerInfo */
+ typeId[1] = OIDOID; /* operator OID */
+ typeId[2] = INTERNALOID; /* args list */
+ typeId[3] = INT2OID; /* jointype */
+
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Fri, May 29, 2015 at 4:28 AM, Alexander Korotkov a.korot...@postgrespro.ru wrote: On Thu, May 28, 2015 at 6:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alexander Korotkov a.korot...@postgrespro.ru writes: Could we address both this problems by denying changing existing commutators and negator? ISTM that setting absent commutator and negator is quite enough for ALTER OPERATOR. User extensions could need setting of commutator and negator because they could add new operators which don't exist before. But it's rather uncommon to unset or change commutator or negator. Note that this functionality is already covered, in that you can specify the commutator/negator linkage when you create the second operator. I'm not particularly convinced that we need to have it in ALTER OPERATOR. Yeah, I didn't notice that CREATE OPERATOR sets commutator and negator on opposite side as well. Then we probably can leave ALTER OPERATOR without altering commutator and negator. BTW, does DROP OPERATOR works correctly? # create operator == (procedure = int8eq, leftarg = bigint, rightarg = bigint); CREATE OPERATOR # create operator !== (procedure = int8ne, leftarg = bigint, rightarg = bigint, negator = ==); CREATE OPERATOR # select oid, * from pg_operator where oprnamespace = (select oid from pg_namespace where nspname = 'public'); oid | oprname | oprnamespace | oprowner | oprkind | oprcanmerge | oprcanhash | oprleft | oprright | oprresult | oprcom | oprnegate | oprcode | oprrest | oprjoin ---+-+--+--+-+-++-+--+---++---+-+-+- 46355 | !== | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46354 | int8ne | - | - 46354 | == | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46355 | int8eq | - | - (2 rows) # create table test (id bigint); CREATE TABLE # explain verbose select * from test where not id == 10::bigint; QUERY PLAN --- Seq Scan on public.test (cost=0.00..38.25 rows=1130 width=8) Output: id Filter: (test.id !== '10'::bigint) (3 rows) # drop operator !== (int8, int8); DROP OPERATOR # select oid, * from pg_operator where oprnamespace = (select oid from pg_namespace where nspname = 'public'); oid | oprname | oprnamespace | oprowner | oprkind | oprcanmerge | oprcanhash | oprleft | oprright | oprresult | oprcom | oprnegate | oprcode | oprrest | oprjoin ---+-+--+--+-+-++-+--+---++---+-+-+- 46354 | == | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46355 | int8eq | - | - (1 row) # explain verbose select * from test where not id == 10::bigint; ERROR: cache lookup failed for function 0 DROP OPERATOR leaves broken reference in oprnegate. Should we fix it? Yeah, that doesn't seem good. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Thu, May 28, 2015 at 6:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alexander Korotkov a.korot...@postgrespro.ru writes: Could we address both this problems by denying changing existing commutators and negator? ISTM that setting absent commutator and negator is quite enough for ALTER OPERATOR. User extensions could need setting of commutator and negator because they could add new operators which don't exist before. But it's rather uncommon to unset or change commutator or negator. Note that this functionality is already covered, in that you can specify the commutator/negator linkage when you create the second operator. I'm not particularly convinced that we need to have it in ALTER OPERATOR. Yeah, I didn't notice that CREATE OPERATOR sets commutator and negator on opposite side as well. Then we probably can leave ALTER OPERATOR without altering commutator and negator. BTW, does DROP OPERATOR works correctly? # create operator == (procedure = int8eq, leftarg = bigint, rightarg = bigint); CREATE OPERATOR # create operator !== (procedure = int8ne, leftarg = bigint, rightarg = bigint, negator = ==); CREATE OPERATOR # select oid, * from pg_operator where oprnamespace = (select oid from pg_namespace where nspname = 'public'); oid | oprname | oprnamespace | oprowner | oprkind | oprcanmerge | oprcanhash | oprleft | oprright | oprresult | oprcom | oprnegate | oprcode | oprrest | oprjoin ---+-+--+--+-+-++-+--+---++---+-+-+- 46355 | !== | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46354 | int8ne | - | - 46354 | == | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46355 | int8eq | - | - (2 rows) # create table test (id bigint); CREATE TABLE # explain verbose select * from test where not id == 10::bigint; QUERY PLAN --- Seq Scan on public.test (cost=0.00..38.25 rows=1130 width=8) Output: id Filter: (test.id !== '10'::bigint) (3 rows) # drop operator !== (int8, int8); DROP OPERATOR # select oid, * from pg_operator where oprnamespace = (select oid from pg_namespace where nspname = 'public'); oid | oprname | oprnamespace | oprowner | oprkind | oprcanmerge | oprcanhash | oprleft | oprright | oprresult | oprcom | oprnegate | oprcode | oprrest | oprjoin ---+-+--+--+-+-++-+--+---++---+-+-+- 46354 | == | 2200 | 10 | b | f | f | 20 | 20 |16 | 0 | 46355 | int8eq | - | - (1 row) # explain verbose select * from test where not id == 10::bigint; ERROR: cache lookup failed for function 0 DROP OPERATOR leaves broken reference in oprnegate. Should we fix it? -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Wed, May 27, 2015 at 9:28 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, May 18, 2015 at 10:21 AM, Tom Lane t...@sss.pgh.pa.us wrote: Uriy Zhuravlev u.zhurav...@postgrespro.ru writes: I have attached a patch that extends ALTER OPERATOR to support COMMUTATOR, NEGATOR, RESTRICT and JOIN. There are fairly significant reasons why we have not done this, based on the difficulty of updating existing cached plans that might have incidentally depended on those operator properties during creation. Perhaps it's all right to simply ignore such concerns, but I would like to see a defense of why. I don't think there's a direct problem with cached plans, because it looks like plancache.c blows away the entire plan cache for any change to pg_operator. OperatorUpd() can already update oprcom and oprnegate, which seems to stand for the proposition that it's OK to set those from InvalidOid to something else. But that doesn't prove that other kinds of changes are safe. A search of other places where oprcom is used in the code led me to ComputeIndexAttrs(). If an operator whose commutator is itself were changed so that the commutator was anything else, then we'd end up with a broken exclusion constraint. That's a problem. targetIsInSortList is run during parse analysis, and that too tests whether sortop == get_commutator(scl-sortop). Those decisions wouldn't get reevaluated if the truth of that expression changed after the fact, which I suspect is also a problem. Could we address both this problems by denying changing existing commutators and negator? ISTM that setting absent commutator and negator is quite enough for ALTER OPERATOR. User extensions could need setting of commutator and negator because they could add new operators which don't exist before. But it's rather uncommon to unset or change commutator or negator. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Alexander Korotkov a.korot...@postgrespro.ru writes: Could we address both this problems by denying changing existing commutators and negator? ISTM that setting absent commutator and negator is quite enough for ALTER OPERATOR. User extensions could need setting of commutator and negator because they could add new operators which don't exist before. But it's rather uncommon to unset or change commutator or negator. Note that this functionality is already covered, in that you can specify the commutator/negator linkage when you create the second operator. I'm not particularly convinced that we need to have it in ALTER OPERATOR. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Mon, May 18, 2015 at 10:21 AM, Tom Lane t...@sss.pgh.pa.us wrote: Uriy Zhuravlev u.zhurav...@postgrespro.ru writes: I have attached a patch that extends ALTER OPERATOR to support COMMUTATOR, NEGATOR, RESTRICT and JOIN. There are fairly significant reasons why we have not done this, based on the difficulty of updating existing cached plans that might have incidentally depended on those operator properties during creation. Perhaps it's all right to simply ignore such concerns, but I would like to see a defense of why. I don't think there's a direct problem with cached plans, because it looks like plancache.c blows away the entire plan cache for any change to pg_operator. OperatorUpd() can already update oprcom and oprnegate, which seems to stand for the proposition that it's OK to set those from InvalidOid to something else. But that doesn't prove that other kinds of changes are safe. A search of other places where oprcom is used in the code led me to ComputeIndexAttrs(). If an operator whose commutator is itself were changed so that the commutator was anything else, then we'd end up with a broken exclusion constraint. That's a problem. targetIsInSortList is run during parse analysis, and that too tests whether sortop == get_commutator(scl-sortop). Those decisions wouldn't get reevaluated if the truth of that expression changed after the fact, which I suspect is also a problem. On a quick survey, I did not find similar hazards related to oprnegate, oprrest, or oprjoin. AFAICS, they are used only by the planner. I *think* that means that if our plan invalidation code is smart enough, those instances will be OK. But I haven't audited them in detail. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hello Hackers.
I reworked the patch. Now, the syntax is similar to CREATE OPERATOR.
And as I wrote earlier problems with the cache I have not found. If someone
can suggest how it could be verified that would be happy.
New syntax example:
ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = =);
ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = =);
ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = example_func);
ALTER OPERATOR === (boolean, boolean) SET (JOIN = example_func);
ALTER OPERATOR === (boolean, boolean) SET (
COMMUTATOR = NULL,
NEGATOR = NULL,
RESTRICT = NULL,
JOIN = NULL
);
Thanks!
--
Uriy Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 072f530..83e020b 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -28,8 +28,10 @@
#include catalog/pg_operator.h
#include catalog/pg_proc.h
#include catalog/pg_type.h
+#include commands/defrem.h
#include miscadmin.h
#include parser/parse_oper.h
+#include parser/parse_func.h
#include utils/acl.h
#include utils/builtins.h
#include utils/lsyscache.h
@@ -53,7 +55,7 @@ static Oid OperatorShellMake(const char *operatorName,
Oid leftTypeId,
Oid rightTypeId);
-static void OperatorUpd(Oid baseId, Oid commId, Oid negId);
+static void ShellOperatorUpd(Oid baseId, Oid commId, Oid negId);
static Oid get_other_operator(List *otherOp,
Oid otherLeftTypeId, Oid otherRightTypeId,
@@ -563,7 +565,7 @@ OperatorCreate(const char *operatorName,
commutatorId = operatorObjectId;
if (OidIsValid(commutatorId) || OidIsValid(negatorId))
- OperatorUpd(operatorObjectId, commutatorId, negatorId);
+ ShellOperatorUpd(operatorObjectId, commutatorId, negatorId);
return address;
}
@@ -633,7 +635,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
}
/*
- * OperatorUpd
+ * ShellOperatorUpd
*
* For a given operator, look up its negator and commutator operators.
* If they are defined, but their negator and commutator fields
@@ -642,7 +644,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
* which are the negator or commutator of each other.
*/
static void
-OperatorUpd(Oid baseId, Oid commId, Oid negId)
+ShellOperatorUpd(Oid baseId, Oid commId, Oid negId)
{
int i;
Relation pg_operator_desc;
@@ -864,3 +866,187 @@ makeOperatorDependencies(HeapTuple tuple)
return myself;
}
+
+/*
+ * Operator update aka ALTER OPERATOR for COMMUTATOR, NEGATOR, RESTRICT, JOIN
+ */
+void OperatorUpd(Oid classId,
+ Oid baseId,
+ List *operator_params)
+{
+ int i;
+ ListCell *pl;
+ Relation catalog;
+ HeapTuple tup;
+ Oid operator_param_id = 0;
+ Form_pg_operator DstOperatorData;
+ bool otherDefined;
+ bool nulls[Natts_pg_operator];
+ bool replaces[Natts_pg_operator];
+ Datum values[Natts_pg_operator];
+
+ for (i = 0; i Natts_pg_operator; ++i)
+ {
+ values[i] = (Datum) 0;
+ replaces[i] = false;
+ nulls[i] = false;
+ }
+
+ catalog = heap_open(classId, RowExclusiveLock);
+ tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(baseId));
+ if (HeapTupleIsValid(tup))
+ {
+ DstOperatorData = (Form_pg_operator) GETSTRUCT(tup);
+
+ /*
+ * loop over the definition list and extract the information we need.
+ */
+ foreach(pl, operator_params)
+ {
+ DefElem*defel = (DefElem *) lfirst(pl);
+ List *param = defGetQualifiedName(defel);
+ int param_type;
+
+ if (pg_strcasecmp(defel-defname, commutator) == 0)
+param_type = Anum_pg_operator_oprcom;
+ else if (pg_strcasecmp(defel-defname, negator) == 0)
+param_type = Anum_pg_operator_oprnegate;
+ else if (pg_strcasecmp(defel-defname, restrict) == 0)
+param_type = Anum_pg_operator_oprrest;
+ else if (pg_strcasecmp(defel-defname, join) == 0)
+param_type = Anum_pg_operator_oprjoin;
+ else
+ {
+ereport(WARNING,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(operator attribute \%s\ not recognized,
+defel-defname)));
+continue;
+ }
+
+ /* Resets if written NULL */
+ if (pg_strcasecmp(NameListToString(param), null) == 0)
+ {
+values[param_type - 1] = ObjectIdGetDatum(InvalidOid);
+replaces[param_type - 1] = true;
+continue;
+ }
+
+ /*
+ * Prepare tuple to upgrade the operator
+ * considering the type of the parameter.
+ */
+ if (param_type == Anum_pg_operator_oprcom ||
+param_type == Anum_pg_operator_oprnegate)
+ {
+otherDefined = true;
+if (PointerIsValid(param))
+ operator_param_id = OperatorLookup(param,
+DstOperatorData-oprleft,
+DstOperatorData-oprright,
+otherDefined);
+else
+ operator_param_id = InvalidOid;
+
+if (!otherDefined OidIsValid(operator_param_id)) {
+ ereport(ERROR,
+ (errmsg_internal(You can't set shell (fake) operator)));
+}
+ }
+
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Wednesday 20 May 2015 20:50:41 Andres Freund wrote: Note that this very likely wasn't actually using a prepared plan. Due to the custom plan infrastructure the first few invocations are going to be replanned. Hello. I tested it on 30 and 50 iterations, and it feels good. Thanks. -- Uriy Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Monday 18 May 2015 10:21:10 you wrote: difficulty of updating existing cached plans Could you specify more precisely about some caches we talking about? PREPARE working correctly: CREATE TABLE test_ints(i int4); CREATE TABLE CREATE INDEX idx ON test_ints(i); CREATE INDEX set enable_bitmapscan=off; SET set enable_seqscan=off; SET PREPARE test_plan (int) AS SELECT * FROM test_ints WHERE $1::int4 i; PREPARE EXPLAIN (COSTS OFF) EXECUTE test_plan(5); QUERY PLAN Index Only Scan using idx on test_ints Index Cond: (i 5) ALTER OPERATOR (int4, int4) SET COMMUTATOR NONE; ALTER OPERATOR EXPLAIN (COSTS OFF) EXECUTE test_plan(5); QUERY PLAN Index Only Scan using idx on test_ints Filter: (5 i) And can you explain more about the syntax? Thanks. -- Uriy Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Hi, On 2015-05-20 12:22:34 +0300, Uriy Zhuravlev wrote: On Monday 18 May 2015 10:21:10 you wrote: difficulty of updating existing cached plans Could you specify more precisely about some caches we talking about? PREPARE working correctly: CREATE TABLE test_ints(i int4); CREATE TABLE CREATE INDEX idx ON test_ints(i); CREATE INDEX set enable_bitmapscan=off; SET set enable_seqscan=off; SET PREPARE test_plan (int) AS SELECT * FROM test_ints WHERE $1::int4 i; PREPARE EXPLAIN (COSTS OFF) EXECUTE test_plan(5); QUERY PLAN Index Only Scan using idx on test_ints Index Cond: (i 5) ALTER OPERATOR (int4, int4) SET COMMUTATOR NONE; ALTER OPERATOR EXPLAIN (COSTS OFF) EXECUTE test_plan(5); QUERY PLAN Index Only Scan using idx on test_ints Filter: (5 i) Note that this very likely wasn't actually using a prepared plan. Due to the custom plan infrastructure the first few invocations are going to be replanned. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Uriy Zhuravlev wrote: And can you explain more about the syntax? I think he means to treat COMMUTATOR etc like a generic element list, i.e. don't define new keywords in kwlist.h/gram.y at all but rather pass the names as strings (probably using a list of DefElem) and strcmp() them in OperatorUpd() or something. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Alvaro Herrera alvhe...@2ndquadrant.com writes: Uriy Zhuravlev wrote: And can you explain more about the syntax? I think he means to treat COMMUTATOR etc like a generic element list, i.e. don't define new keywords in kwlist.h/gram.y at all but rather pass the names as strings (probably using a list of DefElem) and strcmp() them in OperatorUpd() or something. Yeah. If they aren't keywords in CREATE OPERATOR, I don't think they should be in ALTER OPERATOR either. Indeed, the syntax of the two commands probably ought to be similar. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Mon, May 18, 2015 at 5:44 PM, Uriy Zhuravlev u.zhurav...@postgrespro.ru wrote: On Monday 18 May 2015 10:21:10 Tom Lane wrote: There are fairly significant reasons why we have not done this, based on the difficulty of updating existing cached plans that might have incidentally depended on those operator properties during creation. Perhaps it's all right to simply ignore such concerns, but I would like to see a defense of why. Then need to prohibit the use of shells operators (stub operators) to create self-linkage. Implicitly changing commutators and negators working for a long time through the shell operators. I could give another motivation. AFAICS, typically ALTER OPERATOR should introduce enchancements. For instance, some version of extension didn't have negator for operator. In the next version extension introduce such negator. Or the same situation with selectivity estimation. If ALTER OPERATOR introduce only enchancements then old plans could be not optimal but they don't lead to invalid query answers. From this point of view cache invalidation after ALTER OPERATOR is term of optimization. We could include into the patch documentation statement about possible side effects with cached query plans. -- Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
On Monday 18 May 2015 10:21:10 Tom Lane wrote: There are fairly significant reasons why we have not done this, based on the difficulty of updating existing cached plans that might have incidentally depended on those operator properties during creation. Perhaps it's all right to simply ignore such concerns, but I would like to see a defense of why. Then need to prohibit the use of shells operators (stub operators) to create self-linkage. Implicitly changing commutators and negators working for a long time through the shell operators. On a more practical level, please change the syntax so that it does not require making all those things into keywords. Do you say about CREATE OPERATOR like syntax? Thanks. -- Uriy Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: Enhanced ALTER OPERATOR
Uriy Zhuravlev u.zhurav...@postgrespro.ru writes: I have attached a patch that extends ALTER OPERATOR to support COMMUTATOR, NEGATOR, RESTRICT and JOIN. There are fairly significant reasons why we have not done this, based on the difficulty of updating existing cached plans that might have incidentally depended on those operator properties during creation. Perhaps it's all right to simply ignore such concerns, but I would like to see a defense of why. On a more practical level, please change the syntax so that it does not require making all those things into keywords. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
