From 3d3d6150401906dad0b4381f24630a39881c0106 Mon Sep 17 00:00:00 2001
From: Markus Sintonen <markus.sintonen@smartly.io>
Date: Sat, 15 Feb 2020 16:54:38 +0200
Subject: [PATCH] Add comment about NOT_EXPR. Treat it as invalid constraint
 for safety.

---
 .../distributed/planner/shard_pruning.c       |  7 +++
 .../regress/expected/multi_hash_pruning.out   | 61 +++++++++++++++++++
 src/test/regress/expected/null_parameters.out |  3 +
 src/test/regress/sql/multi_hash_pruning.sql   | 16 +++++
 4 files changed, 87 insertions(+)

diff --git a/src/backend/distributed/planner/shard_pruning.c b/src/backend/distributed/planner/shard_pruning.c
index 427feb157..25200e39d 100644
--- a/src/backend/distributed/planner/shard_pruning.c
+++ b/src/backend/distributed/planner/shard_pruning.c
@@ -559,6 +559,13 @@ BuildPruningTree(Node *node, PruningTreeBuildContext *context)
 
 		if (boolExpr->boolop == NOT_EXPR)
 		{
+			/*
+			 * We should not encounter NOT_EXPR nodes.
+			 * Postgres standard planner applies De Morgan's laws to remove them.
+			 * But if we encounter one, we treat it as invalid constraint for pruning.
+			 */
+			context->current->hasInvalidConstraints = true;
+
 			return false;
 		}
 		else if (context->current->boolop != boolExpr->boolop)
diff --git a/src/test/regress/expected/multi_hash_pruning.out b/src/test/regress/expected/multi_hash_pruning.out
index d293e3f5e..a6676a70e 100644
--- a/src/test/regress/expected/multi_hash_pruning.out
+++ b/src/test/regress/expected/multi_hash_pruning.out
@@ -995,4 +995,65 @@ DEBUG:  assigned task to node localhost:xxxxx
      4
 (1 row)
 
+-- Check that NOT is handled with NEQs ORed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey != 2 OR o_orderkey != 3);
+DEBUG:  Creating router plan
+DEBUG:  Plan is router executable
+ count
+---------------------------------------------------------------------
+     0
+(1 row)
+
+-- Check that NOT is handled with EQs ORed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey = 2 OR o_orderkey = 3);
+DEBUG:  no valid constraints found
+DEBUG:  shard count: 4
+DEBUG:  Router planner cannot handle multi-shard select queries
+DEBUG:  no valid constraints found
+DEBUG:  shard count: 4
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+ count
+---------------------------------------------------------------------
+     2
+(1 row)
+
+-- Check that NOT is handled with NEQs ANDed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey != 2 AND o_orderkey != 3);
+DEBUG:  constraint value: 2
+DEBUG:  constraint value: 3
+DEBUG:  shard count: 2
+DEBUG:  Router planner cannot handle multi-shard select queries
+DEBUG:  constraint value: 2
+DEBUG:  constraint value: 3
+DEBUG:  shard count: 2
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+ count
+---------------------------------------------------------------------
+     2
+(1 row)
+
+-- Check that NOT is handled with EQs ANDed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey = 2 AND o_orderkey = 3);
+DEBUG:  no valid constraints found
+DEBUG:  shard count: 4
+DEBUG:  Router planner cannot handle multi-shard select queries
+DEBUG:  no valid constraints found
+DEBUG:  shard count: 4
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+DEBUG:  assigned task to node localhost:xxxxx
+ count
+---------------------------------------------------------------------
+     4
+(1 row)
+
 SET client_min_messages TO DEFAULT;
diff --git a/src/test/regress/expected/null_parameters.out b/src/test/regress/expected/null_parameters.out
index 8f6c17922..6661ae8ec 100644
--- a/src/test/regress/expected/null_parameters.out
+++ b/src/test/regress/expected/null_parameters.out
@@ -1429,6 +1429,9 @@ DEBUG:  Deferred pruning for a fast-path router query
 DEBUG:  Creating router plan
 DEBUG:  Plan is router executable
 EXECUTE null_update_on_text_param_and_in(NULL);
+DEBUG:  Deferred pruning for a fast-path router query
+DEBUG:  Creating router plan
+DEBUG:  Plan is router executable
 PREPARE null_update_on_text_param_and_in_2(text) AS UPDATE text_dist_column SET value = '' WHERE key IN ($1, 'test');
 EXECUTE null_update_on_text_param_and_in_2(NULL);
 DEBUG:  Creating router plan
diff --git a/src/test/regress/sql/multi_hash_pruning.sql b/src/test/regress/sql/multi_hash_pruning.sql
index 19e024510..91a63a8e9 100644
--- a/src/test/regress/sql/multi_hash_pruning.sql
+++ b/src/test/regress/sql/multi_hash_pruning.sql
@@ -258,4 +258,20 @@ SELECT count(*) FROM orders_hash_partitioned
 SELECT count(*) FROM orders_hash_partitioned
 	WHERE o_orderkey = 1 OR o_orderkey IS NOT NULL;
 
+-- Check that NOT is handled with NEQs ORed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey != 2 OR o_orderkey != 3);
+
+-- Check that NOT is handled with EQs ORed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey = 2 OR o_orderkey = 3);
+
+-- Check that NOT is handled with NEQs ANDed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey != 2 AND o_orderkey != 3);
+
+-- Check that NOT is handled with EQs ANDed
+SELECT count(*) FROM orders_hash_partitioned
+	WHERE NOT (o_orderkey = 2 AND o_orderkey = 3);
+
 SET client_min_messages TO DEFAULT;