Fix issue 1986: Failure creating label name close to MAX_LABEL_NAME_L…

…EN (apache#1993)

Fixed the issue with creating a label name close to MAX_LABEL_NAME_LEN. Well,
sort of.

The issue here is that a label name is the name of a relation in PostgreSQL.
While the relation name is a char * it is basically converted to a PG type Name
when it is used to create a relation. This conversion is a silent truncation,
there are no warnings or error messages. The Name type is defined as follows -

    /*
     * Representation of a Name: effectively just a C string, but null-padded to
     * exactly NAMEDATALEN bytes.  The use of a struct is historical.
     */
    typedef struct nameData
    {
        char data[NAMEDATALEN];
    } NameData;
    typedef NameData *Name;

This effectively gives us a max label name length of NAMEDATALEN - 1.

    /*
     * Maximum length for identifiers (e.g. table names, column names,
     * function names).  Names actually are limited to one fewer byte than this,
     * because the length must include a trailing zero byte.
     *
     * This should be at least as much as NAMEDATALEN of the database the
     * applications run against.
     */
    #define NAMEDATALEN 64

Since there isn't a way to get around this, the code was modified to reflect the
usage of NAMEDATALEN for the length of label names. This required modifying the
input parameters to be cstrings, for the functions `create_vlabel` and
`create_elabel` which allows us to tell when NAMEDATALEN has been exceeded.

Additionally, the function `is_valid_label` was renamed to `is_valid_label_name`
for consistency.

Regression tests were updated and added.

age--1.5.0--y.y.y.sql

@@ -129,3 +129,20 @@ CREATE FUNCTION ag_catalog.graph_exists(graph_name name)
     RETURNS agtype
     LANGUAGE c
     AS 'MODULE_PATHNAME', 'age_graph_exists';
+
+CREATE FUNCTION ag_catalog.age_is_valid_label_name(agtype)
+    RETURNS boolean
+    LANGUAGE c
+    IMMUTABLE
+PARALLEL SAFE
+AS 'MODULE_PATHNAME';
+
+CREATE OR REPLACE FUNCTION ag_catalog.create_vlabel(graph_name cstring, label_name cstring)
+    RETURNS void
+    LANGUAGE c
+    AS 'MODULE_PATHNAME';
+
+CREATE OR REPLACE FUNCTION ag_catalog.create_elabel(graph_name cstring, label_name cstring)
+    RETURNS void
+    LANGUAGE c
+    AS 'MODULE_PATHNAME';

regress/expected/name_validation.out

@@ -233,8 +233,10 @@ NOTICE:  graph "graph123" has been created
 -- length
 -- invalid
 SELECT create_vlabel('graph123', '');
+WARNING:  label name length not in range (1 <= length <= 63) length = 0
 ERROR:  label name is invalid
 SELECT create_elabel('graph123', '');
+WARNING:  label name length not in range (1 <= length <= 63) length = 0
 ERROR:  label name is invalid
 -- valid
 SELECT create_vlabel('graph123', 'labelx');
@@ -396,9 +398,57 @@ SELECT * from cypher('graph123', $$ return is_valid_label_name('label2') $$) as
  true
 (1 row)
 
+-- issue 1986: label name validation of long names.
+-- Label names are relation names which are restricted to NAMEDATALEN-1 in size.
+-- However, we can't validate PG type Names due to namein() truncating anything
+-- over NAMEDATALEN-1. To allow the label names to be checked over NAMEDATELEN-1
+-- we changed the input type from PG's Name to cstring. These checks are to
+-- verify that these can now be caught.
+--
+-- should return false and a warning.
+SELECT * from cypher('graph123', $$ return is_valid_label_name('label01234567890123456789012345678901234567890123456789012345678') $$) as (result agtype);
+WARNING:  label name length not in range (1 <= length <= 63) length = 64
+ result 
+--------
+ false
+(1 row)
+
+-- should be successful
+SELECT * from cypher('graph123', $$ return is_valid_label_name('label0123456789012345678901234567890123456789012345678901234567') $$) as (result agtype);
+ result 
+--------
+ true
+(1 row)
+
+--
+-- now check vlabel creation, should fail
+SELECT create_vlabel('graph123', 'vlabel01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678');
+WARNING:  label name length not in range (1 <= length <= 63) length = 95
+ERROR:  label name is invalid
+-- should be successful
+SELECT create_vlabel('graph123', 'vlabel012345678901234567890123456789012345678901234567890123456');
+NOTICE:  VLabel "vlabel012345678901234567890123456789012345678901234567890123456" has been created
+ create_vlabel 
+---------------
+
+(1 row)
+
+--
+-- now check elabel creation, should fail
+SELECT create_elabel('graph123', 'elabel0123456789012345678901234567890123456789012345678901234567');
+WARNING:  label name length not in range (1 <= length <= 63) length = 64
+ERROR:  label name is invalid
+-- should be okay
+SELECT create_elabel('graph123', 'elabel012345678901234567890123456789012345678901234567890123456');
+NOTICE:  ELabel "elabel012345678901234567890123456789012345678901234567890123456" has been created
+ create_elabel 
+---------------
+
+(1 row)
+
 -- clean up
 SELECT drop_graph('graph123', true);
-NOTICE:  drop cascades to 18 other objects
+NOTICE:  drop cascades to 20 other objects
 DETAIL:  drop cascades to table graph123._ag_label_vertex
 drop cascades to table graph123._ag_label_edge
 drop cascades to table graph123.labelx
@@ -417,6 +467,8 @@ drop cascades to table graph123.mylabel
 drop cascades to table graph123."A"
 drop cascades to table graph123.mylabel2
 drop cascades to table graph123."C"
+drop cascades to table graph123.vlabel012345678901234567890123456789012345678901234567890123456
+drop cascades to table graph123.elabel012345678901234567890123456789012345678901234567890123456
 NOTICE:  graph "graph123" has been dropped
  drop_graph 
 ------------

regress/sql/name_validation.sql

@@ -153,6 +153,28 @@ SELECT * from cypher('graph123', $$ return is_valid_label_name('2label') $$) as
 SELECT * from cypher('graph123', $$ return is_valid_label_name('label1') $$) as (result agtype);
 SELECT * from cypher('graph123', $$ return is_valid_label_name('label2') $$) as (result agtype);
 
+-- issue 1986: label name validation of long names.
+-- Label names are relation names which are restricted to NAMEDATALEN-1 in size.
+-- However, we can't validate PG type Names due to namein() truncating anything
+-- over NAMEDATALEN-1. To allow the label names to be checked over NAMEDATELEN-1
+-- we changed the input type from PG's Name to cstring. These checks are to
+-- verify that these can now be caught.
+--
+-- should return false and a warning.
+SELECT * from cypher('graph123', $$ return is_valid_label_name('label01234567890123456789012345678901234567890123456789012345678') $$) as (result agtype);
+-- should be successful
+SELECT * from cypher('graph123', $$ return is_valid_label_name('label0123456789012345678901234567890123456789012345678901234567') $$) as (result agtype);
+--
+-- now check vlabel creation, should fail
+SELECT create_vlabel('graph123', 'vlabel01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678');
+-- should be successful
+SELECT create_vlabel('graph123', 'vlabel012345678901234567890123456789012345678901234567890123456');
+--
+-- now check elabel creation, should fail
+SELECT create_elabel('graph123', 'elabel0123456789012345678901234567890123456789012345678901234567');
+-- should be okay
+SELECT create_elabel('graph123', 'elabel012345678901234567890123456789012345678901234567890123456');
+
 -- clean up
 SELECT drop_graph('graph123', true);
 

sql/age_main.sql

@@ -100,12 +100,12 @@ CREATE FUNCTION ag_catalog.drop_graph(graph_name name, cascade boolean = false)
     LANGUAGE c
     AS 'MODULE_PATHNAME';
 
-CREATE FUNCTION ag_catalog.create_vlabel(graph_name name, label_name name)
+CREATE FUNCTION ag_catalog.create_vlabel(graph_name cstring, label_name cstring)
     RETURNS void
     LANGUAGE c
     AS 'MODULE_PATHNAME';
 
-CREATE FUNCTION ag_catalog.create_elabel(graph_name name, label_name name)
+CREATE FUNCTION ag_catalog.create_elabel(graph_name cstring, label_name cstring)
     RETURNS void
     LANGUAGE c
     AS 'MODULE_PATHNAME';

src/backend/commands/label_commands.c

@@ -130,7 +130,7 @@ Datum age_is_valid_label_name(PG_FUNCTION_ARGS)
     label_name = pnstrdup(agtv_value->val.string.val,
                           agtv_value->val.string.len);
 
-    is_valid = is_valid_label(label_name, 0);
+    is_valid = is_valid_label_name(label_name, 0);
     pfree(label_name);
 
     if (is_valid)
@@ -155,17 +155,11 @@ PG_FUNCTION_INFO_V1(create_vlabel);
 
 Datum create_vlabel(PG_FUNCTION_ARGS)
 {
-    char *graph;
-    Name graph_name;
-    char *graph_name_str;
+    char *graph_name;
     Oid graph_oid;
     List *parent;
-
     RangeVar *rv;
-
-    char *label;
-    Name label_name;
-    char *label_name_str;
+    char *label_name;
 
     /* checking if user has not provided the graph name */
     if (PG_ARGISNULL(0))
@@ -181,42 +175,49 @@ Datum create_vlabel(PG_FUNCTION_ARGS)
                 errmsg("label name must not be NULL")));
     }
 
