Postgres Pro
Downloads
Home   >   mailing lists

Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry? - Mailing list pgsql-hackers

From Farias de Oliveira
Subject Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?
Date
Msg-id CANQ0oxdA8TZvc3Nn=7L5L9a2Yxd0aUOr78oqkns7R299EZb9Eg@mail.gmail.com
Whole thread Raw
In response to Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?
List pgsql-hackers
Tree view
I believe I have found something interesting that might be the root of the problem with RTEPermissionInfo. But I do not know how to fix it exactly. In AGE's code, the execution of it goes through a function called analyze_cypher_clause() which does the following:

static Query *analyze_cypher_clause(transform_method transform,
                                    cypher_clause *clause,
                                    cypher_parsestate *parent_cpstate)
{
    cypher_parsestate *cpstate;
    Query *query;
    ParseState *parent_pstate = (ParseState*)parent_cpstate;
    ParseState *pstate;

    cpstate = make_cypher_parsestate(parent_cpstate);
    pstate = (ParseState*)cpstate;

    /* copy the expr_kind down to the child */
    pstate->p_expr_kind = parent_pstate->p_expr_kind;

    query = transform(cpstate, clause);

    advance_transform_entities_to_next_clause(cpstate->entities);

    parent_cpstate->entities = list_concat(parent_cpstate->entities,
                                           cpstate->entities);

    free_cypher_parsestate(cpstate);

    return query;
}

the free_cypher_parsestate() function calls the free_parsestate() function:

void free_cypher_parsestate(cypher_parsestate *cpstate)
{
    free_parsestate((ParseState *)cpstate);
}

So, before that happens the cpstate struct contains the following data:

{pstate = {parentParseState = 0x2b06ab0, p_sourcetext = 0x2b06ef0 "MATCH (n) SET n.i = 3", p_rtable = 0x2bdb370,
    p_rteperminfos = 0x2bdb320, p_joinexprs = 0x0, p_nullingrels = 0x0, p_joinlist = 0x2bdb478, p_namespace = 0x2bdb4c8,
    p_lateral_active = false, p_ctenamespace = 0x0, p_future_ctes = 0x0, p_parent_cte = 0x0, p_target_relation = 0x0,
    p_target_nsitem = 0x0, p_is_insert = false, p_windowdefs = 0x0, p_expr_kind = EXPR_KIND_FROM_SUBSELECT, p_next_resno = 2,
    p_multiassign_exprs = 0x0, p_locking_clause = 0x0, p_locked_from_parent = false, p_resolve_unknowns = true,
    p_queryEnv = 0x0, p_hasAggs = false, p_hasWindowFuncs = false, p_hasTargetSRFs = false, p_hasSubLinks = false,
    p_hasModifyingCTE = false, p_last_srf = 0x0, p_pre_columnref_hook = 0x0, p_post_columnref_hook = 0x0,
    p_paramref_hook = 0x0, p_coerce_param_hook = 0x0, p_ref_hook_state = 0x0}, graph_name = 0x2b06e50 "cypher_set",
  graph_oid = 16942, params = 0x0, default_alias_num = 0, entities = 0x2c6e158, property_constraint_quals = 0x0,
  exprHasAgg = false, p_opt_match = false}

And then after that the pstate gets all wiped out:

{pstate = {parentParseState = 0x0, p_sourcetext = 0x2b06ef0 "MATCH (n) SET n.i = 3", p_rtable = 0x0,
    p_rteperminfos = 0x0, p_joinexprs = 0x0, p_nullingrels = 0x0, p_joinlist = 0x0, p_namespace = 0x0,
    p_lateral_active = false, p_ctenamespace = 0x0, p_future_ctes = 0x0, p_parent_cte = 0x0, p_target_relation = 0x0,
    p_target_nsitem = 0x0, p_is_insert = false, p_windowdefs = 0x0, p_expr_kind = EXPR_KIND_FROM_SUBSELECT, p_next_resno = 1,
    p_multiassign_exprs = 0x0, p_locking_clause = 0x0, p_locked_from_parent = false, p_resolve_unknowns = true,
    p_queryEnv = 0x0, p_hasAggs = false, p_hasWindowFuncs = false, p_hasTargetSRFs = false, p_hasSubLinks = false,
    p_hasModifyingCTE = false, p_last_srf = 0x0, p_pre_columnref_hook = 0x0, p_post_columnref_hook = 0x0,
    p_paramref_hook = 0x0, p_coerce_param_hook = 0x0, p_ref_hook_state = 0x0}, graph_name = 0x2b06e50 "cypher_set",
  graph_oid = 16942, params = 0x0, default_alias_num = 0, entities = 0x2c6e228, property_constraint_quals = 0x0,
  exprHasAgg = false, p_opt_match = false}

But in transform_cypher_clause_as_subquery(), we use the same pstate. And when we assign 

