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.
Usually we don't use curlies for single line if branches.
I have rebased the patch to v4 and removed the unnecessary braces.
As your comments, Options 2 is still used in patch v4.
Agree that diskquota extension may use more hooks in future.
Currently the behavior of diskquota extension is that we use smgr hooks to detect active tables and record them in the shared memory. Bgworkers of diskquota extension will read these active tables from shared memory and calculate the latest table size and sum them into the size of schema or role. If size of schema of role exceeds their quota limit, they will be put into a black list in shared memory. When a new query comes, ExecutorCheckPerms_hook will be used to check the black list the cancel the query if needed.
