Horton

Thread

Commits

  1. Revert unstable test cases from commit 7d80441d2.

  2. Allow psql's \dt and \di to show TOAST tables and their indexes.

  1. allow to \dtS+ pg_toast.*

    Justin Pryzby <pryzby@telsasoft.com> — 2020-11-30T16:54:36Z

    This makes toast tables a bit less special and easier to inspect.

postgres=# \dtS+ pg_toast.pg_toast_2619
 pg_toast | pg_toast_2619 | toast table | pryzbyj | permanent   | heap          | 56 kB | 

This follows commit from last year:
| eb5472da9 make \d pg_toast.foo show its indices ; and, \d toast show its main table

-- 
Justin

  2. Re: allow to \dtS+ pg_toast.*

    Laurenz Albe <laurenz.albe@cybertec.at> — 2020-12-17T15:16:52Z

    On Mon, 2020-11-30 at 10:54 -0600, Justin Pryzby wrote:
> This makes toast tables a bit less special and easier to inspect.
> 
> postgres=# \dtS+ pg_toast.pg_toast_2619
>  pg_toast | pg_toast_2619 | toast table | pryzbyj | permanent   | heap          | 56 kB | 
> 
> This follows commit from last year:
> | eb5472da9 make \d pg_toast.foo show its indices ; and, \d toast show its main table

This would indeed be convenient.

The patch passes regression tests.

While playing around with it, I found the following oddity:

regression=# \dtS+ pg_toast.pg_toast_30701
                                            List of relations
  Schema  |      Name      |    Type     |  Owner  | Persistence | Access Method |  Size   | Description 
----------+----------------+-------------+---------+-------------+---------------+---------+-------------
 pg_toast | pg_toast_30701 | toast table | laurenz | permanent   | heap          | 0 bytes | 
(1 row)

regression=# \dtS pg_toast.pg_toast_30701
                 List of relations
  Schema  |      Name      |    Type     |  Owner  
----------+----------------+-------------+---------
 pg_toast | pg_toast_30701 | toast table | laurenz
(1 row)

regression=# \dt pg_toast.pg_toast_30701
Did not find any relation named "pg_toast.pg_toast_30701".

Now this doesn't seem right.  To my understanding, \dtS should do the same as \dt,
except that it should also search in "pg_catalog" if no schema was provided.

Another thing that is missing is tab completion for

regression=# \dtS pg_toast.pg_

This should work just like for \d and \dS.


Both of these effects can also be observed with \di and toast indexes.

Yours,
Laurenz Albe

  3. Re: allow to \dtS+ pg_toast.*

    Justin Pryzby <pryzby@telsasoft.com> — 2020-12-18T06:58:08Z

    Thanks for looking

On Thu, Dec 17, 2020 at 04:16:52PM +0100, Laurenz Albe wrote:
> On Mon, 2020-11-30 at 10:54 -0600, Justin Pryzby wrote:
> > This makes toast tables a bit less special and easier to inspect.
> > 
> > postgres=# \dtS+ pg_toast.pg_toast_2619
> >  pg_toast | pg_toast_2619 | toast table | pryzbyj | permanent   | heap          | 56 kB | 
> > 
> > This follows commit from last year:
> > | eb5472da9 make \d pg_toast.foo show its indices ; and, \d toast show its main table
> 
> This would indeed be convenient.
> 
> While playing around with it, I found the following oddity:
> 
> regression=# \dtS pg_toast.pg_toast_30701
>  pg_toast | pg_toast_30701 | toast table | laurenz
> 
> regression=# \dt pg_toast.pg_toast_30701
> Did not find any relation named "pg_toast.pg_toast_30701".
> 
> Now this doesn't seem right.  To my understanding, \dtS should do the same as \dt,
> except that it should also search in "pg_catalog" if no schema was provided.

You mean that if pg_toast.* should be shown if a matching "pattern" is given,
even if "S" was not used.  I think you're right.  The behavior I implemented
was intended to provide a bit of historic compatibility towards hiding toast
tables, but I think it's not needed, since they're not shown anyway unless
someone includes "S", specifies the "pg_toast." schema, or pg_toast is in their
search path.  See attached.

> Another thing that is missing is tab completion for
> regression=# \dtS pg_toast.pg_
> This should work just like for \d and \dS.

Tom objected to this in the past, humorously to me:

