On Thu, Nov 21, 2019 at 2:10 PM Amit Kapila wrote:
>
> Thanks for finding the remaining places, the patch looks good to me.
> I hope this covers the entire code. BTW, are you using some script to
> find this or is this a result of manual inspection of code? I have
> modified the commit message i
On Fri, Nov 22, 2019 at 8:07 PM Tom Lane wrote:
>
> vignesh C writes:
> > After the inclusion they have define and further include based on #if
> > defined. In few cases I had seen the include happens at the end of the
> > file like in regcomp.c as there may be impact. I felt it is better not
> >
vignesh C writes:
> After the inclusion they have define and further include based on #if
> defined. In few cases I had seen the include happens at the end of the
> file like in regcomp.c as there may be impact. I felt it is better not
> to change these files. Let me know your thoughts on the same
On Thu, Nov 21, 2019 at 2:11 PM Amit Kapila wrote:
>
> On Sat, Nov 16, 2019 at 7:01 AM vignesh C wrote:
> >
> > On Tue, Nov 12, 2019 at 11:19 AM Amit Kapila
> > wrote:
> > >
> > > On Tue, Nov 12, 2019 at 6:33 AM vignesh C wrote:
> > > >
> > > >
> > > > Thanks Amit for your comments. Please fin
On Sat, Nov 16, 2019 at 7:01 AM vignesh C wrote:
>
> On Tue, Nov 12, 2019 at 11:19 AM Amit Kapila wrote:
> >
> > On Tue, Nov 12, 2019 at 6:33 AM vignesh C wrote:
> > >
> > >
> > > Thanks Amit for your comments. Please find the updated patch which
> > > does not include the changes mentioned abov
On Tue, Nov 12, 2019 at 11:19 AM Amit Kapila wrote:
>
> On Tue, Nov 12, 2019 at 6:33 AM vignesh C wrote:
> >
> >
> > Thanks Amit for your comments. Please find the updated patch which
> > does not include the changes mentioned above.
> >
>
> Thanks for working on this. I have pushed your latest
On Tue, Nov 12, 2019 at 6:33 AM vignesh C wrote:
>
>
> Thanks Amit for your comments. Please find the updated patch which
> does not include the changes mentioned above.
>
Thanks for working on this. I have pushed your latest patch.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterp
On Sun, Nov 10, 2019 at 5:30 PM vignesh C wrote:
>
[review_latest_patch]:
Do we want to consider the ordering of map file inclusions as well
(see the changes pointed out below)? If so, what all we should
validate, is compilation of these modules sufficient? Tom, anyone, do
you have any opinion
On Sun, Nov 10, 2019 at 5:30 PM vignesh C wrote:
>
> On Fri, Nov 8, 2019 at 2:22 PM Kuntal Ghosh
> wrote:
> >
> > On Sat, Nov 2, 2019 at 7:42 AM vignesh C wrote:
> > >
> > > >
> > > Thanks Amit for committing the changes.
> > > I found couple of more inconsistencies, the attached patch include
On Sat, Nov 2, 2019 at 7:42 AM vignesh C wrote:
>
> >
> Thanks Amit for committing the changes.
> I found couple of more inconsistencies, the attached patch includes
> the fix for the same.
>
Thanks for the patch. It seems you've to rebase the patch as it
doesn't apply on the latest HEAD. Apart f
On Thu, Oct 24, 2019 at 2:43 PM Amit Kapila wrote:
>
> On Wed, Oct 23, 2019 at 10:23 AM Amit Kapila wrote:
> >
> > Attached are patches for (a) and (b) after another round of review and
> > fixes by Vignesh. I am planning to commit the first one (a) tomorrow
> > morning and then if everything i
On Wed, Oct 23, 2019 at 10:23 AM Amit Kapila wrote:
>
> Attached are patches for (a) and (b) after another round of review and
> fixes by Vignesh. I am planning to commit the first one (a) tomorrow
> morning and then if everything is fine on buildfarm, I will commit the
> second one (b) and once
On Tue, Oct 22, 2019 at 3:41 PM Amit Kapila wrote:
>
> On Tue, Oct 22, 2019 at 12:56 PM Amit Kapila wrote:
> >
> > On Mon, Oct 21, 2019 at 11:04 PM vignesh C wrote:
> > >
> > > On Mon, Oct 21, 2019 at 8:47 AM Amit Kapila
> > > wrote:
>
> This patch series has broadly changed the code to organi
On Tue, Oct 22, 2019 at 11:05 PM vignesh C wrote:
>
> On Tue, Oct 22, 2019 at 3:42 PM Amit Kapila wrote:
> > Review for 0003-Ordering-of-header-files-remaining-dir-oct21
> > -
> > 1.
> > --- a/src/bin/pg_baseb
On Tue, Oct 22, 2019 at 3:42 PM Amit Kapila wrote:
> Review for 0003-Ordering-of-header-files-remaining-dir-oct21
> -
> 1.
> --- a/src/bin/pg_basebackup/pg_recvlogical.c
> +++ b/src/bin/pg_basebackup/pg_recvlog
On Tue, Oct 22, 2019 at 12:56 PM Amit Kapila wrote:
> Few comments on 0001-Ordering-of-header-files-in-contrib-dir-oct21.patch
> ---
> 1.
> --- a/contrib/isn/isn.c
> +++ b/contrib/isn/isn.c
> @@
On Tue, Oct 22, 2019 at 12:56 PM Amit Kapila wrote:
>
> On Mon, Oct 21, 2019 at 11:04 PM vignesh C wrote:
> >
> > On Mon, Oct 21, 2019 at 8:47 AM Amit Kapila wrote:
> > >
> > >
> > Thanks for the suggestions.
> > Updated patch contains the fix based on Tom Lane's Suggestion.
> > Let me know your
On Mon, Oct 21, 2019 at 11:04 PM vignesh C wrote:
>
> On Mon, Oct 21, 2019 at 8:47 AM Amit Kapila wrote:
> >
> >
> Thanks for the suggestions.
> Updated patch contains the fix based on Tom Lane's Suggestion.
> Let me know your thoughts for further revision if required.
>
Few comments on 0001-Ord
Amit Kapila writes:
> On Sun, Oct 20, 2019 at 10:58 PM vignesh C wrote:
>> Should we make this changes only in master branch or should we make in
>> back branches also.
> I am in favor of doing this only for HEAD, but I am fine if others
> want to see for back branches as well and you can prepar
On Sun, Oct 20, 2019 at 10:58 PM vignesh C wrote:
>
> On Thu, Oct 17, 2019 at 4:44 PM Amit Kapila wrote:
> >
> >
> > I haven't reviewed it completely, but generally, the changes seem to
> > be fine. Please see if you can be consistent in extra space between
> > includes. Kindly check the same t
On Sun, Oct 20, 2019 at 2:44 AM Andres Freund wrote:
>
> Hi,
>
> On 2019-10-19 21:50:03 +0200, Peter Eisentraut wrote:
> > diff --git a/contrib/bloom/blcost.c b/contrib/bloom/blcost.c
> > index f9fe57f..6224735 100644
> > --- a/contrib/bloom/blcost.c
> > +++ b/contrib/bloom/blcost.c
> > @@ -12,10
On Sun, Oct 20, 2019 at 1:20 AM Peter Eisentraut
wrote:
>
> diff --git a/contrib/bloom/blcost.c b/contrib/bloom/blcost.c
> index f9fe57f..6224735 100644
> --- a/contrib/bloom/blcost.c
> +++ b/contrib/bloom/blcost.c
> @@ -12,10 +12,10 @@
> */
> #include "postgres.h"
>
> +#include "bloom.h"
> #i
Andres Freund writes:
> On 2019-10-19 21:50:03 +0200, Peter Eisentraut wrote:
>> This class of change I don't like.
>> The existing arrangement keeps "other" header files separate from the
>> header file of the module itself. It seems useful to keep that separate.
> If we were to do so, we ought
Hi,
On 2019-10-19 21:50:03 +0200, Peter Eisentraut wrote:
> diff --git a/contrib/bloom/blcost.c b/contrib/bloom/blcost.c
> index f9fe57f..6224735 100644
> --- a/contrib/bloom/blcost.c
> +++ b/contrib/bloom/blcost.c
> @@ -12,10 +12,10 @@
> */
> #include "postgres.h"
>
> +#include "bloom.h"
> #
diff --git a/contrib/bloom/blcost.c b/contrib/bloom/blcost.c
index f9fe57f..6224735 100644
--- a/contrib/bloom/blcost.c
+++ b/contrib/bloom/blcost.c
@@ -12,10 +12,10 @@
*/
#include "postgres.h"
+#include "bloom.h"
#include "fmgr.h"
#include "utils/selfuncs.h"
-#include "bloom.h"
/*
* Est
On Tue, Oct 15, 2019 at 10:57 PM vignesh C wrote:
>
> 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.
>
Hmm, I am not sure if that is better in all cases. It seems to be
making the code loo
On Wed, Oct 16, 2019 at 8:10 AM Amit Kapila wrote:
>
> Thanks for working on this. I will look into this in the coming few
> days or during next CF. Can you please register it for the next CF
> (https://commitfest.postgresql.org/25/)?
>
Thanks, I have added it to the commitfest.
Regards,
Vignes
On Tue, Oct 15, 2019 at 10:57 PM vignesh C wrote:
>
> On Wed, Oct 9, 2019 at 11:37 AM Amit Kapila wrote:
> >
> > On Tue, Oct 8, 2019 at 8:19 PM Tom Lane wrote:
> > >
> > > Amit Kapila writes:
> > > > On Wed, Oct 2, 2019 at 2:57 PM vignesh C wrote:
> > > >> I noticed that some of the header fil
On Tue, Oct 8, 2019 at 8:19 PM Tom Lane wrote:
>
> Amit Kapila writes:
> > On Wed, Oct 2, 2019 at 2:57 PM vignesh C 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
Amit Kapila writes:
> On Wed, Oct 2, 2019 at 2:57 PM vignesh C 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 y
On Wed, Oct 2, 2019 at 2:57 PM vignesh C wrote:
>
> Hi,
>
> 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 th
31 matches
Mail list logo