-    graph_name = PG_GETARG_NAME(0);
-    label_name = PG_GETARG_NAME(1);
+    graph_name = PG_GETARG_CSTRING(0);
+    label_name = PG_GETARG_CSTRING(1);
 
-    graph_name_str = NameStr(*graph_name);
-    label_name_str = NameStr(*label_name);
+    /* validate the graph and label names */
+    if (is_valid_graph_name(graph_name) == 0)
+    {
+        ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("graph name is invalid")));
+    }
+
+    if (is_valid_label_name(label_name, 0) == 0)
+    {
+        ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("label name is invalid")));
+    }
 
     /* Check if graph does not exist */
-    if (!graph_exists(graph_name_str))
+    if (!graph_exists(graph_name))
     {
         ereport(ERROR,
                 (errcode(ERRCODE_UNDEFINED_SCHEMA),
-                        errmsg("graph \"%s\" does not exist.", graph_name_str)));
+                        errmsg("graph \"%s\" does not exist.", graph_name)));
     }
 
-    graph_oid = get_graph_oid(graph_name_str);
+    graph_oid = get_graph_oid(graph_name);
 
     /* Check if label with the input name already exists */
-    if (label_exists(label_name_str, graph_oid))
+    if (label_exists(label_name, graph_oid))
     {
         ereport(ERROR,
                 (errcode(ERRCODE_UNDEFINED_SCHEMA),
-                        errmsg("label \"%s\" already exists", label_name_str)));
+                        errmsg("label \"%s\" already exists", label_name)));
     }
 
     /* Create the default label tables */
