]> granicus.if.org Git - postgresql/commitdiff
Fix multiple problems with satisfies_hash_partition.
authorRobert Haas <rhaas@postgresql.org>
Tue, 21 Nov 2017 18:06:32 +0000 (13:06 -0500)
committerRobert Haas <rhaas@postgresql.org>
Tue, 21 Nov 2017 18:06:32 +0000 (13:06 -0500)
Fix the function header comment to describe the actual behavior.
Check that table OID, modulus, and remainder arguments are not NULL
before accessing them.  Check that the modulus and remainder are
sensible.  If the table OID doesn't exist, return NULL instead of
emitting an internal error, similar to what we do elsewhere.  Check
that the actual argument types match, or at least are binary coercible
to, the expected argument types.  Correctly handle invocation of this
function using the VARIADIC syntax.  Add regression tests.

Robert Haas and Amul Sul, per a report by Andreas Seltenreich and
subsequent followup investigation.

Discussion: http://postgr.es/m/871sl4sdrv.fsf@ansel.ydns.eu

src/backend/catalog/partition.c
src/test/regress/expected/hash_part.out [new file with mode: 0644]
src/test/regress/parallel_schedule
src/test/regress/serial_schedule
src/test/regress/sql/hash_part.sql [new file with mode: 0644]

index 67d4c2a09bc1a3d96c6a985da80389491a4c51ea..9a44cceb226b4c7840ab8e1de8f05210d604f53e 100644 (file)
@@ -40,6 +40,7 @@
 #include "optimizer/planmain.h"
 #include "optimizer/prep.h"
 #include "optimizer/var.h"
+#include "parser/parse_coerce.h"
 #include "rewrite/rewriteManip.h"
 #include "storage/lmgr.h"
 #include "utils/array.h"
@@ -3085,9 +3086,11 @@ compute_hash_value(PartitionKey key, Datum *values, bool *isnull)
 /*
  * satisfies_hash_partition
  *
- * This is a SQL-callable function for use in hash partition constraints takes
- * an already computed hash values of each partition key attribute, and combine
- * them into a single hash value by calling hash_combine64.
+ * This is an SQL-callable function for use in hash partition constraints.
+ * The first three arguments are the parent table OID, modulus, and remainder.
+ * The remaining arguments are the value of the partitioning columns (or
+ * expressions); these are hashed and the results are combined into a single
+ * hash value by calling hash_combine64.
  *
  * Returns true if remainder produced when this computed single hash value is
  * divided by the given modulus is equal to given remainder, otherwise false.
@@ -3100,60 +3103,160 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
        typedef struct ColumnsHashData
        {
                Oid                     relid;
-               int16           nkeys;
+               int                     nkeys;
+               Oid                     variadic_type;
+               int16           variadic_typlen;
+               bool            variadic_typbyval;
+               char            variadic_typalign;
                FmgrInfo        partsupfunc[PARTITION_MAX_KEYS];
        }                       ColumnsHashData;
-       Oid                     parentId = PG_GETARG_OID(0);
-       int                     modulus = PG_GETARG_INT32(1);
-       int                     remainder = PG_GETARG_INT32(2);
-       short           nkeys = PG_NARGS() - 3;
-       int                     i;
+       Oid                     parentId;
+       int                     modulus;
+       int                     remainder;
        Datum           seed = UInt64GetDatum(HASH_PARTITION_SEED);
        ColumnsHashData *my_extra;
        uint64          rowHash = 0;
 
+       /* Return null if the parent OID, modulus, or remainder is NULL. */
+       if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2))
+               PG_RETURN_NULL();
+       parentId = PG_GETARG_OID(0);
+       modulus = PG_GETARG_INT32(1);
+       remainder = PG_GETARG_INT32(2);
+
+       /* Sanity check modulus and remainder. */
+       if (modulus <= 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("modulus for hash partition must be a positive integer")));
+       if (remainder < 0)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("remainder for hash partition must be a non-negative integer")));
+       if (remainder >= modulus)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("remainder for hash partition must be less than modulus")));
+
        /*
         * Cache hash function information.
         */
        my_extra = (ColumnsHashData *) fcinfo->flinfo->fn_extra;
