Re: ALTER TABLE behind-the-scenes effects' CONTEXT

2016-01-04 21:44 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> Pavel Stehule wrote:
> > 2015-10-05 0:08 GMT+02:00 Marko Tiikkaja <marko(at)joh(dot)to>:
> >
> > > In the past I've found the error message in cases such as this somewhat
> > > less helpful than it could be:
> > >
> > > =# CREATE TABLE qqq (a int);
> > > =# CREATE UNIQUE INDEX IF NOT EXISTS qqq_a_idx ON qqq(a);
> > > =# ALTER TABLE qqq ALTER COLUMN a TYPE json USING NULL;
> > > ERROR: data type json has no default operator class for access method
> > > "btree"
> > > HINT: You must specify an operator class for the index or define a
> > > default operator class for the data type.
> > >
> > > The attached patch adds a CONTEXT line to index and constraint
> rebuilds,
> > > e.g:
> > >
> > > CONTEXT: while rebuilding index qqq_a_idx
> >
> > I prefer using DETAIL field for this case.
>
> I think DETAIL doesn't work for this; the advantage of CONTEXT is that
> it can be set not at the location of the actual error but at the calling
> code, by setting up a callback. I think Marko got that right.
>

ok

Regards

Pavel

>
> I'd add some quoting to the object name in the message, and make sure we
> don't crash if the name isn't set up (i.e. test for nullness). But
> also, why do we set the name only in ATPostAlterTypeParse()? Maybe
> there are more ALTER TABLE subcommands that should be setting something
> up? In cases where multiple subcommands are being run, it might be
> useful to see which one caused a certain error message.
>
> I think some additional tests wouldn't hurt.
>
> I await feedback from Simon Riggs, who set himself up as reviewer a
> couple of days ago. Simon, do you also intend to be committer? If so,
> please mark yourself as such in the commitfest app.
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>