Re: Proposal: Automatic partition creation - Mailing list pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Jul 6, 2020 at 6:46 AM Anastasia Lubennikova
<a.lubennikova@postgrespro.ru> wrote:

I am going to implement this via SPI, which allow to simplify checks and
calculations. Do you see any pitfalls in this approach?

I don't really see why we need SPI here.

I would vote against any core facility that is implemented via SPI
queries.  It is just too darn hard to control the semantics completely in
the face of fun stuff like varying search_path.  Look at what a mess the
queries generated by the RI triggers are --- and they only have a very
small set of behaviors to worry about.  I'm still only about 95% confident
they don't have security issues, too.

If you're using SPI to try to look up appropriate operators, I think
the chances of being vulnerable to security problems are 100%.

Good to know, thank you for that. I had doubts about the internal usage of SPI,
but didn't know what exactly can go wrong.

I think the big problem here is identifying the operator to use. We
have no way of identifying the "plus" or "minus" operator associated
with a datatype; indeed, that constant doesn't exist.

We did indeed solve this in connection with window functions, cf
0a459cec9.  I may be misunderstanding what the problem is here,
but I think trying to reuse that infrastructure might help.

Do we need to introduce a new support function? Is there a reason why we can
not rely on '+' operator? I understand that the addition operator may lack or
be overloaded for some complex datatypes, but I haven't found any examples that
are useful for range partitioning. Both pg_pathman and pg_partman also use '+'
to generate bounds.

I explored the code a bit more and came up with this function, which is very
similar to generate_series_* functions, but it doesn't use SPI and looks for
the function that implements the '+' operator, instead of direct call:

// almost pseudocode

static Const *
generate_next_bound(Const *start, Const *interval)
{
    ObjectWithArgs *sum_oper_object = makeNode(ObjectWithArgs);

    sum_oper_object->type = OBJECT_OPERATOR;
    /* hardcode '+' operator for addition */
    sum_oper_object->objname = list_make1(makeString("+"));

    ltype = makeTypeNameFromOid(start->consttype, start->consttypmod);
    rtype = makeTypeNameFromOid(interval->consttype, interval->consttypmod);

    sum_oper_object->objargs = list_make2(ltype, rtype);

    sum_oper_oid = LookupOperWithArgs(sum_oper_object, false);
    oprcode = get_opcode(sum_oper_oid);
    fmgr_info(oprcode, &opproc);

    next_bound->constvalue = FunctionCall2(&opproc,
                             start->constvalue,
                             interval->constvalue);
}

Thoughts?

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company