Cosmetic improvements in ltree code.

Add more comments in ltree.h, and correct a misstatement or two.

Use a symbol, rather than hardwired constants, for the maximum length
of an ltree label.  The max length is still hardwired in the associated
error messages, but I want to clean that up as part of a separate patch
to improve the error messages.

diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 2c483045dbbb8f52cba2fc2c65f3da50d1deb41d..e5f2710c90ecf734f8515f752720fe694781bcd1 100644 (file)
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -7,9 +7,19 @@
 #include "tsearch/ts_locale.h"
 #include "utils/memutils.h"
 
+
+/* ltree */
+
+/*
+ * We want the maximum length of a label to be encoding-independent, so
+ * set it somewhat arbitrarily at 255 characters (not bytes), while using
+ * uint16 fields to hold the byte length.
+ */
+#define LTREE_LABEL_MAX_CHARS 255
+
 typedef struct
 {
-       uint16          len;
+       uint16          len;                    /* label string length in bytes */
        char            name[FLEXIBLE_ARRAY_MEMBER];
 } ltree_level;
 
@@ -19,7 +29,8 @@ typedef struct
 typedef struct
 {
        int32           vl_len_;                /* varlena header (do not touch directly!) */
-       uint16          numlevel;
+       uint16          numlevel;               /* number of labels */
+       /* Array of maxalign'd ltree_level structs follows: */
        char            data[FLEXIBLE_ARRAY_MEMBER];
 } ltree;
 
@@ -30,25 +41,35 @@ typedef struct
 
 /* lquery */
 
+/* lquery_variant: one branch of some OR'ed alternatives */
 typedef struct
 {
-       int32           val;
-       uint16          len;
+       int32           val;                    /* CRC of label string */
+       uint16          len;                    /* label string length in bytes */
        uint8           flag;                   /* see LVAR_xxx flags below */
        char            name[FLEXIBLE_ARRAY_MEMBER];
 } lquery_variant;
 
+/*
+ * Note: these macros contain too many MAXALIGN calls and so will sometimes
+ * overestimate the space needed for an lquery_variant.  However, we can't
+ * change it without breaking on-disk compatibility for lquery.
+ */
 #define LVAR_HDRSIZE   MAXALIGN(offsetof(lquery_variant, name))
 #define LVAR_NEXT(x)   ( (lquery_variant*)( ((char*)(x)) + MAXALIGN(((lquery_variant*)(x))->len) + LVAR_HDRSIZE ) )
 
-#define LVAR_ANYEND 0x01
-#define LVAR_INCASE 0x02
-#define LVAR_SUBLEXEME 0x04
+#define LVAR_ANYEND 0x01               /* '*' flag: prefix match */
+#define LVAR_INCASE 0x02               /* '@' flag: case-insensitive match */
+#define LVAR_SUBLEXEME 0x04    /* '%' flag: word-wise match */
 
