On Tue, Jul 19, 2022 at 10:57 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2022-07-19 14:30:34 +0700, John Naylor wrote:
> > I'm thinking where the first few attributes are fixed length, not null,
and
> > (because of AIX) not double-aligned, we can do a single memcpy on
multiple
> > columns at once. That will still be a common pattern after namedata is
> > varlen. Otherwise, use helper functions/macros similar to the above but
> > instead of passing a tuple descriptor, use info we have at compile time.
>
> I think that might be over-optimizing things. I don't think we do these
> conversions at a rate that's high enough to warrant it - the common stuff
> should be in relcache etc. It's possible that we might want to optimize
the
> catcache case specifically - but that'd be more optimizing memory usage
than
> "conversion" imo.
Okay, here is a hackish experiment that applies on top of v2 but also
invalidates some of that earlier work. Since there is already a pg_cast.c,
I demoed a new function there which looks like this:
void
Deform_pg_cast_tuple(Form_pg_cast pg_cast_struct, HeapTuple pg_cast_tuple,
TupleDesc pg_cast_desc)
{
Datum values[Natts_pg_cast];
bool isnull[Natts_pg_cast];
heap_deform_tuple(pg_cast_tuple, pg_cast_desc, values, isnull);
pg_cast_struct->oid = DatumGetObjectId(values[Anum_pg_cast_oid - 1]);
pg_cast_struct->castsource =
DatumGetObjectId(values[Anum_pg_cast_castsource - 1]);
pg_cast_struct->casttarget =
DatumGetObjectId(values[Anum_pg_cast_casttarget - 1]);
pg_cast_struct->castfunc =
DatumGetObjectId(values[Anum_pg_cast_castfunc - 1]);
pg_cast_struct->castcontext =
DatumGetChar(values[Anum_pg_cast_castcontext - 1]);
pg_cast_struct->castmethod =
DatumGetChar(values[Anum_pg_cast_castmethod - 1]);
}
For the general case we can use pg_*_deform.c or something like that, with
extern declarations in the main headers. To get this to work, I had to add
a couple pointless table open/close calls to get the tuple descriptor,
since currently the whole tuple is stored in the syscache, but that's not
good even as a temporary measure. Storing the full struct in the syscache
is a good future step, as noted upthread, but to get there without a bunch
more churn, maybe the above function can copy the tuple descriptor into a
local stack variable from an expanded version of schemapg.h. Once the
deformed structs are stored in caches, I imagine most of the times we want
to deform are when we have the table open, and we can pass the descriptor
as above without additional code.
--
John Naylor
EDB: http://www.enterprisedb.com
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 66e98daad1..cdc26b1118 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -3028,7 +3028,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
break;
}
- GETSTRUCT_MEMCPY(pg_cast, &castForm, tup);
+ GETSTRUCT_MEMCPY(pg_cast, &castForm, tup, RelationGetDescr(castDesc));
appendStringInfo(&buffer, _("cast from %s to %s"),
format_type_be(castForm.castsource),
@@ -4870,7 +4870,7 @@ getObjectIdentityParts(const ObjectAddress *object,
break;
}
- GETSTRUCT_MEMCPY(pg_cast, &castForm, tup);
+ GETSTRUCT_MEMCPY(pg_cast, &castForm, tup, RelationGetDescr(castRel));
appendStringInfo(&buffer, "(%s AS %s)",
format_type_be_qualified(castForm.castsource),
diff --git a/src/backend/catalog/pg_cast.c b/src/backend/catalog/pg_cast.c
index 1812bb7fcc..83379f3505 100644
--- a/src/backend/catalog/pg_cast.c
+++ b/src/backend/catalog/pg_cast.c
@@ -27,6 +27,23 @@
#include "utils/rel.h"
#include "utils/syscache.h"
+// WIP: #include from generated .c file?
+void
+Deform_pg_cast_tuple(Form_pg_cast pg_cast_struct, HeapTuple pg_cast_tuple, TupleDesc pg_cast_desc)
+{
+ Datum values[Natts_pg_cast];
+ bool isnull[Natts_pg_cast];
+
+ heap_deform_tuple(pg_cast_tuple, pg_cast_desc, values, isnull);
+
+ pg_cast_struct->oid = DatumGetObjectId(values[Anum_pg_cast_oid - 1]);
+ pg_cast_struct->castsource = DatumGetObjectId(values[Anum_pg_cast_castsource - 1]);
+ pg_cast_struct->casttarget = DatumGetObjectId(values[Anum_pg_cast_casttarget - 1]);
+ pg_cast_struct->castfunc = DatumGetObjectId(values[Anum_pg_cast_castfunc - 1]);
+ pg_cast_struct->castcontext = DatumGetChar(values[Anum_pg_cast_castcontext - 1]);
+ pg_cast_struct->castmethod = DatumGetChar(values[Anum_pg_cast_castmethod - 1]);
+}
+
/*
* ----------------------------------------------------------------
* CastCreate
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 0bd90fd5e8..f4bac9d171 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -30,6 +30,9 @@
#include "utils/lsyscache.h"
#include "utils/syscache.h"
#include "utils/typcache.h"
+// WIP
+#include "access/table.h"
+#include "utils/rel.h"
static Node *coerce_type_typmod(Node *node,
@@ -2997,6 +3000,7 @@ IsBinaryCoercible(Oid srctype, Oid targettype)
HeapTuple tuple;
FormData_pg_cast castForm;
bool result;
+ Relation castDesc;
/* Fast path if same type */
if (srctype == targettype)
@@ -3056,12 +3060,15 @@ IsBinaryCoercible(Oid srctype, Oid targettype)
ObjectIdGetDatum(targettype));
if (!HeapTupleIsValid(tuple))
return false; /* no cast */
- GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple);
+
+ castDesc = table_open(CastRelationId, AccessShareLock); // WIP: this would be better with generated "compact tuple descriptors"
+ GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple, RelationGetDescr(castDesc));
result = (castForm.castmethod == COERCION_METHOD_BINARY &&
castForm.castcontext == COERCION_CODE_IMPLICIT);
ReleaseSysCache(tuple);
+ table_close(castDesc, AccessShareLock); // FIXME
return result;
}
@@ -3123,7 +3130,9 @@ find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
FormData_pg_cast castForm;
CoercionContext castcontext;
- GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple);
+ // TODO: the syscache should have the deformed struct instead of the tuple
+ Relation castDesc = table_open(CastRelationId, AccessShareLock);
+ GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple, RelationGetDescr(castDesc));
/* convert char value for castcontext to CoercionContext enum */
switch (castForm.castcontext)
@@ -3167,6 +3176,7 @@ find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
}
ReleaseSysCache(tuple);
+ table_close(castDesc, AccessShareLock); // FIXME
}
else
{
@@ -3290,10 +3300,12 @@ find_typmod_coercion_function(Oid typeId,
if (HeapTupleIsValid(tuple))
{
FormData_pg_cast castForm;
+ Relation castDesc = table_open(CastRelationId, AccessShareLock); // WIP
- GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple);
+ GETSTRUCT_MEMCPY(pg_cast, &castForm, tuple, RelationGetDescr(castDesc));
*funcid = castForm.castfunc;
ReleaseSysCache(tuple);
+ table_close(castDesc, AccessShareLock); // FIXME
}
if (!OidIsValid(*funcid))
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index f300030504..ac9ecabc52 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -657,7 +657,7 @@ struct MinimalTupleData
* GETSTRUCT - given a HeapTuple pointer, deform the user data into the passed struct
*/
// WIP: The _MEMCPY suffix is a temporary scaffold to allow partial progress one catalog at a time.
-#define GETSTRUCT_MEMCPY(CAT, STRUCT, TUP) Deform_##CAT##_tuple(STRUCT, (char *) ((TUP)->t_data) + (TUP)->t_data->t_hoff)
+#define GETSTRUCT_MEMCPY(CAT, STRUCT, TUP, DESC) Deform_##CAT##_tuple((STRUCT), (TUP), (DESC))
/*
* Accessor macros to be used with HeapTuple pointers.
diff --git a/src/include/catalog/pg_cast.h b/src/include/catalog/pg_cast.h
index 4887ea16bd..fde557477a 100644
--- a/src/include/catalog/pg_cast.h
+++ b/src/include/catalog/pg_cast.h
@@ -56,9 +56,6 @@ CATALOG(pg_cast,2605,CastRelationId)
*/
typedef FormData_pg_cast *Form_pg_cast;
-/* now that we've defined the struct type, include the deform function */
-#include "catalog/pg_cast_deform.h"
-
DECLARE_UNIQUE_INDEX_PKEY(pg_cast_oid_index, 2660, CastOidIndexId, on pg_cast using btree(oid oid_ops));
DECLARE_UNIQUE_INDEX(pg_cast_source_target_index, 2661, CastSourceTargetIndexId, on pg_cast using btree(castsource oid_ops, casttarget oid_ops));
@@ -94,6 +91,7 @@ typedef enum CoercionMethod
#endif /* EXPOSE_TO_CLIENT_CODE */
+extern void Deform_pg_cast_tuple(Form_pg_cast pg_cast_struct, HeapTuple pg_cast_tuple, TupleDesc pg_cast_desc);
extern ObjectAddress CastCreate(Oid sourcetypeid,
Oid targettypeid,
