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