CODEWITHSUNDEEP
cdatabasepostgresqlapache-age
rrrokhtar
rrrokhtar

Reputation: 1113

RTE (RangeTblEntry) Replacement with PNSI (ParseNamespaceItem) in make_edge_expr function

Currently I am working as a part of a team which is working on replacing the RTE from apache-age source code to PNSI so that the versions became or unified in terms of the code wise.

I have checked the code which is being used at Apache AGE for PostgreSQL v12 and it contains a function that I am targeting to change it like wise that one on v13.

That is the definition of the function at AGE_PG13

static Node *make_vertex_expr(cypher_parsestate *cpstate,
                              ParseNamespaceItem *pnsi,
                              char *label);

And that what is was on AGE_PG12

static Node *make_vertex_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
                              char *label);

We can notice here the change happens in the parameter *RangeTblEntry rte, and *ParseNamespaceItem pnsi That's my goal

I have updated all of the changes that is required within the functions.

For those who are not aware: PNSI is a container of RTE so that RTE is property of PNSI which make us feels it's okay if we have replaced every needed part which uses RTE to PNSI->RTE.

Here is the changes happened on AGE_PG12

changes @src/backend/parser/cypher_clause.c

@@ -4574,20 +4575,22 @@ static Expr *transform_cypher_node(cypher_parsestate *cpstate,
 
     label_range_var = makeRangeVar(schema_name, rel_name, -1);
     alias = makeAlias(node->name, NIL);
-
-    rte = addRangeTableEntry(pstate, label_range_var, alias,
+    pnsi = malloc(sizeof(ParseNamespaceItem));
+    pnsi->p_rte = addRangeTableEntry(pstate, label_range_var, alias,
                              label_range_var->inh, true);
+
+    Assert(pnsi != NULL);
     /*
      * relation is visible (r.a in expression works) but attributes in the
      * relation are not visible (a in expression doesn't work)
      */
-    addRTEtoQuery(pstate, rte, true, true, true);
+    addRTEtoQuery(pstate, pnsi->p_rte, true, true, true);
 
     resno = pstate->p_next_resno++;
 
     if (valid_label)
     {
-        expr = (Expr *)make_vertex_expr(cpstate, rte, node->label);
+        expr = (Expr *)make_vertex_expr(cpstate, pnsi, node->label);
     }
     else
     {

@@ -4650,7 +4653,7 @@ static Node *make_edge_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
 
     return (Node *)func_expr;
 }
-static Node *make_vertex_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
+static Node *make_vertex_expr(cypher_parsestate *cpstate, ParseNamespaceItem *pnsi,
                               char *label)
 {

@@ -4666,7 +4669,7 @@ static Node *make_vertex_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
     func_oid = get_ag_func_oid("_agtype_build_vertex", 3, GRAPHIDOID,
                                CSTRINGOID, AGTYPEOID);
 
-    id = scanRTEForColumn(pstate, rte, AG_VERTEX_COLNAME_ID, -1, 0, NULL);
+    id = scanRTEForColumn(pstate, pnsi->p_rte, AG_VERTEX_COLNAME_ID, -1, 0, NULL);

@@ -4682,9 +4685,8 @@ static Node *make_vertex_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
                                         InvalidOid, COERCE_EXPLICIT_CALL);
     label_name_func_expr->location = -1;
 
-    props = scanRTEForColumn(pstate, rte, AG_VERTEX_COLNAME_PROPERTIES, -1, 0,
+    props = scanRTEForColumn(pstate, pnsi->p_rte, AG_VERTEX_COLNAME_PROPERTIES, -1, 0,
                              NULL);

The questions comes here; I am seeing that is a waste of memory because for every single instance where RTE was being used I will need to replace it with an allocated memory with size of PNSI (which is larger) to be able to use RTE within that PNSI? Is there any another way efficient that I can follow to achieve my goal?

References

Upvotes: 0

Views: 61

Answers (3)

adil shahid
adil shahid

Reputation: 29

Yes, you are right that PNSI requires more memory overhead than RTE but you can make use of pointers with PNSI to minimize the memory allocation.

While using pointers, you just need to refer to PNSI through a pointer for every instance. With this technique, you can point and use PNSI wherever you need for instances.

Upvotes: 0

rrrokhtar
rrrokhtar

Reputation: 1113

Regarding this we have decided not to move forward with this replacement as it will be out of context. As AGE of Postgresql 12 depends on the Postgresql 12 api and the supported functions which are included on Postgresql 13 are not fully supported in version 12 so that the replacement that happened on v13 is not easily can be done on v12 because there are some functions are newly added on v13 those needed for v12 to make this replacement successfully

Upvotes: 0

Laurenz Albe
Laurenz Albe

Reputation: 247800

You don't malloc() in a PostgreSQL function. PostgreSQL has built its own memory management system on top of C. So you should palloc() the memory in the appropriate memory context, and it will be freed automatically at the appropriate time (when the statement or the transaction ends).

If using a ParseNamespaceItem is a good idea or not is a question you have to discuss with the people who wrote Apache AGE.

Upvotes: 2

Related Questions