agensgraph icon indicating copy to clipboard operation
agensgraph copied to clipboard

Property indexes with attributes needing double quotes

Open jshen-r7 opened this issue 2 years ago • 3 comments

Our team uses Agensgraph with attributes that have non-alphabetic characters, e.g. :. We create property indexes on these attribute, but the issue is we cannot recreate those indexes from the index definitions. This is particularly a problem in an upgrade scenario using pg_upgrade.

We are currently running Agensgraph 2.12.0, attempting to upgrade to 2.13.x.

Example:

beta=#  \d foo."bar"
                                                                  Table "foo.bar"
   Column   |  Type   | Collation | Nullable |                                                          Default
------------+---------+-----------+----------+---------------------------------------------------------------------------------------------------------------------------
 id         | graphid |           | not null | graphid(graph_labid('foo."bar"'::cstring), nextval('foo."bar_id_seq"'::regclass))
 properties | jsonb   |           | not null | jsonb_build_object()
 
 Indexes:
    "bar_pkey" PRIMARY KEY, btree (id)
    "bar_id_idx" btree ((properties.'Enum:id'::text), (properties.'_partition'::text))
    
beta=# select indexdef from pg_catalog.pg_indexes i where tablename='bar';
                                                                               indexdef
----------- 
CREATE UNIQUE INDEX "bar_pkey" ON foo."bar" USING btree (id)
CREATE INDEX "bar_id_idx" ON foo."bar" USING btree ((properties.'Enum:id'::text), (properties.'_partition'::text))

We create the indexes with double quotes around the attribute, i.e. "Enum:id", so the single quotes are a surprise here.

The second index as written can't be created as a property index because of the single quotes, which causes pg_upgrade to fail when run from a dump file. This is the relevant output:

pg_restore: from TOC entry 10917; 1259 24943 INDEX bar_id_idx beta
pg_restore: error: could not execute query: ERROR:  syntax error at or near "'Enum:id'"
LINE 6: ..._id_idx" ON "bar" USING "btree" ('Enum:id',...
                                                             ^
Command was:
-- For binary upgrade, must preserve pg_class oids
SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('24943'::pg_catalog.oid);


SET GRAPH_PATH = "foo";
CREATE PROPERTY INDEX "bar_id_idx" ON "bar" USING "btree" ('Enum:id', _partition);

jshen-r7 avatar Aug 14 '23 16:08 jshen-r7

This is a quick patch I used to move forward in our upgrade, since it looked like the parser wasn't identifying those expressions as constants. It may not be 100% correct but seems to work for our case.

--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10110,7 +10110,14 @@ get_const_expr(Const *constval, deparse_context *context, int showtype)
                        }

                default:
-                       simple_quote_literal(buf, extval);
+                       // Our attribute names get labeled string constants
+                       // But we want to treat them as identifiers
+                       // in our index xpressions
+                       if (context->cypherexpr && context->special_exprkind == EXPR_KIND_INDEX_EXPRESSION) {
+                               appendStringInfoString(buf, quote_identifier(extval));
+                       } else {
+                               simple_quote_literal(buf, extval);
+                       }
                        break;
        }

@@ -12381,7 +12388,7 @@ deparse_prop_expression_pretty(Node *expr, List *dpcontext, int prettyFlags)
        context.prettyFlags = prettyFlags;
        context.wrapColumn = WRAP_COLUMN_DEFAULT;
        context.indentLevel = 0;
-       context.special_exprkind = EXPR_KIND_NONE;
+       context.special_exprkind = EXPR_KIND_INDEX_EXPRESSION;
        context.cypherexpr = true;

        get_rule_expr(expr, &context, false);

jshen-r7 avatar Aug 16 '23 20:08 jshen-r7

Thanks for your suggest. Maybe that issue is that using ' and " at the same expression. So we tested, but some test environment doesn't occured.

If you okay, would you share your system enviromnent?

yjy44 avatar Aug 31 '23 05:08 yjy44

I'll prepare a packaged Agensgraph image and share it with you.

There is another issue we've noticed around the 2.13.1 upgrade, where the base "ag_vertex" and "ag_edge" relations don't come with the properties column not null and default. Below are two graphs, one created before the upgrade and one after (called native):


upgraded=# \d noetic."ag_vertex"
   Table "noetic.ag_vertex"

   Column   |  Type   | Collation | Nullable |                                                   Default
------------+---------+-----------+----------+-----------------------------------------------------------------------------------------
id         | graphid |           | not null | graphid(graph_labid('noetic."ag_vertex"'::cstring), nextval('noetic."ag_vertex_id_seq"'::regclass))
properties | jsonb   |           |          |

Indexes:
    "ag_vertex_pkey" PRIMARY KEY, btree (id)

native=# \d noetic."ag_vertex"
    Table "noetic.ag_vertex"
   Column   |  Type   | Collation | Nullable |                                                   Default
------------+---------+-----------+----------+-----------------------------------------------------------------------------------------
id         | graphid |           | not null | graphid(graph_labid('noetic."ag_vertex"'::cstring), nextval('noetic."ag_vertex_id_seq"'::regclass))
properties | jsonb   |           | not null | jsonb_build_object()

Indexes:
    "ag_vertex_pkey" PRIMARY KEY, btree (id)

Our current planned workaround is simply to apply a migration after the upgrade, but it would be preferable to have this patched.

jshen-r7 avatar Sep 01 '23 01:09 jshen-r7