Re: [HACKERS] operator does not exist: character varying[] character[]
On 12/12/14, 7:16 PM, Tom Lane wrote: Jim Nasby jim.na...@bluetreble.com writes: I'd say that array_eq (and probably _cmp) just needs to be taught to fall back to what oper() does, but this part of the commit message gives me pause: Change the operator search algorithms to look for appropriate btree or hash index opclasses, instead of assuming operators named '' or '=' have the right semantics. As it should. array_cmp is the basis for a btree opclass, therefore it must *not* use operators that are not themselves btree operators. Quite aside from that, we need to move even further away from having internal system operations depend on operator-name-based lookups; see for instance the recent complaints over stuff like IS DISTINCT FROM failing for types whose operators aren't in the search path. Agreed, but in a way that's not what we're doing here; we're trying to utilize an operator the user asked us to use. Of course, array_eq assumes that we want equality; I think that's a problem itself. rowtypes suffer from this too, but presumably it's not as big a deal because we require typid's to match exactly. It's arguable that the typcache code should be taught to look for binary-compatible opclasses if it can't find one directly for the specified type. I'm not sure offhand what rules we'd need to make to ensure such a search would yield deterministic results, though. The risk I see is what happens when someone adds a new operator or cast and suddenly we have multiple paths. That should be fine for regular comparisons, but probably not in an index. Another possibility is that we might be able to extend the text_ops btree operator family to include an opclass entry for varchar, rather than relying on binary compatibility to find the text opclass. But that would also require some careful thought to understand what the relaxed invariants should be for the opfamily structure as a whole. We don't want to add more actual operators, for fear of creating ambiguous-operator lookup failures. Yeah, to me that sounds like heading back down the road of assuming = means = and the other fun we had before classes and families... but maybe I'm just being paranoid. I have an alternative idea, though I'm not sure it's worth the work. Instead of having special array-only operators we could instead apply regular operators to arrays. I believe we can do this and reuse existing operators, if we store an expression of how to combine the result from the previous iteration to the current one (ie: for this would be (prev AND current), if there is a result value that should stop iteration (for comparison operators that would be false), and what to do with different size arrays. In the last case, you're either going to use that to provide a final result, substitute a specific value for any missing elements, or throw an error. Pros: With some additional information in the catalog, we could provide a lot more array operations, using existing operator functions. We can use the same operator search logic we use for elements. If you can perform an operation on 2 elements and that operator has array support, then we're good to go. If we'd perform casting on the elements, we'd do the same casting with the array values. We no longer need to assume things like = means equal. If text = int actually meant length(text) = int then as long as that operator had array support you could do text[] = int[]. This keeps all operator logic together, regardless of whether the input is an array or a base element. Not Cons: Could this cause problems in the planner? Selectivity estimation comes to mind. transformAExprOp/make_op would need to do something different for arrays, because the function would need more information. If we can just extend OpExpr then maybe this isn't a big deal. (A simple grep shows 401 instances of OpExpr in src/backend). I don't think we could use this same technique with rowtypes. We'd still allow for array-specific operators; perhaps that might cause issues or at least confusion. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] operator does not exist: character varying[] character[]
On 12/9/14, 5:06 PM, Jim Nasby wrote: On 12/9/14, 4:30 PM, Tom Lane wrote: Jim Nasby jim.na...@bluetreble.com writes: On 12/9/14, 4:19 PM, Jim Nasby wrote: Is there any particular reason we don't allow comparing char and varchar arrays? If not I'll submit a patch. We're also missing operators on text and varchar arrays. Adding operators would be an incorrect fix. Right, I'm assuming this is a problem somewhere else (haven't looked into it yet). I just wanted confirmation that this is unexpected before I try and fix it. I'll take your silence on that point as confirmation that this is a bug. :) I've tracked down what's going on here; array_eq is lazy about finding an equality operator. It asks lookup_type_cache for TYPECACHE_EQ_OPR_FINFO, which means it looks first for a Btree Opclass, then a Hash Opclass. If neither is found then we fail. OTOH, the path taken in transformAExprOp is very different. It ends up at oper(), which looks for an exact operator match; if that fails we look for binary operators we can coerce to. That's the path that allows this to work in the non-array case. The question is why. :) array_eq's call to lookup_type_cache was created in 2003 [1] and hasn't been touched since. Previously it called equality_oper, which called compatible_oper, which called oper (same as transforAExprOp does). I'd say that array_eq (and probably _cmp) just needs to be taught to fall back to what oper() does, but this part of the commit message gives me pause: Change the operator search algorithms to look for appropriate btree or hash index opclasses, instead of assuming operators named '' or '=' have the right semantics. I can see where there are many places where we don't want to just assume than an oprname of = actually means =, but does that apply to arrays? If the user says array = array, isn't it safe to assume that that's the same thing as if tried to compare two values of the respective typelem's? Wouldn't the same be true for row comparison as well? [1] https://github.com/postgres/postgres/blame/master/src/backend/utils/adt/arrayfuncs.c#L3231 -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] operator does not exist: character varying[] character[]
Jim Nasby jim.na...@bluetreble.com writes: I'd say that array_eq (and probably _cmp) just needs to be taught to fall back to what oper() does, but this part of the commit message gives me pause: Change the operator search algorithms to look for appropriate btree or hash index opclasses, instead of assuming operators named '' or '=' have the right semantics. As it should. array_cmp is the basis for a btree opclass, therefore it must *not* use operators that are not themselves btree operators. Quite aside from that, we need to move even further away from having internal system operations depend on operator-name-based lookups; see for instance the recent complaints over stuff like IS DISTINCT FROM failing for types whose operators aren't in the search path. It's arguable that the typcache code should be taught to look for binary-compatible opclasses if it can't find one directly for the specified type. I'm not sure offhand what rules we'd need to make to ensure such a search would yield deterministic results, though. Another possibility is that we might be able to extend the text_ops btree operator family to include an opclass entry for varchar, rather than relying on binary compatibility to find the text opclass. But that would also require some careful thought to understand what the relaxed invariants should be for the opfamily structure as a whole. We don't want to add more actual operators, for fear of creating ambiguous-operator lookup failures. 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
[HACKERS] operator does not exist: character varying[] character[]
Is there any particular reason we don't allow comparing char and varchar arrays? If not I'll submit a patch. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] operator does not exist: character varying[] character[]
Jim Nasby jim.na...@bluetreble.com writes: On 12/9/14, 4:19 PM, Jim Nasby wrote: Is there any particular reason we don't allow comparing char and varchar arrays? If not I'll submit a patch. We're also missing operators on text and varchar arrays. Adding operators would be an incorrect fix. 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] operator does not exist: character varying[] character[]
On 12/9/14, 4:19 PM, Jim Nasby wrote: Is there any particular reason we don't allow comparing char and varchar arrays? If not I'll submit a patch. We're also missing operators on text and varchar arrays. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] operator does not exist: character varying[] character[]
On 12/9/14, 4:30 PM, Tom Lane wrote: Jim Nasby jim.na...@bluetreble.com writes: On 12/9/14, 4:19 PM, Jim Nasby wrote: Is there any particular reason we don't allow comparing char and varchar arrays? If not I'll submit a patch. We're also missing operators on text and varchar arrays. Adding operators would be an incorrect fix. Right, I'm assuming this is a problem somewhere else (haven't looked into it yet). I just wanted confirmation that this is unexpected before I try and fix it. I'll take your silence on that point as confirmation that this is a bug. :) -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- 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] operator precedence issues
On Fri, Aug 30, 2013 at 5:48 PM, Andres Freund and...@2ndquadrant.com wrote: Hi, On 2013-08-30 17:35:04 -0500, Merlin Moncure wrote: When a schema-qualified operator name is used in the OPERATOR syntax, as for example in: SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence shown in Table 4-2 for any other operator. This is true no matter which specific operator appears inside OPERATOR(). That rule seems intentionally designed to make it impossible to to override mathematical behaviors. Mainly curious -- was that intentional? You can change your search_path to include your schema before an explicitly listed pg_catalog afair. Not nice, but should work... hurk -- wish I had known that last week, but that's a nifty trick! It satisfies my particular problem (safe division) since in this case the problem is handled 'in function' and I can temporarily hack the search_path. Interestingly, if you do this the database doesn't match Ideally though you could specify operator precedence in the operator name itself though in such a way that bison pick it up. I don't know if that's possible since so many operator names have been given out without any thought to reserving characters for precedence, or if it would be worth the extra parsing time even if you could do it. Overriding stock operator behaviors is a really dodgy practice with the limited but important exception of handling certain classes of mathematical errors. While playing around with Andres's trick, I noticed that it works but will not match against operators taking any although those will match with explicit schema declaration (FWICT it goes through the search_path trying to explicitly match int/int operator then goes again matches any). That's pretty weird: postgres=# CREATE OR REPLACE FUNCTION SafeDiv( postgres(# anyelement, postgres(# anyelement) RETURNS anyelement AS postgres-# $$ postgres$# SELECT $1 OPERATOR(pg_catalog./) NULLIF($2, 0); postgres$# $$ LANGUAGE SQL; CREATE FUNCTION postgres=# set search_path to safediv, pg_catalog, public; SET postgres=# CREATE OPERATOR safediv./ postgres-# ( postgres(# PROCEDURE = SafeDiv, postgres(# LEFTARG = anyelement, postgres(# RIGHTARG = anyelement, postgres(# COMMUTATOR = / postgres(# ); CREATE OPERATOR postgres=# select 1/0; ERROR: division by zero postgres=# select 1 operator(safediv./) 0; ?column? -- (1 row) postgres=# CREATE OR REPLACE FUNCTION SafeDiv( postgres(# int4, postgres(# int4) RETURNS int4 AS postgres-# $$ postgres$# SELECT $1 OPERATOR(pg_catalog./) NULLIF($2, 0); postgres$# $$ LANGUAGE SQL; CREATE FUNCTION postgres=# postgres=# CREATE OPERATOR safediv./ postgres-# ( postgres(# PROCEDURE = SafeDiv, postgres(# LEFTARG = int4, postgres(# RIGHTARG = int4, postgres(# COMMUTATOR = / postgres(# ); CREATE OPERATOR postgres=# select 1/0; ?column? -- (1 row) merlin -- 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] operator precedence issues
On 2013-09-03 08:59:53 -0500, Merlin Moncure wrote: While playing around with Andres's trick, I noticed that it works but will not match against operators taking any although those will match with explicit schema declaration (FWICT it goes through the search_path trying to explicitly match int/int operator then goes again matches any). That's pretty weird: Not surprising. We look for the best match for an operator and explicitly matching types will be that. If there were no operator(int, int) your anyelement variant should get called. Ideally though you could specify operator precedence in the operator name itself though in such a way that bison pick it up. I don't know if that's possible since so many operator names have been given out without any thought to reserving characters for precedence, or if it would be worth the extra parsing time even if you could do it. Overriding stock operator behaviors is a really dodgy practice with the limited but important exception of handling certain classes of mathematical errors. I have to say, even those it seems like it's primary advantage is making it harder to read the code, but YMMV. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] operator precedence issues
Andres Freund and...@2ndquadrant.com writes: On 2013-09-03 08:59:53 -0500, Merlin Moncure wrote: While playing around with Andres's trick, I noticed that it works but will not match against operators taking any although those will match with explicit schema declaration (FWICT it goes through the search_path trying to explicitly match int/int operator then goes again matches any). That's pretty weird: Not surprising. We look for the best match for an operator and explicitly matching types will be that. If there were no operator(int, int) your anyelement variant should get called. Yeah, this has exactly nothing to do with operator precedence. Precedence is about which operator binds tighter in cases like A+B*C. 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
[HACKERS] operator precedence issues
On Tue, Sep 3, 2013 at 9:13 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-09-03 08:59:53 -0500, Merlin Moncure wrote: While playing around with Andres's trick, I noticed that it works but will not match against operators taking any although those will match with explicit schema declaration (FWICT it goes through the search_path trying to explicitly match int/int operator then goes again matches any). That's pretty weird: Not surprising. We look for the best match for an operator and explicitly matching types will be that. If there were no operator(int, int) your anyelement variant should get called. Yeah, this has exactly nothing to do with operator precedence. Precedence is about which operator binds tighter in cases like A+B*C. That all makes perfect sense -- thanks guys. For posterity, Andres's trick worked and did end up saving me some coding after all -- in my case I have to eval() some externally generated fairly complex expressions in SQL (via pl/pgsql EXECUTE) in the context of a much larger query. My de-parsing code ended up having bugs and it was much easier to tip-toe around the search_path (via SET LOCAL) and force the modified operator. merlin
[HACKERS] operator precedence issues
Hackers, The operator precedence rules seem hard wired to not be able to be worked around, not matter what. The pain point for me has always been the division operator -- once in a while I end up in a situation where I want to override it so that it wraps the divisor with NULLIF. There is no way I can see to do that: custom operator (for example '//') names evaluate in different precedence order which is a non-starter essentially. That I'm ok with given the reasoning in the docs, but I'm really scratching my head over this rule (via http://www.postgresql.org/docs/9.3/static/sql-syntax-lexical.html#SQL-PRECEDENCE): When a schema-qualified operator name is used in the OPERATOR syntax, as for example in: SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence shown in Table 4-2 for any other operator. This is true no matter which specific operator appears inside OPERATOR(). That rule seems intentionally designed to make it impossible to to override mathematical behaviors. Mainly curious -- was that intentional? merlin -- 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] operator precedence issues
Merlin Moncure mmonc...@gmail.com writes: The operator precedence rules seem hard wired to not be able to be worked around, not matter what. That's right. In the first place, bison is incapable of doing anything other than hard-wired operator precedence. In the second, if we did try to allow catalog-driven precedence, it would require catalog lookups during the raw parser phase, which isn't going to work for a number of implementation reasons; but worse than the implementation troubles is that the grammar would then become fundamentally ambiguous, eg there could be multiple correct parsings of A+B*C depending on what data types A,B,C have. So precedence is hard-wired based on the operator name. I'm really scratching my head over this rule (via http://www.postgresql.org/docs/9.3/static/sql-syntax-lexical.html#SQL-PRECEDENCE): When a schema-qualified operator name is used in the OPERATOR syntax, as for example in: SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence shown in Table 4-2 for any other operator. This is true no matter which specific operator appears inside OPERATOR(). Yeah. I'd rather have said that it's the same precedence as for the undecorated operator name, but again bison doesn't really have a way to do that. 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] operator precedence issues
Hi, On 2013-08-30 17:35:04 -0500, Merlin Moncure wrote: When a schema-qualified operator name is used in the OPERATOR syntax, as for example in: SELECT 3 OPERATOR(pg_catalog.+) 4; the OPERATOR construct is taken to have the default precedence shown in Table 4-2 for any other operator. This is true no matter which specific operator appears inside OPERATOR(). That rule seems intentionally designed to make it impossible to to override mathematical behaviors. Mainly curious -- was that intentional? You can change your search_path to include your schema before an explicitly listed pg_catalog afair. Not nice, but should work... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] operator dependency of commutator and negator, redux
Brendan Jurd dire...@gmail.com writes: On 20 December 2012 11:51, Tom Lane t...@sss.pgh.pa.us wrote: While reconsidering the various not-too-satisfactory fixes we thought of back then, I had a sudden thought. Instead of having a COMMUTATOR or NEGATOR forward reference create a shell operator and link to it, why not simply *ignore* such references? Then when the second operator is defined, go ahead and fill in both links? Ignore with warning sounds pretty good. So it would go something like this? # CREATE OPERATOR (... COMMUTATOR ); WARNING: COMMUTATOR (foo, foo) undefined, ignoring. CREATE OPERATOR # CREATE OPERATOR (... COMMUTATOR ); CREATE OPERATOR I was thinking a NOTICE at most. If it's a WARNING then restoring perfectly valid pg_dump files will result in lots of scary-looking chatter. You could make an argument for printing nothing at all, but that would probably mislead people who'd fat-fingered their COMMUTATOR entries. 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] operator dependency of commutator and negator, redux
On Thu, Dec 20, 2012 at 10:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: Brendan Jurd dire...@gmail.com writes: On 20 December 2012 11:51, Tom Lane t...@sss.pgh.pa.us wrote: While reconsidering the various not-too-satisfactory fixes we thought of back then, I had a sudden thought. Instead of having a COMMUTATOR or NEGATOR forward reference create a shell operator and link to it, why not simply *ignore* such references? Then when the second operator is defined, go ahead and fill in both links? Ignore with warning sounds pretty good. So it would go something like this? # CREATE OPERATOR (... COMMUTATOR ); WARNING: COMMUTATOR (foo, foo) undefined, ignoring. CREATE OPERATOR # CREATE OPERATOR (... COMMUTATOR ); CREATE OPERATOR I was thinking a NOTICE at most. If it's a WARNING then restoring perfectly valid pg_dump files will result in lots of scary-looking chatter. You could make an argument for printing nothing at all, but that would probably mislead people who'd fat-fingered their COMMUTATOR entries. What about jiggering the dump so that only the second of the two operators to be dumped includes the COMMUTATOR clause? Or even adding a separate ALTER OPERATOR COMMUTATOR statement (or something of the sort) that pg_dump can emit as a separate item. Even a NOTICE in pg_dump seems like too much chatter (witness recent quieting of some other NOTICE messages we've all grown tired of) but silently ignoring the problem doesn't seem smart either, for the reason you mention. -- 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] operator dependency of commutator and negator, redux
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 20, 2012 at 10:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: I was thinking a NOTICE at most. If it's a WARNING then restoring perfectly valid pg_dump files will result in lots of scary-looking chatter. You could make an argument for printing nothing at all, but that would probably mislead people who'd fat-fingered their COMMUTATOR entries. What about jiggering the dump so that only the second of the two operators to be dumped includes the COMMUTATOR clause? Seems messy and fragile. In particular this'd represent a lot of work in order to make it more likely that the restore malfunctions if someone makes use of pg_restore's -l switch to reorder the entries. Also it would not retroactively fix the problem for restoring dumps made with existing pg_dump versions. Even a NOTICE in pg_dump seems like too much chatter (witness recent quieting of some other NOTICE messages we've all grown tired of) pg_dump has included set client_min_messages = warning in its output for quite some time now. So as long as we don't insist on making this a WARNING, people won't see it in that usage. 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] operator dependency of commutator and negator, redux
Robert Haas robertmh...@gmail.com writes: a separate ALTER OPERATOR COMMUTATOR statement (or something of the sort) that pg_dump can emit as a separate item. Even a NOTICE in I like that capability, but it's not helping us in the backward compatibility section where we will still read commutator declarations as operator properties. And maintaining an extension with different syntax for CREATE OPERATOR depending on the major version would be a pain (it's the case already for create type by the way, painfully so). So I think Tom's idea is better to fix the problem at hand. About dropping the Operator Shell, we've been talking in the past about adding more properties to our operators to allow for some more optimizer tricks (reducing expressions to constants or straigth variable references at parse time, reducing joins, adding parametrized paths etc). I can think about assiociativity and neutral element, but that's a property of one operator only. Now there's the distributive property that happens in between two different operators and that maybe would better be added as an ALTER OPERATOR statement rather than a possibly forward reference when we come to that. I'm not too sure about other concepts that we might want to tackle down the road here, another angle here would be about support for parallelism where maybe operators property could tell the planner how to spread a complex where clause or output column computation… All in all, it looks to me like the current proposals on the table would allow us to dispose of the Operator Shell idea entirely. If we ever need it back, the ALTER OPERATOR trick looks like a better tool. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] operator dependency of commutator and negator, redux
Bug #7758 seems to be a rediscovery of the behavior that Itagaki-san complained of a couple years ago: http://archives.postgresql.org/pgsql-hackers/2010-09/msg02035.php While reconsidering the various not-too-satisfactory fixes we thought of back then, I had a sudden thought. Instead of having a COMMUTATOR or NEGATOR forward reference create a shell operator and link to it, why not simply *ignore* such references? Then when the second operator is defined, go ahead and fill in both links? The only case where this could result in an unsatisfactory outcome is if the second operator's CREATE command fails to include the converse COMMUTATOR or NEGATOR reference ... but that doesn't work very nicely today anyway, as you end up with a unidirectional reference, hardly a desirable state of affairs. Not only does this solve the problem complained of, but it allows for much stronger error checking, as there is no longer any need to allow inconsistent catalog states even transiently. We could start treating commutator/negator references as true dependencies, permanently preventing dangling references. We could probably even get rid of the notion of shell operators altogether. Thoughts? 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] operator dependency of commutator and negator, redux
On 20 December 2012 11:51, Tom Lane t...@sss.pgh.pa.us wrote: While reconsidering the various not-too-satisfactory fixes we thought of back then, I had a sudden thought. Instead of having a COMMUTATOR or NEGATOR forward reference create a shell operator and link to it, why not simply *ignore* such references? Then when the second operator is defined, go ahead and fill in both links? Ignore with warning sounds pretty good. So it would go something like this? # CREATE OPERATOR (... COMMUTATOR ); WARNING: COMMUTATOR (foo, foo) undefined, ignoring. CREATE OPERATOR # CREATE OPERATOR (... COMMUTATOR ); CREATE OPERATOR Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] OPERATOR FAMILY and pg_dump
Hi, If a basic operator family is created, e.g., create operator family of1 using btree; shouldn't pg_dump include this in its output? If not, why? Joe -- 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] OPERATOR FAMILY and pg_dump
Joe Abbate j...@freedomcircle.com writes: If a basic operator family is created, e.g., create operator family of1 using btree; shouldn't pg_dump include this in its output? If not, why? Quoting from the pg_dump source code: * We want to dump the opfamily only if (1) it contains loose operators * or functions, or (2) it contains an opclass with a different name or * owner. Otherwise it's sufficient to let it be created during creation * of the contained opclass, and not dumping it improves portability of * the dump. I guess if it contains no opclasses and no operators either, this code won't dump it, but isn't it rather useless in such a case? 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] OPERATOR FAMILY and pg_dump
On 09/07/2011 12:10 PM, Tom Lane wrote: I guess if it contains no opclasses and no operators either, this code won't dump it, but isn't it rather useless in such a case? Yes, I think it's useless, like a book cover without the contents, but ISTM it should still be dumped (perhaps someone started defining a family and forgot about it--oh, the puns that come to mind). Joe -- 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] Operator families vs. casts
Noah, Providing my thoughts on the 'mundane' question first. On Tue, May 24, 2011 at 1:40 PM, Noah Misch n...@leadboat.com wrote: I also had a more mundane design question in the second paragraph of [2]. It can probably wait for the review of the next version of the patch. However, given that it affects a large percentage of the patch, I'd appreciate any early feedback on it. Here's the relevant part of the original post: ATPostAlterTypeCleanup has this comment: /* * Re-parse the index and constraint definitions, and attach them to the * appropriate work queue entries. We do this before dropping because in * the case of a FOREIGN KEY constraint, we might not yet have exclusive * lock on the table the constraint is attached to, and we need to get * that before dropping. It's safe because the parser won't actually look * at the catalogs to detect the existing entry. */ Is the second sentence true? I don't think so, so I deleted it for now. Here is the sequence of lock requests against the table possessing the FOREIGN KEY constraint when we alter the parent/upstream column: transformAlterTableStmt - ShareRowExclusiveLock ATPostAlterTypeParse - lockmode of original ALTER TABLE RemoveTriggerById() for update trigger - ShareRowExclusiveLock RemoveTriggerById() for insert trigger - ShareRowExclusiveLock RemoveConstraintById() - AccessExclusiveLock CreateTrigger() for insert trigger - ShareRowExclusiveLock CreateTrigger() for update trigger - ShareRowExclusiveLock RI_Initial_Check() - AccessShareLock (3x) I think the statement in the second sentence of the comment is true. ATPostAlterTypeParse is called only from ATPostAlterTypeCleanup. It has to grab the lock on the table the constraint is attached to before dropping the constraint. What it does is it opens that relation with the same lock that is grabbed for AT_AlterColumnType subtype, i.e. AccessExclusiveLock. I think there is no preceding place in AlterTable chain, that grabs stronger lock on this relation. ShareRowExclusiveLock is taken in transformAlterTableStmt (1st function in your sequence), but this function is ultimately called from ATPostAlterTypeParse, just before the latter grabs the AccessExclusiveLock, so ultimately at the point where this comment is located no locks are taken for the table with a FOREIGN KEY constraint. Alexey. -- 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] Operator families vs. casts
Alexey, On Fri, Jun 10, 2011 at 05:46:42PM +0300, Alexey Klyukin wrote: Providing my thoughts on the 'mundane' question first. I was actually referring to this paragraph: The standing code handled index/constraint dependencies of changing columns by extracting the SQL definition using pg_get_indexdef or pg_get_constraintdef, dropping the object, and recreating it afresh. To implement this optimization for indexes and index-backed constraints, we need to update the index definition without perturbing its storage. I found two major ways to do this, and I'm unsure which will be preferable, so I'm attaching both as alternate implementations of the same outcome. I'd appreciate feedback on which is preferable. The first skips the drop and updates pg_index.indclass, pg_attribute, and pg_constraint.conexclop. The alternate patch retains the drop and create, then resurrects the old relfilenode and assigns it to the new object. The second approach is significantly simpler and smaller, but it seems less-like anything else we do. As a further variation on the second approach, I also considered drilling holes through the performDeletion() and DefineIndex() stacks to avoid the drop-and-later-preserve dynamic, but that seemed uglier. However, while we're on the topic you looked at: Here's the relevant part of the original post: ATPostAlterTypeCleanup has this comment: /* * Re-parse the index and constraint definitions, and attach them to the * appropriate work queue entries. We do this before dropping because in * the case of a FOREIGN KEY constraint, we might not yet have exclusive * lock on the table the constraint is attached to, and we need to get * that before dropping. It's safe because the parser won't actually look * at the catalogs to detect the existing entry. */ Is the second sentence true? I don't think so, so I deleted it for now. Here is the sequence of lock requests against the table possessing the FOREIGN KEY constraint when we alter the parent/upstream column: transformAlterTableStmt - ShareRowExclusiveLock ATPostAlterTypeParse - lockmode of original ALTER TABLE RemoveTriggerById() for update trigger - ShareRowExclusiveLock RemoveTriggerById() for insert trigger - ShareRowExclusiveLock RemoveConstraintById() - AccessExclusiveLock CreateTrigger() for insert trigger - ShareRowExclusiveLock CreateTrigger() for update trigger - ShareRowExclusiveLock RI_Initial_Check() - AccessShareLock (3x) I think the statement in the second sentence of the comment is true. ATPostAlterTypeParse is called only from ATPostAlterTypeCleanup. It has to grab the lock on the table the constraint is attached to before dropping the constraint. What it does is it opens that relation with the same lock that is grabbed for AT_AlterColumnType subtype, i.e. AccessExclusiveLock. I think there is no preceding place in AlterTable chain, that grabs stronger lock on this relation. ShareRowExclusiveLock is taken in transformAlterTableStmt (1st function in your sequence), but this function is ultimately called from ATPostAlterTypeParse, just before the latter grabs the AccessExclusiveLock, so ultimately at the point where this comment is located no locks are taken for the table with a FOREIGN KEY constraint. The comment is correct that we don't yet have a lock on the remote table at that point. But why do we need a lock before RemoveTriggerById() acquires one? True, it is bad form to get a ShareRowExclusiveLock followed by upgrading to an AccessExclusiveLock, but the solution there is that RemoveConstraintById() only needs a ShareRowExclusiveLock. Granted, in retrospect, I had no business editorializing on this matter. It's proximate to the patch's changes but unrelated to them. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Operator families vs. casts
PostgreSQL 9.1 will implement ALTER TABLE ALTER TYPE operations that use a binary coercion cast without rewriting the table or unrelated indexes. It will always rewrite any indexes and recheck any foreign key constraints that depend on a changing column. This is unnecessary for 100% of core binary coercion casts. In my original design[1], I planned to detect this by comparing the operator families of the old and would-be-new indexes. (This still yields some unnecessary rewrites; oid_ops and int4_ops are actually compatible, for example.) When I implemented[2] it, I found that the contracts[3] for operator families are not strong enough to prove that the existing indexes and constraints remain valid. Specifically, I wished assume val0 = val1 iff val0::a = val1::b for any val0, val1, a, b such that we resolve both equality operators in the same operator family. The operator family contracts say nothing about consistency with casts. Is there a credible use case for violating that assumption? If not, I'd like to document it as a requirement for operator family implementors. The above covers B-tree and hash operator families. GIN and GiST have no operator family contracts. Here was the comment in my first patch intended to sweep that under the table: ! * We do not document a contract for GIN or GiST operator families. Only the ! * GIN operator family array_ops has more than one constituent operator class, ! * and only typmod-only changes to arrays can avoid a rewrite. Preserving a GIN ! * index across such a change is safe. We therefore support GiST and GIN here ! * using the same rules as for B-tree and hash indexes, but that is mostly ! * academic. Any forthcoming contract for GiST or GIN operator families should, ! * all other things being equal, bolster the validity of this assumption. ! * ! * Exclusion constraints raise the question: can we trust that the operator has ! * the same semantics with the new type? The operator will fall in the index's ! * operator family. For B-tree or hash, the operator will be = or , ! * yielding an affirmative answer from contractual requirements. For GiST and ! * GIN, we assume that a similar requirement would fall out of any contract for ! * their operator families, should one arise. We therefore support exclusion ! * constraints without any special treatment, but this is again mostly academic. Any thoughts on what to do here? We could just add basic operator family contracts requiring what we need. Perhaps, instead, the ALTER TABLE code should require an operator family match for B-tree and hash but an operator class match for other access methods. For now, I plan to always rewrite indexes on expressions or having predicates. With effort, we could detect compatible changes there, too. I also had a more mundane design question in the second paragraph of [2]. It can probably wait for the review of the next version of the patch. However, given that it affects a large percentage of the patch, I'd appreciate any early feedback on it. Thanks, nm [1] http://archives.postgresql.org/message-id/20101229125625.ga27...@tornado.gateway.2wire.net [2] http://archives.postgresql.org/message-id/20110113230124.ga18...@tornado.gateway.2wire.net [3] http://www.postgresql.org/docs/9.0/interactive/xindex.html#XINDEX-OPFAMILY -- 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] Operator families vs. casts
Noah Misch n...@leadboat.com writes: PostgreSQL 9.1 will implement ALTER TABLE ALTER TYPE operations that use a binary coercion cast without rewriting the table or unrelated indexes. It will always rewrite any indexes and recheck any foreign key constraints that depend on a changing column. This is unnecessary for 100% of core binary coercion casts. In my original design[1], I planned to detect this by comparing the operator families of the old and would-be-new indexes. (This still yields some unnecessary rewrites; oid_ops and int4_ops are actually compatible, for example.) No, they aren't: signed and unsigned comparisons do not yield the same sort order. I think that example may destroy the rest of your argument. 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] Operator families vs. casts
On Tue, May 24, 2011 at 10:10:34AM -0400, Tom Lane wrote: Noah Misch n...@leadboat.com writes: PostgreSQL 9.1 will implement ALTER TABLE ALTER TYPE operations that use a binary coercion cast without rewriting the table or unrelated indexes. It will always rewrite any indexes and recheck any foreign key constraints that depend on a changing column. This is unnecessary for 100% of core binary coercion casts. In my original design[1], I planned to detect this by comparing the operator families of the old and would-be-new indexes. (This still yields some unnecessary rewrites; oid_ops and int4_ops are actually compatible, for example.) No, they aren't: signed and unsigned comparisons do not yield the same sort order. True; scratch the parenthetical comment. I think that example may destroy the rest of your argument. Not that I'm aware of. -- 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] operator classes for index?
Am 26.04.2011 17:37, schrieb Tom Lane: =?ISO-8859-1?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: Am 26.04.2011 14:28, schrieb Robert Haas: On Tue, Apr 26, 2011 at 5:18 AM, Yves Weißig weis...@rbg.informatik.tu-darmstadt.de wrote: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = (abstime,abstime), FUNCTION 1 hashint4(abstime,abstime); it yields: ERROR: function hashint4(abstime, abstime) does not exist My copy of PostgreSQL has a hashint4(integer) function, but no hashint4(abstime, abstime) function. Yes, I know, maybe my question wasn't clear enough. Following statement: ... I get: hash;abstime_ops;hashint4;2227;702;702;1;hashint4;abstime;abstime as an entry and suppose that hashint4 also takes abstime How is it done? How is hashint4 used to hash a value of abstime? Cheating ;-). That entry is hard-wired in pg_amproc.h so it does not pass through the same kind of error checking that CREATE OPERATOR CLASS applies. It works, physically, because abstime and integer are binary compatible (both 4-byte int-aligned pass-by-value types), but the catalog entries are a bit inconsistent. If we wanted to make this look completely clean, we'd have to create an alias function that was declared to take abstime. For instance you could do it like this: create function hashabstime(abstime) returns int4 as 'hashint4' language internal strict immutable; and then say FUNCTION 1 hashabstime(abstime) in CREATE OPERATOR CLASS. You might find this extract from the opr_sanity regression test instructive: -- For hash we can also do a little better: the support routines must be -- of the form hash(lefttype) returns int4. There are several cases where -- we cheat and use a hash function that is physically compatible with the -- datatype even though there's no cast, so this check does find a small -- number of entries. SELECT p1.amprocfamily, p1.amprocnum, p2.proname, p3.opfname FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3 WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND (amprocnum != 1 OR proretset OR prorettype != 'int4'::regtype OR pronargs != 1 OR NOT physically_coercible(amproclefttype, proargtypes[0]) OR amproclefttype != amprocrighttype) ORDER BY 1; amprocfamily | amprocnum |proname | opfname --+---++- 435 | 1 | hashint4 | date_ops 1999 | 1 | timestamp_hash | timestamptz_ops | 1 | hashchar | bool_ops 2223 | 1 | hashvarlena| bytea_ops 2225 | 1 | hashint4 | xid_ops 2226 | 1 | hashint4 | cid_ops (6 rows) regards, tom lane Thanks so much Tom, I was really loosing my mind on this one... now it works! Awesome. Yves -- 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] operator classes for index?
Am 26.04.2011 01:15, schrieb Tom Lane: =?ISO-8859-15?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: But anyway I am having trouble creating an operator class: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = , FUNCTION 1 abstimeeq(abstime,abstime); yields: ERROR: invalid procedure number 1, must be between 1 and 0 Apparently you've got zero in pg_am.amsupport for your new index AM. You need to set that to the number of support-procedure types your AM defines. Have you been through http://developer.postgresql.org/pgdocs/postgres/indexam.html and the docs and source code for the pg_am, pg_amop, pg_amproc catalogs? See http://developer.postgresql.org/pgdocs/postgres/catalogs.html as well as the src/include/catalog/ files for those catalogs. Additional, I don't know yet how to create index method support routines. I want to re-use the hash functions from hashfunc.c (because I do kind of a mapping). Is this possible? Just list them in your CREATE OPERATOR CLASS commands. Alright, now I starting to get the point. Still I have a problem, when I am trying to execute CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = (abstime,abstime), FUNCTION 1 hashint4(abstime,abstime); it yields: ERROR: function hashint4(abstime, abstime) does not exist though it exists (it is part of the hash AM), do I have to note the namespace or something else? pg_proc has a row for hashint4, but of course with different parameter types, int4 namely. Where do I cast them? Or is a implict conversion performed? Thanks again! How does index_getprocinfo(); now which support routine belongs to my index? It looks in pg_amproc to find the routines that are entered for the opclass associated with the index. This is a pretty direct representation of the FUNCTION entries from your previous CREATE OPERATOR CLASS (or if you prefer, CREATE OPERATOR CLASS is designed to provide the information needed to populate pg_amop and pg_amproc). regards, tom lane Greetz, Yves -- 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] operator classes for index?
On Tue, Apr 26, 2011 at 5:18 AM, Yves Weißig weis...@rbg.informatik.tu-darmstadt.de wrote: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = (abstime,abstime), FUNCTION 1 hashint4(abstime,abstime); it yields: ERROR: function hashint4(abstime, abstime) does not exist though it exists (it is part of the hash AM), do I have to note the My copy of PostgreSQL has a hashint4(integer) function, but no hashint4(abstime, abstime) function. -- 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] operator classes for index?
Am 26.04.2011 14:28, schrieb Robert Haas: On Tue, Apr 26, 2011 at 5:18 AM, Yves Weißig weis...@rbg.informatik.tu-darmstadt.de wrote: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = (abstime,abstime), FUNCTION 1 hashint4(abstime,abstime); it yields: ERROR: function hashint4(abstime, abstime) does not exist though it exists (it is part of the hash AM), do I have to note the My copy of PostgreSQL has a hashint4(integer) function, but no hashint4(abstime, abstime) function. Sorry. Yes, I know, maybe my question wasn't clear enough. Following statement: SELECT am.amname AS index_method, opfamily.opfname AS opfamily_name, proc.proname AS procedure_name, amproc.*, typel.typname AS left_typname, typer.typname AS right_typname FROM pg_am am, pg_amproc amproc, pg_proc proc, pg_opfamily opfamily, pg_type typel, pg_type typer WHERE amproc.amprocfamily = opfamily.oid AND amproc.amproc = proc.oid AND opfamily.opfmethod = am.oid AND am.amname = 'hash' AND amproc.amproclefttype = typel.oid AND amproc.amprocrighttype = typer.oid ORDER BY opfamily_name, procedure_name; I get: hash;abstime_ops;hashint4;2227;702;702;1;hashint4;abstime;abstime as an entry and suppose that hashint4 also takes abstime How is it done? How is hashint4 used to hash a value of abstime? -- 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] operator classes for index?
=?ISO-8859-1?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: Am 26.04.2011 14:28, schrieb Robert Haas: On Tue, Apr 26, 2011 at 5:18 AM, Yves Weißig weis...@rbg.informatik.tu-darmstadt.de wrote: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = (abstime,abstime), FUNCTION 1 hashint4(abstime,abstime); it yields: ERROR: function hashint4(abstime, abstime) does not exist My copy of PostgreSQL has a hashint4(integer) function, but no hashint4(abstime, abstime) function. Yes, I know, maybe my question wasn't clear enough. Following statement: ... I get: hash;abstime_ops;hashint4;2227;702;702;1;hashint4;abstime;abstime as an entry and suppose that hashint4 also takes abstime How is it done? How is hashint4 used to hash a value of abstime? Cheating ;-). That entry is hard-wired in pg_amproc.h so it does not pass through the same kind of error checking that CREATE OPERATOR CLASS applies. It works, physically, because abstime and integer are binary compatible (both 4-byte int-aligned pass-by-value types), but the catalog entries are a bit inconsistent. If we wanted to make this look completely clean, we'd have to create an alias function that was declared to take abstime. For instance you could do it like this: create function hashabstime(abstime) returns int4 as 'hashint4' language internal strict immutable; and then say FUNCTION 1 hashabstime(abstime) in CREATE OPERATOR CLASS. You might find this extract from the opr_sanity regression test instructive: -- For hash we can also do a little better: the support routines must be -- of the form hash(lefttype) returns int4. There are several cases where -- we cheat and use a hash function that is physically compatible with the -- datatype even though there's no cast, so this check does find a small -- number of entries. SELECT p1.amprocfamily, p1.amprocnum, p2.proname, p3.opfname FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3 WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND (amprocnum != 1 OR proretset OR prorettype != 'int4'::regtype OR pronargs != 1 OR NOT physically_coercible(amproclefttype, proargtypes[0]) OR amproclefttype != amprocrighttype) ORDER BY 1; amprocfamily | amprocnum |proname | opfname --+---++- 435 | 1 | hashint4 | date_ops 1999 | 1 | timestamp_hash | timestamptz_ops | 1 | hashchar | bool_ops 2223 | 1 | hashvarlena| bytea_ops 2225 | 1 | hashint4 | xid_ops 2226 | 1 | hashint4 | cid_ops (6 rows) 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] operator classes for index?
Am 24.04.2011 23:33, schrieb Tom Lane: =?ISO-8859-15?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: again index access methods, can somebody shed some light into operator classes for indexes? The documentation is an entry point, but after reading I still don't have a clue how exactly they are used and created? Perhaps somebody with great knowledge can supply an 101 on opeartor classes? Because I keep getting the hint: You must specify an operator class for the index or define a default operator class for the data type. Have you read http://developer.postgresql.org/pgdocs/postgres/indexes-opclass.html http://developer.postgresql.org/pgdocs/postgres/xindex.html and the reference pages for CREATE OPERATOR CLASS/FAMILY? Thanks Tom, those links helped me understanding! Especially the contrib modules served as good examples. But anyway I am having trouble creating an operator class: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = , FUNCTION 1 abstimeeq(abstime,abstime); yields: ERROR: invalid procedure number 1, must be between 1 and 0 SQL Status:42P17 I couldn't find additional information to the error via google, what is wrong with the create statement? Additional, I don't know yet how to create index method support routines. I want to re-use the hash functions from hashfunc.c (because I do kind of a mapping). Is this possible? How does index_getprocinfo(); now which support routine belongs to my index? If it's still not coming together for you, there are numerous examples of creating operator classes in the contrib modules. The GIST and GIN documentation might be relevant as well: http://developer.postgresql.org/pgdocs/postgres/gist.html http://developer.postgresql.org/pgdocs/postgres/gin.html regards, tom lane Greetz, Yves -- 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] operator classes for index?
=?ISO-8859-15?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: But anyway I am having trouble creating an operator class: CREATE OPERATOR CLASS abstime_ops DEFAULT FOR TYPE abstime USING ebi FAMILY abstime_ops AS OPERATOR 1 = , FUNCTION 1 abstimeeq(abstime,abstime); yields: ERROR: invalid procedure number 1, must be between 1 and 0 Apparently you've got zero in pg_am.amsupport for your new index AM. You need to set that to the number of support-procedure types your AM defines. Have you been through http://developer.postgresql.org/pgdocs/postgres/indexam.html and the docs and source code for the pg_am, pg_amop, pg_amproc catalogs? See http://developer.postgresql.org/pgdocs/postgres/catalogs.html as well as the src/include/catalog/ files for those catalogs. Additional, I don't know yet how to create index method support routines. I want to re-use the hash functions from hashfunc.c (because I do kind of a mapping). Is this possible? Just list them in your CREATE OPERATOR CLASS commands. How does index_getprocinfo(); now which support routine belongs to my index? It looks in pg_amproc to find the routines that are entered for the opclass associated with the index. This is a pretty direct representation of the FUNCTION entries from your previous CREATE OPERATOR CLASS (or if you prefer, CREATE OPERATOR CLASS is designed to provide the information needed to populate pg_amop and pg_amproc). 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
[HACKERS] operator classes for index?
Hi, again index access methods, can somebody shed some light into operator classes for indexes? The documentation is an entry point, but after reading I still don't have a clue how exactly they are used and created? Perhaps somebody with great knowledge can supply an 101 on opeartor classes? Because I keep getting the hint: You must specify an operator class for the index or define a default operator class for the data type. Greets, Yves -- 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] operator classes for index?
=?ISO-8859-15?Q?Yves_Wei=DFig?= weis...@rbg.informatik.tu-darmstadt.de writes: again index access methods, can somebody shed some light into operator classes for indexes? The documentation is an entry point, but after reading I still don't have a clue how exactly they are used and created? Perhaps somebody with great knowledge can supply an 101 on opeartor classes? Because I keep getting the hint: You must specify an operator class for the index or define a default operator class for the data type. Have you read http://developer.postgresql.org/pgdocs/postgres/indexes-opclass.html http://developer.postgresql.org/pgdocs/postgres/xindex.html and the reference pages for CREATE OPERATOR CLASS/FAMILY? If it's still not coming together for you, there are numerous examples of creating operator classes in the contrib modules. The GIST and GIN documentation might be relevant as well: http://developer.postgresql.org/pgdocs/postgres/gist.html http://developer.postgresql.org/pgdocs/postgres/gin.html 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
[HACKERS] operator dependency of commutator and negator
When we drop an operator used by other operators as COMMUTATOR or NEGATOR, pg_dump generates an invalid SQL command for the operators depending on the dropped one. Is it an unavoidable restriction? CREATE OPERATOR ( PROCEDURE = text_lt, LEFTARG = text, RIGHTARG = text, COMMUTATOR = ); CREATE OPERATOR ( PROCEDURE = text_gt, LEFTARG = text, RIGHTARG = text, COMMUTATOR = ); DROP OPERATOR (text, text); $ pg_dump -- -- Name: ; Type: OPERATOR; Schema: public; Owner: postgres -- CREATE OPERATOR ( PROCEDURE = text_lt, LEFTARG = text, RIGHTARG = text, COMMUTATOR = 16395 == HERE ); -- Itagaki Takahiro -- 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] operator dependency of commutator and negator
Excerpts from Itagaki Takahiro's message of mié sep 29 03:56:33 -0400 2010: When we drop an operator used by other operators as COMMUTATOR or NEGATOR, pg_dump generates an invalid SQL command for the operators depending on the dropped one. Is it an unavoidable restriction? Maybe we need a pg_depend entry from each pg_operator entry to the other one. The problem is that this creates a cycle in the depends graph; not sure how well these are handled in the code, if at all. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] operator dependency of commutator and negator
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Itagaki Takahiro's message of mié sep 29 03:56:33 -0400 2010: When we drop an operator used by other operators as COMMUTATOR or NEGATOR, pg_dump generates an invalid SQL command for the operators depending on the dropped one. Is it an unavoidable restriction? Maybe we need a pg_depend entry from each pg_operator entry to the other one. The problem is that this creates a cycle in the depends graph; not sure how well these are handled in the code, if at all. See the comment in catalog/pg_operator.c: /* * NOTE: we do not consider the operator to depend on the associated * operators oprcom and oprnegate. We would not want to delete this * operator if those go away, but only reset the link fields; which is not * a function that the dependency code can presently handle. (Something * could perhaps be done with objectSubId though.) For now, it's okay to * let those links dangle if a referenced operator is removed. */ I'm not sure that fixing this case is worth the amount of work it'd take. How often do you drop just one member of a commutator pair? 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] operator dependency of commutator and negator
On Wed, Sep 29, 2010 at 11:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure that fixing this case is worth the amount of work it'd take. How often do you drop just one member of a commutator pair? I found the issue when an user tries to write a safe installer script under DROP before CREATE coding rule: 1. DROP OPERATOR IF EXISTS ... ; 2. CREATE OPERATOR (... COMMUTATOR ); 3. DROP OPERATOR IF EXISTS ... ; 4. CREATE OPERATOR (... COMMUTATOR ); 3 drops catalog-only added at 2, and 4 adds a operator that has a different oid from 's commutator. The operator becomes broken state in system catalog. Anyway, it must be a rare case, and we can just avoid the usage. -- Itagaki Takahiro -- 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] operator dependency of commutator and negator
Itagaki Takahiro itagaki.takah...@gmail.com writes: On Wed, Sep 29, 2010 at 11:56 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure that fixing this case is worth the amount of work it'd take. How often do you drop just one member of a commutator pair? I found the issue when an user tries to write a safe installer script under DROP before CREATE coding rule: 1. DROP OPERATOR IF EXISTS ... ; 2. CREATE OPERATOR (... COMMUTATOR ); 3. DROP OPERATOR IF EXISTS ... ; 4. CREATE OPERATOR (... COMMUTATOR ); 3 drops catalog-only added at 2, and 4 adds a operator that has a different oid from 's commutator. The operator becomes broken state in system catalog. Anyway, it must be a rare case, and we can just avoid the usage. Yeah. The above script seems incorrect anyway: if we did clean up the commutator links fully then step 3 would undo the effect of step 2. So really you should drop all the operators first and then start creating new ones. On the other hand ... the above script pattern would do the right thing if OperatorUpd() were willing to overwrite existing nonzero values in the referenced operators' entries. I'm not sure if this is a good idea though. I think that the reason it doesn't do it now is so that you don't accidentally damage the links in an existing unrelated operator. But AFAICS there are no cases where commutator and negator pairs shouldn't be symmetrical, so simply doing nothing doesn't seem like the right thing either: if you don't modify the other operator then you're definitely leaving an inconsistent state in the catalogs. Maybe what we should do is require the user to own the referenced operator and then unconditionally force the referenced operator's link to match. 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] operator exclusion constraints
On Thu, Mar 11, 2010 at 5:29 AM, Tom Lane t...@sss.pgh.pa.us wrote: Indexes: foo_pkey PRIMARY KEY, btree (f1), tablespace ts1 foo_f2_exclusion btree (f2), tablespace ts1 foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED Exclusion constraints: foo_f2_exclusion EXCLUDE USING btree (f2 WITH =) foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY DEFERRED This might have been defensible back when the idea was to keep constraints decoupled from indexes, but now it just looks bizarre. The only really bizarre part is the DEFERRABLE INITIALLY DEFERRED on the index. We should either get rid of the Exclusion constraints: display and attach the info to the index entries, or hide indexes that are attached to exclusion constraints. I lean to the former on the grounds of the precedent for unique/pkey indexes --- which is not totally arbitrary, since an index is usable as a query index regardless of its function as a constraint. It's probably a debatable point though. There is a third option -- print PRIMARY keys twice, once as a btree index and again as a constraint where it says somehting like USING index foo_pkey I think in the long term that would be best -- especially if we combine it with a patch to be able to create a new primary key constraint using an existing index. That's something people have been asking for anyways and I think it's a somewhat important property that these lines can be copy pasted and run nearly as-is to recreate the objects. I definitely agree that your other proposed way to go is worse. I think people need a list of indexes in one place. So given the current syntax for creating these I think your proposed change is the least worst alternative. -- greg -- 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] operator exclusion constraints
Greg Stark gsst...@mit.edu writes: There is a third option -- print PRIMARY keys twice, once as a btree index and again as a constraint where it says somehting like USING index foo_pkey No, that's exactly what I hate about the current behavior for exclusion constraints, and I'd like it even less for more-common options like primary or unique constraints. \d is too d*mn verbose already; there is no percentage in making it even longer by printing redundant entries for many indexes. One thing I did think about was converting PK/UNIQUE indexes to be printed by pg_get_constraintdef() too, rather than assembling an ad-hoc output for them as we do now. This would make the code a bit simpler but would involve some small changes in the output --- in particular, you wouldn't see any indication that they were btrees, since there's no place for that in standard constraint syntax. On balance it didn't seem like an improvement, although it would partially respond to your desire to have the output be cut-and-pasteable. 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] operator exclusion constraints
On Thu, 2010-03-11 at 00:29 -0500, Tom Lane wrote: Patch changes: Indexes: foo_pkey PRIMARY KEY, btree (f1), tablespace ts1 foo_f2_exclusion btree (f2), tablespace ts1 foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED Exclusion constraints: foo_f2_exclusion EXCLUDE USING btree (f2 WITH =) foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY DEFERRED To: Indexes: foo_pkey PRIMARY KEY, btree (f1), tablespace ts1 foo_f2_exclusion EXCLUDE USING btree (f2 WITH =), tablespace ts1 foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY DEFERRED Any objections? Looks good to me. Regards, Jeff Davis -- 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] operator exclusion constraints
Awhile back I wrote:
* I'm not too satisfied with the behavior of psql's \d:
regression=# create table foo (f1 int primary key using index tablespace ts1,
regression(# f2 int, EXCLUDE USING btree (f2 WITH =) using index tablespace
ts1,
regression(# f3 int, EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY
DEFERRED);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index foo_pkey for
table foo
NOTICE: CREATE TABLE / EXCLUDE will create implicit index foo_f2_exclusion
for table foo
NOTICE: CREATE TABLE / EXCLUDE will create implicit index foo_f3_exclusion
for table foo
CREATE TABLE
regression=# \d foo
Table public.foo
Column | Type | Modifiers
+-+---
f1 | integer | not null
f2 | integer |
f3 | integer |
Indexes:
foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
foo_f2_exclusion btree (f2), tablespace ts1
foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED
Exclusion constraints:
foo_f2_exclusion EXCLUDE USING btree (f2 WITH =)
foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY
DEFERRED
regression=#
This might have been defensible back when the idea was to keep constraints
decoupled from indexes, but now it just looks bizarre. We should either
get rid of the Exclusion constraints: display and attach the info to
the index entries, or hide indexes that are attached to exclusion
constraints. I lean to the former on the grounds of the precedent for
unique/pkey indexes --- which is not totally arbitrary, since an index
is usable as a query index regardless of its function as a constraint.
It's probably a debatable point though.
Attached is a patch against HEAD that folds exclusion constraints into
\d's regular indexes list. With this, the above example produces
Table public.foo
Column | Type | Modifiers
+-+---
f1 | integer | not null
f2 | integer |
f3 | integer |
Indexes:
foo_pkey PRIMARY KEY, btree (f1), tablespace ts1
foo_f2_exclusion EXCLUDE USING btree (f2 WITH =), tablespace ts1
foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY
DEFERRED
Any objections?
regards, tom lane
? psql
Index: describe.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.240
diff -c -r1.240 describe.c
*** describe.c 11 Mar 2010 04:36:43 - 1.240
--- describe.c 11 Mar 2010 05:18:28 -
***
*** 1105,
bool hasrules;
bool hastriggers;
bool hasoids;
- bool hasexclusion;
Oid tablespace;
char *reloptions;
char *reloftype;
--- 1105,1110
***
*** 1128,1135
printfPQExpBuffer(buf,
SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules,
c.relhastriggers, c.relhasoids,
! %s, c.reltablespace, c.relhasexclusion,
! CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::text END\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n
WHERE c.oid = '%s'\n,
--- 1127,1134
printfPQExpBuffer(buf,
SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules,
c.relhastriggers, c.relhasoids,
! %s, c.reltablespace,
! CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END\n
FROM pg_catalog.pg_class c\n
LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid)\n
WHERE c.oid = '%s'\n,
***
*** 1207,1216
strdup(PQgetvalue(res, 0, 6)) : 0;
tableinfo.tablespace = (pset.sversion = 8) ?
atooid(PQgetvalue(res, 0, 7)) : 0;
! tableinfo.hasexclusion = (pset.sversion = 9) ?
! strcmp(PQgetvalue(res, 0, 8), t) == 0 : false;
! tableinfo.reloftype = (pset.sversion = 9 strcmp(PQgetvalue(res, 0, 9), ) != 0) ?
! strdup(PQgetvalue(res, 0, 9)) : 0;
PQclear(res);
res = NULL;
--- 1206,1213
strdup(PQgetvalue(res, 0, 6)) : 0;
tableinfo.tablespace = (pset.sversion = 8) ?
atooid(PQgetvalue(res, 0, 7)) : 0;
! tableinfo.reloftype = (pset.sversion = 9 strcmp(PQgetvalue(res, 0, 8), ) != 0) ?
! strdup(PQgetvalue(res, 0, 8)) : 0;
PQclear(res);
res = NULL;
***
*** 1545,1571
appendPQExpBuffer(buf, i.indisvalid, );
else
appendPQExpBuffer(buf, true as indisvalid, );
! appendPQExpBuffer(buf, pg_catalog.pg_get_indexdef(i.indexrelid, 0, true));
if (pset.sversion = 9)
appendPQExpBuffer(buf,
! ,\n (NOT i.indimmediate) AND
! EXISTS (SELECT 1 FROM pg_catalog.pg_constraint
! WHERE conrelid = i.indrelid AND
! conindid = i.indexrelid AND
! contype IN ('p','u','x') AND
! condeferrable) AS condeferrable
! ,\n (NOT
Re: [HACKERS] operator exclusion constraints
On Mon, 2009-12-07 at 00:26 -0500, Tom Lane wrote: Jeff Davis pg...@j-davis.com writes: [ exclusion constraint patch ] Applied after quite a lot of editorialization. For future reference, here is a summary of what I did: Thank you for the suggestions, and the other constructive criticism during development. Regards, Jeff Davis -- 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: [ exclusion constraints patch ] Still working on this patch. I ran into something I didn't like at all: + if (newrel != NULL) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg(cannot rewrite table while adding + operator exclusion constraint))); This would be bad enough if the restriction were what the message alleges, ie, you can't write an ALTER TABLE that both rewrites the heap and adds an exclusion constraint. However, actually the error also occurs if you issue a rewriting ALTER TABLE against a table that already has an exclusion constraint. That raises it from annoyance to showstopper IMO. There is not a whole lot that can be done to fix this given the design that exclusion checking is supposed to be done in ATRewriteTable --- when that runs, we of course have not built the new index yet, so there's no way to use it to check the constraint. I think the right way to go at it is to drop that part of the patch and instead have index_build execute a separate pass to check the constraint after it's built an exclusion-constraint index. In the typical case where you're just doing ALTER TABLE ADD CONSTRAINT EXCLUDE, this ends up being the same amount of work since there's no need to run an ATRewriteTable scan at all. It would be extra work if someone tried to add multiple exclusion constraints in one ALTER; but I have a hard time getting excited about that, especially in view of the fact that the cost of the index searches is going to swamp the heapscan anyway. This approach would also mean that the constraint is re-verified if someone does a REINDEX. I think this is appropriate behavior since reindexing a regular unique index also causes re-verification. However I can imagine someone objecting on grounds of cost --- it would probably about double the cost of reindexing compared to assuming the constraint is still good. We could probably teach index_build to skip the check pass during REINDEX, but I don't really want to. Comments? 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] operator exclusion constraints
On Sun, 2009-12-06 at 10:46 -0500, Tom Lane wrote: This would be bad enough if the restriction were what the message alleges, ie, you can't write an ALTER TABLE that both rewrites the heap and adds an exclusion constraint. However, actually the error also occurs if you issue a rewriting ALTER TABLE against a table that already has an exclusion constraint. That raises it from annoyance to showstopper IMO. The following works as expected for me: create table foo(i int, j int, k int); -- the following causes the error in question: alter table foo add exclude(i with =), alter column k type text; alter table foo add exclude(i with =); -- works alter table foo alter column k type text; -- works So the workaround is simply to break the ALTER into two statements. (Aside: the error message should probably have a DETAIL component telling the user to break up the ALTER commands into separate actions.) Aha -- I think I see the problem you're having: if you try to rewrite one of the columns contained in the exclusion constraint, you get that error: create table bar_exclude(i int, exclude (i with =)); -- raises same error, which is confusing alter table bar_exclude alter column i type text; Compared with UNIQUE: create table bar_unique(i int unique); alter table bar_unique alter column i type text; -- works However, I think we _want_ exclusion constraints to fail in that case. Unique constraints can succeed because both types support UNIQUE, and the semantics are the same. But in the case of exclusion constraints, it's quite likely that the semantics will be different or the named operator won't exist at all. I think it's more fair to compare with a unique index on an expression: create table bar_unique2(i int); create unique index bar_unique2_idx on bar_unique2 ((i + 1)); alter table bar_unique2 alter column i type text; ERROR: operator does not exist: text + integer You could make the argument that we should do the same thing: try to re-apply the expression on top of the new column. The only situation where I can imagine that would be useful is if you are using exclusion constraints in place of UNIQUE (even then, it's different, because it uses operator names, not BTEqualStrategyNumber for the default btree opclass). If the user alters the type of a column that's part of an exclusion constraint, the semantics are complex enough that I think we should inform them that they need to drop the constraint, change the column, and re-add it. So, my personal opinion is that we need to change the error message (and probably have two distinct error messages for the two cases) rather than changing the algorithm. Comments? Regards, Jeff Davis -- 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: Aha -- I think I see the problem you're having: if you try to rewrite one of the columns contained in the exclusion constraint, you get that error: It fails for me regardless of which column is actually modified. It could be this is a consequence of other changes I've been making, but given the way ALTER TABLE works it's hard to see why the specific column being modified would matter. Anything that forces a table rewrite would lead to running through this code path. I don't really agree with your argument that it's okay to reject the case of altering the underlying column type, anyhow. There's enough commonality of operator names that it's sensible to try to preserve the constraint across a change. Consider replacing a circle with a polygon, say. A no-overlap restriction is still sensible. 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] operator exclusion constraints
On Sun, 2009-12-06 at 14:06 -0500, Tom Lane wrote: It fails for me regardless of which column is actually modified. It could be this is a consequence of other changes I've been making, but given the way ALTER TABLE works it's hard to see why the specific column being modified would matter. Anything that forces a table rewrite would lead to running through this code path. In ATAddOperatorExclusionConstraint(): tab-constraints = lappend(tab-constraints, newcon); if that isn't done, it doesn't go into the code path with that error message at all. I don't really agree with your argument that it's okay to reject the case of altering the underlying column type, anyhow. There's enough commonality of operator names that it's sensible to try to preserve the constraint across a change. Consider replacing a circle with a polygon, say. A no-overlap restriction is still sensible. Let's say someone is changing from box to a postgis geometry representing a box. For boxes, = actually means equal area (aside: this is insane); but for polygons, = mean equal. Changing in the other direction is bad, as well. I know operators mostly follow convention most of the time, but I'm concerned with the subtle (and surprising) differences. But if someone is changing a column type, which causes a table rewrite, hopefully they've planned it. I'll look into doing it as you suggest. Regards, Jeff Davis -- 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: ... I'll look into doing it as you suggest. I'm already working with a pretty-heavily-editorialized version. Don't worry about it. 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: [ exclusion constraint patch ] Applied after quite a lot of editorialization. For future reference, here is a summary of what I did: * Reworded and renamed stuff to try to be consistent about calling these things exclusion constraints. The original code and docs bore quite a few traces of the various other terminologies, which is not too surprising given the history but would have been confusing to future readers of the code. * Moved the verification of new exclusion constraints into index_build processing as per discussion. * Unified the EXCLUDE parsing path with the existing unique/pkey path by the expedient of adding an excludeOpNames list to IndexStmt. This got rid of quite a lot of duplicated code, and fixed a number of bizarre corner cases like the bogus wording of the index creation NOTICE messages. * Cleaned up some things that didn't really meet project practices. To mention a couple: one aspect of the try to make the patch look like it had always been there rule is to insert new stuff in unsurprising places. Adding code at the end of a list or file very often doesn't meet this test. I tried to put the EXCLUDE constraint stuff beside UNIQUE/PRIMARY KEY handling where possible. Another pet peeve that was triggered a lot in this patch is that you seemed to be intent on fitting ereport() calls into 80 columns no matter what, and would break the error message texts in random places to make it fit. There's a good practical reason not to do that: it makes it hard to grep the source code for an error message. You can break at phrase boundaries if you must, but in general I prefer to just let the text go past the right margin. There are a couple of loose ends yet: * I made CREATE...LIKE processing handle exclusion constraints the same as unique/pkey constraints, ie, they're processed by INCLUDING INDEXES. There was some discussion of rearranging things to make these be processed by INCLUDING CONSTRAINTS instead, but no consensus that I recall. In any case, failing to copy them at all is clearly no good. * I'm not too satisfied with the behavior of psql's \d: regression=# create table foo (f1 int primary key using index tablespace ts1, regression(# f2 int, EXCLUDE USING btree (f2 WITH =) using index tablespace ts1, regression(# f3 int, EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY DEFERRED); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index foo_pkey for table foo NOTICE: CREATE TABLE / EXCLUDE will create implicit index foo_f2_exclusion for table foo NOTICE: CREATE TABLE / EXCLUDE will create implicit index foo_f3_exclusion for table foo CREATE TABLE regression=# \d foo Table public.foo Column | Type | Modifiers +-+--- f1 | integer | not null f2 | integer | f3 | integer | Indexes: foo_pkey PRIMARY KEY, btree (f1), tablespace ts1 foo_f2_exclusion btree (f2), tablespace ts1 foo_f3_exclusion btree (f3) DEFERRABLE INITIALLY DEFERRED Exclusion constraints: foo_f2_exclusion EXCLUDE USING btree (f2 WITH =) foo_f3_exclusion EXCLUDE USING btree (f3 WITH =) DEFERRABLE INITIALLY DEFERRED regression=# This might have been defensible back when the idea was to keep constraints decoupled from indexes, but now it just looks bizarre. We should either get rid of the Exclusion constraints: display and attach the info to the index entries, or hide indexes that are attached to exclusion constraints. I lean to the former on the grounds of the precedent for unique/pkey indexes --- which is not totally arbitrary, since an index is usable as a query index regardless of its function as a constraint. It's probably a debatable point though. 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] operator exclusion constraints
On Fri, Dec 04, 2009 at 11:35:52AM -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Dec 3, 2009 at 7:42 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. �I thought the consensus was to refer to them as just exclusion constraints? �I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) Well, unique constraints are tied to the opclass/index machinery too. Unless there's loud squawks I'm going to exercise committer's prerogative and make all the docs and messages just say exclusion constraint. We have constraint exclusion already, which could make this confusing. How about either the original operator exclusion constraint or the slightly easier whatever constraint names? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] operator exclusion constraints
Robert Haas robertmh...@gmail.com writes: On Thu, Dec 3, 2009 at 7:42 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. I thought the consensus was to refer to them as just exclusion constraints? I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) Well, unique constraints are tied to the opclass/index machinery too. Unless there's loud squawks I'm going to exercise committer's prerogative and make all the docs and messages just say exclusion constraint. 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] operator exclusion constraints
On Fri, 2009-12-04 at 11:35 -0500, Tom Lane wrote: Unless there's loud squawks I'm going to exercise committer's prerogative and make all the docs and messages just say exclusion constraint. Sounds fine to me. Regards, Jeff Davis -- 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] operator exclusion constraints
On Dec 4, 2009, at 11:35 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Dec 3, 2009 at 7:42 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. I thought the consensus was to refer to them as just exclusion constraints? I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) Well, unique constraints are tied to the opclass/index machinery too. Unless there's loud squawks I'm going to exercise committer's prerogative and make all the docs and messages just say exclusion constraint. Go for it. Membership has its privileges. :-) ...Robert -- 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: On Tue, 2009-12-01 at 23:19 -0500, Robert Haas wrote: For parity with unique constraints, I think that the message: operator exclusion constraint violation detected: %s should be changed to: conflicting key value violates operator exclusion constraint %s Done, and updated tests. I'm starting to go through this patch now. I thought the consensus was to refer to them as just exclusion constraints? I'm not seeing that the word operator really adds anything. 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] operator exclusion constraints
On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. I thought the consensus was to refer to them as just exclusion constraints? I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Regards, Jeff Davis -- 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] operator exclusion constraints
On Thu, Dec 3, 2009 at 7:42 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-12-03 at 19:00 -0500, Tom Lane wrote: I'm starting to go through this patch now. I thought the consensus was to refer to them as just exclusion constraints? I'm not seeing that the word operator really adds anything. I assume you're referring to the name used in documentation and error messages. I didn't see a clear consensus, but the relevant thread is here: http://archives.postgresql.org/message-id/1258227283.708.108.ca...@jdavis Exclusion Constraints is fine with me, as are the other options listed in that email. Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) ...Robert -- 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] operator exclusion constraints
On Dec 3, 2009, at 6:26 PM, Robert Haas wrote: Yeah, I don't remember any such consensus either, but it's not a dumb name. I have been idly wondering throughout this process whether we should try to pick a name that conveys the fact that these constraints are inextricably tied to the opclass/index machinery - but I'm not sure it's possible to really give that flavor in a short phrase, or that it's actually important to do so. IOW... whatever. :-) Whatever constraints? Operator Whatevers? WhatEVERs? I like it. David -- 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] operator exclusion constraints
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Thu, Dec 03, 2009 at 08:38:06PM -0800, David E. Wheeler wrote: [...] Whatever constraints? Operator Whatevers? WhatEVERs? I like it. drigting serioulsy off-topic: there's precedent for that in the most venerable piece of free software; TeX has a whatsit node (basically an extension mechanism). Regards - -- tomás -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFLGKM7Bcgs9XrR2kYRAuHFAJ0ZZYzlXHJEgwEbsraAlKVI58yLAgCfU4Cz n+0vobY0HxROigSHUGog7QI= =MWH+ -END PGP SIGNATURE- -- 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] operator exclusion constraints
On Wed, Dec 2, 2009 at 12:18 AM, Jeff Davis pg...@j-davis.com wrote: On Tue, 2009-12-01 at 23:19 -0500, Robert Haas wrote: For parity with unique constraints, I think that the message: operator exclusion constraint violation detected: %s should be changed to: conflicting key value violates operator exclusion constraint %s Done, and updated tests. In ATAddOperatorExclusionConstraint, streatagy is misspelled. Fixed. Other than that, it looks good to me. Great, thanks for the detailed review! Marked as Ready for Committer. ...Robert -- 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] operator exclusion constraints
On Fri, Nov 27, 2009 at 10:18 PM, Jeff Davis pg...@j-davis.com wrote: On Thu, 2009-11-26 at 01:33 -0800, Jeff Davis wrote: Remaining issues: * represent operator IDs in catalog, rather than strategy numbers Done, attached. * if someone thinks it's an issue, support search strategies that require binary-incompatible casts of the inputs I've already solved the original problem involving ANYARRAY. If someone comes up with a good use case, or provides me with a little more guidance, I'll reconsider this problem again. Otherwise, I feel like I'm solving a problem that doesn't exist (after all, none of the contrib modules seem to have a problem with the current assumptions, nor does postgis, nor does my PERIOD module). So, I'm considering this a non-issue until further notice. To summarize, the problem as I understand it is this: You have two types, T1 and T2, and there's an implicit cast (with function or with inout) from T1 to T2. And you have an opclass like: CREATE OPERATOR CLASS t1_ops FOR TYPE t1 ... OPERATOR 17 %%(t2, t2) ... And then you have an exclusion constraint like: CREATE TABLE foo ( a t1, EXCLUDE (a t1_ops WITH %%) ); What should the behavior be in the following two cases? 1. Only operator %%(t2, t2) exists. 2. Operator %%(t1, t1) and %%(t2, t2) exist. If left unsolved, #1 results in an error because the operator requires binary-incompatible coercion. #2 results in an error because %%(t1, t1) is not in the opclass. Arguably either one of them could succeed by finding %%(t2, t2) and performing the appropriate conversion; but I think it's fair to just say the opclass is poorly defined. Note that if the opclass is on type t2, you can simply cast a to t2 explicitly in the expression, like so: EXCLUDE((a::t2) t2_ops WITH %%) For parity with unique constraints, I think that the message: operator exclusion constraint violation detected: %s should be changed to: conflicting key value violates operator exclusion constraint %s In ATAddOperatorExclusionConstraint, streatagy is misspelled. Other than that, it looks good to me. ...Robert -- 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] operator exclusion constraints
On Thu, Nov 26, 2009 at 4:33 AM, Jeff Davis pg...@j-davis.com wrote: Thanks, I applied it. The only significant thing I changed was that I did not inline the index_elem because it made it fairly hard to read. Instead, I renamed it exclude_elem to make it a little more meaningful, which I assume may have been your motivation for inlining it. No, it was really just that I didn't see any point in defining a token that was only used in one place. It seems like it just represents a jumble of tokens without any real connection between them, so I didn't really see why we should break out that piece as opposed to anything else. Changes this patch: * doc changes * changed constraint violation message to be more like btree unique violation * improved error message when an operator is specified that doesn't have a search strategy Remaining issues: * represent operator IDs in catalog, rather than strategy numbers * if someone thinks it's an issue, support search strategies that require binary-incompatible casts of the inputs I'm (still) not an expert on this topic, but here's one more thought: maybe we should try to set this up so that it works in any situation in which an opclass itself would work. IOW, if you manage to define an opclass, you should probably be able to define an operator-exclusion constraint against it, rather than having some opclasses work and others arbitrarily fail. Just my $0.02, ...Robert -- 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] operator exclusion constraints
On Thu, 2009-11-26 at 16:31 -0500, Robert Haas wrote: On Thu, Nov 26, 2009 at 4:33 AM, Jeff Davis pg...@j-davis.com wrote: Thanks, I applied it. The only significant thing I changed was that I did not inline the index_elem because it made it fairly hard to read. Instead, I renamed it exclude_elem to make it a little more meaningful, which I assume may have been your motivation for inlining it. No, it was really just that I didn't see any point in defining a token that was only used in one place. It seems like it just represents a jumble of tokens without any real connection between them, so I didn't really see why we should break out that piece as opposed to anything else. table_constraint and column_constraint are only used one place. I found it convenient to do so because, in the common case, exclude_elem is just a column name. So the user clearly sees: exclude_elem WITH operator and the pairing isn't obscured by a bunch of optional stuff. I'm (still) not an expert on this topic, but here's one more thought: maybe we should try to set this up so that it works in any situation in which an opclass itself would work. IOW, if you manage to define an opclass, you should probably be able to define an operator-exclusion constraint against it, rather than having some opclasses work and others arbitrarily fail. That's what I was aiming for, but it's theoretically possible for that case to require casts. I will do a little more investigation and write a test case. If it looks reasonable, I'll refactor a little and just do it. It is a pretty obscure case (seeing as I have yet to observe it, including all contrib modules plus postgis), but I might as well do it right as long as it's reasonable to do. If it gets even messier to implement, maybe I'll reconsider. Regards, Jeff Davis -- 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] operator exclusion constraints
On Wed, Nov 25, 2009 at 3:23 AM, Jeff Davis pg...@j-davis.com wrote: I was thinking maybe you call BuildIndexValueDescription twice and make the error message say something like output of first call conflicts with output of second call. Do you really think that's a better error message, or are you just trying to re-use similar code? Let's start from how the error message should read, and then see if we can re-use some code to make it look that way. It's one of the most visible aspects of the feature, and it needs to be reasonably concise and understandable in the simple case, but contain all of the necessary information. I think it's better to avoid the = when describing the conflict. I tend to read it as equals even though it's just punctuation in this case, so it would be distracting. I could change it to a colon, I suppose. I disagree wholeheartedly. :-) My ideal error message is something like: DETAIL: (a, b, c)=(1, 2, 3) conflicts with (a, b, c)=(4, 5, 6) In particular, I think it's very important that we only emit the columns which are part of the operator exclusion constraints, and NOT all the columns of the tuple. The entire tuple could be very large - one of the columns not involved in the constraint could be a 4K block of text, for example, and spitting that out only obscures the real source of the problem. You could argue that one of the columns involved in the constraint could be a 4K block text, too, but in practice I think the constraint columns are likely to be short nine times out of ten. The equals sign is equating the column names to the column values, not the values to each other, so I don't see that as confusing. create table test (a int4[], exclude using gist (a with =)); ERROR: operator does not exist: integer[] = integer[] Thanks, fixed. I am now using compatible_oper_opid(), which will find any operators that don't require binary-incompatible coercion of the operands. Do you think there's any reason to support binary-incompatible coercion of the operands? I can't think of a single use case, and if you really need to, you can coerce the types explicitly in the expression. My operator-class-fu is insufficient to render judgment on this point. I think the thing to do is look at a bunch of non-built-in opclasses and check for POLA violations. ...Robert -- 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] operator exclusion constraints
On Wed, 2009-11-25 at 09:02 -0500, Robert Haas wrote: I disagree wholeheartedly. :-) My ideal error message is something like: DETAIL: (a, b, c)=(1, 2, 3) conflicts with (a, b, c)=(4, 5, 6) In particular, I think it's very important that we only emit the columns which are part of the operator exclusion constraints, and NOT all the columns of the tuple. The entire tuple could be very large - one of the columns not involved in the constraint could be a 4K block of text, for example, and spitting that out only obscures the real source of the problem. You could argue that one of the columns involved in the constraint could be a 4K block text, too, but in practice I think the constraint columns are likely to be short nine times out of ten. The equals sign is equating the column names to the column values, not the values to each other, so I don't see that as confusing. Ok, fair enough. But how do you feel about: (a: 1, b: 2, c: 3) as a tuple representation instead? I think it's closer to the way people expect a tuple to be represented. I can change it in one place so that the unique violations are reported that way, as well (as long as there are no objections). It still doesn't contain the operators, but they can look at the constraint definition for that. My operator-class-fu is insufficient to render judgment on this point. I think the thing to do is look at a bunch of non-built-in opclasses and check for POLA violations. Ok, I'll consider this more. Regards, Jeff Davis -- 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] operator exclusion constraints
Jeff Davis pg...@j-davis.com writes: On Wed, 2009-11-25 at 09:02 -0500, Robert Haas wrote: I disagree wholeheartedly. :-) My ideal error message is something like: DETAIL: (a, b, c)=(1, 2, 3) conflicts with (a, b, c)=(4, 5, 6) Ok, fair enough. But how do you feel about: (a: 1, b: 2, c: 3) as a tuple representation instead? This seems like change for the sake of change. We've been reporting this type of error (in the context of foreign keys) using the first syntax for a very long time. I don't feel a need to rearrange it. 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] operator exclusion constraints
On Wed, 2009-11-25 at 15:59 -0800, Jeff Davis wrote: My operator-class-fu is insufficient to render judgment on this point. I think the thing to do is look at a bunch of non-built-in opclasses and check for POLA violations. Ok, I'll consider this more. In cases where the operator class type is different from the search operator's operands' types, one of the following is usually true: * There is a binary cast from the opclass type (same as expression type) to the search operator's operands' types, or it is otherwise compatible (e.g. ANYARRAY). * There is a candidate function that's a better match (e.g. there may be an =(int8, int8) on int2_ops, but there's also =(int2, int2)). * The left and right operand types are different, and therefore do not work with operator exclusion constraints anyway (e.g. full text search @@). After installing all contrib modules, plus my period module, and postgis, there appear to be no instances that violate these assumptions (according to a join query and some manual testing). In theory there could be, however. It's kind of ugly to make it work, and a challenge to test it, so for now I'll only accept operators returned by compatible_oper(). If you disagree, I can make it work. Regards, Jeff Davis -- 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] operator exclusion constraints
On Fri, Nov 20, 2009 at 01:36:59PM +0900, Josh Berkus wrote: RObert, I guess I'm going to have to vote -1 on this proposal. I code see inventing a pgsql-specific SQLSTATE value for exclusion constraints, since they will be a pgsql-specific extension, but reusing the unique key violation value seems misleading. I admit it may help in a limited number of cases, but IMHO it's not worth the confusion. I'd rather have a new one than just using contstraint violation which is terribly non-specific, and generally makes the application developer think that a value is too large. What, if anything, does the standard have to say about violations of ASSERTIONs? I know these aren't ASSERTIONs, but they much more closely resemble them than they do UNIQUE constraints. For example, if the operator is instead of =, a violation is actually the opposite of a UNIQUE violation. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] operator exclusion constraints
On sön, 2009-11-22 at 16:03 -0800, David Fetter wrote: What, if anything, does the standard have to say about violations of ASSERTIONs? I know these aren't ASSERTIONs, but they much more closely resemble them than they do UNIQUE constraints. An assertion is by definition a constraint that is a schema component independent of a table. Which an exclusion constraint may or may not be, but it's an independent issue. (To clarify: It currently can't be, because assertions are not implemented, but when they are, it could be.) For the same reason, assertions don't have separate error codes, because they are just constraints after all. -- 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] operator exclusion constraints
On Wed, Nov 18, 2009 at 9:00 AM, Jeff Davis pg...@j-davis.com wrote: I'm in Tokyo right now, so please excuse my abbreviated reply. On Tue, 2009-11-17 at 23:13 -0500, Robert Haas wrote: Forgive me if this is discussed before, but why does this store the strategy numbers of the relevant operators instead of the operators themselves? At constraint definition time, I need to make sure that the strategy numbers can be identified anyway, so it wouldn't save any work in ATAddOperatorExclusionConstraint. At the time it seemed slightly more direct to use strategy numbers in index_check_constraint, but it's probably about the same. It sets off a red flag for me any time I see code that asks for A from the user and then actually stores B under the hood, for fear that the relationship that A and B might change. However... It seems like this could lead to surprising behavior if the user modifies the definition of the operator class. Right now, operator classes can't be modified in any meaningful way. Am I missing something? ...poking at it, I have to agree that at least as things stand right now, I can't find a way to break it. Not sure if it's future-proof. I'm wondering if we can't use the existing BuildIndexValueDescription() rather than the new function tuple_as_string(). I realize there are two tuples, but maybe it makes sense to just call it twice? Are you suggesting I change the error output, or reorganize the code to try to reuse BuildIndexValueDescription, or both? I was thinking maybe you call BuildIndexValueDescription twice and make the error message say something like output of first call conflicts with output of second call. One other thing I noticed tonight while poking at this. If I install contrib/citext, I can do this: create table test (a citext, exclude using hash (a with =)); But if I install contrib/intarray, I can't do this: create table test (a int4[], exclude using gist (a with =)); ERROR: operator does not exist: integer[] = integer[] Not sure if I'm doing something wrong, or if this is a limitation of the design, or if it's a bug, but it seems strange. I'm guessing it's because intarray uses the anyarray operator rather than a dedicated operator for int[], but it seems like that ought to work. ...Robert -- 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] operator exclusion constraints
On Wed, Nov 18, 2009 at 9:21 AM, Josh Berkus j...@agliodbs.com wrote: All, FWIW, I'm doing a redesign of a client's production web application right now. I was able, by combining OEC and the Period type from pgfoundry, to make a set of constraints for declaratively asserting in a sports database: That the same player couldn't belong to two different teams at the same time; That the same player couldn't belong to the same team in two different positions with overlapping time periods. This worked as spec'd, and would be extremely useful for this real-world app if it was ready to use in production now. However, I do have an issue with the SQLSTATE returned from the OEC violation. Currently it returns constraint violation, which, from the perspective of an application developer, is not useful. OECs are, in application terms, materially identical to UNIQUE constraints and serve the same purpose. As such, I'd far rather see OECs return unique key violation instead, as any existing application error-trapping code would handle the violation more intelligently if it did. I guess I'm going to have to vote -1 on this proposal. I code see inventing a pgsql-specific SQLSTATE value for exclusion constraints, since they will be a pgsql-specific extension, but reusing the unique key violation value seems misleading. I admit it may help in a limited number of cases, but IMHO it's not worth the confusion. That's just my $0.02, though. ...Robert -- 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] operator exclusion constraints
RObert, I guess I'm going to have to vote -1 on this proposal. I code see inventing a pgsql-specific SQLSTATE value for exclusion constraints, since they will be a pgsql-specific extension, but reusing the unique key violation value seems misleading. I admit it may help in a limited number of cases, but IMHO it's not worth the confusion. I'd rather have a new one than just using contstraint violation which is terribly non-specific, and generally makes the application developer think that a value is too large. --Josh BErkus -- 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] operator exclusion constraints
I'm in Tokyo right now, so please excuse my abbreviated reply. On Tue, 2009-11-17 at 23:13 -0500, Robert Haas wrote: Forgive me if this is discussed before, but why does this store the strategy numbers of the relevant operators instead of the operators themselves? At constraint definition time, I need to make sure that the strategy numbers can be identified anyway, so it wouldn't save any work in ATAddOperatorExclusionConstraint. At the time it seemed slightly more direct to use strategy numbers in index_check_constraint, but it's probably about the same. It seems like this could lead to surprising behavior if the user modifies the definition of the operator class. Right now, operator classes can't be modified in any meaningful way. Am I missing something? I'm wondering if we can't use the existing BuildIndexValueDescription() rather than the new function tuple_as_string(). I realize there are two tuples, but maybe it makes sense to just call it twice? Are you suggesting I change the error output, or reorganize the code to try to reuse BuildIndexValueDescription, or both? Regards, Jeff Davis -- 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] operator exclusion constraints
Robert Haas robertmh...@gmail.com writes: Forgive me if this is discussed before, but why does this store the strategy numbers of the relevant operators instead of the operators themselves? It seems like this could lead to surprising behavior if the user modifies the definition of the operator class. Wild guess: http://www.postgresql.org/docs/8.4/static/xindex.html 34.14.2. Index Method Strategies The operators associated with an operator class are identified by strategy numbers, which serve to identify the semantics of each operator within the context of its operator class. For example, B-trees impose a strict ordering on keys, lesser to greater, and so operators like less than and greater than or equal to are interesting with respect to a B-tree. Because PostgreSQL allows the user to define operators, PostgreSQL cannot look at the name of an operator (e.g., or =) and tell what kind of comparison it is. Instead, the index method defines a set of strategies, which can be thought of as generalized operators. Each operator class specifies which actual operator corresponds to each strategy for a particular data type and interpretation of the index semantics. Regards, -- dim -- 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] operator exclusion constraints
All, FWIW, I'm doing a redesign of a client's production web application right now. I was able, by combining OEC and the Period type from pgfoundry, to make a set of constraints for declaratively asserting in a sports database: That the same player couldn't belong to two different teams at the same time; That the same player couldn't belong to the same team in two different positions with overlapping time periods. This worked as spec'd, and would be extremely useful for this real-world app if it was ready to use in production now. However, I do have an issue with the SQLSTATE returned from the OEC violation. Currently it returns constraint violation, which, from the perspective of an application developer, is not useful. OECs are, in application terms, materially identical to UNIQUE constraints and serve the same purpose. As such, I'd far rather see OECs return unique key violation instead, as any existing application error-trapping code would handle the violation more intelligently if it did. --Josh Berkus -- 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 2:27 PM, Jeff Davis pg...@j-davis.com wrote:
New patches attached.
Forgive me if this is discussed before, but why does this store the
strategy numbers of the relevant operators instead of the operators
themselves? It seems like this could lead to surprising behavior if
the user modifies the definition of the operator class.
I'm wondering if we can't use the existing
BuildIndexValueDescription() rather than the new function
tuple_as_string(). I realize there are two tuples, but maybe it makes
sense to just call it twice?
I'm attaching a revised doc patch for your consideration.
...Robert
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 082dfe4..a73c015 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -51,10 +51,13 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PAR
PRIMARY KEY ( replaceable class=PARAMETERcolumn_name/replaceable [, ... ] ) replaceable class=PARAMETERindex_parameters/replaceable |
CHECK ( replaceable class=PARAMETERexpression/replaceable ) |
FOREIGN KEY ( replaceable class=PARAMETERcolumn_name/replaceable [, ... ] ) REFERENCES replaceable class=PARAMETERreftable/replaceable [ ( replaceable class=PARAMETERrefcolumn/replaceable [, ... ] ) ]
-[ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE replaceable class=parameteraction/replaceable ] [ ON UPDATE replaceable class=parameteraction/replaceable ] }
+[ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE replaceable class=parameteraction/replaceable ] [ ON UPDATE replaceable class=parameteraction/replaceable ] |
+ EXCLUDE [ USING replaceable class=parameterindex_method/replaceable ]
+( { replaceable class=parametercolumn/replaceable | replaceable class=parametercolumn/replaceable | ( replaceable class=parameterexpression/replaceable ) } [ replaceable class=parameteropclass/replaceable ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] WITH replaceable class=parameteroperator/replaceable [, ... ] )
+replaceable class=parameterindex_parameters/replaceable [ WHERE ( replaceable class=parameterpredicate/replaceable ) ] }
[ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
-phrasereplaceable class=PARAMETERindex_parameters/replaceable in literalUNIQUE/literal and literalPRIMARY KEY/literal constraints are:/phrase
+phrasereplaceable class=PARAMETERindex_parameters/replaceable in literalUNIQUE/literal, literalPRIMARY KEY/literal, and literalEXCLUDE/literal constraints are:/phrase
[ WITH ( replaceable class=PARAMETERstorage_parameter/replaceable [= replaceable class=PARAMETERvalue/replaceable] [, ... ] ) ]
[ USING INDEX TABLESPACE replaceable class=PARAMETERtablespace/replaceable ]
@@ -547,6 +550,43 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceable class=PAR
/varlistentry
varlistentry
+termliteralEXCLUDE [ USING replaceable class=parameterindex_method/replaceable ]
+ ( {replaceable class=parametercolumn/replaceable | replaceable class=parametercolumn/replaceable | (replaceable class=parameterexpression/replaceable ) } [ replaceable class=parameteropclass/replaceable ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] WITH replaceable class=parameteroperator/replaceable [, ... ] )
+ replaceable class=parameterindex_parameters/replaceable [ WHERE ( replaceable class=parameterpredicate/replaceable ) ] }/literal/term
+listitem
+ para
+ The literalEXCLUDE/ clause specifies an operator exclusion
+ constraint. An operator exclusion constraint guarantees that if any two
+ tuples are compared on the specified columns or expressions using the
+ specified operators, at least one such comparison will return false.
+ If all of the specified operators test for equality, it is equivalent
+ to a UNIQUE constraint, although an ordinary unique constraint will
+ normally be faster. However, operator exclusion constraints can use
+ index methods other than btree, and can specify more general constraints.
+ For instance, you can specify the constraint that no two tuples in the
+ table contain overlapping circles
+ (see xref linkend=datatype-geometric) by using the
+ literal/literal operator.
+ /para
+
+ para
+ Operator exclusion constraints are implemented internally using
+ an index, so the specified operators must be associated with an
+ appropriate operator class for the given access method, and the
+ access method must support amgettuple (see xref linkend=indexam
+ for details). The operators are also required to be their own
+ commutators (see xref linkend=sql-createoperator).
+ /para
+
+ para
+ The replaceable class=parameterpredicate/replaceable
+ allows you to specify a constraint on a subset of the table,
+ internally using a partial index.
+ /para
+/listitem
+ /varlistentry
+
+
Re: [HACKERS] operator exclusion constraints
On Nov 13, 2009, at 8:39 PM, Robert Haas wrote: alter table foo add constraint bar exclude (a check with =, b check with =); I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. Best, David -- 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] operator exclusion constraints
David E. Wheeler da...@kineticode.com writes: On Nov 13, 2009, at 8:39 PM, Robert Haas wrote: alter table foo add constraint bar exclude (a check with =, b check with =); I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. 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] operator exclusion constraints
On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. Best, David -- 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 12:11 PM, David E. Wheeler da...@kineticode.com wrote: On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I've been meaning to comment on this syntax one more time; apologies for the bike-shedding. But I'm wondering if the CHECK is strictly necessary there, since the WITH seems adequate, and there was some discussion before about the CHECK keyword possibly causing confusion with check constraints. I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. I haven't thought about this too deeply, but could we allow the with = part to be optional? And would it be a good idea? Seems like you would commonly have one or more keys that exclude on equality and then the last one would use an overlap-type operator. ...Robert -- 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] operator exclusion constraints
Robert Haas robertmh...@gmail.com writes: I haven't thought about this too deeply, but could we allow the with = part to be optional? And would it be a good idea? I don't think so. We generally do not believe in defaulting operators based on name. If there were a way to select the standard exclusion operator based on opclass membership it might make sense, but almost by definition this facility is going to be working with unusual opclasses that might not even have an equality slot. 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] operator exclusion constraints
On Fri, 2009-11-13 at 23:39 -0500, Robert Haas wrote: [ reviewing ] Thank you for the comments so far. In index_create(), the elog() where relopxconstraints 0 should just complain about the value being negative, I think, rather than listing the value. If you just say the value is -3, it doesn't give the user a clue why that's bad. Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. I tried to make all user-visible errors into ereport()s. In ATAddOperatorExclusionConstraint(), the message method %s does not support gettuple seems a bit user-unfriendly. Can we explain the problem by referring to the functionality of getttuple(), rather than the name of it? How about operator exclusion constraints don't support method X? Then perhaps have a detail-level message to explain that the access method needs to support the gettuple interface. Trying to describe what gettuple does doesn't help a humble user much. All they care about is can't use gin. I don't really like this message, for a number of reasons. alter table foo add constraint bar exclude (a check with =, b check with =); ERROR: operator exclusion constraint violation detected: foo_a_exclusion DETAIL: Tuple (1, 1, 2) conflicts with existing tuple (1, 1, 3). The corresponding error for a UNIQUE index is: could not create unique index bar, which I like better. Only the relevant columns from the tuples are dumped, and the tuple is not surrounded by double quotes; any reason not to parallel that here? By relevant columns I assume you mean the entire index tuple. That means we need to have column names represented somewhere, because we don't want the user to have to match up ordinal index columns. Also, with exclusion constraints, both values are always relevant, not just the one being inserted. What if the user just wants to adjust their request slightly to avoid an overlap -- how do they know how far to go? I know this could be accomplished with extra queries, as well, but that doesn't always work for someone looking through the logs after the fact, when the original values may be gone. So, the kind of error message you're suggesting starts to get awkward: (a: 1 = 1, b: 1 = 1) or something? And then with more complex type output functions, and expression indexes, it starts to look very complex. What do you think is the cleanest approach? Also, the message is all lower-case. I know the error conventions are documented somewhere, but I completely forgot where. Can you please point me to the right place? I thought most error messages were supposed to be lower case, and detail messages were supposed to read like sentences. Similarly, for an insert/update situation, it seems that a message like key value violates exclusion constraint \%s\ would be better than the existing message. I can certainly simplify it, but I was trying to match the usefulness of UNIQUE constraint error messages. As a quick performance test, I inserted a million 3-integer tuples into a 3-column table with a unique constraint or an operator exclusion constraint over all three columns. The former took ~ 15 s, the latter ~ 21.5 s. That seems acceptable. Great news. I had similar results, though they depend on the conflict percentage as well (I assume you had zero conflicts). Regards, Jeff Davis -- 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] operator exclusion constraints
2009/11/15 Jeff Davis pg...@j-davis.com: I know the error conventions are documented somewhere, but I completely forgot where. Can you please point me to the right place? I thought most error messages were supposed to be lower case, and detail messages were supposed to read like sentences. http://www.postgresql.org/docs/current/static/error-style-guide.html And you are correct. Cheers, BJ -- 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 6:00 PM, Jeff Davis pg...@j-davis.com wrote: Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. Yeah, I think that's right. I think part of the rationale is that if the admin mucks around with catalog tables or does some DDL with inconsistent definitions (like an operator class that isn't internally consistent for example) then we don't treat those errors as user-visible errors that need to be translated but they shouldn't cause a database crash either. If it's possible for the case to arrive through users doing things through entirely supported means then they might need to be real ereports(). But I guess there are grey areas. -- greg -- 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 1:58 PM, Greg Stark gsst...@mit.edu wrote: On Sat, Nov 14, 2009 at 6:00 PM, Jeff Davis pg...@j-davis.com wrote: Hopefully the user never sees that message -- it's almost an Assert. PostgreSQL uses elog(ERROR,...) in many places that should be unreachable, but might happen due to bugs in distant places or corruption. I'm not sure the exact convention there, but I figure that some details are appropriate. Yeah, I think that's right. I think part of the rationale is that if the admin mucks around with catalog tables or does some DDL with inconsistent definitions (like an operator class that isn't internally consistent for example) then we don't treat those errors as user-visible errors that need to be translated but they shouldn't cause a database crash either. If it's possible for the case to arrive through users doing things through entirely supported means then they might need to be real ereports(). But I guess there are grey areas. I guess my point wasn't that the message was likely to be exercised, but rather that it isn't obvious that it's describing an error condition at all. If you get this message: relation whatever has relopxconstraints = -3 ...you can't even tell that it's an error message unless it happens to be preceded by ERROR: . If you get something like: relation whatever has invalid relopxconstraints value (-3) ...it's much more clear that this is not just informational. ...Robert -- 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] operator exclusion constraints
On Sat, 2009-11-14 at 09:11 -0800, David E. Wheeler wrote: On Nov 14, 2009, at 8:55 AM, Tom Lane wrote: I had been manfully restraining myself from re-opening this discussion, but yeah I was thinking the same thing. The original objection to using just WITH was that it wasn't very clear what you were doing with the operator; but that was back when we had a different initial keyword for the construct. EXCLUDE ... WITH ... seems to match up pretty naturally. You're more man than I, Tom, but yeah, with EXCLUDE, WITH works well on its own, methinks. Changed in new patch here: http://archives.postgresql.org/message-id/1258226849.708.97.ca...@jdavis Regards, Jeff Davis -- 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] operator exclusion constraints
On Sat, 2009-11-14 at 14:36 -0500, Robert Haas wrote: I guess my point wasn't that the message was likely to be exercised, but rather that it isn't obvious that it's describing an error condition at all. If you get this message: relation whatever has relopxconstraints = -3 I pretty much copied similar code for relchecks, see pg_constraint.c:570. If you have a suggestion, I'll make the change. It doesn't sound too urgent though, to me. Regards, Jeff Davis -- 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] operator exclusion constraints
On Sat, Nov 14, 2009 at 2:39 PM, Jeff Davis pg...@j-davis.com wrote: If you have a suggestion, I'll make the change. It doesn't sound too urgent though, to me. Yeah, probably not. ...Robert -- 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] operator exclusion constraints
On Mon, 2009-11-09 at 09:12 -0800, David E. Wheeler wrote: On Nov 8, 2009, at 7:43 PM, Jeff Davis wrote: Either of those names are fine with me, too. The current name is a somewhat shortened version of the name operator-based exclusion constraints, so we can also just use that name. Or, just exclusion constraints. (exclusion constraints)++ Ok, I guess this is another issue that requires consensus. Note: this is purely for documentation, release notes, and user-visible error messages. This does not have any impact on the syntax, I think we've got a strong consensus on that already and I would prefer not to break that discussion open again. 1. Operator Exclusion Constraints (current) 2. Generic/Generalized/General Exclusion Constraints 3. Exclusion Constraints (has the potential to cause confusion with constraint_exclusion) Regards, Jeff Davis -- 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] operator exclusion constraints
On Sun, Nov 8, 2009 at 4:41 PM, Jeff Davis pg...@j-davis.com wrote: On Sat, 2009-11-07 at 10:56 -0800, Jeff Davis wrote: EXCLUDE probably flows most nicely with the optional USING clause or without. My only complaint was that it's a transitive verb, so it seems to impart more meaning than it actually can. I doubt anyone would actually be more confused in practice, though. If a couple of people agree, I'll change it to EXCLUDE. It looks like EXCLUDE is the winner. Updated patch attached. The feature is still called operator exclusion constraints, and the docs still make reference to that name, but the syntax specification has been updated. [ reviewing ] First thoughts: I think the create_table documentation gets into a little too much gorey detail. I'm willing to take a pass at improving it, if you'd like, but generally I think it should avoid discussion of implementation details. For example, saying that it's not as fast as a UNIQUE constraint is good; the fact that an extra index lookup is involved is probably overkill. Incidentally, the wording in the first paragraph fails to take into account the possibility that there are multiple operators. In index_create(), the elog() where relopxconstraints 0 should just complain about the value being negative, I think, rather than listing the value. If you just say the value is -3, it doesn't give the user a clue why that's bad. There is a spurious diff hunk for reindex_relation(). In ATRewriteTable() you reindent a bunch of variable declarations; pg_indent will undo this, so you should nix this part. In ATAddOperatorExclusionConstraint(), the message method %s does not support gettuple seems a bit user-unfriendly. Can we explain the problem by referring to the functionality of getttuple(), rather than the name of it? I don't really like this message, for a number of reasons. alter table foo add constraint bar exclude (a check with =, b check with =); ERROR: operator exclusion constraint violation detected: foo_a_exclusion DETAIL: Tuple (1, 1, 2) conflicts with existing tuple (1, 1, 3). The corresponding error for a UNIQUE index is: could not create unique index bar, which I like better. Only the relevant columns from the tuples are dumped, and the tuple is not surrounded by double quotes; any reason not to parallel that here? Also, the message is all lower-case. Similarly, for an insert/update situation, it seems that a message like key value violates exclusion constraint \%s\ would be better than the existing message. alter table X add constraint Y exclude (...) seems to ignore the value of Y and create both the constraint and the index with a name of its own choosing. As a quick performance test, I inserted a million 3-integer tuples into a 3-column table with a unique constraint or an operator exclusion constraint over all three columns. The former took ~ 15 s, the latter ~ 21.5 s. That seems acceptable. I think preprocessOpExConstraints should be called transformOpxConstraints - opx rather than opex because that's what you most frequently use elsewhere in the patch, and transform rather than preprocess for consistency with other, similar functions. In ruleutils.c, the prototype for pg_get_opxdef_worker() has a small whitespace inconsistency relative to the surrounding declarations. I haven't really grokked the substantive things that this patch is doing yet - these are just preliminary comments based on a quick read-through. I'll write more after I have a chance to look at it in more detail. ...Robert -- 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] operator exclusion constraints
On Nov 8, 2009, at 7:43 PM, Jeff Davis wrote: Either of those names are fine with me, too. The current name is a somewhat shortened version of the name operator-based exclusion constraints, so we can also just use that name. Or, just exclusion constraints. (exclusion constraints)++ David -- 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] operator exclusion constraints
On Mon, Nov 9, 2009 at 5:12 PM, David E. Wheeler da...@kineticode.com wrote: On Nov 8, 2009, at 7:43 PM, Jeff Davis wrote: Either of those names are fine with me, too. The current name is a somewhat shortened version of the name operator-based exclusion constraints, so we can also just use that name. Or, just exclusion constraints. (exclusion constraints)++ Out of curiosity, is this feature at all similar to SQL assertions? What would we be missing to turn this into them? -- greg -- 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] operator exclusion constraints
Greg Stark escribió: Out of curiosity, is this feature at all similar to SQL assertions? What would we be missing to turn this into them? I see no relationship to assertions. Those are not tied to any particular table, and are defined with any random expression you care to think of. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] operator exclusion constraints
Tom Lane escribió: Andrew Dunstan and...@dunslane.net writes: This is a pretty good short explanation of how to deal with shift/reduce problems in bison. With your permission I'm going to copy it to the Wiki If you like, but I think the part about figuring out which production is the problem seemed to be at least as important for Jeff ... I agree that it would be worth an entry here http://wiki.postgresql.org/wiki/Developer_FAQ -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] operator exclusion constraints
On Mon, 2009-11-09 at 18:03 +, Greg Stark wrote: Out of curiosity, is this feature at all similar to SQL assertions? What would we be missing to turn this into them? I addressed that here: http://archives.postgresql.org/pgsql-hackers/2009-11/msg00049.php The exclusion constraint mechanism can enforce a subset of the constraints that ASSERT can express; although the same goes for all other constraints, because ASSERT is very general. The exclusion constraint mechanism requires finding the physical tuples that cause a conflict, so that we know when to wait and on which transaction to wait. Otherwise, we have to wait on all transactions; i.e. serialize. The problem with ASSERT is that it expresses a constraint based on a query, which can return arbitrary logical records after an arbitrary amount of manipulation. So there's no way to work backwards. If we try, we'll end up either: (a) supporting only a tiny subset, and throwing bizarre errors that users don't understand when they try to work outside the template; or (b) deciding to serialize when we can't do better, and again, users will be confused about the performance and locking characteristics. Regards, Jeff Davis -- 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] operator exclusion constraints
On Sun, 2009-11-08 at 13:41 -0800, Jeff Davis wrote: On Sat, 2009-11-07 at 10:56 -0800, Jeff Davis wrote: EXCLUDE probably flows most nicely with the optional USING clause or without. My only complaint was that it's a transitive verb, so it seems to impart more meaning than it actually can. I doubt anyone would actually be more confused in practice, though. If a couple of people agree, I'll change it to EXCLUDE. It looks like EXCLUDE is the winner. Updated patch attached. The feature is still called operator exclusion constraints, and the docs still make reference to that name, but the syntax specification has been updated. Don't think that name is very useful either... sounds like you want to exclude operators, which is why I got lost in the first place. I'd call them generic exclusion constraints or user-defined exclusion constraints. Sorry for this. -- Simon Riggs www.2ndQuadrant.com -- 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] operator exclusion constraints
On Sun, 2009-11-08 at 22:03 +, Simon Riggs wrote: Don't think that name is very useful either... sounds like you want to exclude operators, which is why I got lost in the first place. I'd call them generic exclusion constraints or user-defined exclusion constraints. Sorry for this. Either of those names are fine with me, too. The current name is a somewhat shortened version of the name operator-based exclusion constraints, so we can also just use that name. Or, just exclusion constraints. I'll leave the current patch as-is for now, and wait for some reviewer comments. This is purely a documentation issue, so there are bound to be a few of these things that I can clarify at once. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
