MIME-Version: 1.0
References: 
 <CAB0yre=RP_ho6Bq4cV23ELKxRcfhV2Yqrb1zHp0RfUPEWCnBRw@mail.gmail.com>
 <eface0f387a6f7530862ef40e3e3e55ac915ea83.camel@2ndquadrant.com>
 <CAB0yrenHV2iw4hQEnHGkTTpXZN8jMUJNtsCV4=BmWckjnn16Mg@mail.gmail.com>
 <CAL_NLp+jLqn9P+hzy1vYSqoUuCsJ4AtTF4QYH5=950rB9S3dSw@mail.gmail.com>
 <CA+Tgmoa9m74nhU8LpPfsiW-WS2MvWkipCEn4VW40kFrMMkbhKw@mail.gmail.com>
 <20181226055647.GK2234@paquier.xyz>
 <CAL_NLpKMknkagF0WzgjeJAjAL=ZosoNifbnqo7YgbtcYgSBepA@mail.gmail.com>
 <CAB0yrek4pbh-x7vYmKh70YRfbXhd7k1Ceqwhyy35jUYdYRrXAg@mail.gmail.com>
 <CAB0yre=PbBY-obEFqmkLpSvL=VRx_D3DPsnBn7kAKOah8Z-aVg@mail.gmail.com>
 <20190216045351.q344ex2utwk7ndtr@alap3.anarazel.de>
 <CAB0yrekg=fMNpf6jXoxd3iGDCdf4JpRqqX2g9POUpm9g1PYbVQ@mail.gmail.com>
In-Reply-To: 
 <CAB0yrekg=fMNpf6jXoxd3iGDCdf4JpRqqX2g9POUpm9g1PYbVQ@mail.gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 8 Jul 2019 10:47:06 +1200
Message-ID: 
 <CA+hUKGKCNSCj0XW3pUF8Yd75GhXmD0=UNcZw07Ntsht+XE0_-g@mail.gmail.com>
Subject: Re: Control your disk usage in PG: Introduction to Disk Quota
 Extension
To: Hubert Zhang <hzhang@pivotal.io>
Cc: Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Heikki Linnakangas <hlinnakangas@pivotal.io>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 18, 2019 at 7:39 PM Hubert Zhang <hzhang@pivotal.io> wrote:
> Based on the assumption we use smgr as hook position, hook API option1 or option2 which is better?
> Or we could find some balanced API between option1 and option2?
>
> Again comments on other better hook positions are also appreciated!

Hi Hubert,

The July Commitfest is now running, and this entry is in "needs
review" state.  Could you please post a rebased patch?

I have questions about how disk quotas should work and I think we'll
probably eventually want more hooks than these, but simply adding
these hooks so extensions can do whatever they want doesn't seem very
risky for core.  I think it's highly likely that the hook signatures
will have to change in future releases too, but that seems OK for such
detailed internal hooks.  As for your question, my first reaction was
that I preferred your option 1, because SMgrRelation seems quite
private and there are no existing examples of that object being
exposed to extensions.  But on reflection, other callbacks don't take
such a mollycoddling approach.  So my vote is for option 2 "just pass
all the arguments to the callback", which I understand to be the
approach of patch you have posted.

+    if (smgrcreate_hook)
+    {
+        (*smgrcreate_hook)(reln, forknum, isRedo);
+    }

Usually we don't use curlies for single line if branches.

-- 
Thomas Munro
https://enterprisedb.com