-       if (my_extra == NULL || my_extra->nkeys != nkeys ||
-               my_extra->relid != parentId)
+       if (my_extra == NULL || my_extra->relid != parentId)
        {
                Relation        parent;
                PartitionKey key;
-               int j;
-
-               fcinfo->flinfo->fn_extra =
-                       MemoryContextAllocZero(fcinfo->flinfo->fn_mcxt,
-                                                                  offsetof(ColumnsHashData, partsupfunc) +
-                                                                  sizeof(FmgrInfo) * nkeys);
-               my_extra = (ColumnsHashData *) fcinfo->flinfo->fn_extra;
-               my_extra->nkeys = nkeys;
-               my_extra->relid = parentId;
+               int                     j;
 
                /* Open parent relation and fetch partition keyinfo */
-               parent = heap_open(parentId, AccessShareLock);
+               parent = try_relation_open(parentId, AccessShareLock);
+               if (parent == NULL)
+                       PG_RETURN_NULL();
                key = RelationGetPartitionKey(parent);
 
-               Assert(key->partnatts == nkeys);
-               for (j = 0; j < nkeys; ++j)
-                       fmgr_info_copy(&my_extra->partsupfunc[j],
-                                                  key->partsupfunc,
+               /* Reject parent table that is not hash-partitioned. */
+               if (parent->rd_rel->relkind != RELKIND_PARTITIONED_TABLE ||
+                       key->strategy != PARTITION_STRATEGY_HASH)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("\"%s\" is not a hash partitioned table",
+                                                       get_rel_name(parentId))));
+
+               if (!get_fn_expr_variadic(fcinfo->flinfo))
+               {
+                       int                     nargs = PG_NARGS() - 3;
+
+                       /* complain if wrong number of column values */
+                       if (key->partnatts != nargs)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                errmsg("number of partitioning columns (%d) does not match number of partition keys provided (%d)",
+                                                               key->partnatts, nargs)));
+
+                       /* allocate space for our cache */
+                       fcinfo->flinfo->fn_extra =
+                               MemoryContextAllocZero(fcinfo->flinfo->fn_mcxt,
+                                                                          offsetof(ColumnsHashData, partsupfunc) +
+                                                                          sizeof(FmgrInfo) * nargs);
+                       my_extra = (ColumnsHashData *) fcinfo->flinfo->fn_extra;
+                       my_extra->relid = parentId;
+                       my_extra->nkeys = key->partnatts;
+
+                       /* check argument types and save fmgr_infos */
+                       for (j = 0; j < key->partnatts; ++j)
+                       {
+                               Oid                     argtype = get_fn_expr_argtype(fcinfo->flinfo, j + 3);
+
+                               if (argtype != key->parttypid[j] && !IsBinaryCoercible(argtype, key->parttypid[j]))
+                                       ereport(ERROR,
+                                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                        errmsg("column %d of the partition key has type \"%s\", but supplied value is of type \"%s\"",
+                                                                       j + 1, format_type_be(key->parttypid[j]), format_type_be(argtype))));
+
+                               fmgr_info_copy(&my_extra->partsupfunc[j],
+                                                          &key->partsupfunc[j],
+                                                          fcinfo->flinfo->fn_mcxt);
+                       }
+
+               }
+               else
+               {
+                       ArrayType  *variadic_array = PG_GETARG_ARRAYTYPE_P(3);
+
+                       /* allocate space for our cache -- just one FmgrInfo in this case */
+                       fcinfo->flinfo->fn_extra =
+                               MemoryContextAllocZero(fcinfo->flinfo->fn_mcxt,
+                                                                          offsetof(ColumnsHashData, partsupfunc) +
+                                                                          sizeof(FmgrInfo));
+                       my_extra = (ColumnsHashData *) fcinfo->flinfo->fn_extra;
+                       my_extra->relid = parentId;
+                       my_extra->nkeys = key->partnatts;
+                       my_extra->variadic_type = ARR_ELEMTYPE(variadic_array);
+                       get_typlenbyvalalign(my_extra->variadic_type,
+                                                                &my_extra->variadic_typlen,
+                                                                &my_extra->variadic_typbyval,
+                                                                &my_extra->variadic_typalign);
+
+                       /* check argument types */
+                       for (j = 0; j < key->partnatts; ++j)
+                               if (key->parttypid[j] != my_extra->variadic_type)
+                                       ereport(ERROR,
+                                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                        errmsg("column %d of the partition key has type \"%s\", but supplied value is of type \"%s\"",
+                                                                       j + 1,
+                                                                       format_type_be(key->parttypid[j]),
+                                                                       format_type_be(my_extra->variadic_type))));
+
+                       fmgr_info_copy(&my_extra->partsupfunc[0],
+                                                  &key->partsupfunc[0],
                                                   fcinfo->flinfo->fn_mcxt);
