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

On Fri, Mar 11, 2022 at 11:51 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Fri, Mar 11, 2022 at 1:10 PM Robert Haas <robertmhaas(at)gmail(dot)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