[incubator-age] branch master updated: Bug fix for JIRA issue AGE2-307

[jg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jgemignani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-age.git


The following commit(s) were added to refs/heads/master by this push:
     new bd68d9a  Bug fix for JIRA issue AGE2-307
bd68d9a is described below

commit bd68d9a4b682825c98d829da56d40d8d27019412
Author: John Gemignani <jr...@gmail.com>
AuthorDate: Wed Jul 28 11:05:45 2021 -0700

    Bug fix for JIRA issue AGE2-307
    
    This fixes the logic for Edge, Vertex, and Path serialization.
    
    Background - When a type is serialized, it stores the length of the
    buffer that it used, in a header. This information is then used by
    the deserialization process to locate the next type which follows
    it in a container. In other words, in the deserialization process,
    that length (from the header) is used to reconstruct the offset to
    the next type data that is stored in a container.
    
    Additionally, during serialization, data that is added to the buffer
    is being integer aligned for disk storage.
    
    However, there is an issue in the deserialization phase due to how
    these offsets work, or rather, add. In the deserialization phase, the
    offsets are added together and then later integer aligned before
    usage, to get the beginning of the next stored type. This addition of
    these offsets, however, doesn't account for the accumulated integer
    aligned offsets. This cumulative affect is what causes the data to
    get out of sync and generate the following error -
    
        Invalid AGT header value.
    
    The header is actually there, it's just not at the expected location.
    
    After understanding this, there are 2 ways to fix this issue -
    
    The first is modifying the iterator's advancement logic to account for
    these cumulative alignments. This will address the issue in one spot
    and for everything. However, this will impact everything that uses an
    iterator. It is unknown how large of an impact this could be.
    
    The other is to adjust the serialization routines for each type (EDGE,
    VERTEX, & PATH) that is impacted. This adjustment will cause the
    length returned to include the ending integer alignment. This will then
    allow the deserialization logic to add these offsets together and get
    the correct value of the next type's position.
    
    The latter solution was implemented as it was deemed safer.
---
 src/backend/utils/adt/agtype_ext.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/backend/utils/adt/agtype_ext.c b/src/backend/utils/adt/agtype_ext.c
index 9362f6f..b22d2ec 100644
--- a/src/backend/utils/adt/agtype_ext.c
+++ b/src/backend/utils/adt/agtype_ext.c
@@ -78,9 +78,18 @@ bool ag_serialize_extended_type(StringInfo buffer, agtentry *agtentry,
     case AGTV_VERTEX:
     {
         uint32 object_ae = 0;
+
         padlen = ag_serialize_header(buffer, AGT_HEADER_VERTEX);
         convert_extended_object(buffer, &object_ae, scalar_val);
 
+        /*
+         * Make sure that the end of the buffer is padded to the next offset and
+         * add this padding to the length of the buffer used. This ensures that
+         * everything stays aligned and eliminates errors caused by compounded
+         * offsets in the deserialization routines.
+         */
+        object_ae += pad_buffer_to_int(buffer);
+
         *agtentry = AGTENTRY_IS_AGTYPE |
                     ((AGTENTRY_OFFLENMASK & (int)object_ae) + AGT_HEADER_SIZE);
         break;
@@ -89,9 +98,18 @@ bool ag_serialize_extended_type(StringInfo buffer, agtentry *agtentry,
     case AGTV_EDGE:
     {
         uint32 object_ae = 0;
+
         padlen = ag_serialize_header(buffer, AGT_HEADER_EDGE);
         convert_extended_object(buffer, &object_ae, scalar_val);
 
+        /*
+         * Make sure that the end of the buffer is padded to the next offset and
+         * add this padding to the length of the buffer used. This ensures that
+         * everything stays aligned and eliminates errors caused by compounded
+         * offsets in the deserialization routines.
+         */
+        object_ae += pad_buffer_to_int(buffer);
+
         *agtentry = AGTENTRY_IS_AGTYPE |
                     ((AGTENTRY_OFFLENMASK & (int)object_ae) + AGT_HEADER_SIZE);
         break;
@@ -100,9 +118,18 @@ bool ag_serialize_extended_type(StringInfo buffer, agtentry *agtentry,
     case AGTV_PATH:
     {
         uint32 object_ae = 0;
+
         padlen = ag_serialize_header(buffer, AGT_HEADER_PATH);
         convert_extended_array(buffer, &object_ae, scalar_val);
 
+        /*
+         * Make sure that the end of the buffer is padded to the next offset and
+         * add this padding to the length of the buffer used. This ensures that
+         * everything stays aligned and eliminates errors caused by compounded
+         * offsets in the deserialization routines.
+         */
+        object_ae += pad_buffer_to_int(buffer);
+
         *agtentry = AGTENTRY_IS_AGTYPE |
                     ((AGTENTRY_OFFLENMASK & (int)object_ae) + AGT_HEADER_SIZE);
         break;