From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
Cc: | Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Steve Singer <steve(at)ssinger(dot)info>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Jose Luis Tallon <jltallon(at)adv-solutions(dot)net> |
Subject: | Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly |
Date: | 2021-01-21 01:41:46 |
Message-ID: | YAjb2kYBKH07H5Xv@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jan 20, 2021 at 03:34:39PM -0300, Alvaro Herrera wrote:
> On 2021-Jan-20, Alexey Kondratov wrote:
>> Ugh, forgot to attach the patches. Here they are.
>
> Yeah, looks reasonable.
Patch 0002 still has a whole set of issues as I pointed out a couple
of hours ago, but if we agree on 0001 as being useful if done
independently, I'd rather get that done first. This way or just
merging both things in a single commit is not a big deal seeing the
amount of code, so I am fine with any approach. It may be possible
that 0001 requires more changes depending on the work to-be-done for
0002 though?
>> + /* No work if no change in tablespace. */
>> + oldTablespaceOid = rd_rel->reltablespace;
>> + if (tablespaceOid != oldTablespaceOid ||
>> + (tablespaceOid == MyDatabaseTableSpace && OidIsValid(oldTablespaceOid)))
>> + {
>> + /* Update the pg_class row. */
>> + rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
>> + InvalidOid : tablespaceOid;
>> + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
>> +
>> + changed = true;
>> + }
>> +
>> + if (changed)
>> + /* Record dependency on tablespace */
>> + changeDependencyOnTablespace(RelationRelationId,
>> + reloid, rd_rel->reltablespace);
>
> Why have a separate "if (changed)" block here instead of merging with
> the above?
Yep.
+ if (SetRelTablespace(reloid, newTableSpace))
+ /* Make sure the reltablespace change is visible */
+ CommandCounterIncrement();
At quick glance, I am wondering why you just don't do a CCI within
SetRelTablespace().
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2021-01-21 01:42:03 | Re: POC: postgres_fdw insert batching |
Previous Message | tsunakawa.takay@fujitsu.com | 2021-01-21 01:37:35 | RE: POC: postgres_fdw insert batching |