diff --git a/doc/src/sgml/ref/alter_operator.sgml b/doc/src/sgml/ref/alter_operator.sgml index a4a1af564f..673dcce2f5 100644 --- a/doc/src/sgml/ref/alter_operator.sgml +++ b/doc/src/sgml/ref/alter_operator.sgml @@ -30,7 +30,11 @@ ALTER OPERATOR name ( { left_typename ( { left_type | NONE } , right_type ) SET ( { RESTRICT = { res_proc | NONE } | JOIN = { join_proc | NONE } - } [, ... ] ) + | COMMUTATOR = com_op + | NEGATOR = neg_op + | HASHES + | MERGES + } [, ... ] ) @@ -121,9 +125,69 @@ ALTER OPERATOR name ( { left_type + + com_op + + + The commutator of this operator. Can only be changed if the operator + does not have an existing commutator. + + + + + + neg_op + + + The negator of this operator. Can only be changed if the operator does + not have an existing negator. + + + + + + HASHES + + + Indicates this operator can support a hash join. Can only be enabled and + not disabled. + + + + + + MERGES + + + Indicates this operator can support a merge join. Can only be enabled + and not disabled. + + + + + + Notes + + + Refer to and + for further information. + + + + Since commutators come in pairs that are commutators of each other, + ALTER OPERATOR SET COMMUTATOR will also set the + commutator of the com_op + to be the target operator. Likewise, ALTER OPERATOR SET + NEGATOR will also set the negator of + the neg_op to be the + target operator. Therefore, you must own the commutator or negator + operator as well as the target operator. + + + Examples @@ -131,13 +195,25 @@ ALTER OPERATOR name ( { left_typea @@ b for type text: ALTER OPERATOR @@ (text, text) OWNER TO joe; - + + - Change the restriction and join selectivity estimator functions of a custom operator a && b for type int[]: + Change the restriction and join selectivity estimator functions of a + custom operator a && b for + type int[]: -ALTER OPERATOR && (_int4, _int4) SET (RESTRICT = _int_contsel, JOIN = _int_contjoinsel); - +ALTER OPERATOR && (int[], int[]) SET (RESTRICT = _int_contsel, JOIN = _int_contjoinsel); + + + + + Mark the && operator as being its own + commutator: + +ALTER OPERATOR && (int[], int[]) SET (COMMUTATOR = &&); + + diff --git a/doc/src/sgml/ref/create_operator.sgml b/doc/src/sgml/ref/create_operator.sgml index e27512ff39..c421fd21e9 100644 --- a/doc/src/sgml/ref/create_operator.sgml +++ b/doc/src/sgml/ref/create_operator.sgml @@ -104,7 +104,7 @@ CREATE OPERATOR name ( - The other clauses specify optional operator optimization clauses. + The other clauses specify optional operator optimization attributes. Their meaning is detailed in . @@ -112,7 +112,7 @@ CREATE OPERATOR name ( To be able to create an operator, you must have USAGE privilege on the argument types and the return type, as well as EXECUTE privilege on the underlying function. If a - commutator or negator operator is specified, you must own these operators. + commutator or negator operator is specified, you must own those operators. @@ -231,7 +231,67 @@ COMMUTATOR = OPERATOR(myschema.===) , Notes - Refer to for further information. + Refer to and + for further information. + + + + When you are defining a self-commutative operator, you just do it. + When you are defining a pair of commutative operators, things are + a little trickier: how can the first one to be defined refer to the + other one, which you haven't defined yet? There are three solutions + to this problem: + + + + + One way is to omit the COMMUTATOR clause in the + first operator that you define, and then provide one in the second + operator's definition. Since PostgreSQL + knows that commutative operators come in pairs, when it sees the + second definition it will automatically go back and fill in the + missing COMMUTATOR clause in the first + definition. + + + + + + Another, more straightforward way is just to + include COMMUTATOR clauses in both definitions. + When PostgreSQL processes the first + definition and realizes that COMMUTATOR refers to + a nonexistent operator, the system will make a dummy entry for that + operator in the system catalog. This dummy entry will have valid + data only for the operator name, left and right operand types, and + owner, since that's all that PostgreSQL + can deduce at this point. The first operator's catalog entry will + link to this dummy entry. Later, when you define the second + operator, the system updates the dummy entry with the additional + information from the second definition. If you try to use the dummy + operator before it's been filled in, you'll just get an error + message. + + + + + + Alternatively, both operators can be defined + without COMMUTATOR clauses + and then ALTER OPERATOR can be used to set their + commutator links. It's sufficient to ALTER + either one of the pair. + + + + + In all three cases, you must own both operators in order to mark + them as commutators. + + + + Pairs of negator operators can be defined using the same methods + as for commutator pairs. diff --git a/doc/src/sgml/xoper.sgml b/doc/src/sgml/xoper.sgml index a929ced07d..954a90d77d 100644 --- a/doc/src/sgml/xoper.sgml +++ b/doc/src/sgml/xoper.sgml @@ -146,44 +146,6 @@ SELECT (a + b) AS c FROM test_complex; = operator must specify that it is valid, by marking the operator with commutator information. - - - When you are defining a self-commutative operator, you just do it. - When you are defining a pair of commutative operators, things are - a little trickier: how can the first one to be defined refer to the - other one, which you haven't defined yet? There are two solutions - to this problem: - - - - - One way is to omit the COMMUTATOR clause in the first operator that - you define, and then provide one in the second operator's definition. - Since PostgreSQL knows that commutative - operators come in pairs, when it sees the second definition it will - automatically go back and fill in the missing COMMUTATOR clause in - the first definition. - - - - - - The other, more straightforward way is just to include COMMUTATOR clauses - in both definitions. When PostgreSQL processes - the first definition and realizes that COMMUTATOR refers to a nonexistent - operator, the system will make a dummy entry for that operator in the - system catalog. This dummy entry will have valid data only - for the operator name, left and right operand types, and result type, - since that's all that PostgreSQL can deduce - at this point. The first operator's catalog entry will link to this - dummy entry. Later, when you define the second operator, the system - updates the dummy entry with the additional information from the second - definition. If you try to use the dummy operator before it's been filled - in, you'll just get an error message. - - - - @@ -217,12 +179,6 @@ SELECT (a + b) AS c FROM test_complex; x <> y. This comes up more often than you might think, because NOT operations can be inserted as a consequence of other rearrangements. - - - Pairs of negator operators can be defined using the same methods - explained above for commutator pairs. - - diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 95918a77a1..4d1b9758fa 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -44,11 +44,6 @@ static Oid OperatorGet(const char *operatorName, Oid rightObjectId, bool *defined); -static Oid OperatorLookup(List *operatorName, - Oid leftObjectId, - Oid rightObjectId, - bool *defined); - static Oid OperatorShellMake(const char *operatorName, Oid operatorNamespace, Oid leftTypeId, @@ -57,8 +52,7 @@ static Oid OperatorShellMake(const char *operatorName, static Oid get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, const char *operatorName, Oid operatorNamespace, - Oid leftTypeId, Oid rightTypeId, - bool isCommutator); + Oid leftTypeId, Oid rightTypeId); /* @@ -166,7 +160,7 @@ OperatorGet(const char *operatorName, * * *defined is set true if defined (not a shell) */ -static Oid +Oid OperatorLookup(List *operatorName, Oid leftObjectId, Oid rightObjectId, @@ -361,53 +355,17 @@ OperatorCreate(const char *operatorName, errmsg("\"%s\" is not a valid operator name", operatorName))); - if (!(OidIsValid(leftTypeId) && OidIsValid(rightTypeId))) - { - /* If it's not a binary op, these things mustn't be set: */ - if (commutatorName) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only binary operators can have commutators"))); - if (OidIsValid(joinId)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only binary operators can have join selectivity"))); - if (canMerge) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only binary operators can merge join"))); - if (canHash) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only binary operators can hash"))); - } - operResultType = get_func_rettype(procedureId); - if (operResultType != BOOLOID) - { - /* If it's not a boolean op, these things mustn't be set: */ - if (negatorName) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can have negators"))); - if (OidIsValid(restrictionId)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can have restriction selectivity"))); - if (OidIsValid(joinId)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can have join selectivity"))); - if (canMerge) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can merge join"))); - if (canHash) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can hash"))); - } + OperatorValidateParams(leftTypeId, + rightTypeId, + operResultType, + commutatorName != NIL, + negatorName != NIL, + OidIsValid(restrictionId), + OidIsValid(joinId), + canMerge, + canHash); operatorObjectId = OperatorGet(operatorName, operatorNamespace, @@ -442,8 +400,7 @@ OperatorCreate(const char *operatorName, commutatorId = get_other_operator(commutatorName, rightTypeId, leftTypeId, operatorName, operatorNamespace, - leftTypeId, rightTypeId, - true); + leftTypeId, rightTypeId); /* Permission check: must own other operator */ if (OidIsValid(commutatorId) && @@ -452,8 +409,9 @@ OperatorCreate(const char *operatorName, NameListToString(commutatorName)); /* - * self-linkage to this operator; will fix below. Note that only - * self-linkage for commutation makes sense. + * If self-linkage to the new operator is requested, we'll fix it + * below. (In case of self-linkage to an existing shell operator, we + * need do nothing special.) */ if (!OidIsValid(commutatorId)) selfCommutator = true; @@ -467,14 +425,24 @@ OperatorCreate(const char *operatorName, negatorId = get_other_operator(negatorName, leftTypeId, rightTypeId, operatorName, operatorNamespace, - leftTypeId, rightTypeId, - false); + leftTypeId, rightTypeId); /* Permission check: must own other operator */ if (OidIsValid(negatorId) && !object_ownercheck(OperatorRelationId, negatorId, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_OPERATOR, NameListToString(negatorName)); + + /* + * Prevent self negation, as it doesn't make sense. It's self + * negation if result is InvalidOid (negator would be the same + * operator but it doesn't exist yet) or operatorObjectId (we are + * replacing a shell that would need to be its own negator). + */ + if (!OidIsValid(negatorId) || negatorId == operatorObjectId) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("operator cannot be its own negator"))); } else negatorId = InvalidOid; @@ -548,11 +516,6 @@ OperatorCreate(const char *operatorName, /* Add dependencies for the entry */ address = makeOperatorDependencies(tup, true, isUpdate); - /* Post creation hook for new operator */ - InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); - - table_close(pg_operator_desc, RowExclusiveLock); - /* * If a commutator and/or negator link is provided, update the other * operator(s) to point at this one, if they don't already have a link. @@ -570,21 +533,95 @@ OperatorCreate(const char *operatorName, if (OidIsValid(commutatorId) || OidIsValid(negatorId)) OperatorUpd(operatorObjectId, commutatorId, negatorId, false); + /* Post creation hook for new operator */ + InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); + + table_close(pg_operator_desc, RowExclusiveLock); + return address; } /* - * Try to lookup another operator (commutator, etc) + * OperatorValidateParams * - * If not found, check to see if it is exactly the operator we are trying - * to define; if so, return InvalidOid. (Note that this case is only - * sensible for a commutator, so we error out otherwise.) If it is not - * the same operator, create a shell operator. + * Check that an operator with argument types leftTypeId and rightTypeId, + * returning operResultType, can have the attributes that are set to true. + * Raise an error for any disallowed attribute. + * + * Note: in ALTER OPERATOR, we only bother to pass "true" for attributes + * the command is trying to set, not those that may already be set. + * This is OK as long as the attribute checks are independent. + */ +void +OperatorValidateParams(Oid leftTypeId, + Oid rightTypeId, + Oid operResultType, + bool hasCommutator, + bool hasNegator, + bool hasRestrictionSelectivity, + bool hasJoinSelectivity, + bool canMerge, + bool canHash) +{ + if (!(OidIsValid(leftTypeId) && OidIsValid(rightTypeId))) + { + /* If it's not a binary op, these things mustn't be set: */ + if (hasCommutator) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only binary operators can have commutators"))); + if (hasJoinSelectivity) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only binary operators can have join selectivity"))); + if (canMerge) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only binary operators can merge join"))); + if (canHash) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only binary operators can hash"))); + } + + if (operResultType != BOOLOID) + { + /* If it's not a boolean op, these things mustn't be set: */ + if (hasNegator) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only boolean operators can have negators"))); + if (hasRestrictionSelectivity) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only boolean operators can have restriction selectivity"))); + if (hasJoinSelectivity) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only boolean operators can have join selectivity"))); + if (canMerge) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only boolean operators can merge join"))); + if (canHash) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("only boolean operators can hash"))); + } +} + +/* + * Try to lookup another operator (commutator, etc); return its OID + * + * If not found, check to see if it would be the same operator we are trying + * to define; if so, return InvalidOid. (Caller must decide whether + * that is sensible.) If it is not the same operator, create a shell + * operator. */ static Oid get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, const char *operatorName, Oid operatorNamespace, - Oid leftTypeId, Oid rightTypeId, bool isCommutator) + Oid leftTypeId, Oid rightTypeId) { Oid other_oid; bool otherDefined; @@ -611,14 +648,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, otherLeftTypeId == leftTypeId && otherRightTypeId == rightTypeId) { - /* - * self-linkage to this operator; caller will fix later. Note that - * only self-linkage for commutation makes sense. - */ - if (!isCommutator) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("operator cannot be its own negator or sort operator"))); + /* self-linkage to new operator; caller must handle this */ return InvalidOid; } @@ -643,7 +673,7 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, * For a given operator, look up its negator and commutator operators. * When isDelete is false, update their negator and commutator fields to * point back to the given operator; when isDelete is true, update those - * fields to no longer point back to the given operator. + * fields to be InvalidOid. * * The !isDelete case solves a problem for users who need to insert two new * operators that are the negator or commutator of each other, while the @@ -681,17 +711,40 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete) bool update_commutator = false; /* - * Out of due caution, we only change the commutator's oprcom field if - * it has the exact value we expected: InvalidOid when creating an - * operator, or baseId when dropping one. + * We can skip doing anything if the commutator's oprcom field is + * already what we want. While that's not expected in the isDelete + * case, it's perfectly possible when filling in a shell operator. */ - if (isDelete && t->oprcom == baseId) + if (isDelete && OidIsValid(t->oprcom)) { t->oprcom = InvalidOid; update_commutator = true; } - else if (!isDelete && !OidIsValid(t->oprcom)) + else if (!isDelete && t->oprcom != baseId) { + /* + * If commutator's oprcom field is already set to point to some + * third operator, it's an error. Changing its link would be + * unsafe, and letting the inconsistency stand would not be good + * either. This might be indicative of catalog corruption, so + * don't assume t->oprcom is necessarily a valid operator. + */ + if (OidIsValid(t->oprcom)) + { + char *thirdop = get_opname(t->oprcom); + + if (thirdop != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("commutator operator %s is already the commutator of operator %s", + NameStr(t->oprname), thirdop))); + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("commutator operator %s is already the commutator of operator %u", + NameStr(t->oprname), t->oprcom))); + } + t->oprcom = baseId; update_commutator = true; } @@ -726,17 +779,40 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete) bool update_negator = false; /* - * Out of due caution, we only change the negator's oprnegate field if - * it has the exact value we expected: InvalidOid when creating an - * operator, or baseId when dropping one. + * We can skip doing anything if the negator's oprnegate field is + * already what we want. While that's not expected in the isDelete + * case, it's perfectly possible when filling in a shell operator. */ - if (isDelete && t->oprnegate == baseId) + if (isDelete && OidIsValid(t->oprnegate)) { t->oprnegate = InvalidOid; update_negator = true; } - else if (!isDelete && !OidIsValid(t->oprnegate)) + else if (!isDelete && t->oprnegate != baseId) { + /* + * If negator's oprnegate field is already set to point to some + * third operator, it's an error. Changing its link would be + * unsafe, and letting the inconsistency stand would not be good + * either. This might be indicative of catalog corruption, so + * don't assume t->oprnegate is necessarily a valid operator. + */ + if (OidIsValid(t->oprnegate)) + { + char *thirdop = get_opname(t->oprnegate); + + if (thirdop != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("negator operator %s is already the negator of operator %s", + NameStr(t->oprname), thirdop))); + else + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("negator operator %s is already the negator of operator %u", + NameStr(t->oprname), t->oprnegate))); + } + t->oprnegate = baseId; update_negator = true; } diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index cd7f83136f..df84b839f5 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -54,6 +54,9 @@ static Oid ValidateRestrictionEstimator(List *restrictionName); static Oid ValidateJoinEstimator(List *joinName); +static Oid ValidateOperatorReference(List *name, + Oid leftTypeId, + Oid rightTypeId); /* * DefineOperator @@ -359,6 +362,53 @@ ValidateJoinEstimator(List *joinName) return joinOid; } +/* + * Look up and return the OID of an operator, + * given a possibly-qualified name and left and right type IDs. + * + * Verifies that the operator is defined (not a shell) and owned by + * the current user, so that we have permission to associate it with + * the operator being altered. Rejecting shell operators is a policy + * choice to help catch mistakes, rather than something essential. + */ +static Oid +ValidateOperatorReference(List *name, + Oid leftTypeId, + Oid rightTypeId) +{ + Oid oid; + bool defined; + + oid = OperatorLookup(name, + leftTypeId, + rightTypeId, + &defined); + + /* These message strings are chosen to match parse_oper.c */ + if (!OidIsValid(oid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("operator does not exist: %s", + op_signature_string(name, + leftTypeId, + rightTypeId)))); + + if (!defined) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("operator is only a shell: %s", + op_signature_string(name, + leftTypeId, + rightTypeId)))); + + if (!object_ownercheck(OperatorRelationId, oid, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_OPERATOR, + NameListToString(name)); + + return oid; +} + + /* * Guts of operator deletion. */ @@ -406,6 +456,10 @@ RemoveOperatorById(Oid operOid) * routine implementing ALTER OPERATOR SET (option = ...). * * Currently, only RESTRICT and JOIN estimator functions can be changed. + * COMMUTATOR, NEGATOR, MERGES, and HASHES attributes can be set if they + * have not been set previously. (Changing or removing one of these + * attributes could invalidate existing plans, which seems more trouble + * than it's worth.) */ ObjectAddress AlterOperator(AlterOperatorStmt *stmt) @@ -426,6 +480,14 @@ AlterOperator(AlterOperatorStmt *stmt) List *joinName = NIL; /* optional join sel. function */ bool updateJoin = false; Oid joinOid; + List *commutatorName = NIL; /* optional commutator operator name */ + Oid commutatorOid; + List *negatorName = NIL; /* optional negator operator name */ + Oid negatorOid; + bool canMerge = false; + bool updateMerges = false; + bool canHash = false; + bool updateHashes = false; /* Look up the operator */ oprId = LookupOperWithArgs(stmt->opername, false); @@ -456,6 +518,24 @@ AlterOperator(AlterOperatorStmt *stmt) joinName = param; updateJoin = true; } + else if (strcmp(defel->defname, "commutator") == 0) + { + commutatorName = defGetQualifiedName(defel); + } + else if (strcmp(defel->defname, "negator") == 0) + { + negatorName = defGetQualifiedName(defel); + } + else if (strcmp(defel->defname, "merges") == 0) + { + canMerge = defGetBoolean(defel); + updateMerges = true; + } + else if (strcmp(defel->defname, "hashes") == 0) + { + canHash = defGetBoolean(defel); + updateHashes = true; + } /* * The rest of the options that CREATE accepts cannot be changed. @@ -464,11 +544,7 @@ AlterOperator(AlterOperatorStmt *stmt) else if (strcmp(defel->defname, "leftarg") == 0 || strcmp(defel->defname, "rightarg") == 0 || strcmp(defel->defname, "function") == 0 || - strcmp(defel->defname, "procedure") == 0 || - strcmp(defel->defname, "commutator") == 0 || - strcmp(defel->defname, "negator") == 0 || - strcmp(defel->defname, "hashes") == 0 || - strcmp(defel->defname, "merges") == 0) + strcmp(defel->defname, "procedure") == 0) { ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -488,7 +564,7 @@ AlterOperator(AlterOperatorStmt *stmt) NameStr(oprForm->oprname)); /* - * Look up restriction and join estimators if specified + * Look up OIDs for any parameters specified */ if (restrictionName) restrictionOid = ValidateRestrictionEstimator(restrictionName); @@ -499,27 +575,78 @@ AlterOperator(AlterOperatorStmt *stmt) else joinOid = InvalidOid; - /* Perform additional checks, like OperatorCreate does */ - if (!(OidIsValid(oprForm->oprleft) && OidIsValid(oprForm->oprright))) + if (commutatorName) { - /* If it's not a binary op, these things mustn't be set: */ - if (OidIsValid(joinOid)) + /* commutator has reversed arg types */ + commutatorOid = ValidateOperatorReference(commutatorName, + oprForm->oprright, + oprForm->oprleft); + + /* + * We don't need to do anything extra for a self commutator as in + * OperatorCreate, since the operator surely exists already. + */ + } + else + commutatorOid = InvalidOid; + + if (negatorName) + { + negatorOid = ValidateOperatorReference(negatorName, + oprForm->oprleft, + oprForm->oprright); + + /* Must reject self-negation */ + if (negatorOid == oprForm->oid) ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only binary operators can have join selectivity"))); + errmsg("operator cannot be its own negator"))); + } + else + { + negatorOid = InvalidOid; } - if (oprForm->oprresult != BOOLOID) - { - if (OidIsValid(restrictionOid)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can have restriction selectivity"))); - if (OidIsValid(joinOid)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only boolean operators can have join selectivity"))); - } + /* + * Check that we're not changing any attributes that might be depended on + * by plans, while allowing no-op updates. + */ + if (OidIsValid(commutatorOid) && OidIsValid(oprForm->oprcom) && + commutatorOid != oprForm->oprcom) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("operator attribute \"%s\" cannot be changed if it has already been set", + "commutator"))); + + if (OidIsValid(negatorOid) && OidIsValid(oprForm->oprnegate) && + negatorOid != oprForm->oprnegate) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("operator attribute \"%s\" cannot be changed if it has already been set", + "negator"))); + + if (updateMerges && oprForm->oprcanmerge && !canMerge) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("operator attribute \"%s\" cannot be changed if it has already been set", + "merges"))); + + if (updateHashes && oprForm->oprcanhash && !canHash) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("operator attribute \"%s\" cannot be changed if it has already been set", + "hashes"))); + + /* Perform additional checks, like OperatorCreate does */ + OperatorValidateParams(oprForm->oprleft, + oprForm->oprright, + oprForm->oprresult, + OidIsValid(commutatorOid), + OidIsValid(negatorOid), + OidIsValid(restrictionOid), + OidIsValid(joinOid), + canMerge, + canHash); /* Update the tuple */ for (i = 0; i < Natts_pg_operator; ++i) @@ -531,12 +658,32 @@ AlterOperator(AlterOperatorStmt *stmt) if (updateRestriction) { replaces[Anum_pg_operator_oprrest - 1] = true; - values[Anum_pg_operator_oprrest - 1] = restrictionOid; + values[Anum_pg_operator_oprrest - 1] = ObjectIdGetDatum(restrictionOid); } if (updateJoin) { replaces[Anum_pg_operator_oprjoin - 1] = true; - values[Anum_pg_operator_oprjoin - 1] = joinOid; + values[Anum_pg_operator_oprjoin - 1] = ObjectIdGetDatum(joinOid); + } + if (OidIsValid(commutatorOid)) + { + replaces[Anum_pg_operator_oprcom - 1] = true; + values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(commutatorOid); + } + if (OidIsValid(negatorOid)) + { + replaces[Anum_pg_operator_oprnegate - 1] = true; + values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(negatorOid); + } + if (updateMerges) + { + replaces[Anum_pg_operator_oprcanmerge - 1] = true; + values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(canMerge); + } + if (updateHashes) + { + replaces[Anum_pg_operator_oprcanhash - 1] = true; + values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(canHash); } tup = heap_modify_tuple(tup, RelationGetDescr(catalog), @@ -546,6 +693,9 @@ AlterOperator(AlterOperatorStmt *stmt) address = makeOperatorDependencies(tup, false, true); + if (OidIsValid(commutatorOid) || OidIsValid(negatorOid)) + OperatorUpd(oprId, commutatorOid, negatorOid, false); + InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0); table_close(catalog, NoLock); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 50ed504e5a..c224df4ecc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10151,6 +10151,8 @@ operator_def_elem: ColLabel '=' NONE { $$ = makeDefElem($1, NULL, @1); } | ColLabel '=' operator_def_arg { $$ = makeDefElem($1, (Node *) $3, @1); } + | ColLabel + { $$ = makeDefElem($1, NULL, @1); } ; /* must be similar enough to def_arg to avoid reduce/reduce conflicts */ diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c index bdc8f8e26a..d08fc23f5b 100644 --- a/src/backend/parser/parse_oper.c +++ b/src/backend/parser/parse_oper.c @@ -70,9 +70,7 @@ static FuncDetailCode oper_select_candidate(int nargs, Oid *input_typeids, FuncCandidateList candidates, Oid *operOid); -static const char *op_signature_string(List *op, char oprkind, - Oid arg1, Oid arg2); -static void op_error(ParseState *pstate, List *op, char oprkind, +static void op_error(ParseState *pstate, List *op, Oid arg1, Oid arg2, FuncDetailCode fdresult, int location); static bool make_oper_cache_key(ParseState *pstate, OprCacheKey *key, @@ -110,26 +108,16 @@ LookupOperName(ParseState *pstate, List *opername, Oid oprleft, Oid oprright, /* we don't use op_error here because only an exact match is wanted */ if (!noError) { - char oprkind; - - if (!OidIsValid(oprleft)) - oprkind = 'l'; - else if (OidIsValid(oprright)) - oprkind = 'b'; - else - { + if (!OidIsValid(oprright)) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("postfix operators are not supported"), parser_errposition(pstate, location))); - oprkind = 0; /* keep compiler quiet */ - } ereport(ERROR, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("operator does not exist: %s", - op_signature_string(opername, oprkind, - oprleft, oprright)), + op_signature_string(opername, oprleft, oprright)), parser_errposition(pstate, location))); } @@ -446,7 +434,7 @@ oper(ParseState *pstate, List *opname, Oid ltypeId, Oid rtypeId, make_oper_cache_entry(&key, operOid); } else if (!noError) - op_error(pstate, opname, 'b', ltypeId, rtypeId, fdresult, location); + op_error(pstate, opname, ltypeId, rtypeId, fdresult, location); return (Operator) tup; } @@ -483,7 +471,7 @@ compatible_oper(ParseState *pstate, List *op, Oid arg1, Oid arg2, ereport(ERROR, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("operator requires run-time type coercion: %s", - op_signature_string(op, 'b', arg1, arg2)), + op_signature_string(op, arg1, arg2)), parser_errposition(pstate, location))); return (Operator) NULL; @@ -597,7 +585,7 @@ left_oper(ParseState *pstate, List *op, Oid arg, bool noError, int location) make_oper_cache_entry(&key, operOid); } else if (!noError) - op_error(pstate, op, 'l', InvalidOid, arg, fdresult, location); + op_error(pstate, op, InvalidOid, arg, fdresult, location); return (Operator) tup; } @@ -610,14 +598,14 @@ left_oper(ParseState *pstate, List *op, Oid arg, bool noError, int location) * This is typically used in the construction of operator-not-found error * messages. */ -static const char * -op_signature_string(List *op, char oprkind, Oid arg1, Oid arg2) +const char * +op_signature_string(List *op, Oid arg1, Oid arg2) { StringInfoData argbuf; initStringInfo(&argbuf); - if (oprkind != 'l') + if (OidIsValid(arg1)) appendStringInfo(&argbuf, "%s ", format_type_be(arg1)); appendStringInfoString(&argbuf, NameListToString(op)); @@ -631,7 +619,7 @@ op_signature_string(List *op, char oprkind, Oid arg1, Oid arg2) * op_error - utility routine to complain about an unresolvable operator */ static void -op_error(ParseState *pstate, List *op, char oprkind, +op_error(ParseState *pstate, List *op, Oid arg1, Oid arg2, FuncDetailCode fdresult, int location) { @@ -639,7 +627,7 @@ op_error(ParseState *pstate, List *op, char oprkind, ereport(ERROR, (errcode(ERRCODE_AMBIGUOUS_FUNCTION), errmsg("operator is not unique: %s", - op_signature_string(op, oprkind, arg1, arg2)), + op_signature_string(op, arg1, arg2)), errhint("Could not choose a best candidate operator. " "You might need to add explicit type casts."), parser_errposition(pstate, location))); @@ -647,7 +635,7 @@ op_error(ParseState *pstate, List *op, char oprkind, ereport(ERROR, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("operator does not exist: %s", - op_signature_string(op, oprkind, arg1, arg2)), + op_signature_string(op, arg1, arg2)), (!arg1 || !arg2) ? errhint("No operator matches the given name and argument type. " "You might need to add an explicit type cast.") : @@ -713,7 +701,6 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("operator is only a shell: %s", op_signature_string(opname, - opform->oprkind, opform->oprleft, opform->oprright)), parser_errposition(pstate, location))); @@ -827,7 +814,6 @@ make_scalar_array_op(ParseState *pstate, List *opname, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("operator is only a shell: %s", op_signature_string(opname, - opform->oprkind, opform->oprleft, opform->oprright)), parser_errposition(pstate, location))); diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h index aff372b4bb..e890cee2df 100644 --- a/src/include/catalog/pg_operator.h +++ b/src/include/catalog/pg_operator.h @@ -86,6 +86,11 @@ DECLARE_UNIQUE_INDEX_PKEY(pg_operator_oid_index, 2688, OperatorOidIndexId, pg_op DECLARE_UNIQUE_INDEX(pg_operator_oprname_l_r_n_index, 2689, OperatorNameNspIndexId, pg_operator, btree(oprname name_ops, oprleft oid_ops, oprright oid_ops, oprnamespace oid_ops)); +extern Oid OperatorLookup(List *operatorName, + Oid leftObjectId, + Oid rightObjectId, + bool *defined); + extern ObjectAddress OperatorCreate(const char *operatorName, Oid operatorNamespace, Oid leftTypeId, @@ -102,6 +107,16 @@ extern ObjectAddress makeOperatorDependencies(HeapTuple tuple, bool makeExtensionDep, bool isUpdate); +extern void OperatorValidateParams(Oid leftTypeId, + Oid rightTypeId, + Oid operResultType, + bool hasCommutator, + bool hasNegator, + bool hasRestrictionSelectivity, + bool hasJoinSelectivity, + bool canMerge, + bool canHash); + extern void OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete); #endif /* PG_OPERATOR_H */ diff --git a/src/include/parser/parse_oper.h b/src/include/parser/parse_oper.h index 5768a1ce87..c756caa360 100644 --- a/src/include/parser/parse_oper.h +++ b/src/include/parser/parse_oper.h @@ -42,6 +42,9 @@ extern Operator compatible_oper(ParseState *pstate, List *op, /* currently no need for compatible_left_oper/compatible_right_oper */ +/* Error reporting support */ +extern const char *op_signature_string(List *op, Oid arg1, Oid arg2); + /* Routines for identifying "<", "=", ">" operators for a type */ extern void get_sort_group_operators(Oid argtype, bool needLT, bool needEQ, bool needGT, diff --git a/src/test/regress/expected/alter_operator.out b/src/test/regress/expected/alter_operator.out index 71bd484282..4217ba15de 100644 --- a/src/test/regress/expected/alter_operator.out +++ b/src/test/regress/expected/alter_operator.out @@ -25,7 +25,7 @@ ORDER BY 1; (3 rows) -- --- Reset and set params +-- Test resetting and setting restrict and join attributes. -- ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); ALTER OPERATOR === (boolean, boolean) SET (JOIN = NONE); @@ -109,18 +109,10 @@ ORDER BY 1; -- -- Test invalid options. -- -ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = ====); -ERROR: operator attribute "commutator" cannot be changed -ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = ====); -ERROR: operator attribute "negator" cannot be changed ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = non_existent_func); ERROR: function non_existent_func(internal, oid, internal, integer) does not exist ALTER OPERATOR === (boolean, boolean) SET (JOIN = non_existent_func); ERROR: function non_existent_func(internal, oid, internal, smallint, internal) does not exist -ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = !==); -ERROR: operator attribute "commutator" cannot be changed -ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = !==); -ERROR: operator attribute "negator" cannot be changed -- invalid: non-lowercase quoted identifiers ALTER OPERATOR & (bit, bit) SET ("Restrict" = _int_contsel, "Join" = _int_contjoinsel); ERROR: operator attribute "Restrict" not recognized @@ -131,9 +123,145 @@ CREATE USER regress_alter_op_user; SET SESSION AUTHORIZATION regress_alter_op_user; ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); ERROR: must be owner of operator === --- Clean up RESET SESSION AUTHORIZATION; +-- +-- Test setting commutator, negator, merges, and hashes attributes, +-- which can only be set if not already set +-- +CREATE FUNCTION alter_op_test_fn_bool_real(boolean, real) +RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; +CREATE FUNCTION alter_op_test_fn_real_bool(real, boolean) +RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; +-- operator +CREATE OPERATOR === ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); +-- commutator +CREATE OPERATOR ==== ( + LEFTARG = real, + RIGHTARG = boolean, + PROCEDURE = alter_op_test_fn_real_bool +); +-- negator +CREATE OPERATOR !==== ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); +-- No-op setting already false hashes and merges to false works +ALTER OPERATOR === (boolean, real) SET (MERGES = false); +ALTER OPERATOR === (boolean, real) SET (HASHES = false); +-- Test setting merges and hashes +ALTER OPERATOR === (boolean, real) SET (MERGES); +ALTER OPERATOR === (boolean, real) SET (HASHES); +SELECT oprcanmerge, oprcanhash +FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + oprcanmerge | oprcanhash +-------------+------------ + t | t +(1 row) + +-- Test setting commutator +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====); +-- Check that oprcom has been set on both the operator and commutator, +-- that they reference each other, and that the operator used is the existing +-- one we created and not a new shell operator. +SELECT op.oprname AS operator_name, com.oprname AS commutator_name, + com.oprcode AS commutator_func + FROM pg_operator op + INNER JOIN pg_operator com ON (op.oid = com.oprcom AND op.oprcom = com.oid) + WHERE op.oprname = '===' + AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype; + operator_name | commutator_name | commutator_func +---------------+-----------------+---------------------------- + === | ==== | alter_op_test_fn_real_bool +(1 row) + +-- Cannot set self as negator +ALTER OPERATOR === (boolean, real) SET (NEGATOR = ===); +ERROR: operator cannot be its own negator +-- Test setting negator +ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====); +-- Check that oprnegate has been set on both the operator and negator, +-- that they reference each other, and that the operator used is the existing +-- one we created and not a new shell operator. +SELECT op.oprname AS operator_name, neg.oprname AS negator_name, + neg.oprcode AS negator_func + FROM pg_operator op + INNER JOIN pg_operator neg ON (op.oid = neg.oprnegate AND op.oprnegate = neg.oid) + WHERE op.oprname = '===' + AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype; + operator_name | negator_name | negator_func +---------------+--------------+---------------------------- + === | !==== | alter_op_test_fn_bool_real +(1 row) + +-- Test that no-op set succeeds +ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====); +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====); +ALTER OPERATOR === (boolean, real) SET (MERGES); +ALTER OPERATOR === (boolean, real) SET (HASHES); +-- Check that the final state of the operator is as we expect +SELECT oprcanmerge, oprcanhash, + pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator, + pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator + FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + oprcanmerge | oprcanhash | commutator | negator +-------------+------------+-----------------------------+------------------------------ + t | t | operator ====(real,boolean) | operator !====(boolean,real) +(1 row) + +-- Cannot change commutator, negator, merges, and hashes when already set +CREATE OPERATOR @= ( + LEFTARG = real, + RIGHTARG = boolean, + PROCEDURE = alter_op_test_fn_real_bool +); +CREATE OPERATOR @!= ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = @=); +ERROR: operator attribute "commutator" cannot be changed if it has already been set +ALTER OPERATOR === (boolean, real) SET (NEGATOR = @!=); +ERROR: operator attribute "negator" cannot be changed if it has already been set +ALTER OPERATOR === (boolean, real) SET (MERGES = false); +ERROR: operator attribute "merges" cannot be changed if it has already been set +ALTER OPERATOR === (boolean, real) SET (HASHES = false); +ERROR: operator attribute "hashes" cannot be changed if it has already been set +-- Cannot set an operator that already has a commutator as the commutator +ALTER OPERATOR @=(real, boolean) SET (COMMUTATOR = ===); +ERROR: commutator operator === is already the commutator of operator ==== +-- Cannot set an operator that already has a negator as the negator +ALTER OPERATOR @!=(boolean, real) SET (NEGATOR = ===); +ERROR: negator operator === is already the negator of operator !==== +-- Check no changes made +SELECT oprcanmerge, oprcanhash, + pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator, + pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator + FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + oprcanmerge | oprcanhash | commutator | negator +-------------+------------+-----------------------------+------------------------------ + t | t | operator ====(real,boolean) | operator !====(boolean,real) +(1 row) + +-- +-- Clean up +-- DROP USER regress_alter_op_user; DROP OPERATOR === (boolean, boolean); +DROP OPERATOR === (boolean, real); +DROP OPERATOR ==== (real, boolean); +DROP OPERATOR !==== (boolean, real); +DROP OPERATOR @= (real, boolean); +DROP OPERATOR @!= (boolean, real); DROP FUNCTION customcontsel(internal, oid, internal, integer); DROP FUNCTION alter_op_test_fn(boolean, boolean); +DROP FUNCTION alter_op_test_fn_bool_real(boolean, real); +DROP FUNCTION alter_op_test_fn_real_bool(real, boolean); diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out index f71b601f2d..d776d9c18c 100644 --- a/src/test/regress/expected/create_operator.out +++ b/src/test/regress/expected/create_operator.out @@ -260,6 +260,50 @@ CREATE OPERATOR #*# ( ); ERROR: permission denied for type type_op6 ROLLBACK; +-- Should fail. An operator cannot be its own negator. +BEGIN TRANSACTION; +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + negator = === +); +ERROR: operator cannot be its own negator +ROLLBACK; +-- Should fail. An operator cannot be its own negator. Here we check that +-- this error is detected when replacing a shell operator. +BEGIN TRANSACTION; +-- create a shell operator for ===!!! by referencing it as a commutator +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + commutator = ===!!! +); +CREATE OPERATOR ===!!! ( + leftarg = integer, + rightarg = integer, + procedure = int4ne, + negator = ===!!! +); +ERROR: operator cannot be its own negator +ROLLBACK; +-- test that we can't use part of an existing commutator or negator pair +-- as a commutator or negator +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + commutator = = +); +ERROR: commutator operator = is already the commutator of operator = +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + negator = <> +); +ERROR: negator operator <> is already the negator of operator = -- invalid: non-lowercase quoted identifiers CREATE OPERATOR === ( diff --git a/src/test/regress/sql/alter_operator.sql b/src/test/regress/sql/alter_operator.sql index fd40370165..8faecf7830 100644 --- a/src/test/regress/sql/alter_operator.sql +++ b/src/test/regress/sql/alter_operator.sql @@ -22,7 +22,7 @@ WHERE classid = 'pg_operator'::regclass AND ORDER BY 1; -- --- Reset and set params +-- Test resetting and setting restrict and join attributes. -- ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); @@ -74,12 +74,8 @@ ORDER BY 1; -- -- Test invalid options. -- -ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = ====); -ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = ====); ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = non_existent_func); ALTER OPERATOR === (boolean, boolean) SET (JOIN = non_existent_func); -ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = !==); -ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = !==); -- invalid: non-lowercase quoted identifiers ALTER OPERATOR & (bit, bit) SET ("Restrict" = _int_contsel, "Join" = _int_contjoinsel); @@ -92,9 +88,138 @@ SET SESSION AUTHORIZATION regress_alter_op_user; ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE); --- Clean up RESET SESSION AUTHORIZATION; + +-- +-- Test setting commutator, negator, merges, and hashes attributes, +-- which can only be set if not already set +-- + +CREATE FUNCTION alter_op_test_fn_bool_real(boolean, real) +RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; + +CREATE FUNCTION alter_op_test_fn_real_bool(real, boolean) +RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE; + +-- operator +CREATE OPERATOR === ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); + +-- commutator +CREATE OPERATOR ==== ( + LEFTARG = real, + RIGHTARG = boolean, + PROCEDURE = alter_op_test_fn_real_bool +); + +-- negator +CREATE OPERATOR !==== ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); + +-- No-op setting already false hashes and merges to false works +ALTER OPERATOR === (boolean, real) SET (MERGES = false); +ALTER OPERATOR === (boolean, real) SET (HASHES = false); + +-- Test setting merges and hashes +ALTER OPERATOR === (boolean, real) SET (MERGES); +ALTER OPERATOR === (boolean, real) SET (HASHES); +SELECT oprcanmerge, oprcanhash +FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + +-- Test setting commutator +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====); + +-- Check that oprcom has been set on both the operator and commutator, +-- that they reference each other, and that the operator used is the existing +-- one we created and not a new shell operator. +SELECT op.oprname AS operator_name, com.oprname AS commutator_name, + com.oprcode AS commutator_func + FROM pg_operator op + INNER JOIN pg_operator com ON (op.oid = com.oprcom AND op.oprcom = com.oid) + WHERE op.oprname = '===' + AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype; + +-- Cannot set self as negator +ALTER OPERATOR === (boolean, real) SET (NEGATOR = ===); + +-- Test setting negator +ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====); + +-- Check that oprnegate has been set on both the operator and negator, +-- that they reference each other, and that the operator used is the existing +-- one we created and not a new shell operator. +SELECT op.oprname AS operator_name, neg.oprname AS negator_name, + neg.oprcode AS negator_func + FROM pg_operator op + INNER JOIN pg_operator neg ON (op.oid = neg.oprnegate AND op.oprnegate = neg.oid) + WHERE op.oprname = '===' + AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype; + +-- Test that no-op set succeeds +ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====); +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====); +ALTER OPERATOR === (boolean, real) SET (MERGES); +ALTER OPERATOR === (boolean, real) SET (HASHES); + +-- Check that the final state of the operator is as we expect +SELECT oprcanmerge, oprcanhash, + pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator, + pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator + FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + +-- Cannot change commutator, negator, merges, and hashes when already set + +CREATE OPERATOR @= ( + LEFTARG = real, + RIGHTARG = boolean, + PROCEDURE = alter_op_test_fn_real_bool +); +CREATE OPERATOR @!= ( + LEFTARG = boolean, + RIGHTARG = real, + PROCEDURE = alter_op_test_fn_bool_real +); + +ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = @=); +ALTER OPERATOR === (boolean, real) SET (NEGATOR = @!=); +ALTER OPERATOR === (boolean, real) SET (MERGES = false); +ALTER OPERATOR === (boolean, real) SET (HASHES = false); + +-- Cannot set an operator that already has a commutator as the commutator +ALTER OPERATOR @=(real, boolean) SET (COMMUTATOR = ===); + +-- Cannot set an operator that already has a negator as the negator +ALTER OPERATOR @!=(boolean, real) SET (NEGATOR = ===); + +-- Check no changes made +SELECT oprcanmerge, oprcanhash, + pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator, + pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator + FROM pg_operator WHERE oprname = '===' + AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype; + +-- +-- Clean up +-- + DROP USER regress_alter_op_user; + DROP OPERATOR === (boolean, boolean); +DROP OPERATOR === (boolean, real); +DROP OPERATOR ==== (real, boolean); +DROP OPERATOR !==== (boolean, real); +DROP OPERATOR @= (real, boolean); +DROP OPERATOR @!= (boolean, real); + DROP FUNCTION customcontsel(internal, oid, internal, integer); DROP FUNCTION alter_op_test_fn(boolean, boolean); +DROP FUNCTION alter_op_test_fn_bool_real(boolean, real); +DROP FUNCTION alter_op_test_fn_real_bool(real, boolean); diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql index f53e24db3c..a3096f17df 100644 --- a/src/test/regress/sql/create_operator.sql +++ b/src/test/regress/sql/create_operator.sql @@ -210,6 +210,49 @@ CREATE OPERATOR #*# ( ); ROLLBACK; +-- Should fail. An operator cannot be its own negator. +BEGIN TRANSACTION; +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + negator = === +); +ROLLBACK; + +-- Should fail. An operator cannot be its own negator. Here we check that +-- this error is detected when replacing a shell operator. +BEGIN TRANSACTION; +-- create a shell operator for ===!!! by referencing it as a commutator +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + commutator = ===!!! +); +CREATE OPERATOR ===!!! ( + leftarg = integer, + rightarg = integer, + procedure = int4ne, + negator = ===!!! +); +ROLLBACK; + +-- test that we can't use part of an existing commutator or negator pair +-- as a commutator or negator +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + commutator = = +); +CREATE OPERATOR === ( + leftarg = integer, + rightarg = integer, + procedure = int4eq, + negator = <> +); + -- invalid: non-lowercase quoted identifiers CREATE OPERATOR === (