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
signature.asc
Description: PGP signature