On Sun, Oct 11, 2015 at 7:56 PM, Noah Misch <n...@leadboat.com> wrote:
> I see no mention in this thread of varatt_indirect, but I anticipated
> datumSerialize() reacting to it the same way datumCopy() reacts. If
> datumSerialize() can get away without doing so, why is that?
Good point. I don't think it can. Attached is a patch to fix that.
This patch also includes some somewhat-related changes to
plpgsql_param_fetch() upon which I would appreciate any input you can
provide.
plpgsql_param_fetch() assumes that it can detect whether it's being
called from copyParamList() by checking whether params !=
estate->paramLI. I don't know why this works, but I do know that this
test fails to detect the case where it's being called from
SerializeParamList(), which causes failures in exec_eval_datum() as
predicted. Calls from SerializeParamList() need the same treatment as
calls from copyParamList() because it, too, will try to evaluate every
parameter in the list. Here, I've taken the approach of making that
check unconditional, which seems to work, but I'm not sure if some
other approach would be better, such as adding an additional Boolean
(or enum context?) argument to ParamFetchHook. I *think* that
skipping this check is merely a performance optimization rather than
anything that affects correctness, and bms_is_member() is pretty
cheap, so perhaps the way I've done it is OK.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c
index 3d9e354..0d61950 100644
--- a/src/backend/utils/adt/datum.c
+++ b/src/backend/utils/adt/datum.c
@@ -264,6 +264,11 @@ datumEstimateSpace(Datum value, bool isnull, bool typByVal, int typLen)
/* no need to use add_size, can't overflow */
if (typByVal)
sz += sizeof(Datum);
+ else if (VARATT_IS_EXTERNAL_EXPANDED(value))
+ {
+ ExpandedObjectHeader *eoh = DatumGetEOHP(value);
+ sz += EOH_get_flat_size(eoh);
+ }
else
sz += datumGetSize(value, typByVal, typLen);
}
@@ -292,6 +297,7 @@ void
datumSerialize(Datum value, bool isnull, bool typByVal, int typLen,
char **start_address)
{
+ ExpandedObjectHeader *eoh = NULL;
int header;
/* Write header word. */
@@ -299,6 +305,11 @@ datumSerialize(Datum value, bool isnull, bool typByVal, int typLen,
header = -2;
else if (typByVal)
header = -1;
+ else if (VARATT_IS_EXTERNAL_EXPANDED(value))
+ {
+ eoh = DatumGetEOHP(value);
+ header = EOH_get_flat_size(eoh);
+ }
else
header = datumGetSize(value, typByVal, typLen);
memcpy(*start_address, &header, sizeof(int));
@@ -312,6 +323,11 @@ datumSerialize(Datum value, bool isnull, bool typByVal, int typLen,
memcpy(*start_address, &value, sizeof(Datum));
*start_address += sizeof(Datum);
}
+ else if (eoh)
+ {
+ EOH_flatten_into(eoh, (void *) *start_address, header);
+ *start_address += header;
+ }
else
{
memcpy(*start_address, DatumGetPointer(value), header);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index c73f20b..346e8f8 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -5696,21 +5696,17 @@ plpgsql_param_fetch(ParamListInfo params, int paramid)
/* now we can access the target datum */
datum = estate->datums[dno];
- /* need to behave slightly differently for shared and unshared arrays */
- if (params != estate->paramLI)
- {
- /*
- * We're being called, presumably from copyParamList(), for cursor
- * parameters. Since copyParamList() will try to materialize every
- * single parameter slot, it's important to do nothing when asked for
- * a datum that's not supposed to be used by this SQL expression.
- * Otherwise we risk failures in exec_eval_datum(), not to mention
- * possibly copying a lot more data than the cursor actually uses.
- */
- if (!bms_is_member(dno, expr->paramnos))
- return;
- }
- else
+ /*
+ * Since copyParamList() and SerializeParamList() will try to materialize
+ * every single parameter slot, it's important to do nothing when asked for
+ * a datum that's not supposed to be used by this SQL expression.
+ * Otherwise we risk failures in exec_eval_datum(), not to mention
+ * possibly copying a lot more data than the cursor actually uses.
+ */
+ if (!bms_is_member(dno, expr->paramnos))
+ return;
+
+ if (params == estate->paramLI)
{
/*
* Normal evaluation cases. We don't need to sanity-check dno, but we
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
