Thread

1. PATCH: CreateComments: use explicit indexing for ``values''

   richhguard-monotone@yahoo.co.uk — 2011-06-12T11:26:16Z

   Hello,
   I'm new to PostgreSQL and git, but having read through the wiki entries such as http://wiki.postgresql.org/wiki/Submitting_a_Patch, I think I have a patch worthy of submission.
   
   It's a readability improvement in src/backend/commands/comment.c (CreateComments function), which changes the existing code from incrementing a variable for use as the array index, to use explicit ``values'' instead.
   
   This has the following benefits
   
   1) The structure of ``values'' is now clear at first glance.
   2) ``i'' is then only used for 1 reason; the for loop
   
   The patch is based on "master", and all existing tests pass.
   
   Regards
   Richard

2. Re: PATCH: CreateComments: use explicit indexing for ``values''

   Robert Haas <robertmhaas@gmail.com> — 2011-06-13T12:15:47Z

   On Sun, Jun 12, 2011 at 7:26 AM,  <richhguard-monotone@yahoo.co.uk> wrote:
   > Hello,
   > I'm new to PostgreSQL and git, but having read through the wiki entries such as http://wiki.postgresql.org/wiki/Submitting_a_Patch, I think I have a patch worthy of submission.
   >
   > It's a readability improvement in src/backend/commands/comment.c (CreateComments function), which changes the existing code from incrementing a variable for use as the array index, to use explicit ``values'' instead.
   >
   > This has the following benefits
   >
   > 1) The structure of ``values'' is now clear at first glance.
   > 2) ``i'' is then only used for 1 reason; the for loop
   >
   > The patch is based on "master", and all existing tests pass.
   
   Wow.  That code is pretty ugly, all right.  I think, though, that we
   probably ought to be using the Apg_description_<columnname> constants
   instead of writing 0-3.  Care to update the patch?
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   
   

3. Re: PATCH: CreateComments: use explicit indexing for ``values''

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-13T14:03:49Z

   Robert Haas <robertmhaas@gmail.com> writes:
   > On Sun, Jun 12, 2011 at 7:26 AM,  <richhguard-monotone@yahoo.co.uk> wrote:
   >> It's a readability improvement in src/backend/commands/comment.c (CreateComments function), which changes the existing code from incrementing a variable for use as the array index, to use explicit ``values'' instead.
   
   > Wow.  That code is pretty ugly, all right.  I think, though, that we
   > probably ought to be using the Apg_description_<columnname> constants
   > instead of writing 0-3.  Care to update the patch?
   
   Historically this i++ approach has been used in a lot of places that
   fill in system catalog tuples.  We've fixed some of them over time, but
   I doubt this is the only one remaining.  If we're going to try to remove
   it here, maybe we ought to try to fix them all rather than just this
   one.  I agree that the main point of doing so would be to introduce the
   greppable Apg_xxx constants, and so just using hard-coded integers is
   not much of an improvement.
   
   			regards, tom lane