Revisit handling of UNION ALL subqueries with non-Var output columns.

In commit 57664ed25e5dea117158a2e663c29e60b3546e1c I tried to fix a bug
reported by Teodor Sigaev by making non-simple-Var output columns distinct
(by wrapping their expressions with dummy PlaceHolderVar nodes).  This did
not work too well.  Commit b28ffd0fcc583c1811e5295279e7d4366c3cae6c fixed
some ensuing problems with matching to child indexes, but per a recent
report from Claus Stadler, constraint exclusion of UNION ALL subqueries was
still broken, because constant-simplification didn't handle the injected
PlaceHolderVars well either.  On reflection, the original patch was quite
misguided: there is no reason to expect that EquivalenceClass child members
will be distinct.  So instead of trying to make them so, we should ensure
that we can cope with the situation when they're not.

Accordingly, this patch reverts the code changes in the above-mentioned
commits (though the regression test cases they added stay).  Instead, I've
added assorted defenses to make sure that duplicate EC child members don't
cause any problems.  Teodor's original problem ("MergeAppend child's
targetlist doesn't match MergeAppend") is addressed more directly by
revising prepare_sort_from_pathkeys to let the parent MergeAppend's sort
list guide creation of each child's sort list.

In passing, get rid of add_sort_column; as far as I can tell, testing for
duplicate sort keys at this stage is dead code.  Certainly it doesn't
trigger often enough to be worth expending cycles on in ordinary queries.
And keeping the test would've greatly complicated the new logic in
prepare_sort_from_pathkeys, because comparing pathkey list entries against
a previous output array requires that we not skip any entries in the list.

Back-patch to 9.1, like the previous patches.  The only known issue in
this area that wasn't caused by the ill-advised previous patches was the
MergeAppend planning failure, which of course is not relevant before 9.1.
It's possible that we need some of the new defenses against duplicate child
EC entries in older branches, but until there's some clear evidence of that
I'm going to refrain from back-patching further.

4 files changed, 100 insertions, 2 deletions

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 6ef1cbdec79..d8f20e8ce2c 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1067,11 +1067,11 @@ drop cascades to table matest2
 drop cascades to table matest3
 --
 -- Test merge-append for UNION ALL append relations
--- Check handling of duplicated, constant, or volatile targetlist items
 --
 set enable_seqscan = off;
 set enable_indexscan = on;
 set enable_bitmapscan = off;
+-- Check handling of duplicated, constant, or volatile targetlist items
 explain (costs off)
 SELECT thousand, tenthous FROM tenk1
 UNION ALL
@@ -1120,6 +1120,61 @@ ORDER BY thousand, tenthous;
                ->  Index Only Scan using tenk1_thous_tenthous on tenk1
 (7 rows)
 
+-- Check min/max aggregate optimization
+explain (costs off)
+SELECT min(x) FROM
+  (SELECT unique1 AS x FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x FROM tenk1 b) s;
+                             QUERY PLAN                             
+--------------------------------------------------------------------
+ Result
+   InitPlan 1 (returns $0)
+     ->  Limit
+           ->  Merge Append
+                 Sort Key: a.unique1
+                 ->  Index Only Scan using tenk1_unique1 on tenk1 a
+                       Index Cond: (unique1 IS NOT NULL)
+                 ->  Index Only Scan using tenk1_unique2 on tenk1 b
+                       Index Cond: (unique2 IS NOT NULL)
+(9 rows)
+
+explain (costs off)
+SELECT min(y) FROM
+  (SELECT unique1 AS x, unique1 AS y FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x, unique2 AS y FROM tenk1 b) s;
+                             QUERY PLAN                             
+--------------------------------------------------------------------
+ Result
+   InitPlan 1 (returns $0)
+     ->  Limit
+           ->  Merge Append
+                 Sort Key: a.unique1
+                 ->  Index Only Scan using tenk1_unique1 on tenk1 a
+                       Index Cond: (unique1 IS NOT NULL)
+                 ->  Index Only Scan using tenk1_unique2 on tenk1 b
+                       Index Cond: (unique2 IS NOT NULL)
+(9 rows)
+
+-- XXX planner doesn't recognize that index on unique2 is sufficiently sorted
+explain (costs off)
+SELECT x, y FROM
+  (SELECT thousand AS x, tenthous AS y FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x, unique2 AS y FROM tenk1 b) s
+ORDER BY x, y;
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Result
+   ->  Merge Append
+         Sort Key: a.thousand, a.tenthous
+         ->  Index Only Scan using tenk1_thous_tenthous on tenk1 a
+         ->  Sort
+               Sort Key: b.unique2, b.unique2
+               ->  Index Only Scan using tenk1_unique2 on tenk1 b
+(7 rows)
+
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 2913f3d8f37..405c5847cd1 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -503,3 +503,17 @@ explain (costs off)
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
+-- Test constraint exclusion of UNION ALL subqueries
+explain (costs off)
+ SELECT * FROM
+  (SELECT 1 AS t, * FROM tenk1 a
+   UNION ALL
+   SELECT 2 AS t, * FROM tenk1 b) c
+ WHERE t = 2;
+           QUERY PLAN            
+---------------------------------
+ Result
+   ->  Append
+         ->  Seq Scan on tenk1 b
+(3 rows)
+
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 6fdbd18c251..eec8192928d 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -326,13 +326,13 @@ drop table matest0 cascade;
 
 --
 -- Test merge-append for UNION ALL append relations
--- Check handling of duplicated, constant, or volatile targetlist items
 --
 
 set enable_seqscan = off;
 set enable_indexscan = on;
 set enable_bitmapscan = off;
 
+-- Check handling of duplicated, constant, or volatile targetlist items
 explain (costs off)
 SELECT thousand, tenthous FROM tenk1
 UNION ALL
@@ -351,6 +351,27 @@ UNION ALL
 SELECT thousand, random()::integer FROM tenk1
 ORDER BY thousand, tenthous;
 
+-- Check min/max aggregate optimization
+explain (costs off)
+SELECT min(x) FROM
+  (SELECT unique1 AS x FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x FROM tenk1 b) s;
+
+explain (costs off)
+SELECT min(y) FROM
+  (SELECT unique1 AS x, unique1 AS y FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x, unique2 AS y FROM tenk1 b) s;
+
+-- XXX planner doesn't recognize that index on unique2 is sufficiently sorted
+explain (costs off)
+SELECT x, y FROM
+  (SELECT thousand AS x, tenthous AS y FROM tenk1 a
+   UNION ALL
+   SELECT unique2 AS x, unique2 AS y FROM tenk1 b) s
+ORDER BY x, y;
+
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index b5c2128f3f3..752ae470f0d 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -199,3 +199,11 @@ explain (costs off)
 reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
+
+-- Test constraint exclusion of UNION ALL subqueries
+explain (costs off)
+ SELECT * FROM
+  (SELECT 1 AS t, * FROM tenk1 a
+   UNION ALL
+   SELECT 2 AS t, * FROM tenk1 b) c
+ WHERE t = 2;