Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-03 Thread Alvaro Herrera
Tom Lane wrote: > I think Peter's got the error and the detail backwards. It should be > more like > > ERROR: "someview" cannot have constraints > DETAIL: "someview" is a view. > > If we do it like that, we need one ERROR message per error reason, > and one DETAIL per relkind, which should be

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-03 Thread Alvaro Herrera
Joe Conway wrote: > On 08/02/2017 10:52 PM, Ashutosh Bapat wrote: > > On Wed, Aug 2, 2017 at 11:15 PM, Alvaro Herrera > > wrote: > >> Alvaro Herrera wrote: > >>> I think pg_class is a reasonable place to put more generic relkind lists > >>> alongside a matching error

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-03 Thread Joe Conway
On 08/02/2017 10:52 PM, Ashutosh Bapat wrote: > On Wed, Aug 2, 2017 at 11:15 PM, Alvaro Herrera > wrote: >> Alvaro Herrera wrote: >>> I think pg_class is a reasonable place to put more generic relkind lists >>> alongside a matching error message for each, rather than

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-03 Thread Joe Conway
On 08/02/2017 10:30 PM, Ashutosh Bapat wrote: > On Wed, Aug 2, 2017 at 8:47 PM, Robert Haas wrote: >> 0001-RELKIND_HAS_VISIBILITY_MAP.patch - one place >> 0002-RELKIND_HAS_STORAGE.patch - one place >> 0003-RELKIND_HAS_XIDS-macro.patch - one place >>

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Ashutosh Bapat
On Wed, Aug 2, 2017 at 11:15 PM, Alvaro Herrera wrote: > Alvaro Herrera wrote: >> I think pg_class is a reasonable place to put more generic relkind lists >> alongside a matching error message for each, rather than specialized >> "does this relkind have storage" macros.

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Ashutosh Bapat
On Wed, Aug 2, 2017 at 10:28 PM, Tom Lane wrote: > Robert Haas writes: >> I've thought about this kind of thing, too. But the thing is that >> most of these macros you're proposing to introduce only get used in >> one place. > > I think the value would

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Ashutosh Bapat
On Wed, Aug 2, 2017 at 8:47 PM, Robert Haas wrote: > On Mon, Jul 3, 2017 at 3:52 AM, Ashutosh Bapat > wrote: >>> I noticed, that >>> after we introduced RELKIND_PARTITIONED_TABLE, we required to change a >>> number of conditions to include

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Tom Lane
Alvaro Herrera writes: > Peter Eisentraut wrote: >> The actual error, from the perspective of the user, is something like >> ERROR: "someview" is a view >> DETAIL: Views cannot have constraints. > OK. "%s is a %s" is a reasonable set of errors -- we just need one for >

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Alvaro Herrera
Peter Eisentraut wrote: > I don't find this style of error message optimal anyway. If I do, for > example > > ALTER TABLE someview ADD CONSTRAINT ... > ERROR: "someview" is not a table, foreign table, whatever > > then this information is not helpful. It's not like I'm going to turn > my view

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Alvaro Herrera
Alvaro Herrera wrote: > I think pg_class is a reasonable place to put more generic relkind lists > alongside a matching error message for each, rather than specialized > "does this relkind have storage" macros. What about something like a > struct list in pg_class.h, I just noticed that this

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Peter Eisentraut
On 8/2/17 13:28, Alvaro Herrera wrote: > I think pg_class is a reasonable place to put more generic relkind lists > alongside a matching error message for each, rather than specialized > "does this relkind have storage" macros. What about something like a > struct list in pg_class.h, > > { >

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Alvaro Herrera
I think pg_class is a reasonable place to put more generic relkind lists alongside a matching error message for each, rather than specialized "does this relkind have storage" macros. What about something like a struct list in pg_class.h, { { relkinds_r_i_T, { 'r', 'i', 'T' },

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Tom Lane
Robert Haas writes: > I've thought about this kind of thing, too. But the thing is that > most of these macros you're proposing to introduce only get used in > one place. I think the value would be in having a centralized checklist of

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-08-02 Thread Robert Haas
On Mon, Jul 3, 2017 at 3:52 AM, Ashutosh Bapat wrote: >> I noticed, that >> after we introduced RELKIND_PARTITIONED_TABLE, we required to change a >> number of conditions to include this relkind. We missed some places in >> initial commits and fixed those later.

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-07-03 Thread Ashutosh Bapat
Forgot to attach the patch with the earlier mail. On Mon, Jul 3, 2017 at 1:22 PM, Ashutosh Bapat wrote: > On Mon, May 29, 2017 at 12:55 PM, Ashutosh Bapat > wrote: >> >> -- >> I noticed, that >> after we introduced

Re: [HACKERS] Macros bundling RELKIND_* conditions

2017-07-03 Thread Ashutosh Bapat
On Mon, May 29, 2017 at 12:55 PM, Ashutosh Bapat wrote: > > -- > I noticed, that > after we introduced RELKIND_PARTITIONED_TABLE, we required to change a > number of conditions to include this relkind. We missed some places in > initial commits and fixed those

[HACKERS] Macros bundling RELKIND_* conditions

2017-05-29 Thread Ashutosh Bapat
Hi, We saw a handful bugs reported because RELKIND_PARTITIONED_TABLE was not added to appropriate conditions on relkind. One such report is [1]. On that thread I suggested that we encapsulate these conditions as macros. Here's excerpt of my mail. -- I noticed, that after we introduced