Merge branch 'master' into PG16_to_master

jrgemignani · jrgemignani · commit 69d8494b547f · 2023-12-21T11:00:40.000-08:00
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

Modified specifically for PG16 branch:
    .github/workflows/go-driver.yml
    .github/workflows/installcheck.yaml
    .github/workflows/jdbc-driver.yaml
    .github/workflows/nodejs-driver.yaml
    .github/workflows/python-driver.yaml
diff --git a/.github/workflows/go-driver.yml b/.github/workflows/go-driver.yml
@@ -2,10 +2,10 @@ name: Go Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
 jobs:
   build:
@@ -24,15 +24,11 @@ jobs:
     - name: Set tag based on branch
       run: |
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
-          if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
+          if [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         elif [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then
-          if [[ "$GITHUB_BASE_REF" == "master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+          if [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
diff --git a/.github/workflows/installcheck.yaml b/.github/workflows/installcheck.yaml
@@ -2,9 +2,9 @@ name: Build / Regression
 
 on:
   push:
-    branches: [ 'master', 'PG16' ]
+    branches: [ 'PG16' ]
   pull_request:
-    branches: [ 'master', 'PG16' ]
+    branches: [ 'PG16' ]
 
 jobs:
   build:
diff --git a/.github/workflows/jdbc-driver.yaml b/.github/workflows/jdbc-driver.yaml
@@ -2,10 +2,10 @@ name: JDBC Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
 jobs:
   build:
@@ -26,15 +26,11 @@ jobs:
     - name: Set tag based on branch
       run: |
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
-          if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
+          if [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         elif [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then
-          if [[ "$GITHUB_BASE_REF" == "master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+          if [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
diff --git a/.github/workflows/nodejs-driver.yaml b/.github/workflows/nodejs-driver.yaml
@@ -2,10 +2,10 @@ name: Nodejs Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
 jobs:
   build:
@@ -21,15 +21,11 @@ jobs:
     - name: Set tag based on branch
       run: |
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
-          if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
+          if [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         elif [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then
-          if [[ "$GITHUB_BASE_REF" == "master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+          if [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
diff --git a/.github/workflows/python-driver.yaml b/.github/workflows/python-driver.yaml
@@ -2,10 +2,10 @@ name: Python Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG16" ]
+    branches: [ "PG16" ]
 
 jobs:
   build:
@@ -21,15 +21,11 @@ jobs:
     - name: Set tag based on branch
       run: |
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
-          if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
+          if [[ "$GITHUB_REF" == "refs/heads/PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         elif [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then
-          if [[ "$GITHUB_BASE_REF" == "master" ]]; then
-            echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+          if [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
             echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
diff --git a/src/backend/catalog/ag_catalog.c b/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 --git a/src/backend/catalog/ag_label.c b/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 --git a/src/backend/executor/cypher_set.c b/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 --git a/src/backend/executor/cypher_utils.c b/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 --git a/src/backend/nodes/cypher_readfuncs.c b/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 --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c
diff --git a/src/backend/parser/cypher_expr.c b/src/backend/parser/cypher_expr.c
diff --git a/src/backend/parser/cypher_item.c b/src/backend/parser/cypher_item.c
diff --git a/src/backend/parser/cypher_parse_agg.c b/src/backend/parser/cypher_parse_agg.c
diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c

Original file line number	Diff line number	Diff line change
`@@ -485,7 +485,8 @@ static void process_update_list(CustomScanState *node)`
`485`	`485`	`}`
`486`	`486`
`487`	`487`	`// Alter the properties Agtype value.`
`488`		`- if (strcmp(update_item->prop_name, ""))`
	`488`	`+ if (update_item->prop_name != NULL &&`
	`489`	`+ strcmp(update_item->prop_name, "") != 0)`
`489`	`490`	`{`
`490`	`491`	`altered_properties = alter_property_value(original_properties,`
`491`	`492`	`update_item->prop_name,`