Merge branch 'master' into PG16_to_master

Resolved conflicts:
    src/backend/catalog/ag_catalog.c
    src/backend/catalog/ag_label.c
    src/backend/executor/cypher_utils.c
    src/backend/nodes/cypher_readfuncs.c
    src/backend/parser/cypher_expr.c
    src/backend/parser/cypher_item.c
    src/backend/parser/cypher_parse_agg.c
    src/backend/utils/adt/agtype.c

Author: jrgemignani

Diff for: src/backend/catalog/ag_catalog.c

@@ -86,26 +86,29 @@ void process_utility_hook_fini(void)
  * from being thrown, we need to disable the object_access_hook before dropping
  * the extension.
  */
-void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char *queryString, bool readOnlyTree,
-                             ProcessUtilityContext context, ParamListInfo params,
-                             QueryEnvironment *queryEnv, DestReceiver *dest,
-                             QueryCompletion *qc)
+void ag_ProcessUtility_hook(PlannedStmt *pstmt, const char *queryString,
+                            bool readOnlyTree, ProcessUtilityContext context,
+                            ParamListInfo params, QueryEnvironment *queryEnv,
+                            DestReceiver *dest, QueryCompletion *qc)
 {
     if (is_age_drop(pstmt))
+    {
         drop_age_extension((DropStmt *)pstmt->utilityStmt);
+    }
     else if (prev_process_utility_hook)
-        (*prev_process_utility_hook) (pstmt, queryString, readOnlyTree, context, params,
-                                      queryEnv, dest, qc);
+    {
+        (*prev_process_utility_hook) (pstmt, queryString, readOnlyTree, context,
+                                      params, queryEnv, dest, qc);
+    }
     else
     {
         Assert(IsA(pstmt, PlannedStmt));
         Assert(pstmt->commandType == CMD_UTILITY);
         Assert(queryString != NULL);	/* required as of 8.4 */
         Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN);
-        standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv,
-                                dest, qc);
+        standard_ProcessUtility(pstmt, queryString, readOnlyTree, context,
+                                params, queryEnv, dest, qc);
     }
-        
 }
 
 static void drop_age_extension(DropStmt *stmt)

Diff for: src/backend/catalog/ag_label.c

@@ -188,9 +188,10 @@ Datum _label_name(PG_FUNCTION_ARGS)
     uint32 label_id;
 
     if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
-        PG_RETURN_NULL(); 
-        //ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-        //                errmsg("graph_oid and label_id must not be null")));
+    {
+        ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+                        errmsg("graph_oid and label_id must not be null")));
+    }
 
     graph = PG_GETARG_OID(0);
 

Diff for: src/backend/executor/cypher_set.c

@@ -485,7 +485,8 @@ static void process_update_list(CustomScanState *node)
         }
 
         // Alter the properties Agtype value.