+/*
+ * In an lquery_level, "flag" contains the union of the variants' flags
+ * along with possible LQL_xxx flags; so those bit sets can't overlap.
+ */
 typedef struct
 {
        uint16          totallen;               /* total length of this level, in bytes */
-       uint16          flag;                   /* see LQL_xxx flags below */
+       uint16          flag;                   /* see LQL_xxx and LVAR_xxx flags */
        uint16          numvar;                 /* number of variants; 0 means '*' */
        uint16          low;                    /* minimum repeat count for '*' */
        uint16          high;                   /* maximum repeat count for '*' */
@@ -60,7 +81,7 @@ typedef struct
 #define LQL_NEXT(x) ( (lquery_level*)( ((char*)(x)) + MAXALIGN(((lquery_level*)(x))->totallen) ) )
 #define LQL_FIRST(x)   ( (lquery_variant*)( ((char*)(x))+LQL_HDRSIZE ) )
 
-#define LQL_NOT                0x10
+#define LQL_NOT                0x10            /* level has '!' (NOT) prefix */
 
 #ifdef LOWER_NODE
 #define FLG_CANLOOKSIGN(x) ( ( (x) & ( LQL_NOT | LVAR_ANYEND | LVAR_SUBLEXEME ) ) == 0 )
@@ -73,7 +94,7 @@ typedef struct
 {
        int32           vl_len_;                /* varlena header (do not touch directly!) */
        uint16          numlevel;               /* number of lquery_levels */
-       uint16          firstgood;
+       uint16          firstgood;              /* number of leading simple-match levels */
        uint16          flag;                   /* see LQUERY_xxx flags below */
        /* Array of maxalign'd lquery_level structs follows: */
        char            data[FLEXIBLE_ARRAY_MEMBER];

diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c
index 3ae7dc0d069dd12536c0680ea0c38b9aa02e5f5b..2503d47d14f57a4360ee9d940252cb4abc5d9f71 100644 (file)
--- a/contrib/ltree/ltree_io.c
+++ b/contrib/ltree/ltree_io.c
@@ -85,7 +85,7 @@ ltree_in(PG_FUNCTION_ARGS)
                        if (charlen == 1 && t_iseq(ptr, '.'))
                        {
                                lptr->len = ptr - lptr->start;
-                               if (lptr->wlen > 255)
+                               if (lptr->wlen > LTREE_LABEL_MAX_CHARS)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_NAME_TOO_LONG),
                                                         errmsg("name of level is too long"),
@@ -112,7 +112,7 @@ ltree_in(PG_FUNCTION_ARGS)
        if (state == LTPRS_WAITDELIM)
        {
                lptr->len = ptr - lptr->start;
-               if (lptr->wlen > 255)
+               if (lptr->wlen > LTREE_LABEL_MAX_CHARS)
                        ereport(ERROR,
                                        (errcode(ERRCODE_NAME_TOO_LONG),
                                         errmsg("name of level is too long"),
@@ -302,7 +302,7 @@ lquery_in(PG_FUNCTION_ARGS)
                                        ((lptr->flag & LVAR_SUBLEXEME) ? 1 : 0) -
                                        ((lptr->flag & LVAR_INCASE) ? 1 : 0) -
                                        ((lptr->flag & LVAR_ANYEND) ? 1 : 0);
-                               if (lptr->wlen > 255)
+                               if (lptr->wlen > LTREE_LABEL_MAX_CHARS)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_NAME_TOO_LONG),
                                                         errmsg("name of level is too long"),
@@ -318,7 +318,7 @@ lquery_in(PG_FUNCTION_ARGS)
                                        ((lptr->flag & LVAR_SUBLEXEME) ? 1 : 0) -
                                        ((lptr->flag & LVAR_INCASE) ? 1 : 0) -
                                        ((lptr->flag & LVAR_ANYEND) ? 1 : 0);
-                               if (lptr->wlen > 255)
+                               if (lptr->wlen > LTREE_LABEL_MAX_CHARS)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_NAME_TOO_LONG),
                                                         errmsg("name of level is too long"),
@@ -453,7 +453,7 @@ lquery_in(PG_FUNCTION_ARGS)
                                         errmsg("lquery syntax error"),
                                         errdetail("Unexpected end of line.")));
 
-               if (lptr->wlen > 255)
+               if (lptr->wlen > LTREE_LABEL_MAX_CHARS)
                        ereport(ERROR,
                                        (errcode(ERRCODE_NAME_TOO_LONG),
                                         errmsg("name of level is too long"),
@@ -522,12 +522,21 @@ lquery_in(PG_FUNCTION_ARGS)
                        }
                        pfree(GETVAR(curqlevel));
                        if (cur->numvar > 1 || cur->flag != 0)
+                       {
+                               /* Not a simple match */
                                wasbad = true;
+                       }
                        else if (wasbad == false)
+                       {
+                               /* count leading simple matches */
                                (result->firstgood)++;
+                       }
                }
                else
+               {
+                       /* '*', so this isn't a simple match */
                        wasbad = true;
+               }
                curqlevel = NEXTLEV(curqlevel);
                cur = LQL_NEXT(cur);
        }