+               }
 
                /* Hold lock until commit */
-               heap_close(parent, NoLock);
+               relation_close(parent, NoLock);
        }
 
-       for (i = 0; i < nkeys; i++)
+       if (!OidIsValid(my_extra->variadic_type))
        {
-               /* keys start from fourth argument of function. */
-               int                     argno = i + 3;
+               int                     nkeys = my_extra->nkeys;
+               int                     i;
+
+               /*
+                * For a non-variadic call, neither the number of arguments nor their
+                * types can change across calls, so avoid the expense of rechecking
+                * here.
+                */
 
-               if (!PG_ARGISNULL(argno))
+               for (i = 0; i < nkeys; i++)
                {
                        Datum           hash;
 
+                       /* keys start from fourth argument of function. */
+                       int                     argno = i + 3;
+
+                       if (PG_ARGISNULL(argno))
+                               continue;
+
                        Assert(OidIsValid(my_extra->partsupfunc[i].fn_oid));
 
                        hash = FunctionCall2(&my_extra->partsupfunc[i],
@@ -3164,6 +3267,45 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
                        rowHash = hash_combine64(rowHash, DatumGetUInt64(hash));
                }
        }
+       else
+       {
+               ArrayType  *variadic_array = PG_GETARG_ARRAYTYPE_P(3);
+               int                     i;
+               int                     nelems;
+               Datum      *datum;
+               bool       *isnull;
+
+               deconstruct_array(variadic_array,
+                                                 my_extra->variadic_type,
+                                                 my_extra->variadic_typlen,
+                                                 my_extra->variadic_typbyval,
+                                                 my_extra->variadic_typalign,
+                                                 &datum, &isnull, &nelems);
+
+               /* complain if wrong number of column values */
+               if (nelems != my_extra->nkeys)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("number of partitioning columns (%d) does not match number of partition keys provided (%d)",
+                                                       my_extra->nkeys, nelems)));
+
+               for (i = 0; i < nelems; i++)
+               {
+                       Datum           hash;
+
+                       if (isnull[i])
+                               continue;
+
+                       Assert(OidIsValid(my_extra->partsupfunc[0].fn_oid));
+
+                       hash = FunctionCall2(&my_extra->partsupfunc[0],
+                                                                datum[i],
+                                                                seed);
+
+                       /* Form a single 64-bit hash value */
+                       rowHash = hash_combine64(rowHash, DatumGetUInt64(hash));
+               }
+       }
 
        PG_RETURN_BOOL(rowHash % modulus == remainder);
 }
