Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

On Fri, Mar 11, 2022 at 11:51 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Mar 11, 2022 at 1:10 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > I don't think you've adequately considered temporary relations here.
> > It seems to be that ReadBufferWithoutRelcache() could not be safe on a
> > temprel, because we'd need a BackendId to access the underlying
> > storage. So I think that ReadBufferWithoutRelcache can only accept
> > unlogged or permanent, and maybe the argument ought to be a Boolean
> > instead of a relpersistence value. I thought that this problem might
> > be only cosmetic, but I checked the code that actually does the copy,
> > and there's no filter there on relpersistence either. And I think
> > there should be.

Yeah right for now, this api can only support unlogged or permanent.
I will fix this

> I hit "send" too quickly there:
>
> rhaas=# create database fudge;
> CREATE DATABASE
> rhaas=# \c fudge
> You are now connected to database "fudge" as user "rhaas".
> fudge=# create temp table q ();
> CREATE TABLE
> fudge=# ^Z
> [2]+  Stopped                 psql
> [rhaas Downloads]$ pg_ctl stop -mi
> waiting for server to shut down.... done
> server stopped
> [rhaas Downloads]$ %%
> psql
> \c
> You are now connected to database "fudge" as user "rhaas".
> fudge=# select * from pg_class where relpersistence='t';
>   oid  | relname | relnamespace | reltype | reloftype | relowner |
> relam | relfilenode | reltablespace | relpages | reltuples |
> relallvisible | reltoastrelid | relhasindex | relisshared |
> relpersistence | relkind | relnatts | relchecks | relhasrules |
> relhastriggers | relhassubclass | relrowsecurity | relforcerowsecurity
> | relispopulated | relreplident | relispartition | relrewrite |
> relfrozenxid | relminmxid | relacl | reloptions | relpartbound
>
-------+---------+--------------+---------+-----------+----------+-------+-------------+---------------+----------+-----------+---------------+---------------+-------------+-------------+----------------+---------+----------+-----------+-------------+----------------+----------------+----------------+---------------------+----------------+--------------+----------------+------------+--------------+------------+--------+------------+--------------
>  16388 | q       |        16386 |   16390 |         0 |       10 |
> 2 |       16388 |             0 |        0 |        -1 |             0
> |             0 | f           | f           | t              | r
> |        0 |         0 | f           | f              | f
> | f              | f                   | t              | d
> | f              |          0 |          721 |          1 |        |
>          |
> (1 row)
>
> fudge=# \c rhaas
> You are now connected to database "rhaas" as user "rhaas".
> rhaas=# alter database fudge is_template true;
> ALTER DATABASE
> rhaas=# create database cookies template fudge;
> CREATE DATABASE
> rhaas=# \c cookies
> You are now connected to database "cookies" as user "rhaas".
> cookies=# select count(*) from pg_class where relpersistence='t';
>  count
> -------
>      1
> (1 row)

I think this is not a right example to show the problem, no?  Because
you are showing the pg_class entry and the pg_class is not a temp
relation so even if we avoid copying the temp relation pg_class will
be copied right? so you will still see this uncleaned temp relation
entry.  I could reproduce exactly the same issue without my patch as
well.

So I agree we need to avoid copying temp relations but with that the
above behavior will not change.  Am I missing something?

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com