From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Rework manipulation and structure of attribute mappings |
Date: | 2019-11-25 08:55:50 |
Message-ID: | CA+HiwqGamLf3WGgFY3K1C4h0TOBC3JtcMQoHyK7fVgkkLwJE3w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 22, 2019 at 4:57 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Fri, Nov 22, 2019 at 02:21:41PM +0900, Amit Langote wrote:
> > Actually, we should also refactor
> > convert_tuples_by_position() to carve out the code that builds the
> > AttrMap into a separate function and move it to attmap.c.
>
> Not sure how to name that. One logic uses a match based on the
> attribute name, and the other uses the type. So the one based on the
> attribute name should be something like build_attrmap_by_name() and
> the second attrmap_build_by_position()? We could use a better
> convention like AttrMapBuildByPosition for example. Any suggestions
> of names are welcome.
Actually, I was just suggesting that we create a new function
convert_tuples_by_position_map() and put the logic that compares the
two TupleDescs to create the AttrMap in it, just like
convert_tuples_by_name_map(). Now you could say that there would be
no point in having such a function, because no caller currently wants
to use such a map (that is, without the accompanying
TupleConversionMap), but maybe there will be in the future. Though
irrespective of that consideration, I guess you'd agree to group
similar code in a single source file.
Regarding coming up with any new name, having a word in the name that
distinguishes the aspect of mapping by attribute name vs. type
(position) should suffice. We can always do the renaming in a
separate patch.
> Please note that I still have a commit fest to
> run and finish, so I'll unlikely come back to that before December.
> Let's continue with the tuning of the function names though.
As it's mainly just moving around code, I gave it a shot; patch
attached (applies on top of yours). I haven't invented any new names
yet, but let's keep discussing that as you say.
Thanks,
Amit
Attachment | Content-Type | Size |
---|---|---|
0002-Move-more-code-to-attmap.c.patch | application/octet-stream | 22.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Antonin Houska | 2019-11-25 09:02:00 | Re: Attempt to consolidate reading of XLOG page |
Previous Message | Jobin Augustine | 2019-11-25 08:55:38 | Re: [HACKERS] Incomplete startup packet errors |