From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Franck Verrot <franck(at)verrot(dot)fr>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Mention column name in error messages |
Date: | 2016-10-18 21:54:47 |
Message-ID: | CAB7nPqTg9aOqnwDcBBZy-n8D3eYy4Dgz9Wrc-ThTh632E2nqyQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 19, 2016 at 3:14 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Oct 17, 2016 at 3:18 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Andres wrote:
>>> +/* Support for TransformExprCallback */
>>> +typedef struct TransformExprState
>>> +{
>>> + const char *column_name;
>>> + Oid expected_type;
>>> +} TransformExprState;
>>> I see no need for this to be a struct defined in the header. Given that
>>> TransformExprCallback isn't public, and the whole thing is specific to
>>> TransformExprState...
>>
>> That's a matter of taste, really. Personally I find cleaner to declare
>> that with the other static declarations at the top of the fil, and
>> keep the comments of transformAssignedExpr clean of everything.
>
> It's pretty standard practice for PostgreSQL to keep declarations
> private to particular files whenever they are used only in that file.
> And I'd argue that it's good practice in general.
Yes, definitely. My comment was referring to the fact that the
declaration of this structure was not at the top of this .c file as
the last version of the patch is doing (would be better to declare
them at the beginning of this .c file for clarity actually). It seems
like I did not get Andres' review point, sorry for that.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2016-10-18 22:09:10 | Re: Renaming of pg_xlog and pg_clog |
Previous Message | Alexander Korotkov | 2016-10-18 21:46:18 | Re: Indirect indexes |