Master to PostgreSQL version 16 (apache#1451)

* Initial PG16 version (apache#1237)

Fixed empty string handling.

Previously, the PG 16 outToken emitted NULL for an empty string, but now it emits an empty string "". Consequently, our cypher read function required modification to properly decode the empty string as a plain token, thereby addressing the comparison issue.

Compared the branches and added the necessary changes so that the query's rteperminfos variable doesn't stay NULL.

Fix missing include varatt.h (causing undefined symbol VARDATA_ANY) & Fixing some test cases of the failings

Added missing include varatt.h to fix the undefined symbol while loading age into postgresql because usage of VARDATA_ANY needs to import varatt.h in PG16

Modified initialisation of ResultRelInfo and removed unnecessary RTEs

Compared the branches and added the necessary changes so that the query's rteperminfos variable doesn't stay NULL.

Modified initialisation of ResultRelInfo and removed unnecessary RTEs

One of the problems that we were facing was related to the ResultRelInfo pointing at the wrong RTE via its ri_RangeTableIndex. The create_entity_result_rel_info() function does not have the capability of setting the ri_RootResultRelInfo to the correct ResultRelInfo node because it does not have access to the ModifyTableState node. The solution for this was to set the third argument in InitResultRelInfo() to be zero instead of list_length(estate->es_range_table).

In the update_entity_tuple() function, when we call table_tuple_update() and assign the returned value to the result variable, the buffer variable receives the value of 0.
Made a workaround so that the original value isn't lost.

This is a work in progress for the new field that was added to the struct Var called varnullingrels. According to the documentation, this field is responsible for marking the Vars as nullable, if they are coming from a JOIN, either LEFT JOIN, RIGHT JOIN, or FULL OUTER JOIN. The changes were made following an "optional match" clause which is being treated as a LEFT JOIN from our extension.

A function markRelsAsNulledBy is added because its internal in Postgres and doesn't belong in a header file, therefore it can't be exported. This function is added before the creation of the Vars from the make_vertex_expr and make_edge_expr, to correctly mark the specific PNSI as nullable, so later in the planner stage, the Vars will be correctly nulled.

Fix incorrect typecasting in agtype_to_graphid function.

Fix incorrect returns to the fuction _label_name, _ag_build_vertex and _ag_build_edge.
Contributors

Panagiotis Foliadis <[email protected]>
Matheus Farias <[email protected]>
Mohamed Mokhtar <[email protected]>
Hannan Aamir <[email protected]>
John Gemignani <[email protected]>
Muhammad Taha Naveed <[email protected]>
Wendel de Lana <[email protected]>
---------

* Fix Docker files to reflect PostgreSQL version 16 (apache#1448)

Fixed the following Docker files to reflect running under
PostgreSQL version 16.

    modified:   docker/Dockerfile
    modified:   docker/Dockerfile.dev

This impacts the DockerHub builds.

* Mark null-returning RTEs in outer joins as nullable

RTEs that appear in the right side of a left join are marked as 'nullable'.
The column references to a nullable RTE within the join's RTE are also
marked as 'nullable'. This concept is introduced in Postgresql v16.

The change in Postgresql v16's pullup_replace_vars_callback() function causes
different plans to be generated depending on whether appropriate RTEs are
marked nullable. Without marking nullable, in a left join, any function call
expression containing right RTE's columns as arguments are not evaluated at
the scan level, rather it is evaluated after the join is performed. At that
point, the function call may receive null input, which was unexpected in
previous Postgresql versions.

See:
----
  - Postgresql v16's commit: Make Vars be outer-join-aware
    https://www.postgresql.org/message-id/[email protected]

  - The 'Vars and PlaceHolderVars' section in the Postgresql v16's
    optimizer/README.md

* Fix minor code formatting.

---------

Co-authored-by: Shoaib <[email protected]>
Co-authored-by: Rafsun Masud <[email protected]>

.github/workflows/go-driver.yml

@@ -2,10 +2,10 @@ name: Go Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
 jobs:
   build:
@@ -26,14 +26,14 @@ jobs:
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
           if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
             echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$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" == "PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+            echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
 

.github/workflows/installcheck.yaml

@@ -2,45 +2,45 @@ name: Build / Regression
 
 on:
   push:
-    branches: [ 'master', 'PG15' ]
+    branches: [ 'master', 'PG16' ]
   pull_request:
-    branches: [ 'master', 'PG15' ]
+    branches: [ 'master', 'PG16' ]
 
 jobs:
   build:
     runs-on: ubuntu-latest
 
     steps:
-      - name: Get latest commit id of PostgreSQL 15
+      - name: Get latest commit id of PostgreSQL 16
         run: |
-          echo "PG_COMMIT_HASH=$(git ls-remote git://git.postgresql.org/git/postgresql.git refs/heads/REL_15_STABLE | awk '{print $1}')" >> $GITHUB_ENV
+          echo "PG_COMMIT_HASH=$(git ls-remote git://git.postgresql.org/git/postgresql.git refs/heads/REL_16_STABLE | awk '{print $1}')" >> $GITHUB_ENV
 
-      - name: Cache PostgreSQL 15
+      - name: Cache PostgreSQL 16
         uses: actions/cache@v3
-        id: pg15cache
+        id: pg16cache
         with:
-          path: ~/pg15
-          key: ${{ runner.os }}-v1-pg15-${{ env.PG_COMMIT_HASH }}
+          path: ~/pg16
+          key: ${{ runner.os }}-v1-pg16-${{ env.PG_COMMIT_HASH }}
 
-      - name: Install PostgreSQL 15
-        if: steps.pg15cache.outputs.cache-hit != 'true'
+      - name: Install PostgreSQL 16
+        if: steps.pg16cache.outputs.cache-hit != 'true'
         run: |
-          git clone --depth 1 --branch REL_15_STABLE git://git.postgresql.org/git/postgresql.git ~/pg15source
-          cd ~/pg15source
-          ./configure --prefix=$HOME/pg15 CFLAGS="-std=gnu99 -ggdb -O0" --enable-cassert
+          git clone --depth 1 --branch REL_16_STABLE git://git.postgresql.org/git/postgresql.git ~/pg16source
+          cd ~/pg16source
+          ./configure --prefix=$HOME/pg16 CFLAGS="-std=gnu99 -ggdb -O0" --enable-cassert
           make install -j$(nproc) > /dev/null
 
       - uses: actions/checkout@v3
 
       - name: Build
         id: build
         run: |
-          make PG_CONFIG=$HOME/pg15/bin/pg_config install -j$(nproc)
+          make PG_CONFIG=$HOME/pg16/bin/pg_config install -j$(nproc)
 
       - name: Regression tests
         id: regression_tests
         run: |
-          make PG_CONFIG=$HOME/pg15/bin/pg_config installcheck
+          make PG_CONFIG=$HOME/pg16/bin/pg_config installcheck
         continue-on-error: true
 
       - name: Dump regression test errors

.github/workflows/jdbc-driver.yaml

@@ -2,10 +2,10 @@ name: JDBC Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
 jobs:
   build:
@@ -28,14 +28,14 @@ jobs:
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
           if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
             echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$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" == "PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+            echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
 

.github/workflows/nodejs-driver.yaml

@@ -2,10 +2,10 @@ name: Nodejs Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
 jobs:
   build:
@@ -23,14 +23,14 @@ jobs:
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
           if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
             echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$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" == "PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+            echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
 

.github/workflows/python-driver.yaml

@@ -2,10 +2,10 @@ name: Python Driver Tests
 
 on:
   push:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
   pull_request:
-    branches: [ "master", "PG15" ]
+    branches: [ "master", "PG16" ]
 
 jobs:
   build:
@@ -23,14 +23,14 @@ jobs:
         if [[ "$GITHUB_EVENT_NAME" == "push" ]]; then
           if [[ "$GITHUB_REF" == "refs/heads/master" ]]; then
             echo "TAG=latest" >> $GITHUB_ENV
-          elif [[ "$GITHUB_REF" == "refs/heads/PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$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" == "PG15" ]]; then
-            echo "TAG=PG15_latest" >> $GITHUB_ENV
+          elif [[ "$GITHUB_BASE_REF" == "PG16" ]]; then
+            echo "TAG=PG16_latest" >> $GITHUB_ENV
           fi
         fi
 

.gitignore

@@ -7,3 +7,4 @@ age--*.*.*.sql
 .DS_Store
 *.tokens
 *.interp
+*.dylib

docker/Dockerfile

@@ -16,14 +16,14 @@
 # limitations under the License.
 #
 
-FROM postgres:15
+FROM postgres:16
 
 RUN apt-get update \
     && apt-get install -y --no-install-recommends --no-install-suggests \
        bison \
        build-essential \
        flex \
-       postgresql-server-dev-15 \
+       postgresql-server-dev-16 \
        locales
 
 ENV LANG=en_US.UTF-8

docker/Dockerfile.dev

@@ -17,14 +17,14 @@
 #
 
 
-FROM postgres:15
+FROM postgres:16
 
 RUN apt-get update
 RUN apt-get install --assume-yes --no-install-recommends --no-install-suggests \
   bison \
   build-essential \
   flex \
-  postgresql-server-dev-15 \
+  postgresql-server-dev-16 \
   locales
 
 ENV LANG=en_US.UTF-8

src/backend/catalog/ag_catalog.c

@@ -86,19 +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
-        standard_ProcessUtility(pstmt, queryString, readOnlyTree, context, params, queryEnv,
-                                dest, qc);
+    {
+        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);
+    }
 }
 
 static void drop_age_extension(DropStmt *stmt)

src/backend/catalog/ag_graph.c

@@ -49,8 +49,8 @@ void insert_graph(const Name graph_name, const Oid nsp_id)
     HeapTuple tuple;
 
 
-    AssertArg(graph_name);
-    AssertArg(OidIsValid(nsp_id));
+    Assert(graph_name);
+    Assert(OidIsValid(nsp_id));
 
     ag_graph = table_open(ag_graph_relation_id(), RowExclusiveLock);
     values[Anum_ag_graph_oid - 1] = ObjectIdGetDatum(nsp_id);

src/backend/catalog/ag_label.c

@@ -63,12 +63,12 @@ void insert_label(const char *label_name, Oid graph_oid, int32 label_id,
      * NOTE: Is it better to make use of label_id and label_kind domain types
      *       than to use assert to check label_id and label_kind are valid?
      */
-    AssertArg(label_name);
-    AssertArg(label_id_is_valid(label_id));
-    AssertArg(label_kind == LABEL_KIND_VERTEX ||
+    Assert(label_name);
+    Assert(label_id_is_valid(label_id));
+    Assert(label_kind == LABEL_KIND_VERTEX ||
               label_kind == LABEL_KIND_EDGE);
-    AssertArg(OidIsValid(label_relation));
-    AssertArg(seq_name);
+    Assert(OidIsValid(label_relation));
+    Assert(seq_name);
 
     ag_label = table_open(ag_label_relation_id(), RowExclusiveLock);
 
@@ -188,8 +188,10 @@ Datum _label_name(PG_FUNCTION_ARGS)
     uint32 label_id;
 
     if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
+    {
         ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
                         errmsg("graph_oid and label_id must not be null")));
+    }
 
     graph = PG_GETARG_OID(0);
 

src/backend/commands/label_commands.c

@@ -808,7 +808,7 @@ static void remove_relation(List *qname)
     Oid rel_oid;
     ObjectAddress address;
 
-    AssertArg(list_length(qname) == 2);
+    Assert(list_length(qname) == 2);
 
     // concurrent is false so lockmode is AccessExclusiveLock
 
@@ -868,8 +868,7 @@ static void range_var_callback_for_remove_relation(const RangeVar *rel,
 
     // relkind == expected_relkind
 
-    if (!pg_class_ownercheck(rel_oid, GetUserId()) &&
-        !pg_namespace_ownercheck(get_rel_namespace(rel_oid), GetUserId()))
+    if (!object_ownercheck(rel_oid, get_rel_namespace(rel_oid), GetUserId()))
     {
         aclcheck_error(ACLCHECK_NOT_OWNER,
                        get_relkind_objtype(get_rel_relkind(rel_oid)),

src/backend/executor/cypher_create.c

@@ -438,7 +438,7 @@ static void create_edge(cypher_create_custom_scan_state *css,
 
         result = make_edge(
             id, start_id, end_id, CStringGetDatum(node->label_name),
-            PointerGetDatum(scanTupleSlot->tts_values[node->prop_attr_num]));
+            scanTupleSlot->tts_values[node->prop_attr_num]);
 
         if (CYPHER_TARGET_NODE_IN_PATH(node->flags))
         {
@@ -528,7 +528,7 @@ static Datum create_vertex(cypher_create_custom_scan_state *css,
 
             // make the vertex agtype
             result = make_vertex(id, CStringGetDatum(node->label_name),
-                PointerGetDatum(scanTupleSlot->tts_values[node->prop_attr_num]));
+                scanTupleSlot->tts_values[node->prop_attr_num]);
 
             // append to the path list
             if (CYPHER_TARGET_NODE_IN_PATH(node->flags))

src/backend/executor/cypher_merge.c

@@ -472,8 +472,9 @@ static TupleTableSlot *exec_cypher_merge(CustomScanState *node)
              * So we will need to create a TupleTableSlot and populate with the
              * information from the newly created path that the query needs.
              */
-            ExprContext *econtext = node->ss.ps.ps_ExprContext;
-            SubqueryScanState *sss = (SubqueryScanState *)node->ss.ps.lefttree;
+            SubqueryScanState *sss = NULL;
+            econtext = node->ss.ps.ps_ExprContext;
+            sss = (SubqueryScanState *)node->ss.ps.lefttree;
 
             /*
              * Our child execution node is always a subquery. If not there

src/backend/executor/cypher_set.c

@@ -111,7 +111,7 @@ static HeapTuple update_entity_tuple(ResultRelInfo *resultRelInfo,
     TM_FailureData hufd;
     TM_Result lock_result;
     Buffer buffer;
-    bool update_indexes;
+    TU_UpdateIndexes update_indexes;
     TM_Result   result;
     CommandId cid = GetCurrentCommandId(true);
     ResultRelInfo **saved_resultRels = estate->es_result_relations;
@@ -167,9 +167,10 @@ static HeapTuple update_entity_tuple(ResultRelInfo *resultRelInfo,
         }
 
         // Insert index entries for the tuple
-        if (resultRelInfo->ri_NumIndices > 0 && update_indexes)
+        if (resultRelInfo->ri_NumIndices > 0 && update_indexes != TU_None)
         {
-          ExecInsertIndexTuples(resultRelInfo, elemTupleSlot, estate, false, false, NULL, NIL);
+          ExecInsertIndexTuples(resultRelInfo, elemTupleSlot, estate, false, false, NULL, NIL,
+                                (update_indexes == TU_Summarizing));
         }
 
         ExecCloseIndices(resultRelInfo);
@@ -484,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,