Re-allow DISTINCT in pl/pgsql expressions.

tglsfdc · tglsfdc · commit 7cd9765f9bd3 · 2021-01-22T16:26:22.000-05:00
I'd omitted this from the grammar in commit c9d5298, figuring that it wasn't worth supporting. However we already have one complaint, so it seems that judgment was wrong. It doesn't require a huge amount of code, so add it back. (I'm still drawing the line at UNION/INTERSECT/EXCEPT though: those'd require an unreasonable amount of grammar refactoring, and the single-result-row restriction makes them near useless anyway.) Also rethink the documentation: this behavior is a property of all pl/pgsql expressions, not just assignments. Discussion: https://postgr.es/m/20210122134106.e94c5cd7@mail.verfriemelt.org
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
@@ -917,6 +917,24 @@ PREPARE <replaceable>statement_name</replaceable>(integer, integer) AS SELECT $1
      they are useful to know when trying to diagnose a problem.
      More information appears in <xref linkend="plpgsql-plan-caching"/>.
     </para>
+
+    <para>
+     Since an <replaceable>expression</replaceable> is converted to a
+     <literal>SELECT</literal> command, it can contain the same clauses
+     that an ordinary <literal>SELECT</literal> would, except that it
+     cannot include a top-level <literal>UNION</literal>,
+     <literal>INTERSECT</literal>, or <literal>EXCEPT</literal> clause.
+     Thus for example one could test whether a table is non-empty with
+<programlisting>
+IF count(*) &gt; 0 FROM my_table THEN ...
+</programlisting>
+     since the <replaceable>expression</replaceable>
+     between <literal>IF</literal> and <literal>THEN</literal> is parsed as
+     though it were <literal>SELECT count(*) &gt; 0 FROM my_table</literal>.
+     The <literal>SELECT</literal> must produce a single column, and not
+     more than one row.  (If it produces no rows, the result is taken as
+     NULL.)
+    </para>
   </sect1>
 
   <sect1 id="plpgsql-statements">
@@ -973,20 +991,6 @@ my_array[1:3] := array[1,2,3];
 complex_array[n].realpart = 12.3;
 </programlisting>
     </para>
-
-    <para>
-     It's useful to know that what follows the assignment operator is
-     essentially treated as a <literal>SELECT</literal> command; as long
-     as it returns a single row and column, it will work.  Thus for example
-     one can write something like
-<programlisting>
-total_sales := sum(quantity) from sales;
-</programlisting>
-     This provides an effect similar to the single-row <literal>SELECT
-     ... INTO</literal> syntax described in
-     <xref linkend="plpgsql-statements-sql-onerow"/>.  However, that syntax
-     is more portable.
-    </para>
    </sect2>
 
    <sect2 id="plpgsql-statements-sql-noresult">
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
@@ -2466,7 +2466,7 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt)
 
 	/*
 	 * The rest mostly matches transformSelectStmt, except that we needn't
-	 * consider WITH or DISTINCT, and we build a targetlist our own way.
+	 * consider WITH or INTO, and we build a targetlist our own way.
 	 */
 	qry->commandType = CMD_SELECT;
 	pstate->p_is_insert = false;
@@ -2590,10 +2590,29 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt)
 											EXPR_KIND_GROUP_BY,
 											false /* allow SQL92 rules */ );
 
