Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly

On 2020-12-23 08:22, Justin Pryzby wrote:
> On Tue, Dec 22, 2020 at 03:22:19PM -0800, Zhihong Yu wrote:
>> Justin:
>> For reindex_index() :
>> 
>> +   if (options->tablespaceOid == MyDatabaseTableSpace)
>> +       options->tablespaceOid = InvalidOid;
>> ...
>> +   oldTablespaceOid = iRel->rd_rel->reltablespace;
>> +   if (set_tablespace &&
>> +       (options->tablespaceOid != oldTablespaceOid ||
>> +       (options->tablespaceOid == MyDatabaseTableSpace &&
>> OidIsValid(oldTablespaceOid))))
>> 
>> I wonder why the options->tablespaceOid == MyDatabaseTableSpace clause
>> appears again in the second if statement.
>> Since the first if statement would assign InvalidOid
>> to options->tablespaceOid when the first if condition is satisfied.
> 
> Good question.  Alexey mentioned on Sept 23 that he added the first 
> stanza.  to
> avoid storing the DB's tablespace OID (rather than InvalidOid).
> 
> I think the 2nd half of the "or" is unnecessary since that was added 
> setting to
> options->tablespaceOid = InvalidOid.
> If requesting to move to the DB's default tablespace, it'll now hit the 
> first
> part of the OR:
> 
>> +       (options->tablespaceOid != oldTablespaceOid ||
> 
> Without the first stanza setting, it would've hit the 2nd condition:
> 
>> +       (options->tablespaceOid == MyDatabaseTableSpace && 
>> OidIsValid(oldTablespaceOid))))
> 
> which means: "user requested to move a table to the DB's default 
> tblspace, and
> it was previously on a nondefault space".
> 
> So I think we can drop the 2nd half of the OR.  Thanks for noticing.

Yes, I have not noticed that we would have already assigned 
tablespaceOid to InvalidOid in this case. Back to the v7 we were doing 
this assignment a bit later, so this could make sense, but now it seems 
to be redundant. For some reason I have mixed these refactorings 
separated by a dozen of versions...


Thanks
-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company