Re: [v9.1] add makeRangeTblEntry() into makefuncs.c

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [v9.1] add makeRangeTblEntry() into makefuncs.c
Date: 2010-06-14 13:11:02
Message-ID: AANLkTilZFoKq-O1RRU0AzgaJpc0FJUzmgBRqXo8gv4bz@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 14, 2010 at 8:46 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
>> 2010/6/14 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> > It adds makeRangeTblEntry() into makefuncs.c to keep the code more
>> > clean. It shall be also used for the upcoming DML refactor patch.
>> > In this refactoring, a common DML permission checker function take
>> > a list of RangeTblEntry, so the caller has to set up the object.
>>
>> I think this is the epitome of pointless.  It looks to me like this
>> just makes the code harder to read and very slightly slower without
>> actually accomplishing any useful abstraction.
>
> I had suggested to KaiGai that he check if there was an existing
> function for creating an RTE rather than just malloc'ing it- he took
> that to mean he should add one if he couldn't find one.  Wasn't my
> intent, but by the same token I didn't see it as a terribly bad thing
> either.  Perhaps it should be improved or maybe we should just rip it
> out, but I rather prefer some kind of abstraction for that given it's
> use in a number of places.  Of course, I may just be overly thinking it.

Well, there's not much point in having a function that initializes ONE
member of a 20+ member structure and leaves the initialization of all
the rest to the caller. The structure effectively functions like a
disjoint union, so maybe there'd be some value in having a function
for "build a relation RTE", which would set the rtekind to
RTE_RELATION and take arguments for the other fields that pertain to
that case. But the generic function proposed here doesn't really
provide any meaningful abstraction.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-06-14 13:13:29 Re: Proposal for 9.1: WAL streaming from WAL buffers
Previous Message Dimitri Fontaine 2010-06-14 12:55:04 Re: pg_archive_bypass