https://www.postgresql.org/message-id/14255.1536781029@sss.pgh.pa.us
On Wed, Sep 12, 2018 at 03:37:09PM -0400, Tom Lane wrote:
> Arthur Zakirov <a.zakirov@postgrespro.ru> writes:
> > On Sun, Jul 29, 2018 at 07:42:43PM -0500, Justin Pryzby wrote:
> >>> Actually..another thought: since toast tables may be VACUUM-ed, should I
> >>> introduce Query_for_list_of_tpmt ?
> 
> >> I didn't include this one yet though.
> 
> > I think it could be done by a separate patch.
> 
> I don't actually think that's a good idea.  It's more likely to clutter
> peoples' completion lists than offer anything they want.  Even if someone
> actually does want to vacuum a toast table, they are not likely to be
> entering its name via tab completion; they're going to have identified
> which table they want via some query, and then they'll be doing something
> like copy-and-paste out of a query result.

  4. Re: allow to \dtS+ pg_toast.*

    Laurenz Albe <laurenz.albe@cybertec.at> — 2020-12-18T11:43:07Z

    On Fri, 2020-12-18 at 00:58 -0600, Justin Pryzby wrote:
> On Thu, Dec 17, 2020 at 04:16:52PM +0100, Laurenz Albe wrote:
> > On Mon, 2020-11-30 at 10:54 -0600, Justin Pryzby wrote:
> > > This makes toast tables a bit less special and easier to inspect.
> > > 
> > > postgres=# \dtS+ pg_toast.pg_toast_2619
> > >  pg_toast | pg_toast_2619 | toast table | pryzbyj | permanent   | heap          | 56 kB | 
> > > 
> > > This follows commit from last year:
> > > | eb5472da9 make \d pg_toast.foo show its indices ; and, \d toast show its main table
> > 
> > This would indeed be convenient.
> > 
> > While playing around with it, I found the following oddity:
> > 
> > regression=# \dtS pg_toast.pg_toast_30701
> >  pg_toast | pg_toast_30701 | toast table | laurenz
> > 
> > regression=# \dt pg_toast.pg_toast_30701
> > Did not find any relation named "pg_toast.pg_toast_30701".
> > 
> > Now this doesn't seem right.  To my understanding, \dtS should do the same as \dt,
> > except that it should also search in "pg_catalog" if no schema was provided.
> 
> You mean that if pg_toast.* should be shown if a matching "pattern" is given,
> even if "S" was not used.  I think you're right.  The behavior I implemented
> was intended to provide a bit of historic compatibility towards hiding toast
> tables, but I think it's not needed, since they're not shown anyway unless
> someone includes "S", specifies the "pg_toast." schema, or pg_toast is in their
> search path.  See attached.

Yes, exactly.

I wonder why the modification in "listPartitionedTables" is necessary.
Surely there cannot be any partitioned toast tables, can there?

> > Another thing that is missing is tab completion for
> > regression=# \dtS pg_toast.pg_
> > This should work just like for \d and \dS.
> 
> Tom objected to this in the past, humorously to me:
> 
> https://www.postgresql.org/message-id/14255.1536781029@sss.pgh.pa.us

That was about VACUUM and ANALYZE, and I certainly see the point
that this is not a frequent enough use case to warrant guiding
people there with tab completion.

But I don't take that as an argument against tab completion in our case.
If I want to know how big the TOAST table of relation 87654 is,
I think it is convenient if I can tab to

\dt+ pg_toast.pg_toast_

If you want to abolish special treatment for TOAST tables in \dt(S),
go all the way.

Yours,
Laurenz Albe

  5. Re: allow to \dtS+ pg_toast.*

    Justin Pryzby <pryzby@telsasoft.com> — 2020-12-18T17:33:12Z

    On Fri, Dec 18, 2020 at 12:43:07PM +0100, Laurenz Albe wrote:
> On Fri, 2020-12-18 at 00:58 -0600, Justin Pryzby wrote:
> > On Thu, Dec 17, 2020 at 04:16:52PM +0100, Laurenz Albe wrote:
> > > On Mon, 2020-11-30 at 10:54 -0600, Justin Pryzby wrote:
> > > > This makes toast tables a bit less special and easier to inspect.
> > > > 
> > > > postgres=# \dtS+ pg_toast.pg_toast_2619
> > > >  pg_toast | pg_toast_2619 | toast table | pryzbyj | permanent   | heap          | 56 kB | 
> > > > 
> > > > This follows commit from last year:
> > > > | eb5472da9 make \d pg_toast.foo show its indices ; and, \d toast show its main table
> > > 
> > > This would indeed be convenient.
> > > 
> > > While playing around with it, I found the following oddity:
> > > 
> > > regression=# \dtS pg_toast.pg_toast_30701
> > >  pg_toast | pg_toast_30701 | toast table | laurenz
> > > 
> > > regression=# \dt pg_toast.pg_toast_30701
> > > Did not find any relation named "pg_toast.pg_toast_30701".
> > > 
> > > Now this doesn't seem right.  To my understanding, \dtS should do the same as \dt,
> > > except that it should also search in "pg_catalog" if no schema was provided.
> > 
> > You mean that if pg_toast.* should be shown if a matching "pattern" is given,
> > even if "S" was not used.  I think you're right.  The behavior I implemented
> > was intended to provide a bit of historic compatibility towards hiding toast
> > tables, but I think it's not needed, since they're not shown anyway unless
> > someone includes "S", specifies the "pg_toast." schema, or pg_toast is in their
> > search path.  See attached.
> 
> Yes, exactly.
> 
> I wonder why the modification in "listPartitionedTables" is necessary.
> Surely there cannot be any partitioned toast tables, can there?

