Re: [HACKERS] [COMMITTERS] pgsql: Fix a bug in how we generate partition constraints.
On 2017/01/14 13:36, Amit Langote wrote:
> On Sat, Jan 14, 2017 at 6:10 AM, Robert Haas wrote:
>> On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera
>> wrote:
>>>
>>> I'm just saying that the problem at hand is already solved for a related
>>> feature, so ISTM this new code should use the recently added routine
>>> rather than doing the same thing in a different way.
>>
>> Oh, I see. Amit, thoughts?
>
> Hm, perhaps. The code in map_partition_varattnos() that creates the
> map could be replaced by a call to the new
> convert_tuples_by_name_map(). In fact, it could even have used the
> old version of it (convert_tuples_by_name()). I guess I just aped
> what other callers of map_variable_attnos() were doing, which is to
> generate the map themselves (not that they ought to be changed to use
> convert_tuples_by_name_map).
>
> I will send a patch at my earliest convenience. Thanks to Alvaro for
> pointing that out.
And here is the patch.
Thanks,
Amit
>From e04fde063af93e756f15a374611cdb2fa2708ece Mon Sep 17 00:00:00 2001
From: amit
Date: Mon, 16 Jan 2017 14:37:50 +0900
Subject: [PATCH 1/7] Avoid code duplication in map_partition_varattnos()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Code to map attribute numbers in map_partition_varattnos() duplicates
what convert_tuples_by_name_map() does. Avoid that.
Amit Langote, pointed out by Ãlvaro Herrera.
---
src/backend/catalog/partition.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 874e69d8d6..01e0c6c360 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -885,32 +885,19 @@ get_qual_from_partbound(Relation rel, Relation parent, Node *bound)
List *
map_partition_varattnos(List *expr, Relation partrel, Relation parent)
{
- TupleDesc tupdesc = RelationGetDescr(parent);
- AttrNumber attno;
AttrNumber *part_attnos;
bool found_whole_row;
if (expr == NIL)
return NIL;
- part_attnos = (AttrNumber *) palloc0(tupdesc->natts * sizeof(AttrNumber));
- for (attno = 1; attno <= tupdesc->natts; attno++)
- {
- Form_pg_attribute attribute = tupdesc->attrs[attno - 1];
- char *attname = NameStr(attribute->attname);
- AttrNumber part_attno;
-
- if (attribute->attisdropped)
- continue;
-
- part_attno = get_attnum(RelationGetRelid(partrel), attname);
- part_attnos[attno - 1] = part_attno;
- }
-
+ part_attnos = convert_tuples_by_name_map(RelationGetDescr(partrel),
+ RelationGetDescr(parent),
+ gettext_noop("could not convert row type"));
expr = (List *) map_variable_attnos((Node *) expr,
1, 0,
part_attnos,
- tupdesc->natts,
+ RelationGetDescr(parent)->natts,
&found_whole_row);
/* There can never be a whole-row reference here */
if (found_whole_row)
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Fix a bug in how we generate partition constraints.
On Sat, Jan 14, 2017 at 6:10 AM, Robert Haas wrote: > On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera > wrote: >> >> I'm just saying that the problem at hand is already solved for a related >> feature, so ISTM this new code should use the recently added routine >> rather than doing the same thing in a different way. > > Oh, I see. Amit, thoughts? Hm, perhaps. The code in map_partition_varattnos() that creates the map could be replaced by a call to the new convert_tuples_by_name_map(). In fact, it could even have used the old version of it (convert_tuples_by_name()). I guess I just aped what other callers of map_variable_attnos() were doing, which is to generate the map themselves (not that they ought to be changed to use convert_tuples_by_name_map). I will send a patch at my earliest convenience. Thanks to Alvaro for pointing that out. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Fix a bug in how we generate partition constraints.
On Fri, Jan 13, 2017 at 3:09 PM, Alvaro Herrera wrote: > Robert Haas wrote: >> On Fri, Jan 13, 2017 at 2:31 PM, Robert Haas wrote: >> > On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera >> > wrote: > >> >> Hmm, we were discussing this stuff a few days ago, see >> >> https://www.postgresql.org/message-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql >> >> and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code >> >> duplicates that ... >> > >> > Is that bad? >> >> If you are expressing a concern about who wrote this code, I took >> Amit's word for it that he did. > > I'm just saying that the problem at hand is already solved for a related > feature, so ISTM this new code should use the recently added routine > rather than doing the same thing in a different way. Oh, I see. Amit, thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Fix a bug in how we generate partition constraints.
On Fri, Jan 13, 2017 at 2:31 PM, Robert Haas wrote: > On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera > wrote: >> Robert Haas wrote: >>> Fix a bug in how we generate partition constraints. >>> >>> Move the code for doing parent attnos to child attnos mapping for Vars >>> in partition constraint expressions to a separate function >>> map_partition_varattnos() and call it from the appropriate places. >> >> Hmm, we were discussing this stuff a few days ago, see >> https://www.postgresql.org/message-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql >> and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code >> duplicates that ... > > Is that bad? If you are expressing a concern about who wrote this code, I took Amit's word for it that he did. His patch file says: Reported by: n/a Patch by: Amit Langote Reports: n/a If that ain't right, that's bad. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Fix a bug in how we generate partition constraints.
On Fri, Jan 13, 2017 at 2:18 PM, Alvaro Herrera wrote: > Robert Haas wrote: >> Fix a bug in how we generate partition constraints. >> >> Move the code for doing parent attnos to child attnos mapping for Vars >> in partition constraint expressions to a separate function >> map_partition_varattnos() and call it from the appropriate places. > > Hmm, we were discussing this stuff a few days ago, see > https://www.postgresql.org/message-id/20170109182800.qrkae62kmur3gfeg@alvherre.pgsql > and commit 3957b58b8885441c8d03bc1cfc00e47cf8cd7975. Part of this code > duplicates that ... Is that bad? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
