On Sat, Nov 17, 2018 at 7:41 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2018-Nov-17, Amit Kapila wrote:
>
> > On Sat, Nov 17, 2018 at 4:46 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> > > Uh, ouch! Seems to me that this is a high-caliber foot-gun, and will
> > > cause nasty suprises when production stats data disappear inadvertently!
> >
> > What is the alternative in your mind?
>
> Well, as I see this interface, it is as if
> DELETE FROM ... WHERE queryid = <value>
> where passing a NULL value meant to delete *all* rows regardless of
> queryid, rather than only those where queryid is NULL.
>
If one want to understand in query form, it will be like:
DELETE FROM ... WHERE userid = <value> AND dbid = <value> AND queryid = <value>;
Now, if any of the value is NULL, we ignore it, so the case in
question would be:
DELETE FROM ... WHERE userid = <value> AND dbid = <value> OR queryid = NULL;
Now, I think it is quite possible that one can interpret it as you
have written above and then this can lead to some unintended behavior.
I think one thing we can do to avoid such cases is to make this
function strict and make the default values as InvalidOid or -1. So,
on NULL input, we don't do anything, but for -1 or an invalid value
for the parameter, we ignore only that parameter as we are doing now.
Do you think such an interface will address your concern?
> > AFAICS, we have below alternatives:
> >
> > a) Return an error for the NULL value of any argument?
> > b) Silently ignore if there is any NULL value argument and don't
> > remove any stats.
> > c) Just ignore the NULL value parameter and remove the stats based on
> > other parameters.
>
> > Currently, patch implements option (c), but we can do (a) or (b) as
> > well, however, that can make the API usage bit awkward.
>
> I think option d) is to have more functions (seven functions total), and
> have them all be strict (i.e. don't do anything if one argument is
> NULL). One function takes three arguments, and removes all entries
> matching the three. Other functions take two arguments, and remove
> everything matching those, ignoring the third (so behaving like the
> current NULL). Others take one argument.
>
> Maybe it doesn't make sense to provide all combinations, so it's less
> than seven. But having an interface that's inherently easy to misuse
> makes me a bit nervous.
>
Yeah, we can go in that direction if the one interface idea doesn't
work. IIRC, we have discussed having multiple interfaces for this API
and majority people seems to be in favor of single interface. Let us
first see if there is some reasonable way to provide this
functionality with a single interface, if not, then surely we can
explore multiple interface idea.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com