pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] rewrite ExecPartitionCheckEmitError
5+ messages / 4 participants
[nested] [flat]
* [PATCH] rewrite ExecPartitionCheckEmitError
@ 2018-11-14 23:57 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 5+ messages in thread
From: Alvaro Herrera @ 2018-11-14 23:57 UTC (permalink / raw)
---
src/backend/executor/execMain.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d83d296d82..78c8ce4935 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1837,11 +1837,9 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- Relation orig_rel = rel;
- TupleDesc tupdesc = RelationGetDescr(rel);
- char *val_desc;
- Bitmapset *modifiedCols;
+ Oid root_relid;
+ TupleDesc tupdesc;
+ char *valdsc;
Bitmapset *insertedCols;
Bitmapset *updatedCols;
@@ -1851,11 +1849,13 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
*/
if (resultRelInfo->ri_PartitionRoot)
{
- TupleDesc old_tupdesc = RelationGetDescr(rel);
+ TupleDesc old_tupdesc;
AttrNumber *map;
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
+ root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
/* a reverse map */
map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
gettext_noop("could not convert row type"));
@@ -1868,20 +1868,24 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
slot = execute_attr_map_slot(map, slot,
MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
}
+ else
+ {
+ root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ }
insertedCols = GetInsertedColumns(resultRelInfo, estate);
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ valdsc = ExecBuildSlotValueDescription(root_relid,
+ slot,
+ tupdesc,
+ bms_union(insertedCols, updatedCols),
+ 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
- RelationGetRelationName(orig_rel)),
- val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
+ RelationGetRelationName(resultRelInfo->ri_RelationDesc)),
+ valdsc ? errdetail("Failing row contains %s.", valdsc) : 0));
}
/*
--
2.11.0
--ki3z2n5j73ow2eol--
^ permalink raw reply [nested|flat] 5+ messages in thread
* rewrite ExecPartitionCheckEmitError
@ 2018-12-06 22:22 Alvaro Herrera <alvherre@2ndquadrant.com>
2018-12-07 20:32 ` Re: rewrite ExecPartitionCheckEmitError Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Alvaro Herrera @ 2018-12-06 22:22 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Just on cleanliness grounds, I propose to rewrite the function in
$SUBJECT. I came across this while reviewing some already-committed
patch for partition pruning, and it's been sitting in my laptop ever
since.
I think the current coding is too convoluted and hard to follow. The
patch makes it much simpler (IMO).
--
Álvaro Herrera
Attachments:
0001-rewrite-ExecPartitionCheckEmitError.patch (text/x-diff, 2.8K) download
From 8c8ac713ac168b2678925e58cdfc83694a8fa5b9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 14 Nov 2018 20:57:19 -0300
Subject: [PATCH] rewrite ExecPartitionCheckEmitError
---
src/backend/executor/execMain.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d83d296d82..78c8ce4935 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1837,11 +1837,9 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- Relation rel = resultRelInfo->ri_RelationDesc;
- Relation orig_rel = rel;
- TupleDesc tupdesc = RelationGetDescr(rel);
- char *val_desc;
- Bitmapset *modifiedCols;
+ Oid root_relid;
+ TupleDesc tupdesc;
+ char *valdsc;
Bitmapset *insertedCols;
Bitmapset *updatedCols;
@@ -1851,11 +1849,13 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
*/
if (resultRelInfo->ri_PartitionRoot)
{
- TupleDesc old_tupdesc = RelationGetDescr(rel);
+ TupleDesc old_tupdesc;
AttrNumber *map;
- rel = resultRelInfo->ri_PartitionRoot;
- tupdesc = RelationGetDescr(rel);
+ root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
+
+ old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
/* a reverse map */
map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc,
gettext_noop("could not convert row type"));
@@ -1868,20 +1868,24 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
slot = execute_attr_map_slot(map, slot,
MakeTupleTableSlot(tupdesc, &TTSOpsVirtual));
}
+ else
+ {
+ root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
+ tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+ }
insertedCols = GetInsertedColumns(resultRelInfo, estate);
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
- modifiedCols = bms_union(insertedCols, updatedCols);
- val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel),
- slot,
- tupdesc,
- modifiedCols,
- 64);
+ valdsc = ExecBuildSlotValueDescription(root_relid,
+ slot,
+ tupdesc,
+ bms_union(insertedCols, updatedCols),
+ 64);
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
- RelationGetRelationName(orig_rel)),
- val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
+ RelationGetRelationName(resultRelInfo->ri_RelationDesc)),
+ valdsc ? errdetail("Failing row contains %s.", valdsc) : 0));
}
/*
--
2.11.0
* Re: rewrite ExecPartitionCheckEmitError
2018-12-06 22:22 rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2018-12-07 20:32 ` Robert Haas <robertmhaas@gmail.com>
2018-12-28 17:52 ` Re: rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 5+ messages in thread
From: Robert Haas @ 2018-12-07 20:32 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Dec 6, 2018 at 5:22 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Just on cleanliness grounds, I propose to rewrite the function in
> $SUBJECT. I came across this while reviewing some already-committed
> patch for partition pruning, and it's been sitting in my laptop ever
> since.
>
> I think the current coding is too convoluted and hard to follow. The
> patch makes it much simpler (IMO).
I don't really think this is an improvement, but the only part I
really dislike is s/val_desc/valdsc/. That is surely not an
improvement in readability.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: rewrite ExecPartitionCheckEmitError
2018-12-06 22:22 rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
2018-12-07 20:32 ` Re: rewrite ExecPartitionCheckEmitError Robert Haas <robertmhaas@gmail.com>
@ 2018-12-28 17:52 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2019-01-01 02:57 ` Re: rewrite ExecPartitionCheckEmitError David Rowley <david.rowley@2ndquadrant.com>
0 siblings, 1 reply; 5+ messages in thread
From: Alvaro Herrera @ 2018-12-28 17:52 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On 2018-Dec-07, Robert Haas wrote:
> On Thu, Dec 6, 2018 at 5:22 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Just on cleanliness grounds, I propose to rewrite the function in
> > $SUBJECT. I came across this while reviewing some already-committed
> > patch for partition pruning, and it's been sitting in my laptop ever
> > since.
> >
> > I think the current coding is too convoluted and hard to follow. The
> > patch makes it much simpler (IMO).
>
> I don't really think this is an improvement, but the only part I
> really dislike is s/val_desc/valdsc/. That is surely not an
> improvement in readability.
Point taken; pushed with that change.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: rewrite ExecPartitionCheckEmitError
2018-12-06 22:22 rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
2018-12-07 20:32 ` Re: rewrite ExecPartitionCheckEmitError Robert Haas <robertmhaas@gmail.com>
2018-12-28 17:52 ` Re: rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2019-01-01 02:57 ` David Rowley <david.rowley@2ndquadrant.com>
0 siblings, 0 replies; 5+ messages in thread
From: David Rowley @ 2019-01-01 02:57 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sat, 29 Dec 2018 at 06:52, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Point taken; pushed with that change.
Marking this as committed in the CF app.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2019-01-01 02:57 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14 23:57 [PATCH] rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@alvh.no-ip.org>
2018-12-06 22:22 rewrite ExecPartitionCheckEmitError Alvaro Herrera <alvherre@2ndquadrant.com>
2018-12-07 20:32 ` Robert Haas <robertmhaas@gmail.com>
2018-12-28 17:52 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2019-01-01 02:57 ` David Rowley <david.rowley@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox