On Sun, Dec 16, 2018 at 05:47:16PM -0300, Alvaro Herrera wrote: > I don't follow. When a relfilenode is passed by callers, they indicate > that the storage has already been created. Contrariwise, when a > relation kind that *does* have storage but is not yet created, they > pass InvalidOid as relfilenode, and heap_create is in charge of creating > storage. Maybe I'm not quite seeing what problem you mean. Or I could > add a separate boolean, but that seems pointless.
I have been double-checking my thoughts on the matter, and one take is to allow the reuse of relfilenodes for indexes like in TryReuseIndex(). I did not recall that case. Sorry for the noise. > Another possible improvement is to remove the create_storage boolean Yes, this could go away as this is linked with relfilenode. I let it up to you if you want to remove it or keep it. I think that I would just remove it. > Added a test in sanity_check.sql that there's no relation with the > relkinds that aren't supposed to have storage. Without the code fix it > fails in current regression database, but in the failure result set > there isn't any relation of kinds 'p' or 'I', so this isn't a terribly > comprehensive test -- the query runs too early in the regression > sequence. I'm not sure it's worth bothering further. +-- check that relations without storage don't have relfilenode It could be an idea to add a comment mentioning that the set of relkinds in RELKIND_CAN_HAVE_STORAGE are linked with the relkinds of this query. -- Michael
signature.asc
Description: PGP signature