On Wed, Oct 9, 2019 at 11:37 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Oct 8, 2019 at 8:19 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> > > On Wed, Oct 2, 2019 at 2:57 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >> I noticed that some of the header files inclusion is not ordered as
> > >> per the usual standard that is followed.
> > >> The attached patch contains the fix for the order in which the header
> > >> files are included.
> > >> Let me know your thoughts on the same.
> >
> > > +1.
> >
> > FWIW, I'm not on board with reordering system-header inclusions.
> > Some platforms have (had?) ordering dependencies for those, and where
> > that's true, it's seldom alphabetical. It's only our own headers
> > where we can safely expect that any arbitrary order will work.
> >
>
> Okay, that makes sense. However, I noticed that ordering for
> system-header inclusions is somewhat random. For ex. nodeSubPlan.c,
> datetime.c, etc. include limits.h first and then math.h whereas
> knapsack.c, float.c includes them in reverse order. There could be
> more such inconsistencies and the probable reason is that we don't
> have any specific rule, so different people decide to do it
> differently.
>
> > > I think we shouldn't remove the extra line as part of the above change.
> >
> > I would take out the blank lines between our own #includes.
> >
>
> Okay, that would be better, but doing it half-heartedly as done in
> patch might make it worse. So, it is better to remove blank lines
> between our own #includes in all cases.
>
Attached patch contains the fix based on the comments suggested.
I have added/deleted extra lines in certain places so that the
readability is better.
I have removed the duplicate includes of certain header files in same
source file.
In some place postgres header files was getting included as
<postgres_header.h>, I have changed it to "postgres_header.h".
Let me know if any change is required.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com