-    graph = graph_name->data;
-    label = label_name->data;
-
-    rv = get_label_range_var(graph, graph_oid, AG_DEFAULT_LABEL_VERTEX);
+    rv = get_label_range_var(graph_name, graph_oid, AG_DEFAULT_LABEL_VERTEX);
 
     parent = list_make1(rv);
 
-    create_label(graph, label, LABEL_TYPE_VERTEX, parent);
+    create_label(graph_name, label_name, LABEL_TYPE_VERTEX, parent);
 
     ereport(NOTICE,
-            (errmsg("VLabel \"%s\" has been created", NameStr(*label_name))));
+            (errmsg("VLabel \"%s\" has been created", label_name)));
 
     PG_RETURN_VOID();
 }
@@ -235,17 +236,11 @@ PG_FUNCTION_INFO_V1(create_elabel);
 
 Datum create_elabel(PG_FUNCTION_ARGS)
 {
-    char *graph;
-    Name graph_name;
-    char *graph_name_str;
+    char *graph_name;
     Oid graph_oid;
     List *parent;
-
     RangeVar *rv;
-
-    char *label;
-    Name label_name;
-    char *label_name_str;
+    char *label_name;
 
     /* checking if user has not provided the graph name */
     if (PG_ARGISNULL(0))
@@ -261,41 +256,48 @@ Datum create_elabel(PG_FUNCTION_ARGS)
                 errmsg("label name must not be NULL")));
     }
 
