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