-        if (strcmp(update_item->prop_name, ""))
+        if (update_item->prop_name != NULL &&
+            strcmp(update_item->prop_name, "") != 0)
         {
             altered_properties = alter_property_value(original_properties,
                                                       update_item->prop_name,

Diff for: src/backend/executor/cypher_utils.c

@@ -51,16 +51,20 @@
 
 /*
  * Given the graph name and the label name, create a ResultRelInfo for the table
- * those to variables represent. Open the Indices too.
+ * those two variables represent. Open the Indices too.
  */
 ResultRelInfo *create_entity_result_rel_info(EState *estate, char *graph_name,
                                              char *label_name)
 {
-    RangeVar *rv;
-    Relation label_relation;
-    ResultRelInfo *resultRelInfo;
+    RangeVar *rv = NULL;
+    Relation label_relation = NULL;
+    ResultRelInfo *resultRelInfo = NULL;
+    ParseState *pstate = NULL;
+    RangeTblEntry *rte = NULL;
+    int pii = 0;
 
-    ParseState *pstate = make_parsestate(NULL);
+    /* create a new parse state for this operation */
+    pstate = make_parsestate(NULL);
 
     resultRelInfo = palloc(sizeof(ResultRelInfo));
 
@@ -75,12 +79,33 @@ ResultRelInfo *create_entity_result_rel_info(EState *estate, char *graph_name,
 
     label_relation = parserOpenTable(pstate, rv, RowExclusiveLock);
 
-    // initialize the resultRelInfo
-    InitResultRelInfo(resultRelInfo, label_relation,
-                      0, NULL,
+    /*
+     * Get the rte to determine the correct perminfoindex value. Some rtes
+     * may have it set up, some created here (executor) may not.
+     *
+     * Note: The RTEPermissionInfo structure was added in PostgreSQL version 16.
+     *
+     * Note: We use the list_length because exec_rt_fetch starts at 1, not 0.
+     *       Doing this gives us the last rte in the es_range_table list, which
+     *       is the rte in question.
+     *
+     *       If the rte is created here and doesn't have a perminfoindex, we
+     *       need to pass on a 0. Otherwise, later on GetResultRTEPermissionInfo
+     *       will attempt to get the rte's RTEPermissionInfo data, which doesn't
+     *       exist.
+     *
+     * TODO: Ideally, we should consider creating the RTEPermissionInfo data,
+     *       but as this is just a read of the label relation, it is likely
+     *       unnecessary.
+     */
+    rte = exec_rt_fetch(list_length(estate->es_range_table), estate);
+    pii = (rte->perminfoindex == 0) ? 0 : list_length(estate->es_range_table);
+
+    /* initialize the resultRelInfo */
+    InitResultRelInfo(resultRelInfo, label_relation, pii, NULL,
                       estate->es_instrument);
 
-    // open the parse state
+    /* open the indices */
     ExecOpenIndices(resultRelInfo, false);
 
     free_parsestate(pstate);

Diff for: src/backend/nodes/cypher_readfuncs.c

@@ -24,6 +24,7 @@
 #include "nodes/cypher_readfuncs.h"
 #include "nodes/cypher_nodes.h"
 
+static char *nullable_string(const char *token, int length);
 /*
  * Copied From Postgres
  *
@@ -111,7 +112,7 @@
 #define READ_STRING_FIELD(fldname) \
         token = pg_strtok(&length); \
         token = pg_strtok(&length); \
-        local_node->fldname = non_nullable_string(token, length)
+        local_node->fldname = nullable_string(token, length)
 
 // Read a parse location field (and throw away the value, per notes above)
 #define READ_LOCATION_FIELD(fldname) \
@@ -162,11 +163,22 @@
 
 #define strtobool(x)  ((*(x) == 't') ? true : false)
 
-#define nullable_string(token,length)  \
-        ((length) == 0 ? NULL : debackslash(token, length))
-
-#define non_nullable_string(token,length)  \
-        ((length == 2 && token[0] == '"' && token[1] == '"') ? "" : debackslash(token, length))
+/* copied from PG16 function of the same name for consistency */
+static char *nullable_string(const char *token, int length)
+{
+    /* outToken emits <> for NULL, and pg_strtok makes that an empty string */
+    if (length == 0)
+    {
+        return NULL;
+    }
+    /* outToken emits "" for empty string */
+    if (length == 2 && token[0] == '"' && token[1] == '"')
+    {
+        return pstrdup("");
+    }
+    /* otherwise, we must remove protective backslashes added by outToken */
+    return debackslash(token, length);
+}
 
 /*
  * Default read function for cypher nodes. For most nodes, we don't expect

Diff for: src/backend/parser/cypher_clause.c

@@ -45,6 +45,7 @@
 #include "parser/parse_relation.h"
 #include "parser/parse_target.h"
 #include "parser/parsetree.h"
+#include "parser/parse_relation.h"
 #include "rewrite/rewriteHandler.h"
 #include "utils/typcache.h"
 #include "utils/lsyscache.h"
@@ -331,6 +332,8 @@ static List *make_target_list_from_join(ParseState *pstate,
                                         RangeTblEntry *rte);
 static FuncExpr *make_clause_func_expr(char *function_name,
                                        Node *clause_information);
+static void markRelsAsNulledBy(ParseState *pstate, Node *n, int jindex);
+
 /* for VLE support */
 static ParseNamespaceItem *transform_RangeFunction(cypher_parsestate *cpstate,
                                                    RangeFunction *r);
@@ -2472,8 +2475,17 @@ static void get_res_cols(ParseState *pstate, ParseNamespaceItem *l_pnsi,
 
         if (var == NULL)
         {
+            Var *v;
+
+            /*
+             * Each join (left) RTE's Var, that references a column of the
+             * right RTE, needs to be marked 'nullable'.
+             */
+            v = lfirst(r_lvar);
+            markNullableIfNeeded(pstate, v);
+
             colnames = lappend(colnames, lfirst(r_lname));
-            colvars = lappend(colvars, lfirst(r_lvar));
+            colvars = lappend(colvars, v);
         }
     }
 
@@ -2522,6 +2534,13 @@ static RangeTblEntry *transform_cypher_optional_match_clause(cypher_parsestate *
     j->rarg = transform_clause_for_join(cpstate, clause, &r_rte,
                                         &r_nsitem, r_alias);
 
+    /*
+     * Since this is a left join, we need to mark j->rarg as it may potentially
+     * emit NULL. The jindex argument holds rtindex of the join's RTE, which is
+     * created right after j->arg's RTE in this case.
+     */
+    markRelsAsNulledBy(pstate, j->rarg, r_nsitem->p_rtindex + 1);
+
     // we are done transform the lateral left join
     pstate->p_lateral_active = false;
 
@@ -6377,6 +6396,13 @@ transform_merge_make_lateral_join(cypher_parsestate *cpstate, Query *query,
     j->rarg = transform_clause_for_join(cpstate, isolated_merge_clause, &r_rte,
                                             &r_nsitem, r_alias);
 
+    /*
+     * Since this is a left join, we need to mark j->rarg as it may potentially
+     * emit NULL. The jindex argument holds rtindex of the join's RTE, which is
+     * created right after j->arg's RTE in this case.
+     */
+    markRelsAsNulledBy(pstate, j->rarg, r_nsitem->p_rtindex + 1);
+
     // deactivate the lateral flag
     pstate->p_lateral_active = false;
 
@@ -7095,6 +7121,50 @@ static FuncExpr *make_clause_func_expr(char *function_name,
     return func_expr;
 }
 
+/*
+ * This function is borrowed from PG version 16.1.
+ *
+ * It is used in transformations involving left join in Optional Match and
+ * Merge in a similar way PG16's transformFromClauseItem() uses it.
+ */
+static void markRelsAsNulledBy(ParseState *pstate, Node *n, int jindex)
+{
+    int varno;
+    ListCell *lc;
+
+    /* Note: we can't see FromExpr here */
+    if (IsA(n, RangeTblRef))
+    {
+        varno = ((RangeTblRef *) n)->rtindex;
+    }
+    else if (IsA(n, JoinExpr))
+    {
+        JoinExpr   *j = (JoinExpr *) n;
+
+        /* recurse to children */
+        markRelsAsNulledBy(pstate, j->larg, jindex);
+        markRelsAsNulledBy(pstate, j->rarg, jindex);
+        varno = j->rtindex;
+    }
+    else
+    {
+        elog(ERROR, "unrecognized node type: %d", (int) nodeTag(n));
+        varno = 0;				/* keep compiler quiet */
+    }
+
+    /*
+     * Now add jindex to the p_nullingrels set for relation varno.  Since we
+     * maintain the p_nullingrels list lazily, we might need to extend it to
+     * make the varno'th entry exist.
+     */
+    while (list_length(pstate->p_nullingrels) < varno)
+    {
+        pstate->p_nullingrels = lappend(pstate->p_nullingrels, NULL);
+    }
+    lc = list_nth_cell(pstate->p_nullingrels, varno - 1);
+    lfirst(lc) = bms_add_member((Bitmapset *) lfirst(lc), jindex);
+}
+
 /*
  * Utility function that helps a clause add the information needed to
  * the query from the previous clause.

Diff for: src/backend/parser/cypher_expr.c

@@ -249,7 +249,8 @@ static Node *transform_A_Const(cypher_parsestate *cpstate, A_Const *ac)
             }
             else
             {
-                float8 f = float8in_internal(n, NULL, "double precision", n, NULL);
+                float8 f = float8in_internal(n, NULL, "double precision", n,
+                                             NULL);
 
                 d = float_to_agtype(f);
             }

Diff for: src/backend/parser/cypher_item.c

@@ -40,7 +40,8 @@
 
 static List *ExpandAllTables(ParseState *pstate, int location);
 static List *expand_pnsi_attrs(ParseState *pstate, ParseNamespaceItem *pnsi,
-			       int sublevels_up, bool require_col_privs, int location);
+			       int sublevels_up, bool require_col_privs,
+                               int location);
 
 // see transformTargetEntry()
 TargetEntry *transform_cypher_item(cypher_parsestate *cpstate, Node *node,
@@ -161,10 +162,8 @@ static List *ExpandAllTables(ParseState *pstate, int location)
         /* Remember we found a p_cols_visible item */
         found_table = true;
 
-        target = list_concat(target, expand_pnsi_attrs(pstate,
-                                                      nsitem,
-                                                      0,
-                                                      true, location));
+        target = list_concat(target, expand_pnsi_attrs(pstate, nsitem, 0, true,
+                                                       location));
     }
 
     /* Check for "RETURN *;" */
@@ -181,7 +180,8 @@ static List *ExpandAllTables(ParseState *pstate, int location)
  * Modified to exclude hidden variables and aliases in RETURN *
  */
 static List *expand_pnsi_attrs(ParseState *pstate, ParseNamespaceItem *pnsi,
-			       int sublevels_up, bool require_col_privs, int location)
+			       int sublevels_up, bool require_col_privs,
+                               int location)
 {
     RangeTblEntry *rte = pnsi->p_rte;
     RTEPermissionInfo *perminfo = pnsi->p_perminfo;
@@ -190,7 +190,7 @@ static List *expand_pnsi_attrs(ParseState *pstate, ParseNamespaceItem *pnsi,
     List *te_list = NIL;
     int var_prefix_len = strlen(AGE_DEFAULT_VARNAME_PREFIX);
     int alias_prefix_len = strlen(AGE_DEFAULT_ALIAS_PREFIX);
-    
+
     vars = expandNSItemVars(pstate, pnsi, sublevels_up, location, &names);
 
     /*

Diff for: src/backend/parser/cypher_parse_agg.c

@@ -236,7 +236,9 @@ void parse_check_aggregates(ParseState *pstate, Query *qry)
     finalize_grouping_exprs(clause, pstate, qry, groupClauses, root,
                             have_non_var_grouping);
     if (hasJoinRTEs)
+    {
         clause = flatten_join_alias_vars(root, qry, clause);
+    }
     check_ungrouped_columns(clause, pstate, qry, groupClauses,
                             groupClauseCommonVars, have_non_var_grouping,
                             &func_grouped_rels);
@@ -245,7 +247,9 @@ void parse_check_aggregates(ParseState *pstate, Query *qry)
     finalize_grouping_exprs(clause, pstate, qry, groupClauses, root,
                             have_non_var_grouping);
     if (hasJoinRTEs)
+    {
         clause = flatten_join_alias_vars(root, qry, clause);
+    }
     check_ungrouped_columns(clause, pstate, qry, groupClauses,
                             groupClauseCommonVars, have_non_var_grouping,
                             &func_grouped_rels);
@@ -254,10 +258,12 @@ void parse_check_aggregates(ParseState *pstate, Query *qry)
      * Per spec, aggregates can't appear in a recursive term.
      */
     if (pstate->p_hasAggs && hasSelfRefRTEs)
+    {
         ereport(ERROR,
                 (errcode(ERRCODE_INVALID_RECURSION),
                  errmsg("aggregate functions are not allowed in a recursive query's recursive term"),
                  parser_errposition(pstate, locate_agg_of_level((Node *) qry, 0))));
+    }
 }
 
 /*
@@ -562,7 +568,11 @@ static bool finalize_grouping_exprs_walker(Node *node,
                 Index ref = 0;
 
                 if (context->root)
-                    expr = flatten_join_alias_vars(context-> root, (Query*)context->root, expr);
+                {
+                    expr = flatten_join_alias_vars(context->root,
+                                                   (Query *)context->root,
+                                                   expr);
+                }
 
                 /*
                  * Each expression must match a grouping entry at the current