0001-Fix-record-hash-support-variant-1.patch

text/plain

Filename: 0001-Fix-record-hash-support-variant-1.patch
Type: text/plain
Part: 0
Message: Re: BUG #17158: Distinct ROW fails with Postgres 14

Patch

Format: format-patch
Series: patch 0001
Subject: Fix record hash support, variant 1
File+
src/backend/parser/analyze.c 18 5
src/backend/rewrite/rewriteSearchCycle.c 3 3
src/backend/utils/adt/arrayfuncs.c 20 2
src/backend/utils/cache/typcache.c 7 4
src/include/parser/analyze.h 1 1
From 6fd5c5e265099ca0167cdb6db401bfbb08d38c2b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 27 Aug 2021 10:47:00 +0200
Subject: [PATCH] Fix record hash support, variant 1

---
 src/backend/parser/analyze.c             | 23 ++++++++++++++++++-----
 src/backend/rewrite/rewriteSearchCycle.c |  6 +++---
 src/backend/utils/adt/arrayfuncs.c       | 22 ++++++++++++++++++++--
 src/backend/utils/cache/typcache.c       | 11 +++++++----
 src/include/parser/analyze.h             |  2 +-
 5 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 15669c8238..6af6f09f94 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1852,9 +1852,12 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 
 /*
  * Make a SortGroupClause node for a SetOperationStmt's groupClauses
+ *
+ * If need_hash is true, the caller is indicating that they need hash support
+ * or they will fail.  So look extra hard for hash support.
  */
 SortGroupClause *
-makeSortGroupClauseForSetOp(Oid rescoltype)
+makeSortGroupClauseForSetOp(Oid rescoltype, bool need_hash)
 {
 	SortGroupClause *grpcl = makeNode(SortGroupClause);
 	Oid			sortop;
@@ -1867,6 +1870,15 @@ makeSortGroupClauseForSetOp(Oid rescoltype)
 							 &sortop, &eqop, NULL,
 							 &hashable);
 
+	/*
+	 * The type cache doesn't believe that record is hashable (see
+	 * cache_record_field_properties()), but if the caller really needs hash
+	 * support, we can assume it does.  Worst case, if any components of the
+	 * record don't support hashing, we will fail at execution.
+	 */
+	if (need_hash && (rescoltype == RECORDOID || rescoltype == RECORDARRAYOID))
+		hashable = true;
+
 	/* we don't have a tlist yet, so can't assign sortgrouprefs */
 	grpcl->tleSortGroupRef = 0;
 	grpcl->eqop = eqop;
@@ -2027,6 +2039,8 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
 		ListCell   *ltl;
 		ListCell   *rtl;
 		const char *context;
+		bool recursive = (pstate->p_parent_cte &&
+						  pstate->p_parent_cte->cterecursive);
 
 		context = (stmt->op == SETOP_UNION ? "UNION" :
 				   (stmt->op == SETOP_INTERSECT ? "INTERSECT" :
@@ -2048,9 +2062,7 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
 		 * containing CTE as having those result columns.  We should do this
 		 * only at the topmost setop of the CTE, of course.
 		 */
-		if (isTopLevel &&
-			pstate->p_parent_cte &&
-			pstate->p_parent_cte->cterecursive)
+		if (isTopLevel && recursive)
 			determineRecursiveColTypes(pstate, op->larg, ltargetlist);
 
 		/*
@@ -2182,8 +2194,9 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
 				setup_parser_errposition_callback(&pcbstate, pstate,
 												  bestlocation);
 
+				/* If it's a recursive union, we need to require hashing support. */
 				op->groupClauses = lappend(op->groupClauses,
-										   makeSortGroupClauseForSetOp(rescoltype));
+										   makeSortGroupClauseForSetOp(rescoltype, recursive));
 
 				cancel_parser_errposition_callback(&pcbstate);
 			}