diff --git a/src/test/regress/expected/hash_part.out b/src/test/regress/expected/hash_part.out
new file mode 100644 (file)
index 0000000..9e9e56f
--- /dev/null
@@ -0,0 +1,113 @@
+--
+-- Hash partitioning.
+--
+CREATE OR REPLACE FUNCTION hashint4_noop(int4, int8) RETURNS int8 AS
+$$SELECT coalesce($1,0)::int8$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR CLASS test_int4_ops FOR TYPE int4 USING HASH AS
+OPERATOR 1 = , FUNCTION 2 hashint4_noop(int4, int8);
+CREATE OR REPLACE FUNCTION hashtext_length(text, int8) RETURNS int8 AS
+$$SELECT length(coalesce($1,''))::int8$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR CLASS test_text_ops FOR TYPE text USING HASH AS
+OPERATOR 1 = , FUNCTION 2 hashtext_length(text, int8);
+CREATE TABLE mchash (a int, b text, c jsonb)
+  PARTITION BY HASH (a test_int4_ops, b test_text_ops);
+CREATE TABLE mchash1
+  PARTITION OF mchash FOR VALUES WITH (MODULUS 4, REMAINDER 0);
+-- invalid OID, no such table
+SELECT satisfies_hash_partition(0, 4, 0, NULL);
+ satisfies_hash_partition 
+--------------------------
+(1 row)
+
+-- not partitioned
+SELECT satisfies_hash_partition('tenk1'::regclass, 4, 0, NULL);
+ERROR:  "tenk1" is not a hash partitioned table
+-- partition rather than the parent
+SELECT satisfies_hash_partition('mchash1'::regclass, 4, 0, NULL);
+ERROR:  "mchash1" is not a hash partitioned table
+-- invalid modulus
+SELECT satisfies_hash_partition('mchash'::regclass, 0, 0, NULL);
+ERROR:  modulus for hash partition must be a positive integer
+-- remainder too small
+SELECT satisfies_hash_partition('mchash'::regclass, 1, -1, NULL);
+ERROR:  remainder for hash partition must be a non-negative integer
+-- remainder too large
+SELECT satisfies_hash_partition('mchash'::regclass, 1, 1, NULL);
+ERROR:  remainder for hash partition must be less than modulus
+-- modulus is null
+SELECT satisfies_hash_partition('mchash'::regclass, NULL, 0, NULL);
+ satisfies_hash_partition 
+--------------------------
+(1 row)
+
+-- remainder is null
+SELECT satisfies_hash_partition('mchash'::regclass, 4, NULL, NULL);
+ satisfies_hash_partition 
+--------------------------
+(1 row)
+
+-- too many arguments
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, NULL::int, NULL::text, NULL::json);
+ERROR:  number of partitioning columns (2) does not match number of partition keys provided (3)
+-- too few arguments
+SELECT satisfies_hash_partition('mchash'::regclass, 3, 1, NULL::int);
+ERROR:  number of partitioning columns (2) does not match number of partition keys provided (1)
+-- wrong argument type
+SELECT satisfies_hash_partition('mchash'::regclass, 2, 1, NULL::int, NULL::int);
+ERROR:  column 2 of the partition key has type "text", but supplied value is of type "integer"
+-- ok, should be false
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, 0, ''::text);
+ satisfies_hash_partition 
+--------------------------
+ f
+(1 row)
+
+-- ok, should be true
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, 1, ''::text);
+ satisfies_hash_partition 
+--------------------------
+ t
+(1 row)
+
+-- argument via variadic syntax, should fail because not all partitioning
+-- columns are of the correct type
+SELECT satisfies_hash_partition('mchash'::regclass, 2, 1,
+                                                               variadic array[1,2]::int[]);
+ERROR:  column 2 of the partition key has type "text", but supplied value is of type "integer"
+-- multiple partitioning columns of the same type
+CREATE TABLE mcinthash (a int, b int, c jsonb)
+  PARTITION BY HASH (a test_int4_ops, b test_int4_ops);
+-- now variadic should work, should be false
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[0, 0]);
+ satisfies_hash_partition 
+--------------------------
+ f
+(1 row)
+
+-- should be true
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[1, 0]);
+ satisfies_hash_partition 
+--------------------------
+ t
+(1 row)
+
+-- wrong length
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[]::int[]);
+ERROR:  number of partitioning columns (2) does not match number of partition keys provided (0)
+-- wrong type
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[now(), now()]);
+ERROR:  column 1 of the partition key has type "integer", but supplied value is of type "timestamp with time zone"
+-- cleanup
+DROP TABLE mchash;
+DROP TABLE mcinthash;
+DROP OPERATOR CLASS test_text_ops USING hash;
+DROP OPERATOR CLASS test_int4_ops USING hash;
+DROP FUNCTION hashint4_noop(int4, int8);
+DROP FUNCTION hashtext_length(text, int8);
index aa5e6af6218a807418fe3f84c5e111187a0c5713..1a3ac4c1f940d7f2f51d3d97733a7a207c2cdd12 100644 (file)
@@ -116,7 +116,7 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare without_oid c
 # ----------
 # Another group of parallel tests
 # ----------
-test: identity partition_join reloptions
+test: identity partition_join reloptions hash_part
 
 # event triggers cannot run concurrently with any test that runs DDL
 test: event_trigger