pnsi = addRangeTableEntryForSubquery(pstate, query, alias, lateral, true);

The pstate changes, adding a value to p_rtable but nothing in p_rteperminfos.

Then after that addNSItemToQuery(pstate, pnsi, true, false, true) is called, changing the pstate to add values to p_joinlist and p_namespace.

It ends up going inside other functions and changing it more a bit, but at the end of one of these functions it assigns values to some members of the query:

query->targetList = lappend(query->targetList, tle);
query->rtable = pstate->p_rtable;
query->jointree = makeFromExpr(pstate->p_joinlist, NULL);

I assume that here is missing the assignment of query->rteperminfos to be the same as pstate->p_rteperminfos, but the pstate has the following values:

{pstate = {parentParseState = 0x0, p_sourcetext = 0x2b06ef0 "MATCH (n) SET n.i = 3", p_rtable = 0x2c6e590,
    p_rteperminfos = 0x0, p_joinexprs = 0x0, p_nullingrels = 0x0, p_joinlist = 0x2c6e678, p_namespace = 0x2c6e6c8,
    p_lateral_active = false, p_ctenamespace = 0x0, p_future_ctes = 0x0, p_parent_cte = 0x0, p_target_relation = 0x0,
    p_target_nsitem = 0x0, p_is_insert = false, p_windowdefs = 0x0, p_expr_kind = EXPR_KIND_NONE, p_next_resno = 3,
    p_multiassign_exprs = 0x0, p_locking_clause = 0x0, p_locked_from_parent = false, p_resolve_unknowns = true,
    p_queryEnv = 0x0, p_hasAggs = false, p_hasWindowFuncs = false, p_hasTargetSRFs = false, p_hasSubLinks = false,
    p_hasModifyingCTE = false, p_last_srf = 0x0, p_pre_columnref_hook = 0x0, p_post_columnref_hook = 0x0,
    p_paramref_hook = 0x0, p_coerce_param_hook = 0x0, p_ref_hook_state = 0x0}, graph_name = 0x2b06e50 "cypher_set",
  graph_oid = 16942, params = 0x0, default_alias_num = 0, entities = 0x2c6e228, property_constraint_quals = 0x0,
  exprHasAgg = false, p_opt_match = false}

So changing that won't solve the issue.

Em sex., 14 de jul. de 2023 às 12:16, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Farias de Oliveira <matheusfarias519@gmail.com> writes:
> 3905                  elog(ERROR, "invalid perminfoindex %u in RTE with relid %u",
> (gdb) bt
> #0  getRTEPermissionInfo (rteperminfos=0x138a500, rte=0x138a6b0) at parse_relation.c:3905
> #1  0x0000000000676e29 in GetResultRTEPermissionInfo (relinfo=relinfo@entry=0x13b8f50, estate=estate@entry=0x138ce48)
>     at execUtils.c:1412
> #2  0x0000000000677c30 in ExecGetUpdatedCols (relinfo=relinfo@entry=0x13b8f50, estate=estate@entry=0x138ce48)
>     at execUtils.c:1321
> #3  0x0000000000677cd7 in ExecGetAllUpdatedCols (relinfo=relinfo@entry=0x13b8f50, estate=estate@entry=0x138ce48)
>     at execUtils.c:1362
> #4  0x000000000066b9bf in ExecUpdateLockMode (estate=estate@entry=0x138ce48, relinfo=relinfo@entry=0x13b8f50) at execMain.c:2385
> #5  0x00007f197fb19a8d in update_entity_tuple (resultRelInfo=<optimized out>, resultRelInfo@entry=0x13b8f50,
>     elemTupleSlot=elemTupleSlot@entry=0x13b9730, estate=estate@entry=0x138ce48, old_tuple=0x13bae80)
>     at src/backend/executor/cypher_set.c:120
> #6  0x00007f197fb1a2ff in process_update_list (node=node@entry=0x138d0c8) at src/backend/executor/cypher_set.c:595
> #7  0x00007f197fb1a348 in process_all_tuples (node=node@entry=0x138d0c8) at src/backend/executor/cypher_set.c:212
> #8  0x00007f197fb1a455 in exec_cypher_set (node=0x138d0c8) at src/backend/executor/cypher_set.c:641

So apparently, what we have here is a result-relation RTE that has
no permissions info associated.  It's not clear to me whether that
is a bug in building the parse tree, or an expectable situation
that GetResultRTEPermissionInfo ought to be coping with.  I'm
inclined to bet on the former though, and to guess that AGE is
missing dealing with the new RTEPermissionInfo structs in someplace
or other.  I'm afraid that allowing GetResultRTEPermissionInfo to
let this pass without comment would mask actual bugs, so fixing
it at that end doesn't seem attractive.

                        regards, tom lane

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: add non-option reordering to in-tree getopt_long
Next
From: Dave Cramer
Date:
Subject: Re: CommandStatus from insert returning when using a portal.