diff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.c
index c50ebdba24..ef38f4025a 100644
--- a/src/backend/rewrite/rewriteSearchCycle.c
+++ b/src/backend/rewrite/rewriteSearchCycle.c
@@ -594,7 +594,7 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
 		sos->colCollations = lappend_oid(sos->colCollations, InvalidOid);
 		if (!sos->all)
 			sos->groupClauses = lappend(sos->groupClauses,
-										makeSortGroupClauseForSetOp(search_seq_type));
+										makeSortGroupClauseForSetOp(search_seq_type, true));
 	}
 	if (cte->cycle_clause)
 	{
@@ -603,14 +603,14 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
 		sos->colCollations = lappend_oid(sos->colCollations, cte->cycle_clause->cycle_mark_collation);
 		if (!sos->all)
 			sos->groupClauses = lappend(sos->groupClauses,
-										makeSortGroupClauseForSetOp(cte->cycle_clause->cycle_mark_type));
+										makeSortGroupClauseForSetOp(cte->cycle_clause->cycle_mark_type, true));
 
 		sos->colTypes = lappend_oid(sos->colTypes, RECORDARRAYOID);
 		sos->colTypmods = lappend_int(sos->colTypmods, -1);
 		sos->colCollations = lappend_oid(sos->colCollations, InvalidOid);
 		if (!sos->all)
 			sos->groupClauses = lappend(sos->groupClauses,
-										makeSortGroupClauseForSetOp(RECORDARRAYOID));
+										makeSortGroupClauseForSetOp(RECORDARRAYOID, true));
 	}
 
 	/*
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index 04d487c544..e7209b4d6b 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -29,6 +29,7 @@
 #include "utils/arrayaccess.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/selfuncs.h"
@@ -3960,6 +3961,8 @@ hash_array(PG_FUNCTION_ARGS)
 	char		typalign;
 	int			i;
 	array_iter	iter;
+	FmgrInfo	record_hash_proc_finfo;
+	FmgrInfo   *my_hash_proc_finfo;
 
 	/*
 	 * We arrange to look up the hash function only once per series of calls,
@@ -3973,7 +3976,7 @@ hash_array(PG_FUNCTION_ARGS)
 	{
 		typentry = lookup_type_cache(element_type,
 									 TYPECACHE_HASH_PROC_FINFO);
-		if (!OidIsValid(typentry->hash_proc_finfo.fn_oid))
+		if (!OidIsValid(typentry->hash_proc_finfo.fn_oid) && element_type != RECORDOID)
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_FUNCTION),
 					 errmsg("could not identify a hash function for type %s",
@@ -3984,10 +3987,25 @@ hash_array(PG_FUNCTION_ARGS)
 	typbyval = typentry->typbyval;
 	typalign = typentry->typalign;
 
+	/*
+	 * The type cache doesn't believe that record is hashable (see
+	 * cache_record_field_properties()), but since we're here, we're committed
+	 * to hashing, so we can assume it does.  Worst case, if any components of
+	 * the record don't support hashing, we will fail at execution.
+	 */
+	if (element_type == RECORDOID)
+	{
+		fmgr_info_cxt(F_HASH_RECORD, &record_hash_proc_finfo,
+					  CacheMemoryContext);
+		my_hash_proc_finfo = &record_hash_proc_finfo;
+	}
+	else
+		my_hash_proc_finfo = &typentry->hash_proc_finfo;
+
 	/*
 	 * apply the hash function to each array element.
 	 */
-	InitFunctionCallInfoData(*locfcinfo, &typentry->hash_proc_finfo, 1,
+	InitFunctionCallInfoData(*locfcinfo, my_hash_proc_finfo, 1,
 							 PG_GET_COLLATION(), NULL, NULL);
 
 	/* Loop over source data */
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 326fae62e2..70e5c51297 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1515,14 +1515,17 @@ cache_record_field_properties(TypeCacheEntry *typentry)
 	/*
 	 * For type RECORD, we can't really tell what will work, since we don't
 	 * have access here to the specific anonymous type.  Just assume that
-	 * everything will (we may get a failure at runtime ...)
+	 * equality and comparison will (we may get a failure at runtime).  We
+	 * could also claim that hashing works, but then if code that has the
+	 * option between a comparison-based (sort-based) and a hash-based plan
+	 * chooses hashing, stuff could fail that would otherwise work if it chose
+	 * a comparison-based plan.  In practice more types support comparison
+	 * than hashing.
 	 */
 	if (typentry->type_id == RECORDOID)
 	{
 		typentry->flags |= (TCFLAGS_HAVE_FIELD_EQUALITY |
-							TCFLAGS_HAVE_FIELD_COMPARE |
-							TCFLAGS_HAVE_FIELD_HASHING |
-							TCFLAGS_HAVE_FIELD_EXTENDED_HASHING);
+							TCFLAGS_HAVE_FIELD_COMPARE);
 	}
 	else if (typentry->typtype == TYPTYPE_COMPOSITE)
 	{
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index 6716db6c13..89d61238e9 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -48,6 +48,6 @@ extern void applyLockingClause(Query *qry, Index rtindex,
 extern List *BuildOnConflictExcludedTargetlist(Relation targetrel,
 											   Index exclRelIndex);
 
-extern SortGroupClause *makeSortGroupClauseForSetOp(Oid rescoltype);
+extern SortGroupClause *makeSortGroupClauseForSetOp(Oid rescoltype, bool need_hash);
 
 #endif							/* ANALYZE_H */
-- 
2.33.0