Обсуждение: ALTER TABLE behind-the-scenes effects' CONTEXT

Hi,

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

Any feedback welcome.

.m


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel Stehule wrote:
> 2015-10-05 0:08 GMT+02:00 Marko Tiikkaja <marko@joh.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.

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

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.

On 4 January 2016 at 20:44, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

 

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 like the patch.

We should have a message for each subcommand, since there are many that run for a long time and we support the optimization of allowing many subcommands together at once.

There should also be a comment about making name a requirement for any subcommand.

 

I think some additional tests wouldn't hurt.

Each subcommand message should be generated at least once in tests.

 

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.

Happy to be the committer on this.