-	/* No DISTINCT clause */
-	Assert(!sstmt->distinctClause);
-	qry->distinctClause = NIL;
-	qry->hasDistinctOn = false;
+	if (sstmt->distinctClause == NIL)
+	{
+		qry->distinctClause = NIL;
+		qry->hasDistinctOn = false;
+	}
+	else if (linitial(sstmt->distinctClause) == NULL)
+	{
+		/* We had SELECT DISTINCT */
+		qry->distinctClause = transformDistinctClause(pstate,
+													  &qry->targetList,
+													  qry->sortClause,
+													  false);
+		qry->hasDistinctOn = false;
+	}
+	else
+	{
+		/* We had SELECT DISTINCT ON */
+		qry->distinctClause = transformDistinctOnClause(pstate,
+														sstmt->distinctClause,
+														&qry->targetList,
+														qry->sortClause);
+		qry->hasDistinctOn = true;
+	}
 
 	/* transform LIMIT */
 	qry->limitOffset = transformLimitClause(pstate, sstmt->limitOffset,
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
@@ -389,7 +389,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				OptTableElementList TableElementList OptInherit definition
 				OptTypedTableElementList TypedTableElementList
 				reloptions opt_reloptions
-				OptWith distinct_clause opt_definition func_args func_args_list
+				OptWith opt_definition func_args func_args_list
 				func_args_with_defaults func_args_with_defaults_list
 				aggr_args aggr_args_list
 				func_as createfunc_opt_list alterfunc_opt_list
@@ -401,6 +401,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 				name_list role_list from_clause from_list opt_array_bounds
 				qualified_name_list any_name any_name_list type_name_list
 				any_operator expr_list attrs
+				distinct_clause opt_distinct_clause
 				target_list opt_target_list insert_column_list set_target_list
 				set_clause_list set_clause
 				def_list operator_def_list indirection opt_indirection
@@ -11260,6 +11261,11 @@ select_clause:
  * As with select_no_parens, simple_select cannot have outer parentheses,
  * but can have parenthesized subclauses.
  *
+ * It might appear that we could fold the first two alternatives into one
+ * by using opt_distinct_clause.  However, that causes a shift/reduce conflict
+ * against INSERT ... SELECT ... ON CONFLICT.  We avoid the ambiguity by
+ * requiring SELECT DISTINCT [ON] to be followed by a non-empty target_list.
+ *
  * Note that sort clauses cannot be included at this level --- SQL requires
  *		SELECT foo UNION SELECT bar ORDER BY baz
  * to be parsed as
@@ -11497,8 +11503,13 @@ opt_all_clause:
 			| /*EMPTY*/
 		;
 
+opt_distinct_clause:
+			distinct_clause							{ $$ = $1; }
+			| opt_all_clause						{ $$ = NIL; }
+		;
+
 opt_sort_clause:
-			sort_clause								{ $$ = $1;}
+			sort_clause								{ $$ = $1; }
 			| /*EMPTY*/								{ $$ = NIL; }
 		;
 
@@ -15065,32 +15076,33 @@ role_list:	RoleSpec
  * Therefore the returned struct is a SelectStmt.
  *****************************************************************************/
 
-PLpgSQL_Expr: opt_target_list
+PLpgSQL_Expr: opt_distinct_clause opt_target_list
 			from_clause where_clause
 			group_clause having_clause window_clause
 			opt_sort_clause opt_select_limit opt_for_locking_clause
 				{
 					SelectStmt *n = makeNode(SelectStmt);
 
-					n->targetList = $1;
-					n->fromClause = $2;
-					n->whereClause = $3;
-					n->groupClause = $4;
-					n->havingClause = $5;
-					n->windowClause = $6;
-					n->sortClause = $7;
-					if ($8)
+					n->distinctClause = $1;
+					n->targetList = $2;
+					n->fromClause = $3;
+					n->whereClause = $4;
+					n->groupClause = $5;
+					n->havingClause = $6;
+					n->windowClause = $7;
+					n->sortClause = $8;
+					if ($9)
 					{
-						n->limitOffset = $8->limitOffset;
-						n->limitCount = $8->limitCount;
+						n->limitOffset = $9->limitOffset;
+						n->limitCount = $9->limitCount;
 						if (!n->sortClause &&
-							$8->limitOption == LIMIT_OPTION_WITH_TIES)
+							$9->limitOption == LIMIT_OPTION_WITH_TIES)
 							ereport(ERROR,
 									(errcode(ERRCODE_SYNTAX_ERROR),
 									 errmsg("WITH TIES cannot be specified without ORDER BY clause")));
-						n->limitOption = $8->limitOption;
+						n->limitOption = $9->limitOption;
 					}
-					n->lockingClause = $9;
+					n->lockingClause = $10;
 					$$ = (Node *) n;
 				}
 		;
