patch - Report the schema along table name in a referential failure error message

Lists: pgsql-hackers
From: George Gensure <werkt0(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 07:45:17
Message-ID: b47db0340911142345v60ae8c14n4cfd745b7c100a0b@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I've put together a small patch to provide a schema name in an fk
violation in deference to the todo item "Report the schema along table
name in a referential failure error message"

The error message only contains the schema if the table name being
referenced is non-unique or not present in the search_path.

It passes a make check, and I've added a couple of test cases which
expect the schema's output in the cases mentioned above.

Also, it looks like Rev 1.113 added spaces to the values specified in
errdetail for failed FK violations, but the testoutput wasn't updated.
I haven't included that in this patch for clarity, but it probably
should be corrected.

Have at it,
-George

Attachment Content-Type Size
fk_schemas.patch application/octet-stream 8.6 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: George Gensure <werkt0(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 16:21:57
Message-ID: 19906.1258302117@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

George Gensure <werkt0(at)gmail(dot)com> writes:
> I've put together a small patch to provide a schema name in an fk
> violation in deference to the todo item "Report the schema along table
> name in a referential failure error message"

This is not the way forward; if it were, we would have done it years
ago. Despite the poor wording of the TODO item, nobody is particularly
interested in solving this problem one error message at a time.
Furthermore, inserting the schema name into the text as you have done it
is 100% wrong --- in an example like
... table "non_searched_schema.fknsref" violates ...
the reader could be excused for thinking that the report is showing
an unqualified name that happens to include a dot, because that's
what double quotes imply in SQL. And it certainly does not help
client-side tools that want to extract the full table name, which
is the real subtext behind many of the requests for this.

The direction that we really want to move in is to include the table and
schema names as well as other elements of the standard "diagnostics
area" as separate fields in error reports. That will be a great deal
of work unfortunately :-( which is why it hasn't been tackled yet.

regards, tom lane


From: George Gensure <werkt0(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 18:09:56
Message-ID: b47db0340911151009s26aed4a7l5e98de0904d12d8f@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sun, Nov 15, 2009 at 11:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> George Gensure <werkt0(at)gmail(dot)com> writes:
>> I've put together a small patch to provide a schema name in an fk
>> violation in deference to the todo item "Report the schema along table
>> name in a referential failure error message"
>
> This is not the way forward; if it were, we would have done it years
> ago.  Despite the poor wording of the TODO item, nobody is particularly
> interested in solving this problem one error message at a time.
> Furthermore, inserting the schema name into the text as you have done it
> is 100% wrong --- in an example like
>        ... table "non_searched_schema.fknsref" violates ...
> the reader could be excused for thinking that the report is showing
> an unqualified name that happens to include a dot, because that's
> what double quotes imply in SQL.  And it certainly does not help
> client-side tools that want to extract the full table name, which
> is the real subtext behind many of the requests for this.
>
> The direction that we really want to move in is to include the table and
> schema names as well as other elements of the standard "diagnostics
> area" as separate fields in error reports.  That will be a great deal
> of work unfortunately :-( which is why it hasn't been tackled yet.
>
>                        regards, tom lane
>

Fair enough, and I hadn't even considered that dots could be valid
chars in table names. I noted your post in the chain attached to this
todo request in which you said this is a much bigger problem, but
didn't think that you would have left it marked as easy if you thought
there should be something done that makes the original error string
modification pointless.

This begs a bigger question: what's *really* easy or low barrier to
entry for very light contributors like myself? - I've got time, I like
the product, I need to know what's going to get you a win, I may not
be gunning particularly for the feature myself. Its fascinating that
this item also included a mention of straw polling in its thread.

Thanks,
-George


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: George Gensure <werkt0(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 18:43:05
Message-ID: 4B004BB9.5030207@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

George Gensure wrote:
> This begs a bigger question: what's *really* easy or low barrier to
> entry for very light contributors like myself? - I've got time, I like
> the product, I need to know what's going to get you a win, I may not
> be gunning particularly for the feature myself.

The TODO list at <http://wiki.postgresql.org/wiki/Todo> doesn't seem to
have a huge number or [E] items. Maybe we need a bit of a brainstorm to
come up with a few more.

The one I just started talking about (using param names in SQL
functions) might not be terribly hard, depending on your coding skills,
since it would be making use of the new parser hooks feature that Tom
has just done the heavy lifting on.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: George Gensure <werkt0(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 21:55:38
Message-ID: 24296.1258322138@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> George Gensure wrote:
>> This begs a bigger question: what's *really* easy or low barrier to
>> entry for very light contributors like myself?

> The TODO list at <http://wiki.postgresql.org/wiki/Todo> doesn't seem to
> have a huge number or [E] items. Maybe we need a bit of a brainstorm to
> come up with a few more.

The real problem with the entry that George picked up on was that it was
misdescribed and mislabeled as easy because whoever put it in ignored
the fact that there was not a consensus to do a half-baked fix ...
this is a problem with a wiki TODO list :-(

> The one I just started talking about (using param names in SQL
> functions) might not be terribly hard, depending on your coding skills,
> since it would be making use of the new parser hooks feature that Tom
> has just done the heavy lifting on.

It is easy ... as long as you don't move the goalposts by insisting on
inventing some nonstandard syntax. I would envision that given
create function f (x int)
you should be able to refer to the parameter as "x" or "f.x" if you
need to qualify it. This matches plpgsql practice and won't surprise
anybody, and can be implemented with a couple hours' hacking I'd guess.

regards, tom lane


From: Brendan Jurd <direvus(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, George Gensure <werkt0(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 22:18:36
Message-ID: 37ed240d0911151418i74b70a93t90ed407131ddb094@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2009/11/16 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> The real problem with the entry that George picked up on was that it was
> misdescribed and mislabeled as easy because whoever put it in ignored
> the fact that there was not a consensus to do a half-baked fix ...
> this is a problem with a wiki TODO list :-(

Wouldn't it be more accurate to say that it's a problem with *any*
TODO list? I don't see what the wiki has to do with it. Garbage in,
garbage out. A poorly described item will always be trouble
regardless of what form it is in.

However, I'm not sure how productive the [E]asy marker can really be.
Items end up on the TODO generally because a) we couldn't settle on a
way forward, or b) nobody was keen to do it right away. There just
aren't many genuinely "easy" items in there, easy ones usually get
done right away.

Cheers,
BJ


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Brendan Jurd <direvus(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, George Gensure <werkt0(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-15 22:32:33
Message-ID: 24812.1258324353@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Brendan Jurd <direvus(at)gmail(dot)com> writes:
> However, I'm not sure how productive the [E]asy marker can really be.
> Items end up on the TODO generally because a) we couldn't settle on a
> way forward, or b) nobody was keen to do it right away. There just
> aren't many genuinely "easy" items in there, easy ones usually get
> done right away.

Yeah, that is a real problem for new would-be contributors --- there
simply isn't that much low-hanging fruit waiting for them, unless
they focus on areas that no one else has taken much interest in;
and even then there are few really small tasks.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Brendan Jurd <direvus(at)gmail(dot)com>, George Gensure <werkt0(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-16 01:14:36
Message-ID: 4B00A77C.4090709@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
>
> Yeah, that is a real problem for new would-be contributors --- there
> simply isn't that much low-hanging fruit waiting for them, unless
> they focus on areas that no one else has taken much interest in;
> and even then there are few really small tasks.
>
>

Then I think we need to start being more creative about ways to ease the
path for people who want to get people involved.

cheers

andrew


From: Christophe Pettus <xof(at)thebuild(dot)com>
To: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Apprentices? (was =patch - Report the schema...)
Date: 2009-11-16 03:34:12
Message-ID: 1AE13344-F603-40C0-BD75-1DD62B08BD58@thebuild.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On Nov 15, 2009, at 5:14 PM, Andrew Dunstan wrote:
> Then I think we need to start being more creative about ways to ease
> the path for people who want to get people involved.

With that as an inspiration, I'd like to offer a modest proposal:
Apprentices.

In my case, I would be very exciting about participating more in PG
development. I have a reasonable amount of C/C++ experience (20-ish
years), and have been a PG admin/client developer since 1998, but I
lack enough familiarity with the codebase to do patch review on non-
trivial patches, or to launch in and do development on a non-trivial
feature. (And, as has been noted, there are essential no non-trivial
features needing work.)

On the other hand, I'm sure there are those such as myself who are
perfectly capable in *assisting* in some way: Reviewing a patch for
specific things, writing test cases, working on a subsection of a
larger patch. This also is a solution for the intimidating prospect
of working on a large patch and being sold, "Um, that's really
completely the wrong thing. Sorry."

Not every primary contribution or patch reviewer is going to be
comfortable working with other people, because of temperament or work
style, but I'm sure some are. Might this help?
--
-- Christophe Pettus
xof(at)thebuild(dot)com


From: George Gensure <werkt0(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-17 05:41:53
Message-ID: b47db0340911162141r785a3c11va2a993979be680b0@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sun, Nov 15, 2009 at 1:43 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> George Gensure wrote:
>>
>> This begs a bigger question:  what's *really* easy or low barrier to
>> entry for very light contributors like myself? - I've got time, I like
>> the product, I need to know what's going to get you a win, I may not
>> be gunning particularly for the feature myself.
>
>
> The TODO list at <http://wiki.postgresql.org/wiki/Todo> doesn't seem to have
> a huge number or [E] items.  Maybe we need a bit of a brainstorm to come up
> with a few more.
>
> The one I just started talking about (using param names in SQL functions)
> might not be terribly hard, depending on your coding skills, since it would
> be making use of the new parser hooks feature that Tom has just done the
> heavy lifting on.
>
> cheers
>
> andrew
>

There's some tricky stuff in here to say the least. Doesn't look like
param names are kept anywhere past the parser - gonna have to have it
follow through a bunch of functions to reach
parse_(fixed|variable)_parameters. The p_post_columnref_hook you
alluded to will help once I have the names though, so thanks :)

-George


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: George Gensure <werkt0(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-17 14:16:10
Message-ID: 27534.1258467370@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

George Gensure <werkt0(at)gmail(dot)com> writes:
> There's some tricky stuff in here to say the least. Doesn't look like
> param names are kept anywhere past the parser - gonna have to have it
> follow through a bunch of functions to reach
> parse_(fixed|variable)_parameters. The p_post_columnref_hook you
> alluded to will help once I have the names though, so thanks :)

I'm not sure where you're looking, but I would think the place to start
is with pulling the parameter names out of the pg_proc tuple in
init_sql_fcache.

regards, tom lane


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: George Gensure <werkt0(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: patch - Report the schema along table name in a referential failure error message
Date: 2009-11-17 19:53:58
Message-ID: 20091117195358.GC4146@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane escribió:
> George Gensure <werkt0(at)gmail(dot)com> writes:
> > I've put together a small patch to provide a schema name in an fk
> > violation in deference to the todo item "Report the schema along table
> > name in a referential failure error message"
>
> This is not the way forward; if it were, we would have done it years
> ago. Despite the poor wording of the TODO item, nobody is particularly
> interested in solving this problem one error message at a time.

FWIW see this thread
http://archives.postgresql.org/pgsql-hackers/2009-08/msg00213.php

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.