Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?

From: Farias de Oliveira <matheusfarias519(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: In Postgres 16 BETA, should the ParseNamespaceItem have the same index as it's RangeTableEntry?
Date: 2023-07-14 19:43:01
Message-ID: CANQ0oxdA8TZvc3Nn=7L5L9a2Yxd0aUOr78oqkns7R299EZb9Eg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)sss(dot)pgh(dot)pa(dot)us> escreveu:

> Farias de Oliveira <matheusfarias519(at)gmail(dot)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(at)entry=0x13b8f50, estate=estate(at)entry=0x138ce48)
> > at execUtils.c:1412
> > #2 0x0000000000677c30 in ExecGetUpdatedCols (relinfo=relinfo(at)entry=0x13b8f50,
> estate=estate(at)entry=0x138ce48)
> > at execUtils.c:1321
> > #3 0x0000000000677cd7 in ExecGetAllUpdatedCols (relinfo=relinfo(at)entry=0x13b8f50,
> estate=estate(at)entry=0x138ce48)
> > at execUtils.c:1362
> > #4 0x000000000066b9bf in ExecUpdateLockMode (estate=estate(at)entry=0x138ce48,
> relinfo=relinfo(at)entry=0x13b8f50) at execMain.c:2385
> > #5 0x00007f197fb19a8d in update_entity_tuple (resultRelInfo=<optimized
> out>, resultRelInfo(at)entry=0x13b8f50,
> > elemTupleSlot=elemTupleSlot(at)entry=0x13b9730, estate=estate(at)entry=0x138ce48,
> old_tuple=0x13bae80)
> > at src/backend/executor/cypher_set.c:120
> > #6 0x00007f197fb1a2ff in process_update_list (node=node(at)entry=0x138d0c8)
> at src/backend/executor/cypher_set.c:595
> > #7 0x00007f197fb1a348 in process_all_tuples (node=node(at)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
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2023-07-14 19:49:33 Re: CommandStatus from insert returning when using a portal.
Previous Message Nathan Bossart 2023-07-14 19:42:52 Re: add non-option reordering to in-tree getopt_long