-    graph_name = PG_GETARG_NAME(0);
-    label_name = PG_GETARG_NAME(1);
+    graph_name = PG_GETARG_CSTRING(0);
+    label_name = PG_GETARG_CSTRING(1);
 
-    graph_name_str = NameStr(*graph_name);
-    label_name_str = NameStr(*label_name);
+    /* validate the graph and label names */
+    if (is_valid_graph_name(graph_name) == 0)
+    {
+        ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("graph name is invalid")));
+    }
+
+    if (is_valid_label_name(label_name, 0) == 0)
+    {
+        ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("label name is invalid")));
+    }
 
     /* Check if graph does not exist */
-    if (!graph_exists(graph_name_str))
+    if (!graph_exists(graph_name))
     {
         ereport(ERROR,
                 (errcode(ERRCODE_UNDEFINED_SCHEMA),
-                 errmsg("graph \"%s\" does not exist.", graph_name_str)));
+                 errmsg("graph \"%s\" does not exist.", graph_name)));
     }
 
-    graph_oid = get_graph_oid(graph_name_str);
+    graph_oid = get_graph_oid(graph_name);
 
     /* Check if label with the input name already exists */
-    if (label_exists(label_name_str, graph_oid))
+    if (label_exists(label_name, graph_oid))
     {
         ereport(ERROR,
                 (errcode(ERRCODE_UNDEFINED_SCHEMA),
-                        errmsg("label \"%s\" already exists", label_name_str)));
+                        errmsg("label \"%s\" already exists", label_name)));
     }
 
     /* Create the default label tables */
-    graph = graph_name->data;
-    label = label_name->data;
-
-    rv = get_label_range_var(graph, graph_oid, AG_DEFAULT_LABEL_EDGE);
+    rv = get_label_range_var(graph_name, graph_oid, AG_DEFAULT_LABEL_EDGE);
 
     parent = list_make1(rv);
-    create_label(graph, label, LABEL_TYPE_EDGE, parent);
+    create_label(graph_name, label_name, LABEL_TYPE_EDGE, parent);
 
     ereport(NOTICE,
-            (errmsg("ELabel \"%s\" has been created", NameStr(*label_name))));
+            (errmsg("ELabel \"%s\" has been created", label_name)));
 
     PG_RETURN_VOID();
 }
@@ -318,7 +320,7 @@ void create_label(char *graph_name, char *label_name, char label_type,
     int32 label_id;
     Oid relation_id;
 
-    if (!is_valid_label(label_name, label_type))
+    if (!is_valid_label_name(label_name, label_type))
     {
         ereport(ERROR, (errcode(ERRCODE_UNDEFINED_SCHEMA),
                         errmsg("label name is invalid")));

src/backend/utils/name_validation.c

@@ -55,12 +55,17 @@ int is_valid_graph_name(const char *graph_name)
  * @param label_type label type defined in label_commands.h
  * @return int
  */
-int is_valid_label(char *label_name, char label_type)
+int is_valid_label_name(char *label_name, char label_type)
 {
     int len = strlen(label_name);
 
     if (len < MIN_LABEL_NAME_LEN || len > MAX_LABEL_NAME_LEN)
     {
+        ereport(WARNING,
+                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                 errmsg("label name length not in range (%d <= length <= %d) length = %d",
+                        MIN_LABEL_NAME_LEN, MAX_LABEL_NAME_LEN, len)));
+
         return 0;
     }
 

src/include/utils/name_validation.h

@@ -35,10 +35,10 @@
 
 #define MAX_GRAPH_NAME_LEN 63
 #define MIN_GRAPH_NAME_LEN 3
-#define MAX_LABEL_NAME_LEN 65535
+#define MAX_LABEL_NAME_LEN NAMEDATALEN -1
 #define MIN_LABEL_NAME_LEN 1
 
 int is_valid_graph_name(const char *graph_name);
-int is_valid_label(char *label_name, char label_type);
+int is_valid_label_name(char *label_name, char label_type);
 
 #endif