fix empty array expression in get_qual_for_list()

Started by Jeevan Ladheover 8 years ago4 messages
#1Jeevan Ladhe
jeevan.ladhe@enterprisedb.com
1 attachment(s)

Hi,

In case of list partitioned table:
1. If there is a partition accepting only null values and nothing else, then
currently the partition constraints for such a partition are constructed as
"((a IS NULL) OR (a = ANY (ARRAY[]::integer[])))".
I think there it is better to avoid constructing an empty array to avoid
executing ANY expression.

2.Also, we are constructing an expression using index 0 of arrays in
PartitionKey since currently we have only one column for list partition in
key,
added an assert for this.

PFA.

Regards,
Jeevan Ladhe

Attachments:

fix_empty_arry_get_qual_for_list.patchapplication/octet-stream; name=fix_empty_arry_get_qual_for_list.patchDownload
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index af07cd0..b43ae71 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1305,12 +1305,18 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
 	List	   *result;
 	Expr	   *keyCol;
 	ArrayExpr  *arr;
-	Expr	   *opexpr;
+	Expr	   *opexpr = NULL;
 	NullTest   *nulltest;
 	ListCell   *cell;
 	List	   *arrelems = NIL;
 	bool		list_has_null = false;
 
+	/*
+	 * Only single-column list partitioning is supported, so we are worried
+	 * only about the partition key with index 0.
+	 */
+	Assert(key->partnatts == 1);
+
 	/* Construct Var or expression representing the partition column */
 	if (key->partattrs[0] != 0)
 		keyCol = (Expr *) makeVar(1,
@@ -1333,20 +1339,23 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
 			arrelems = lappend(arrelems, copyObject(val));
 	}
 
-	/* Construct an ArrayExpr for the non-null partition values */
-	arr = makeNode(ArrayExpr);
-	arr->array_typeid = !type_is_array(key->parttypid[0])
-		? get_array_type(key->parttypid[0])
-		: key->parttypid[0];
-	arr->array_collid = key->parttypcoll[0];
-	arr->element_typeid = key->parttypid[0];
-	arr->elements = arrelems;
-	arr->multidims = false;
-	arr->location = -1;
-
-	/* Generate the main expression, i.e., keyCol = ANY (arr) */
-	opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
-									keyCol, (Expr *) arr);
+	if (arrelems)
+	{
+		/* Construct an ArrayExpr for the non-null partition values */
+		arr = makeNode(ArrayExpr);
+		arr->array_typeid = !type_is_array(key->parttypid[0])
+			? get_array_type(key->parttypid[0])
+			: key->parttypid[0];
+		arr->array_collid = key->parttypcoll[0];
+		arr->element_typeid = key->parttypid[0];
+		arr->elements = arrelems;
+		arr->multidims = false;
+		arr->location = -1;
+
+		/* Generate the main expression, i.e., keyCol = ANY (arr) */
+		opexpr = make_partition_op_expr(key, 0, BTEqualStrategyNumber,
+										keyCol, (Expr *) arr);
+	}
 
 	if (!list_has_null)
 	{
@@ -1361,7 +1370,7 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
 		nulltest->argisrow = false;
 		nulltest->location = -1;
 
-		result = list_make2(nulltest, opexpr);
+		result = (opexpr) ? list_make2(nulltest, opexpr) : list_make1(nulltest);
 	}
 	else
 	{
@@ -1377,8 +1386,13 @@ get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec)
 		nulltest->argisrow = false;
 		nulltest->location = -1;
 
-		or = makeBoolExpr(OR_EXPR, list_make2(nulltest, opexpr), -1);
-		result = list_make1(or);
+		if (opexpr)
+		{
+			or = makeBoolExpr(OR_EXPR, list_make2(nulltest, opexpr), -1);
+			result = list_make1(or);
+		}
+		else
+			result = list_make1(nulltest);
 	}
 
 	return result;
#2Jeevan Ladhe
jeevan.ladhe@enterprisedb.com
In reply to: Jeevan Ladhe (#1)
Re: fix empty array expression in get_qual_for_list()

Here is an example:

create table t1 ( a int) partition by list (a);
create table t11 partition of t1 for values in (null);

*Current constraints:*
postgres=# \d+ t11;
Table "public.t11"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
Partition of: t1 FOR VALUES IN (NULL)
Partition constraint: ((a IS NULL) OR (a = ANY (ARRAY[]::integer[])))

*Constraints after fix:*
postgres=# \d+ t11;
Table "public.t11"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | | | plain |
|
Partition of: t1 FOR VALUES IN (NULL)
Partition constraint: (a IS NULL)

Regards,
Jeevan Ladhe

On Mon, Jun 26, 2017 at 4:53 PM, Jeevan Ladhe <jeevan.ladhe@enterprisedb.com

Show quoted text

wrote:

Hi,

In case of list partitioned table:
1. If there is a partition accepting only null values and nothing else,
then
currently the partition constraints for such a partition are constructed as
"((a IS NULL) OR (a = ANY (ARRAY[]::integer[])))".
I think there it is better to avoid constructing an empty array to avoid
executing ANY expression.

2.Also, we are constructing an expression using index 0 of arrays in
PartitionKey since currently we have only one column for list partition in
key,
added an assert for this.

PFA.

Regards,
Jeevan Ladhe

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jeevan Ladhe (#1)
Re: fix empty array expression in get_qual_for_list()

Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> writes:

In case of list partitioned table:
1. If there is a partition accepting only null values and nothing else, then
currently the partition constraints for such a partition are constructed as
"((a IS NULL) OR (a = ANY (ARRAY[]::integer[])))".
I think there it is better to avoid constructing an empty array to avoid
executing ANY expression.

Looks like a good idea, pushed with trivial stylistic adjustments.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Jeevan Ladhe
jeevan.ladhe@enterprisedb.com
In reply to: Tom Lane (#3)
Re: fix empty array expression in get_qual_for_list()

On Mon, Jun 26, 2017 at 8:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> writes:

In case of list partitioned table:
1. If there is a partition accepting only null values and nothing else,

then

currently the partition constraints for such a partition are constructed

as

"((a IS NULL) OR (a = ANY (ARRAY[]::integer[])))".
I think there it is better to avoid constructing an empty array to avoid
executing ANY expression.

Looks like a good idea, pushed with trivial stylistic adjustments.

Thanks Tom for taking care of this.

Regards,
Jeevan Ladhe