index 3866314a92241acbfa83c7d95243ca93ec3b0af3..a205e5d05c7a8e52449584a745e5fb352741beab 100644 (file)
@@ -181,5 +181,6 @@ test: xml
 test: identity
 test: partition_join
 test: reloptions
+test: hash_part
 test: event_trigger
 test: stats
diff --git a/src/test/regress/sql/hash_part.sql b/src/test/regress/sql/hash_part.sql
new file mode 100644 (file)
index 0000000..94c5eaa
--- /dev/null
@@ -0,0 +1,90 @@
+--
+-- Hash partitioning.
+--
+
+CREATE OR REPLACE FUNCTION hashint4_noop(int4, int8) RETURNS int8 AS
+$$SELECT coalesce($1,0)::int8$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR CLASS test_int4_ops FOR TYPE int4 USING HASH AS
+OPERATOR 1 = , FUNCTION 2 hashint4_noop(int4, int8);
+
+CREATE OR REPLACE FUNCTION hashtext_length(text, int8) RETURNS int8 AS
+$$SELECT length(coalesce($1,''))::int8$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR CLASS test_text_ops FOR TYPE text USING HASH AS
+OPERATOR 1 = , FUNCTION 2 hashtext_length(text, int8);
+
+CREATE TABLE mchash (a int, b text, c jsonb)
+  PARTITION BY HASH (a test_int4_ops, b test_text_ops);
+CREATE TABLE mchash1
+  PARTITION OF mchash FOR VALUES WITH (MODULUS 4, REMAINDER 0);
+
+-- invalid OID, no such table
+SELECT satisfies_hash_partition(0, 4, 0, NULL);
+
+-- not partitioned
+SELECT satisfies_hash_partition('tenk1'::regclass, 4, 0, NULL);
+
+-- partition rather than the parent
+SELECT satisfies_hash_partition('mchash1'::regclass, 4, 0, NULL);
+
+-- invalid modulus
+SELECT satisfies_hash_partition('mchash'::regclass, 0, 0, NULL);
+
+-- remainder too small
+SELECT satisfies_hash_partition('mchash'::regclass, 1, -1, NULL);
+
+-- remainder too large
+SELECT satisfies_hash_partition('mchash'::regclass, 1, 1, NULL);
+
+-- modulus is null
+SELECT satisfies_hash_partition('mchash'::regclass, NULL, 0, NULL);
+
+-- remainder is null
+SELECT satisfies_hash_partition('mchash'::regclass, 4, NULL, NULL);
+
+-- too many arguments
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, NULL::int, NULL::text, NULL::json);
+
+-- too few arguments
+SELECT satisfies_hash_partition('mchash'::regclass, 3, 1, NULL::int);
+
+-- wrong argument type
+SELECT satisfies_hash_partition('mchash'::regclass, 2, 1, NULL::int, NULL::int);
+
+-- ok, should be false
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, 0, ''::text);
+
+-- ok, should be true
+SELECT satisfies_hash_partition('mchash'::regclass, 4, 0, 1, ''::text);
+
+-- argument via variadic syntax, should fail because not all partitioning
+-- columns are of the correct type
+SELECT satisfies_hash_partition('mchash'::regclass, 2, 1,
+                                                               variadic array[1,2]::int[]);
+
+-- multiple partitioning columns of the same type
+CREATE TABLE mcinthash (a int, b int, c jsonb)
+  PARTITION BY HASH (a test_int4_ops, b test_int4_ops);
+
+-- now variadic should work, should be false
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[0, 0]);
+
+-- should be true
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[1, 0]);
+
+-- wrong length
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[]::int[]);
+
+-- wrong type
+SELECT satisfies_hash_partition('mcinthash'::regclass, 4, 0,
+                                                               variadic array[now(), now()]);
+
+-- cleanup
+DROP TABLE mchash;
+DROP TABLE mcinthash;
+DROP OPERATOR CLASS test_text_ops USING hash;
+DROP OPERATOR CLASS test_int4_ops USING hash;
+DROP FUNCTION hashint4_noop(int4, int8);
+DROP FUNCTION hashtext_length(text, int8);