2009/7/29 Tom Lane t...@sss.pgh.pa.us:
Dean Rasheed dean.a.rash...@googlemail.com writes:
[ deferrable unique constraints patch ]
Applied after rather extensive editorialization.
Excellent! Thanks for all your work sorting out my rookie mistakes. I
haven't had a chance to go through all your
2009/7/28 Tom Lane t...@sss.pgh.pa.us:
... speaking of adding catalog columns, I just discovered that the patch
adds searches of pg_depend and pg_constraint to BuildIndexInfo. This
seems utterly unacceptable on two grounds:
* It's sheer luck that it gets through bootstrap without crashing.
2009/7/29 Tom Lane t...@sss.pgh.pa.us:
Another thought on the index AM API issues: after poking through the
code I realized that there is *nobody* paying any attention to the
existing bool result of aminsert() (ie, did we insert anything or not).
So I think that instead of adding a bool*
Dean Rasheed dean.a.rash...@googlemail.com writes:
2009/7/29 Tom Lane t...@sss.pgh.pa.us:
For non-unique indexes, it is not required that functionaminsert/
do anything; it might for instance refuse to index NULLs.
Doesn't this comment apply equally to unique indexes?
Hmm, I was thinking
Dean Rasheed dean.a.rash...@googlemail.com writes:
[ deferrable unique constraints patch ]
Applied after rather extensive editorialization. Aside from the points
we already discussed, I redid the logic in _bt_check_unique ... it
didn't look right to me, and I also felt that we need a
Tom Lane wrote:
Dean Rasheed dean.a.rash...@googlemail.com writes:
[ deferrable unique constraints patch ]
Applied after rather extensive editorialization.
Kudos!!
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
On Wed, Jul 29, 2009 at 5:00 PM, Tom Lanet...@sss.pgh.pa.us wrote:
Dean Rasheed dean.a.rash...@googlemail.com writes:
[ deferrable unique constraints patch ]
Applied after rather extensive editorialization. Aside from the points
Wow, cool.
...Robert
--
Sent via pgsql-hackers mailing list
... btw, where in the SQL spec do you read that PRIMARY KEY constraints
can't be deferred? I don't see that.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
... speaking of adding catalog columns, I just discovered that the patch
adds searches of pg_depend and pg_constraint to BuildIndexInfo. This
seems utterly unacceptable on two grounds:
* It's sheer luck that it gets through bootstrap without crashing.
Those aren't part of the core set of
On Tue, 2009-07-28 at 13:41 -0400, Tom Lane wrote:
* It's sheer luck that it gets through bootstrap without crashing.
Those aren't part of the core set of catalogs that we expect to be
accessed by primitive catalog searches. I wouldn't be too surprised
if it gets the wrong answer, and the
Jeff Davis pg...@j-davis.com writes:
On Tue, 2009-07-28 at 13:41 -0400, Tom Lane wrote:
I think we had better add the deferrability state to pg_index
to avoid this.
This might make it difficult to allow multiple constraints to use the
same index.
Huh? That hardly seems possible anyway, if
On Tue, 2009-07-28 at 15:15 -0400, Tom Lane wrote:
This might make it difficult to allow multiple constraints to use the
same index.
Huh? That hardly seems possible anyway, if some of them want deferred
checks and others do not.
I don't see why it's completely impossible. You could have:
Jeff Davis pg...@j-davis.com writes:
On Tue, 2009-07-28 at 15:15 -0400, Tom Lane wrote:
Sure it does. Whether the check is immediate must be considered a
property of the index itself. Any checking you do later could be
per-constraint, but the index is either going to fail at insert or not.
Another thought on the index AM API issues: after poking through the
code I realized that there is *nobody* paying any attention to the
existing bool result of aminsert() (ie, did we insert anything or not).
So I think that instead of adding a bool* parameter, we should repurpose
the function
Dean Rasheed dean.a.rash...@googlemail.com writes:
[ latest deferrable-unique patch ]
I'm starting to look at this now. I haven't read the patch itself yet,
but I went through the discussion thread. I'm not sure whether we have
actually decided that we want to commit this, as opposed to
On Mon, 2009-07-27 at 13:14 -0400, Tom Lane wrote:
The main thing that is bothering me is something Dean pointed out at
the very beginning: the patch will not scale well for large numbers of
conflicts.
The way I see it, there are two strategies:
(a) build up a list as you go, and check it
2009/7/27 Jeff Davis pg...@j-davis.com:
On Mon, 2009-07-27 at 13:14 -0400, Tom Lane wrote:
The main thing that is bothering me is something Dean pointed out at
the very beginning: the patch will not scale well for large numbers of
conflicts.
I'd pefer to take option #3, and I want to try to
Jeff Davis pg...@j-davis.com writes:
The way I see it, there are two strategies:
(a) build up a list as you go, and check it later
(b) do a check of the full table at once
The patch seems like a reasonable implementation of (a), so what it's
missing is the ability to fall back to (b)
Dean Rasheed dean.a.rash...@googlemail.com writes:
Actually I see 2 parts to this:
(1). make trigger queue scalable with easy mechanism to switchover to
wholesale check
(2). implement wholesale check for UNIQUE
but (1) seems like to lion's share of the work.
(and then maybe (3). wholesale
2009/7/27 Tom Lane t...@sss.pgh.pa.us:
Jeff Davis pg...@j-davis.com writes:
The way I see it, there are two strategies:
(a) build up a list as you go, and check it later
(b) do a check of the full table at once
The patch seems like a reasonable implementation of (a), so what it's
2009/7/27 Tom Lane t...@sss.pgh.pa.us:
(In fact, in a real sense these ARE join problems ... maybe we should
stop thinking of them as fire-a-bunch-of-triggers and instead think of
executing a single check query with appropriate WHERE clause ...)
Hmm. Presumably that is the same WHERE clause
[ still poking around in this patch ]
Jeff Davis pg...@j-davis.com writes:
10. You're overloading tgconstrrelid to hold the constraint's index's
oid, when normally it holds the referenced table. You should probably
document this a little better, because I don't think that field is used
to
On Mon, 2009-07-27 at 16:33 -0400, Tom Lane wrote:
If we did add another column to pg_trigger, I'd be a bit tempted to add
one to pg_constraint too. tgconstrrelid for RI triggers is a mirror of
a pg_constraint column, and it seems like the index data perhaps should
be as well. (Indeed, one
2009/7/27 Jeff Davis pg...@j-davis.com:
On Mon, 2009-07-27 at 16:33 -0400, Tom Lane wrote:
If we did add another column to pg_trigger, I'd be a bit tempted to add
one to pg_constraint too. tgconstrrelid for RI triggers is a mirror of
a pg_constraint column, and it seems like the index data
On Mon, Jul 27, 2009 at 7:51 PM, Tom Lanet...@sss.pgh.pa.us wrote:
(In fact, in a real sense these ARE join problems ... maybe we should
stop thinking of them as fire-a-bunch-of-triggers and instead think of
executing a single check query with appropriate WHERE clause ...)
A while back I
Greg Stark gsst...@mit.edu writes:
For foreign keys I was picturing some way to issue an SQL statement
like SELECT from tabletocheck where ctid in (magic parameter) and
not exists (select 1 from referenced_table where pk =
tabletocheck.fk) and then somehow pass the list of ctids from the
Dean Rasheed dean.a.rash...@googlemail.com writes:
2009/7/27 Jeff Davis pg...@j-davis.com:
On Mon, 2009-07-27 at 16:33 -0400, Tom Lane wrote:
If we did add another column to pg_trigger, I'd be a bit tempted to add
one to pg_constraint too.
That would work great for me, as I was planning to
On Mon, 2009-07-27 at 19:12 -0400, Tom Lane wrote:
(thinks...) Actually, u for unique might be a poor choice if Jeff's
patch goes in and starts using it for things that aren't exactly
unique indexes. Should it be just conindid?
My thoughts exactly.
Regards,
Jeff Davis
--
Sent via
On Tue, Jul 28, 2009 at 12:04 AM, Tom Lanet...@sss.pgh.pa.us wrote:
Greg Stark gsst...@mit.edu writes:
For foreign keys I was picturing some way to issue an SQL statement
like SELECT from tabletocheck where ctid in (magic parameter) and
not exists (select 1 from referenced_table where pk =
Jeff Davis pg...@j-davis.com writes:
On Mon, 2009-07-27 at 19:12 -0400, Tom Lane wrote:
(thinks...) Actually, u for unique might be a poor choice if Jeff's
patch goes in and starts using it for things that aren't exactly
unique indexes. Should it be just conindid?
My thoughts exactly.
On
On Wed, 2009-07-22 at 12:25 +0100, Dean Rasheed wrote:
OK, here's an updated patch.
One thing that Alvaro mentioned that you didn't do yet is use the macro
to return from the function (either PG_RETURN_VOID() or
PG_RETURN_NULL()).
You seem to be following the document here:
2009/7/22 Dean Rasheed dean.a.rash...@googlemail.com:
OK, here's an updated patch.
In case it's not obvious, I've opted for backend/commands for the new file.
- Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
On Mon, 2009-07-20 at 17:24 +0100, Dean Rasheed wrote:
The same argument could be applied to ruleutils.c, trigfuncs.c and perhaps
one or two others.
And if not there, where then?
I'm moving the patch status back to waiting on author until Alvaro's
concerns are addressed. I don't have an
2009/7/21 Jeff Davis pg...@j-davis.com:
On Mon, 2009-07-20 at 17:24 +0100, Dean Rasheed wrote:
The same argument could be applied to ruleutils.c, trigfuncs.c and perhaps
one or two others.
And if not there, where then?
I'm moving the patch status back to waiting on author until Alvaro's
On Tue, 2009-07-21 at 22:01 +0100, Dean Rasheed wrote:
OK, I'll try to post an updated patch soon.
I'm not seeing an obvious alternative location to utils/adt. Any advice would
be appreciated.
My first reaction is utils/misc.
Regards,
Jeff Davis
--
Sent via pgsql-hackers mailing
Jeff Davis pg...@j-davis.com writes:
On Tue, 2009-07-21 at 22:01 +0100, Dean Rasheed wrote:
I'm not seeing an obvious alternative location to utils/adt. Any advice would
be appreciated.
You could make a fair case for either backend/catalog or
backend/commands. Maybe the latter since that's
2009/7/20 Alvaro Herrera alvhe...@commandprompt.com:
Dean Rasheed wrote:
Thanks for the thorough review. I attach an new version of the patch,
updated to HEAD, and with the index AM change discussed.
Wow, this is a large patch.
I didn't do a thorough review, but some quickies I noticed:
*
Dean Rasheed dean.a.rash...@googlemail.com writes:
* Please move the code that says that it should be in a new file to a
new file.
Ah yes, I forgot about that. Will do.
utils/adt/constraints.c ?
Please, no. Constraints are not a datatype. Do not copy the brain-dead
decision that put
On Mon, Jul 20, 2009 at 10:21:53AM -0400, Tom Lane wrote:
Dean Rasheed dean.a.rash...@googlemail.com writes:
* Please move the code that says that it should be in a new file to a
�new file.
Ah yes, I forgot about that. Will do.
utils/adt/constraints.c ?
Please, no. Constraints are
2009/7/20 David Fetter da...@fetter.org:
On Mon, Jul 20, 2009 at 10:21:53AM -0400, Tom Lane wrote:
Dean Rasheed dean.a.rash...@googlemail.com writes:
* Please move the code that says that it should be in a new file to a
new file.
Ah yes, I forgot about that. Will do.
David Fetter da...@fetter.org writes:
Please, no. Constraints are not a datatype. Do not copy the brain-dead
decision that put ri_triggers there.
Does that mean ri_triggers should come out of there?
Not as long as we're on CVS --- it isn't worth the loss of history.
I might think about it
On Mon, Jul 20, 2009 at 01:00:12PM -0400, Tom Lane wrote:
David Fetter da...@fetter.org writes:
Please, no. Constraints are not a datatype. Do not copy the brain-dead
decision that put ri_triggers there.
Does that mean ri_triggers should come out of there?
Not as long as we're on CVS
David Fetter da...@fetter.org writes:
On Mon, Jul 20, 2009 at 01:00:12PM -0400, Tom Lane wrote:
I might think about it when/if we move to git.
As far as you're concerned, what's blocking that?
Lack of committer familiarity with git, lack of a bulletproof migration
process, uncertainty about
2009/7/20 Alvaro Herrera alvhe...@commandprompt.com:
Seems related to the new list in AfterTriggerSaveEvent, which is
used in ways that seem to conflict with its header comment ...
Reading the comment for that function, I think it is quite misleading
- mainly because the meaning of the word
Dean Rasheed wrote:
Thanks for the thorough review. I attach an new version of the patch,
updated to HEAD, and with the index AM change discussed.
Wow, this is a large patch.
I didn't do a thorough review, but some quickies I noticed:
* Please move the code that says that it should be in a
Review feedback:
1. Compiler warning:
index.c: In function ‘index_create’:
index.c:784: warning: implicit declaration of function ‘SystemFuncName’
2. I know that the GIN, GiST, and Hash AMs don't use the
uniqueness_check argument at all -- in fact it is #ifdef'ed out.
However, for the sake of
On Sun, 2009-07-12 at 14:14 +0100, Dean Rasheed wrote:
Here is an updated version of this patch which should apply to HEAD,
with updated docs, regression tests, pg_dump and psql \d.
It works well for small numbers of temporary uniqueness violations,
and at least as well (in fact about twice
On Tue, Jul 14, 2009 at 09:56:48AM -0700, Jeff Davis wrote:
On Sun, 2009-07-12 at 14:14 +0100, Dean Rasheed wrote:
Here is an updated version of this patch which should apply to HEAD,
with updated docs, regression tests, pg_dump and psql \d.
It works well for small numbers of temporary
Jeff Davis wrote:
The only problem there is telling the btree AM whether or not to do the
insert or not (i.e. fake versus real insert). Perhaps you can just do
that with careful use of a global variable?
Sure, all of this is a little ugly, but we've already acknowledged that
there is some
On Tue, 2009-07-14 at 13:29 -0500, Kenneth Marshall wrote:
I am looking at adding unique support to hash indexes for 8.5 and
they will definitely need to visit the heap.
Have you seen this patch?
http://archives.postgresql.org/message-id/1246840119.19547.126.ca...@jdavis
This patch will
On Tue, 2009-07-14 at 15:00 -0400, Alvaro Herrera wrote:
My 2c on this issue: if this is ugly (and it is) and needs revisiting to
extend it, please by all means let's make it not ugly instead of moving
the ugliness around. I didn't read the original proposal in detail so
IMBFOS, but it
Jeff Davis wrote:
1. Are you saying that an AM API change is the best route? If so, we
should probably start a discussion along those lines. Heikki is already
changing the API for index-only scans, and Dean's API change proposal
may be useful for Kenneth's unique hash indexes. You might as
2009/7/14 Alvaro Herrera alvhe...@commandprompt.com:
Jeff Davis wrote:
The only problem there is telling the btree AM whether or not to do the
insert or not (i.e. fake versus real insert). Perhaps you can just do
that with careful use of a global variable?
Sure, all of this is a little
On Tue, 2009-07-14 at 20:32 +0100, Dean Rasheed wrote:
Well the ugliness referred to here (btree accessing the heap) seems
like a necessary evil. I don't think I want to add to it by
introducing global variables.
Ok, try to coordinate with Kenneth to make sure that the API change
satisfies
On Tue, Jul 14, 2009 at 12:13:33PM -0700, Jeff Davis wrote:
On Tue, 2009-07-14 at 13:29 -0500, Kenneth Marshall wrote:
I am looking at adding unique support to hash indexes for 8.5 and
they will definitely need to visit the heap.
Have you seen this patch?
Jeff Davis wrote:
First, I'm happy that you're working on this; I think it's important. I
am working on another index constraints feature that may have some
interaction:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg00302.php
Let me know if you see any potential conflicts
Jeff Davis wrote:
On Tue, 2009-07-07 at 19:38 +0100, Dean Rasheed wrote:
This approach works well if the number of potential conflicts is
small.
[...]
Curing the scalability problem by spooling the queue to disk shouldn't
be too hard to do, but that doesn't address the problem that if a
First, I'm happy that you're working on this; I think it's important. I
am working on another index constraints feature that may have some
interaction:
http://archives.postgresql.org/pgsql-hackers/2009-07/msg00302.php
Let me know if you see any potential conflicts between our work.
On Tue,
On Tue, 2009-07-07 at 19:38 +0100, Dean Rasheed wrote:
This approach works well if the number of potential conflicts is
small.
[...]
Curing the scalability problem by spooling the queue to disk shouldn't
be too hard to do, but that doesn't address the problem that if a
significant
59 matches
Mail list logo
