On Thu, Sep 12, 2019 at 11:43 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Sep 11, 2019 at 11:22:45PM +0800, Masahiko Sawada wrote:
> > Hmm it will be more consistent with other functions but I think we
> > would need to increase the pageinspect version to 1.8 and need the new
> > sql file to rename the function name. And it will be for PG12, not
> > PG13. If we have to do it someday I think it's better to do it in PG12
> > that the table AM has been introduced to. Anyway I've attached
> > separate patch for it.
>
> Like Alvaro, I would discard this one for now.
>
> > I've attached the updated patch that incorporated all comments. I kept
> > the function as superuser-restricted.
>
> But not this one. So committed.
>
I had a few comments as posted in the previous email which I think we
can address incrementally as the patch for those is produced.
However, one point which I am slightly worried is the last one in my
email. Are we happy with the name of the new parameter in the API
decode_combined? Because if we decide to change that then we need to
change the exposed API and I think in the ideal case we need to change
the version as well, but I might be wrong and maybe the parameter name
as committed is good enough in which case we should be good.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com