The comment should be removed for consistency.
And I changed the code for consistency with listTables (from which I assume
listPartitionedTables was derived - I was involved in the last stages of that
patch).  It doesn't need to exclude pg_catalog or information_schema, either,
but it's kept the same for consistency.  That part could also be removed.

> > > Another thing that is missing is tab completion for
> > > regression=# \dtS pg_toast.pg_
> > > This should work just like for \d and \dS.
..
> If I want to know how big the TOAST table of relation 87654 is,
> I think it is convenient if I can tab to
> 
> \dt+ pg_toast.pg_toast_

I agree that it's nice to complete the schema name, but I'm still not convinced
this part should be included.

The way to include pg_toast.pg_toast is if toast relations are included, which
is exactly what Tom pointed out is usually unhelpful.  If you include toast
relations, tab completion might give "pg_toast.pg_toast_14..." when you wanted
to paste "145678" - you'd need to remove the common suffix that it found.

I considered whether "toast table" should be capitalized (as it is for "\d")
but I think it should stay lowercase.

-- 
Justin

  6. Re: allow to \dtS+ pg_toast.*

    Laurenz Albe <laurenz.albe@cybertec.at> — 2020-12-21T14:20:51Z

    On Fri, 2020-12-18 at 11:33 -0600, Justin Pryzby wrote:
> > > > > This makes toast tables a bit less special and easier to inspect.
> > 
> > I wonder why the modification in "listPartitionedTables" is necessary.
> > Surely there cannot be any partitioned toast tables, can there?
> 
> The comment should be removed for consistency.
> And I changed the code for consistency with listTables (from which I assume
> listPartitionedTables was derived - I was involved in the last stages of that
> patch).  It doesn't need to exclude pg_catalog or information_schema, either,
> but it's kept the same for consistency.  That part could also be removed.

I don't think that consistency with "listTables" is particularly useful here,
but I think this is somewhat academic.

I'll leave that for the committer to decide.

> > > > Another thing that is missing is tab completion for
> > > > regression=# \dtS pg_toast.pg_
> > > > This should work just like for \d and \dS.
> 
> I agree that it's nice to complete the schema name, but I'm still not convinced
> this part should be included.
> 
> The way to include pg_toast.pg_toast is if toast relations are included, which
> is exactly what Tom pointed out is usually unhelpful.  If you include toast
> relations, tab completion might give "pg_toast.pg_toast_14..." when you wanted
> to paste "145678" - you'd need to remove the common suffix that it found.

Again a judgement call.  I am happy with the way the latest patch does it.

> I considered whether "toast table" should be capitalized (as it is for "\d")
> but I think it should stay lowercase.

Then you should also change the way \d does it (upper case).
I think we should be consistent.

I'd use TOAST for both to create no unnecessary change in \d output.

Anyway, I think that this is ready for committer and will mark it as such.

Yours,
Laurenz Albe

  7. Re: allow to \dtS+ pg_toast.*

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-01-05T23:46:01Z

    Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Fri, 2020-12-18 at 11:33 -0600, Justin Pryzby wrote:
>>>> This makes toast tables a bit less special and easier to inspect.

Pushed, except for

>>> Another thing that is missing is tab completion for
>>> regression=# \dtS pg_toast.pg_
>>> This should work just like for \d and \dS.

>> I agree that it's nice to complete the schema name, but I'm still not convinced
>> this part should be included.

> Again a judgement call.  I am happy with the way the latest patch does it.

I remain pretty much against including toast tables in tab completion,
so I left out that part.

>> I considered whether "toast table" should be capitalized (as it is for "\d")
>> but I think it should stay lowercase.

> Then you should also change the way \d does it (upper case).
> I think we should be consistent.

I think capitalized TOAST is correct, because it's an acronym.
I also agree that consistency with what \d shows is important,
and I have no desire to change \d's longstanding output.

			regards, tom lane

  8. Re: allow to \dtS+ pg_toast.*

    Justin Pryzby <pryzby@telsasoft.com> — 2021-01-05T23:50:20Z

    On Tue, Jan 05, 2021 at 06:46:01PM -0500, Tom Lane wrote:
> Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > On Fri, 2020-12-18 at 11:33 -0600, Justin Pryzby wrote:
> >>>> This makes toast tables a bit less special and easier to inspect.
> 
> Pushed, except for

Thank you

-- 
Justin