2009/7/29 Tom Lane :
> Dean Rasheed 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 "editorializations" in
detail yet, but I'm
On Wed, Jul 29, 2009 at 5:00 PM, Tom Lane wrote:
> Dean Rasheed writes:
>> [ deferrable unique constraints patch ]
>
> Applied after rather extensive editorialization. Aside from the points
Wow, cool.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make chan
Tom Lane wrote:
> Dean Rasheed writes:
> > [ deferrable unique constraints patch ]
>
> Applied after rather extensive editorialization.
Kudos!!
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
--
Sent via pgsql-hacke
Dean Rasheed 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 cross-check
to verify that the original
Dean Rasheed writes:
> 2009/7/29 Tom Lane :
>> For non-unique indexes, it is not required that aminsert
>> do anything; it might for instance refuse to index NULLs.
> Doesn't this comment apply equally to unique indexes?
Hmm, I was thinking that a unique-capable index would have to index all
2009/7/29 Tom Lane :
> 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
2009/7/28 Tom Lane :
> ... 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
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 resul
Jeff Davis 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.
> My point
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
Jeff Davis 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 some of the
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
... 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 catalogs
... 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:
http://www.postgresql.org/mailpref/p
Jeff Davis 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 looking cl
On Tue, Jul 28, 2009 at 12:04 AM, Tom Lane wrote:
> Greg Stark writes:
>> For foreign keys I was picturing some way to issue an SQL statement
>> like "SELECT from tabletocheck where ctid in () and
>> not exists (select 1 from referenced_table where pk =
>> tabletocheck.fk)" and then somehow pass t
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 v
Dean Rasheed writes:
> 2009/7/27 Jeff Davis :
>> 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 add such a column
>> anyway for my
Greg Stark writes:
> For foreign keys I was picturing some way to issue an SQL statement
> like "SELECT from tabletocheck where ctid in () and
> not exists (select 1 from referenced_table where pk =
> tabletocheck.fk)" and then somehow pass the list of ctids from the
> deferred list.
I have no pr
On Mon, Jul 27, 2009 at 7:51 PM, Tom Lane 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 suggested keeping the
2009/7/27 Jeff Davis :
> 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 shoul
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, on
[ still poking around in this patch ]
Jeff Davis 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 hold an index oid
2009/7/27 Tom Lane :
> (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 as the UPDATE.
B
2009/7/27 Tom Lane :
> Jeff Davis 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
Dean Rasheed 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 check for RI constraints
Jeff Davis 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) when the list ge
2009/7/27 Jeff Davis :
> 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 tackle the
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
Dean Rasheed 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 treating it
as an investigation exe
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:
http://www.postgresq
2009/7/22 Dean Rasheed :
> 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:
http://www.postgresql.org/mailpref/pgsql-hack
Jeff Davis 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 where the core
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 maili
2009/7/21 Jeff Davis :
> 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
> concer
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/20 Alvaro Herrera :
> 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 "event" mutates half-way
t
David Fetter 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 preferred mult
On Mon, Jul 20, 2009 at 01:00:12PM -0400, Tom Lane wrote:
> David Fetter 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 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 when/if we mo
2009/7/20 David Fetter :
> On Mon, Jul 20, 2009 at 10:21:53AM -0400, Tom Lane wrote:
>> Dean Rasheed 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
On Mon, Jul 20, 2009 at 10:21:53AM -0400, Tom Lane wrote:
> Dean Rasheed 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
Dean Rasheed 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 ri_triggers there.
2009/7/20 Alvaro Herrera :
> 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
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 n
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 d
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?
>
> http://archives.postgre
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 d
2009/7/14 Alvaro Herrera :
> 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 alr
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
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 doe
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 supp
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
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 tem
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 tw
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
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
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 propo
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, 2009-
59 matches
Mail list logo
