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

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

(2010/06/14 22:11), Robert Haas wrote:
> 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.
>
Yes, it is fact that I hesitated about what fields should be initialized
at the makeRangeTblEntry(), because it has more than 20 members but
most of them are meaningful only when rtekind has a certain code.

OK, at least, priority of the patch is not higher than others for me.
I'll cancel the patch submission.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message KaiGai Kohei 2010-06-15 00:09:51 Re: ExecutorCheckPerms() hook
Previous Message KaiGai Kohei 2010-06-14 23:54:05 Re: [v9.1] Add security hook on initialization of instance