Don't apply sortgroupref labels to a tlist that might not match.

tglsfdc · tglsfdc · commit c12f02ffc94f · 2016-06-28T10:43:11.000-04:00
If we need to use a gating Result node for pseudoconstant quals, create_scan_plan() intentionally suppresses use_physical_tlist's checks on whether there are matches for sortgroupref labels, on the grounds that we don't need matches because we can label the Result's projection output properly. However, it then called apply_pathtarget_labeling_to_tlist anyway. This oversight was harmless when written, but in commit aeb9ae6 I made that function throw an error if there was no match. Thus, the combination of a table scan, pseudoconstant quals, and a non-simple-Var sortgroupref column threw the dreaded "ORDER/GROUP BY expression not found in targetlist" error. To fix, just skip applying the labeling in this case. Per report from Rushabh Lathia. Report: <CAGPqQf2iLB8t6t-XrL-zR233DFTXxEsfVZ4WSqaYfLupEoDxXA@mail.gmail.com>
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
@@ -544,8 +544,13 @@ create_scan_plan(PlannerInfo *root, Path *best_path, int flags)
 		{
 			/* For index-only scan, the preferred tlist is the index's */
 			tlist = copyObject(((IndexPath *) best_path)->indexinfo->indextlist);
-			/* Transfer any sortgroupref data to the replacement tlist */
-			apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);
+
+			/*
+			 * Transfer any sortgroupref data to the replacement tlist, unless
+			 * we don't care because the gating Result will handle it.
+			 */
+			if (!gating_clauses)
+				apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);
 		}
 		else
 		{
@@ -557,8 +562,9 @@ create_scan_plan(PlannerInfo *root, Path *best_path, int flags)
 			}
 			else
 			{
-				/* Transfer any sortgroupref data to the replacement tlist */
-				apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);
+				/* As above, transfer sortgroupref data to replacement tlist */
+				if (!gating_clauses)
+					apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);
 			}
 		}
 	}
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
@@ -751,6 +751,25 @@ select max(unique2), generate_series(1,3) as g from tenk1 order by g desc;
  9999 | 1
 (3 rows)
 
+-- interesting corner case: constant gets optimized into a seqscan
+explain (costs off)
+  select max(100) from tenk1;
+                     QUERY PLAN                     
+----------------------------------------------------
+ Result
+   InitPlan 1 (returns $0)
+     ->  Limit
+           ->  Result
+                 One-Time Filter: (100 IS NOT NULL)
+                 ->  Seq Scan on tenk1
+(6 rows)
+
+select max(100) from tenk1;
+ max 
+-----
+ 100
+(1 row)
+
 -- try it on an inheritance tree
 create table minmaxtest(f1 int);
 create table minmaxtest1() inherits (minmaxtest);
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
@@ -270,6 +270,11 @@ explain (costs off)
   select max(unique2), generate_series(1,3) as g from tenk1 order by g desc;
 select max(unique2), generate_series(1,3) as g from tenk1 order by g desc;
 
+-- interesting corner case: constant gets optimized into a seqscan
+explain (costs off)
+  select max(100) from tenk1;
+select max(100) from tenk1;
+
 -- try it on an inheritance tree
 create table minmaxtest(f1 int);
 create table minmaxtest1() inherits (minmaxtest);

Original file line number	Diff line number	Diff line change
`@@ -544,8 +544,13 @@ create_scan_plan(PlannerInfo root, Path best_path, int flags)`
`544`	`544`	`{`
`545`	`545`	`/* For index-only scan, the preferred tlist is the index's */`
`546`	`546`	`tlist = copyObject(((IndexPath *) best_path)->indexinfo->indextlist);`
`547`		`- /* Transfer any sortgroupref data to the replacement tlist */`
`548`		`- apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);`
	`547`	`+`
	`548`	`+ /*`
	`549`	`+ * Transfer any sortgroupref data to the replacement tlist, unless`
	`550`	`+ * we don't care because the gating Result will handle it.`
	`551`	`+ */`
	`552`	`+ if (!gating_clauses)`
	`553`	`+ apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);`
`549`	`554`	`}`
`550`	`555`	`else`
`551`	`556`	`{`
`@@ -557,8 +562,9 @@ create_scan_plan(PlannerInfo root, Path best_path, int flags)`
`557`	`562`	`}`
`558`	`563`	`else`
`559`	`564`	`{`
`560`		`- /* Transfer any sortgroupref data to the replacement tlist */`
`561`		`- apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);`
	`565`	`+ /* As above, transfer sortgroupref data to replacement tlist */`
	`566`	`+ if (!gating_clauses)`
	`567`	`+ apply_pathtarget_labeling_to_tlist(tlist, best_path->pathtarget);`
`562`	`568`	`}`
`563`	`569`	`}`
`564`	`570`	`}`