Обсуждение: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missingcolumn ..."

Ryan Murphy <ryanfmurphy@gmail.com> writes:
> The attached patch is my initial attempt at adding the type, making the
> error message read e.g.:
> ERROR: child table is missing column "name" text

"... column "name" of type text", perhaps?  Does not read very nicely
as is.

> I'm sure it needs work (in particular I borrowed a lot of the get-type-name
> logic from getTypeOutputInfo() so probably needs a factor),

The approved way to do this is with format_type_be(), or
format_type_with_typemod() if you want to include typmod info, which
I think you probably do for the intended use-case.
        regards, tom lane

Ryan Murphy <ryanfmurphy@gmail.com> writes:
> So I tried using format_type_with_typemod() thinking that the "typemod
> info" meant things like NOT NULL, DEFAULT etc.

No, it means atttypmod, which stores info like the max length for
varchar(n).

> when I was hoping for
> user=# alter table temp inherit entity;
> ERROR:  child table is missing column "id" uuid default uuid_generate_v1mc()
> Is there an easy way to get the string that includes all those additional
> constraints/defaults etc?

No, and TBH I would vote strongly against including that much detail in
this error message anyway.  That info could be indefinitely long, and it's
not especially relevant to the stated error condition --- for example, the
presence of a default is *not* relevant to whether the column matches the
parent.  I'm okay with shoehorning column type into this message, but not
much more than that.
        regards, tom lane

On 01/07/2017 08:15 PM, Tom Lane wrote:
> Ryan Murphy <ryanfmurphy@gmail.com> writes:
>> I was hoping for
>> user=# alter table temp inherit entity;
>> ERROR:  child table is missing column "id" uuid default uuid_generate_v1mc()
>> Is there an easy way to get the string that includes all those additional
>> constraints/defaults etc?
> 
> No, and TBH I would vote strongly against including that much detail in
> this error message anyway.  That info could be indefinitely long, and it's
> not especially relevant to the stated error condition --- for example, the
> presence of a default is *not* relevant to whether the column matches the
> parent.  I'm okay with shoehorning column type into this message, but not
> much more than that.

I agree.

Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
could be emitted with the entire column definition (or close to it)?
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Vik Fearing <vik@2ndquadrant.fr> writes:
> On 01/07/2017 08:15 PM, Tom Lane wrote:
>> No, and TBH I would vote strongly against including that much detail in
>> this error message anyway.  That info could be indefinitely long, and it's
>> not especially relevant to the stated error condition --- for example, the
>> presence of a default is *not* relevant to whether the column matches the
>> parent.  I'm okay with shoehorning column type into this message, but not
>> much more than that.

> I agree.

> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
> could be emitted with the entire column definition (or close to it)?

AFAICS, what Ryan is after would be better served by a separate tool that
would produce a "diff" of the two tables' schemas.  Even if we were
willing to overload this error message with everything we know about the
parent column, do you really want to fix discrepancies one column at a
time?  And what about properties that can't be uniquely associated with a
single column, such as constraints covering multiple columns?

Also, I have a feeling that a suitable tool is already out there.  A
moment's digging in the list archives found this thread with links to
several candidates:

https://www.postgresql.org/message-id/flat/561D27E7.5010906%40trustport.com

and I'm pretty sure there have been other such discussions.
        regards, tom lane

On 01/07/2017 11:05 PM, Tom Lane wrote:
> Vik Fearing <vik@2ndquadrant.fr> writes:
>> On 01/07/2017 08:15 PM, Tom Lane wrote:
>>> No, and TBH I would vote strongly against including that much detail in
>>> this error message anyway.  That info could be indefinitely long, and it's
>>> not especially relevant to the stated error condition --- for example, the
>>> presence of a default is *not* relevant to whether the column matches the
>>> parent.  I'm okay with shoehorning column type into this message, but not
>>> much more than that.
> 
>> I agree.
> 
>> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line
>> could be emitted with the entire column definition (or close to it)?
> 
> AFAICS, what Ryan is after would be better served by a separate tool that
> would produce a "diff" of the two tables' schemas.  Even if we were
> willing to overload this error message with everything we know about the
> parent column, do you really want to fix discrepancies one column at a
> time?  And what about properties that can't be uniquely associated with a
> single column, such as constraints covering multiple columns?
> 
> Also, I have a feeling that a suitable tool is already out there.

Well personally, if I were trying to attach one table to another and it
didn't work, I'd use good ol' \d.  Maybe that's just me.
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

On 1/7/17 1:16 PM, Ryan Murphy wrote:
>
>     No, and TBH I would vote strongly against including that much detail in
>     this error message anyway.  That info could be indefinitely long,
>     and it's
>     not especially relevant to the stated error condition --- for
>     example, the
>     presence of a default is *not* relevant to whether the column
>     matches the
>     parent.  I'm okay with shoehorning column type into this message,
>     but not
>     much more than that.
>
>                             regards, tom lane
>
>
> Ok, that makes sense.  How about things like NOT NULL? you get an error
> if your column doesn't have that.

Yeah, anything that we're explicitly testing for needs to be mentioned 
in an error message, otherwise users will be very confused if the column 
*is* in the parent but is failing some other test. Perhaps it would be 
better for each test to spit out a different error message making it 
clear what exactly was wrong.

Related to the other idea of seeing the problems that exist in all the 
columns (instead of one column at a time), I think it'd be reasonable to 
have a SRF that spit out everything you'd need to fix to allow 
inheritance to be added. A schema diff won't know what specifically has 
to match, but our code does.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

Tom Lane wrote:

> AFAICS, what Ryan is after would be better served by a separate tool that
> would produce a "diff" of the two tables' schemas.  Even if we were
> willing to overload this error message with everything we know about the
> parent column, do you really want to fix discrepancies one column at a
> time?  And what about properties that can't be uniquely associated with a
> single column, such as constraints covering multiple columns?
> 
> Also, I have a feeling that a suitable tool is already out there.  A
> moment's digging in the list archives found this thread with links to
> several candidates:
> 
> https://www.postgresql.org/message-id/flat/561D27E7.5010906%40trustport.com
> 
> and I'm pretty sure there have been other such discussions.

I remember looking some time ago, and most of all the possible
candidates were either abandoned or terribly cumbersome to use.  I ran
across Euler Taveira in a conference sometime later and he told me he
had the idea of working on writing such a tool.  I was pleased to see
his announcement recently:
https://www.postgresql.org/message-id/6c9d4a85-55b5-81cc-6f09-8f26a6da2072@timbira.com.br
I admit I have not looked at his code, but he had some very good ideas
for the complex cases.  I think it's worth checking out what he has
done.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services