0001-Fix-record-hash-support-variant-1.patch
text/plain
Filename: 0001-Fix-record-hash-support-variant-1.patch
Type: text/plain
Part: 0
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