jrgemignani commented on code in PR #1192:
URL: https://github.com/apache/age/pull/1192#discussion_r1304875747


##########
src/backend/utils/adt/agtype_gin.c:
##########
@@ -224,33 +224,42 @@ Datum gin_extract_agtype_query(PG_FUNCTION_ARGS)
              strategy == AGTYPE_EXISTS_ALL_STRATEGY_NUMBER)
     {
         /* Query is a text array; each element is treated as a key */
-        ArrayType *query = PG_GETARG_ARRAYTYPE_P(0);
-        Datum *key_datums;
-        bool *key_nulls;
-        int key_count;
-        int i, j;
-
-        deconstruct_array(query, TEXTOID, -1, false, 'i',
-                          &key_datums, &key_nulls, &key_count);
+        agtype *agt = AG_GET_ARG_AGTYPE_P(0);
+        agtype_iterator *it = NULL;
+        agtype_value elem;
+        agtype_iterator_token itok;
+        int key_count = AGTYPE_CONTAINER_SIZE(&agt->root);
+        int index = 0;
+
+        if (AGTYPE_CONTAINER_IS_SCALAR(&agt->root) ||
+            !AGTYPE_CONTAINER_IS_ARRAY(&agt->root))
+        {
+            elog(ERROR, "GIN query requires an agtype array");
+        }
 
         entries = (Datum *) palloc(sizeof(Datum) * key_count);
+        it = agtype_iterator_init(&agt->root);
 
-        for (i = 0, j = 0; i < key_count; i++)
+        /* it should be WAGT_BEGIN_ARRAY */
+        itok = agtype_iterator_next(&it, &elem, true);
+        Assert(itok == WAGT_BEGIN_ARRAY);
+
+        while (WAGT_END_ARRAY != agtype_iterator_next(&it, &elem, true))
         {
-            /* Nulls in the array are ignored */
-            if (key_nulls[i])
+            if (elem.type != AGTV_STRING)
             {
-                continue;
+                elog(ERROR, "unsupport agtype for GIN lookup: %d", elem.type);

Review Comment:
   unsupport or unsupported?



##########
src/backend/utils/adt/agtype_gin.c:
##########
@@ -224,33 +224,42 @@ Datum gin_extract_agtype_query(PG_FUNCTION_ARGS)
              strategy == AGTYPE_EXISTS_ALL_STRATEGY_NUMBER)
     {
         /* Query is a text array; each element is treated as a key */
-        ArrayType *query = PG_GETARG_ARRAYTYPE_P(0);
-        Datum *key_datums;
-        bool *key_nulls;
-        int key_count;
-        int i, j;
-
-        deconstruct_array(query, TEXTOID, -1, false, 'i',
-                          &key_datums, &key_nulls, &key_count);
+        agtype *agt = AG_GET_ARG_AGTYPE_P(0);
+        agtype_iterator *it = NULL;
+        agtype_value elem;
+        agtype_iterator_token itok;
+        int key_count = AGTYPE_CONTAINER_SIZE(&agt->root);
+        int index = 0;
+
+        if (AGTYPE_CONTAINER_IS_SCALAR(&agt->root) ||
+            !AGTYPE_CONTAINER_IS_ARRAY(&agt->root))
+        {
+            elog(ERROR, "GIN query requires an agtype array");
+        }
 
         entries = (Datum *) palloc(sizeof(Datum) * key_count);
+        it = agtype_iterator_init(&agt->root);
 
-        for (i = 0, j = 0; i < key_count; i++)
+        /* it should be WAGT_BEGIN_ARRAY */
+        itok = agtype_iterator_next(&it, &elem, true);
+        Assert(itok == WAGT_BEGIN_ARRAY);
+
+        while (WAGT_END_ARRAY != agtype_iterator_next(&it, &elem, true))
         {
-            /* Nulls in the array are ignored */
-            if (key_nulls[i])
+            if (elem.type != AGTV_STRING)
             {
-                continue;
+                elog(ERROR, "unsupport agtype for GIN lookup: %d", elem.type);
             }
 
-            entries[j++] = make_text_key(AGT_GIN_FLAG_KEY,
-                                         VARDATA(key_datums[i]),
-                                         VARSIZE(key_datums[i]) - VARHDRSZ);
+            entries[index++] = make_text_key(AGT_GIN_FLAG_KEY,
+                                         elem.val.string.val,
+                                         elem.val.string.len);

Review Comment:
   Alignment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to