diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index f4d3087bba..042d003b89 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -233,7 +233,6 @@ static Oid get_partition_operator(PartitionKey key, int col, StrategyNumber strategy, bool *need_relabel); static List *get_qual_for_hash(Relation parent, PartitionBoundSpec *spec); static List *get_qual_for_list(Relation parent, PartitionBoundSpec *spec); -static List *get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec); static List *get_qual_for_range(Relation parent, PartitionBoundSpec *spec, bool for_default); static void get_range_key_properties(PartitionKey key, int keynum, @@ -369,6 +368,7 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts, boundinfo = (PartitionBoundInfoData *) palloc0(sizeof(PartitionBoundInfoData)); boundinfo->strategy = key->strategy; + boundinfo->partnatts = key->partnatts; /* No special hash partitions. */ boundinfo->isnulls = NULL; boundinfo->default_index = -1; @@ -457,8 +457,7 @@ partition_bound_accepts_nulls(PartitionBoundInfo boundinfo) for (i = 0; i < boundinfo->ndatums; i++) { - //TODO: Handle for multi-column cases - for (j = 0; j < 1; j++) + for (j = 0; j < boundinfo->partnatts; j++) { if (boundinfo->isnulls[i][j]) return true; @@ -527,10 +526,12 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts, int next_index = 0; int default_index = -1; Datum *boundDatums; + bool *boundIsNulls; boundinfo = (PartitionBoundInfoData *) palloc0(sizeof(PartitionBoundInfoData)); boundinfo->strategy = key->strategy; + boundinfo->partnatts = key->partnatts; /* Will be set correctly below. */ boundinfo->default_index = -1; @@ -604,7 +605,8 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts, * arrays, here we just allocate a single array and below we'll just * assign a portion of this array per datum. */ - boundDatums = (Datum *) palloc(ndatums * sizeof(Datum)); + boundDatums = (Datum *) palloc(ndatums * key->partnatts * sizeof(Datum)); + boundIsNulls = (bool *) palloc(ndatums * key->partnatts * sizeof(bool)); /* * Copy values. Canonical indexes are values ranging from 0 to (nparts - @@ -616,17 +618,15 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts, { int j = 0; int orig_index = all_values[i]->index; - boundinfo->datums[i] = (Datum *) palloc(key->partnatts * sizeof(Datum)); - boundinfo->isnulls[i] = (bool *) palloc(key->partnatts * sizeof(bool)); - + boundinfo->datums[i] = &boundDatums[i * key->partnatts]; + boundinfo->isnulls[i] = &boundIsNulls[i * key->partnatts]; for (j = 0; j < key->partnatts; j++) { if (!all_values[i]->isnulls[j]) boundinfo->datums[i][j] = datumCopy(all_values[i]->values[j], key->parttypbyval[j], key->parttyplen[j]); - boundinfo->isnulls[i][j] = all_values[i]->isnulls[j]; } @@ -734,6 +734,7 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts, boundinfo = (PartitionBoundInfoData *) palloc0(sizeof(PartitionBoundInfoData)); boundinfo->strategy = key->strategy; + boundinfo->partnatts = key->partnatts; boundinfo->isnulls = NULL; /* Will be set correctly below. */ boundinfo->default_index = -1; @@ -4079,14 +4080,10 @@ make_partition_op_expr(PartitionKey key, int keynum, { case PARTITION_STRATEGY_LIST: { - List *elems = (List *) arg2; - int nelems = list_length(elems); - - Assert(nelems >= 1); - - if (key->partnatts == 1 && nelems > 1 && + if (IsA(arg2, List) && list_length((List *) arg2) > 1 && !type_is_array(key->parttypid[keynum])) { + List *elems = (List *) arg2; ArrayExpr *arrexpr; ScalarArrayOpExpr *saopexpr; @@ -4113,8 +4110,9 @@ make_partition_op_expr(PartitionKey key, int keynum, result = (Expr *) saopexpr; } - else if (key->partnatts == 1) + else if (IsA(arg2, List) && list_length((List *) arg2) > 1) { + List *elems = (List *) arg2; List *elemops = NIL; ListCell *lc; @@ -4132,14 +4130,16 @@ make_partition_op_expr(PartitionKey key, int keynum, elemops = lappend(elemops, elemop); } - result = nelems > 1 ? makeBoolExpr(OR_EXPR, elemops, -1) : linitial(elemops); + result = makeBoolExpr(OR_EXPR, elemops, -1); } else { result = make_opclause(operoid, BOOLOID, false, - arg1, arg2, + arg1, + IsA(arg2, List) ? + linitial((List *) arg2) : arg2, InvalidOid, key->partcollation[keynum]); } @@ -4259,207 +4259,39 @@ static List * get_qual_for_list(Relation parent, PartitionBoundSpec *spec) { PartitionKey key = RelationGetPartitionKey(parent); - List *result; - Expr *keyCol; - Expr *opexpr; - NullTest *nulltest; - ListCell *cell; - List *elems = NIL; - bool list_has_null = false; - - if (key->partnatts > 1) - return get_qual_for_multi_column_list(parent, spec); - - /* Construct Var or expression representing the partition column */ - if (key->partattrs[0] != 0) - keyCol = (Expr *) makeVar(1, - key->partattrs[0], - key->parttypid[0], - key->parttypmod[0], - key->parttypcoll[0], - 0); - else - keyCol = (Expr *) copyObject(linitial(key->partexprs)); - - /* - * For default list partition, collect datums for all the partitions. The - * default partition constraint should check that the partition key is - * equal to none of those. - */ - if (spec->is_default) - { - int i; - int ndatums = 0; - PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); - PartitionBoundInfo boundinfo = pdesc->boundinfo; - - if (boundinfo) - ndatums = boundinfo->ndatums; - - /* - * If default is the only partition, there need not be any partition - * constraint on it. - */ - if (ndatums == 0 && !list_has_null) - return NIL; - - for (i = 0; i < ndatums; i++) - { - Const *val; - - if (boundinfo->isnulls[i][0]) - { - list_has_null = true; - continue; - } - - /* - * Construct Const from known-not-null datum. We must be careful - * to copy the value, because our result has to be able to outlive - * the relcache entry we're copying from. - */ - val = makeConst(key->parttypid[0], - key->parttypmod[0], - key->parttypcoll[0], - key->parttyplen[0], - datumCopy(boundinfo->datums[i][0], - key->parttypbyval[0], - key->parttyplen[0]), - false, /* isnull */ - key->parttypbyval[0]); - - elems = lappend(elems, val); - } - } - else - { - /* - * Create list of Consts for the allowed values, excluding any nulls. - */ - foreach(cell, spec->listdatums) - { - ListCell *cell2 = NULL; - - foreach(cell2, (List *) lfirst(cell)) - { - Const *val = castNode(Const, lfirst(cell2)); - - if (val->constisnull) - list_has_null = true; - else - elems = lappend(elems, copyObject(val)); - } - } - } - - if (elems) - { - /* - * Generate the operator expression from the non-null partition - * values. - */ - opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber, - keyCol, (Expr *) elems); - } - else - { - /* - * If there are no partition values, we don't need an operator - * expression. - */ - opexpr = NULL; - } - - if (!list_has_null) - { - /* - * Gin up a "col IS NOT NULL" test that will be ANDed with the main - * expression. This might seem redundant, but the partition routing - * machinery needs it. - */ - nulltest = makeNode(NullTest); - nulltest->arg = keyCol; - nulltest->nulltesttype = IS_NOT_NULL; - nulltest->argisrow = false; - nulltest->location = -1; - - result = opexpr ? list_make2(nulltest, opexpr) : list_make1(nulltest); - } - else - { - /* - * Gin up a "col IS NULL" test that will be OR'd with the main - * expression. - */ - nulltest = makeNode(NullTest); - nulltest->arg = keyCol; - nulltest->nulltesttype = IS_NULL; - nulltest->argisrow = false; - nulltest->location = -1; - - if (opexpr) - { - Expr *or; - - or = makeBoolExpr(OR_EXPR, list_make2(nulltest, opexpr), -1); - result = list_make1(or); - } - else - result = list_make1(nulltest); - } - - /* - * Note that, in general, applying NOT to a constraint expression doesn't - * necessarily invert the set of rows it accepts, because NOT (NULL) is - * NULL. However, the partition constraints we construct here never - * evaluate to NULL, so applying NOT works as intended. - */ - if (spec->is_default) - { - result = list_make1(make_ands_explicit(result)); - result = list_make1(makeBoolExpr(NOT_EXPR, result, -1)); - } - - return result; -} - -/* - * get_qual_for_list_for_multi_column - * - * Returns a list of expressions to use as a list partition's constraint, - * given the parent relation and partition bound structure. - * - * Returns NIL for a default partition when it's the only partition since - * in that case there is no constraint. - */ -static List * -get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec) -{ - int i = 0; - int j = 0; - PartitionKey key = RelationGetPartitionKey(parent); - List *result; - Expr *opexpr; - NullTest *nulltest; + List *result = NIL; + Expr *datumtest; + Expr *is_null_test = NULL; + List *datum_elems = NIL; ListCell *cell; - List *elems = NIL; + bool key_is_null[PARTITION_MAX_KEYS]; + int i, + j; Expr **keyCol = (Expr **) palloc0 (key->partnatts * sizeof(Expr *)); - /* Construct Var or expression representing the partition columns */ - for (i = 0; i < key->partnatts; i++) + /* Set up partition key Vars/expressions. */ + for (i = 0, j = 0; i < key->partnatts; i++) { if (key->partattrs[i] != 0) + { keyCol[i] = (Expr *) makeVar(1, - key->partattrs[i], - key->parttypid[i], - key->parttypmod[i], - key->parttypcoll[i], - 0); + key->partattrs[i], + key->parttypid[i], + key->parttypmod[i], + key->parttypcoll[i], + 0); + } else { keyCol[i] = (Expr *) copyObject(list_nth(key->partexprs, j)); ++j; } + + /* + * While at it, also initialize IS NULL marker for every key. This is + * set to true if a given key accepts NULL. + */ + key_is_null[i] = false; } /* @@ -4469,6 +4301,7 @@ get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec) */ if (spec->is_default) { + int i; int ndatums = 0; PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); PartitionBoundInfo boundinfo = pdesc->boundinfo; @@ -4480,40 +4313,42 @@ get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec) * If default is the only partition, there need not be any partition * constraint on it. */ - if (ndatums == 0) + if (ndatums == 0 && !partition_bound_accepts_nulls(boundinfo)) return NIL; for (i = 0; i < ndatums; i++) { - List *andexpr = NIL; + List *and_args = NIL; + Expr *datum_elem = NULL; + /* + * For the multi-column case, we must make an BoolExpr that + * ANDs the results of the expressions for various columns, + * where each expresion is either an IS NULL test or an + * OpExpr comparing the column against a non-NULL datum. + */ for (j = 0; j < key->partnatts; j++) { Const *val = NULL; if (boundinfo->isnulls[i][j]) { - nulltest = makeNode(NullTest); + NullTest *nulltest = makeNode(NullTest); + + key_is_null[j] = true; + nulltest->arg = keyCol[j]; nulltest->nulltesttype = IS_NULL; nulltest->argisrow = false; nulltest->location = -1; - andexpr = lappend(andexpr, nulltest); + + if (key->partnatts > 1) + and_args = lappend(and_args, nulltest); + else + is_null_test = (Expr *) nulltest; } else { - /* - * Gin up a "col IS NOT NULL" test that will be ANDed with - * the each column's expression. This might seem redundant, - * but the partition routing machinery needs it. - */ - nulltest = makeNode(NullTest); - nulltest->arg = keyCol[j]; - nulltest->nulltesttype = IS_NOT_NULL; - nulltest->argisrow = false; - nulltest->location = -1; - andexpr = lappend(andexpr, nulltest); - val = makeConst(key->parttypid[j], key->parttypmod[j], key->parttypcoll[j], @@ -4524,68 +4359,143 @@ get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec) false, /* isnull */ key->parttypbyval[j]); - opexpr = make_partition_op_expr(key, j, BTEqualStrategyNumber, - keyCol[j], (Expr *) val); - andexpr = lappend(andexpr, opexpr); + if (key->partnatts > 1) + { + Expr *opexpr = + make_partition_op_expr(key, j, + BTEqualStrategyNumber, + keyCol[j], + (Expr *) val); + and_args = lappend(and_args, opexpr); + } + else + datum_elem = (Expr *) val; } } - opexpr = makeBoolExpr(AND_EXPR, andexpr, -1); - elems = lappend(elems, opexpr); + if (list_length(and_args) > 1) + datum_elem = makeBoolExpr(AND_EXPR, and_args, -1); + + if (datum_elem) + datum_elems = lappend(datum_elems, datum_elem); } } else { - /* - * Create list of Consts for the allowed values. - */ foreach(cell, spec->listdatums) { - List *andexpr = NIL; - ListCell *cell2 = NULL; + List *listbound = (List *) lfirst(cell); + ListCell *cell2; + List *and_args = NIL; + Expr *datum_elem = NULL; + /* + * See the comment above regarding the handling for the + * multi-column case. + */ j = 0; - foreach(cell2, (List *) lfirst(cell)) + foreach(cell2, listbound) { Const *val = castNode(Const, lfirst(cell2)); if (val->constisnull) { - nulltest = makeNode(NullTest); + NullTest *nulltest = makeNode(NullTest); + + key_is_null[j] = true; + nulltest->arg = keyCol[j]; nulltest->nulltesttype = IS_NULL; nulltest->argisrow = false; nulltest->location = -1; - andexpr = lappend(andexpr, nulltest); + + if (key->partnatts > 1) + and_args = lappend(and_args, nulltest); + else + is_null_test = (Expr *) nulltest; } else { - /* - * Gin up a "col IS NOT NULL" test that will be ANDed with - * the each column's expression. This might seem redundant, - * but the partition routing machinery needs it. - */ - nulltest = makeNode(NullTest); - nulltest->arg = keyCol[j]; - nulltest->nulltesttype = IS_NOT_NULL; - nulltest->argisrow = false; - nulltest->location = -1; - andexpr = lappend(andexpr, nulltest); - - opexpr = make_partition_op_expr(key, j, BTEqualStrategyNumber, - keyCol[j], (Expr *) val); - andexpr = lappend(andexpr, opexpr); + if (key->partnatts > 1) + { + Expr *opexpr = + make_partition_op_expr(key, j, + BTEqualStrategyNumber, + keyCol[j], + (Expr *) val); + and_args = lappend(and_args, opexpr); + } + else + datum_elem = (Expr *) val; } j++; } - opexpr = makeBoolExpr(AND_EXPR, andexpr, -1); - elems = lappend(elems, opexpr); + if (list_length(and_args) > 1) + datum_elem = makeBoolExpr(AND_EXPR, and_args, -1); + + if (datum_elem) + datum_elems = lappend(datum_elems, datum_elem); } } - opexpr = makeBoolExpr(OR_EXPR, elems, -1); - result = list_make1(opexpr); + /* + * Gin up a "col IS NOT NULL" test for every column that was not found to + * have a NULL value assigned to it. The test will be ANDed with the + * other tests. This might seem redundant, but the partition routing + * machinery needs it. + */ + for (i = 0; i < key->partnatts; i++) + { + if (!key_is_null[i]) + { + NullTest *notnull_test = NULL; + + notnull_test = makeNode(NullTest); + notnull_test->arg = keyCol[i]; + notnull_test->nulltesttype = IS_NOT_NULL; + notnull_test->argisrow = false; + notnull_test->location = -1; + result = lappend(result, notnull_test); + } + } + + /* + * Create an expression that ORs the results of per-list-bound + * expressions. For the single column case, make_partition_op_expr() + * contains the logic to optionally use a ScalarArrayOpExpr, so + * we use that. XXX fix make_partition_op_expr() to handle the + * multi-column case. + */ + if (datum_elems) + { + if (key->partnatts > 1) + datumtest = makeBoolExpr(OR_EXPR, datum_elems, -1); + else + datumtest = make_partition_op_expr(key, 0, + BTEqualStrategyNumber, + keyCol[0], + (Expr *) datum_elems); + } + else + datumtest = NULL; + + /* + * is_null_test might have been set in the single-column case if + * NULL is allowed, which OR with the datum expression if any. + */ + if (is_null_test && datumtest) + { + Expr *orexpr = makeBoolExpr(OR_EXPR, + list_make2(is_null_test, datumtest), + -1); + + result = lappend(result, orexpr); + } + else if (is_null_test) + result = lappend(result, is_null_test); + else if (datumtest) + result = lappend(result, datumtest); /* * Note that, in general, applying NOT to a constraint expression doesn't @@ -4594,7 +4504,10 @@ get_qual_for_multi_column_list(Relation parent, PartitionBoundSpec *spec) * evaluate to NULL, so applying NOT works as intended. */ if (spec->is_default) + { + result = list_make1(make_ands_explicit(result)); result = list_make1(makeBoolExpr(NOT_EXPR, result, -1)); + } return result; } diff --git a/src/include/partitioning/partbounds.h b/src/include/partitioning/partbounds.h index 0bb851801e..35991a0227 100644 --- a/src/include/partitioning/partbounds.h +++ b/src/include/partitioning/partbounds.h @@ -78,6 +78,7 @@ struct RelOptInfo; /* avoid including pathnodes.h here */ typedef struct PartitionBoundInfoData { char strategy; /* hash, list or range? */ + int partnatts; /* number of partition key columns */ int ndatums; /* Length of the datums[] array */ Datum **datums; bool **isnulls; diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 158c1d9d63..038cc5395e 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -1067,7 +1067,7 @@ Partitions: mclparted_p1 FOR VALUES IN (('a', 1)), a | text | | | | extended | | b | integer | | | | plain | | Partition of: mclparted FOR VALUES IN (('a', 1)) -Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) AND (b = 1))) +Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (((a = 'a'::text) AND (b = 1)))) \d+ mclparted_p2 Table "public.mclparted_p2" @@ -1076,7 +1076,7 @@ Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) a | text | | | | extended | | b | integer | | | | plain | | Partition of: mclparted FOR VALUES IN (('a', 2), ('b', 1), ('c', 3), ('d', 3), ('e', 3)) -Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) AND (b = 2)) OR ((a IS NOT NULL) AND (a = 'b'::text) AND (b IS NOT NULL) AND (b = 1)) OR ((a IS NOT NULL) AND (a = 'c'::text) AND (b IS NOT NULL) AND (b = 3)) OR ((a IS NOT NULL) AND (a = 'd'::text) AND (b IS NOT NULL) AND (b = 3)) OR ((a IS NOT NULL) AND (a = 'e'::text) AND (b IS NOT NULL) AND (b = 3))) +Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (((a = 'a'::text) AND (b = 2)) OR ((a = 'b'::text) AND (b = 1)) OR ((a = 'c'::text) AND (b = 3)) OR ((a = 'd'::text) AND (b = 3)) OR ((a = 'e'::text) AND (b = 3)))) \d+ mclparted_p3 Table "public.mclparted_p3" @@ -1085,7 +1085,7 @@ Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) a | text | | | | extended | | b | integer | | | | plain | | Partition of: mclparted FOR VALUES IN (('a', 3), ('a', 4), ('a', NULL), (NULL, 1)) -Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) AND (b = 3)) OR ((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) AND (b = 4)) OR ((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NULL)) OR ((a IS NULL) AND (b IS NOT NULL) AND (b = 1))) +Partition constraint: (((a = 'a'::text) AND (b = 3)) OR ((a = 'a'::text) AND (b = 4)) OR ((a = 'a'::text) AND (b IS NULL)) OR ((a IS NULL) AND (b = 1))) \d+ mclparted_p4 Table "public.mclparted_p4" @@ -1094,7 +1094,7 @@ Partition constraint: (((a IS NOT NULL) AND (a = 'a'::text) AND (b IS NOT NULL) a | text | | | | extended | | b | integer | | | | plain | | Partition of: mclparted FOR VALUES IN (('b', NULL), (NULL, 2)) -Partition constraint: (((a IS NOT NULL) AND (a = 'b'::text) AND (b IS NULL)) OR ((a IS NULL) AND (b IS NOT NULL) AND (b = 2))) +Partition constraint: (((a = 'b'::text) AND (b IS NULL)) OR ((a IS NULL) AND (b = 2))) \d+ mclparted_p5 Table "public.mclparted_p5"