Re: why doesn't DestroyPartitionDirectory hash_destroy?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: why doesn't DestroyPartitionDirectory hash_destroy?
Date: 2019-03-14 17:40:50
Message-ID: 6099.1552585250@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Mar 14, 2019 at 12:56 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> It's completely not acceptable that every reader of this code should
>> have to reverse-engineer these design assumptions, especially given
>> how shaky they are.

> Well, one reason is that everything you just said is basically
> self-evident. If you spend 5 seconds looking at the header file,
> you'll see that there is a CreatePartitionDirectory() function and a
> DestroyPartitionDirectory() function, and so you'll probably figure
> out that the latter is intended to be called rather than just ignored.
> You will probably also guess that it doesn't need to be called if
> there's an ERROR, just like basically everything else in PostgreSQL.
> And if you want to know why, you can look at the code and you
> shouldn't have any trouble determining that it releases relcache ref
> counts, which may tip you off that if you don't call it, some relcache
> refcounts will not be released.

So here's my problem with that argument: you're effectively saying that
you needn't write any API spec for the PartitionDirectory functions
because you intend that every person calling them will read their code,
carefully and fully, before using them. This is not my idea of sound
software engineering. If you need me to spell out why not, I will do
so, but I'd like to think that I needn't explain abstraction to a senior
committer.

But anyway, it's somewhat moot because I think we need to change this
API spec anyhow, per the other thread. PartitionDirectory should not
be holding on to relcache refcounts, which would make
DestroyPartitionDirectory unnecessary.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2019-03-14 17:56:50 Re: why doesn't DestroyPartitionDirectory hash_destroy?
Previous Message Alvaro Herrera 2019-03-14 17:31:42 Re: partitioned tables referenced by FKs