On 06.07.2020 19:10, Tom Lane wrote:
Robert Haas <robertmh...@gmail.com> writes:
On Mon, Jul 6, 2020 at 6:46 AM Anastasia Lubennikova
<a.lubennik...@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

Reply via email to