On Thu, Feb 27, 2020 at 10:43:50AM +0100, Peter Eisentraut wrote:
> This thread is still in the commit fest, but it's apparently gone as far as
> it will, so I've set it to "Committed" for lack of a "partial" status.
Thanks, that sounds right to me. I was just looking at the latest
patch presente
On 2019-12-18 10:55, Alvaro Herrera wrote:
On 2019-Dec-18, Michael Paquier wrote:
Let's take one example. The changes in pg_dump/ like
/progname/prog_name/ have just been done in haste, without actual
thoughts about how the problem ought to be fixed. And in this case,
something which could be
De: Michael Paquier
Enviadas: Quarta-feira, 18 de Dezembro de 2019 01:18
>Committed that part.
Thanks.
>Let's take one example. The changes in pg_dump/ like
>/progname/prog_name/ have just been done in haste, without actual
>thoughts about how the problem ought to be fixed. And in this case,
>so
On 2019-Dec-18, Michael Paquier wrote:
> Let's take one example. The changes in pg_dump/ like
> /progname/prog_name/ have just been done in haste, without actual
> thoughts about how the problem ought to be fixed. And in this case,
> something which could be more adapted is to remove the argumen
On Tue, Dec 17, 2019 at 09:36:13AM +0900, Michael Paquier wrote:
> As that's a confusion I introduced with d9fadbf, I would like to fix
> that and backpatch this change down to 11. (Ranier gets the
> authorship per se as that's extracted from a larger patch).
Committed that part.
I got to look a
De: Michael Paquier
Enviadas: Terça-feira, 17 de Dezembro de 2019 00:36
>Hmm. In the case of this logic, we are referring to the current end
>of WAL with endptr, and what you are calling the startptr is really
>the redo LSN of the last checkpoint in all the routines which are now
>confused with R
On Mon, Dec 09, 2019 at 05:11:10PM -0500, Tom Lane wrote:
> Alvaro Herrera writes:
>> We have a not-consistently-used convention that names in CamelCase are
>> used for global variables. Naming a function parameter in that style
>> seems pointlessly confusing. I would rather use redorecptr as To
Greetings,
* Ranier Vilela (ranier_...@hotmail.com) wrote:
> 1.So I would like to ask you if at least what has consensus could be used.
> Or is it better to leave everything as it is?
As I tried to say before- I'm all for working to eliminate the shadow
variables, but it should be on a case-by-ca
De: Stephen Frost
Enviadas: Quarta-feira, 11 de Dezembro de 2019 18:57
>I really don't have any doubts that it's going to lead to confusion,
>particularly in a case like the numTables vs. nTables thing you're
>proposing in the one case that I spent some time looking at, and that
>confusion certainl
Greetings,
* Ranier Vilela (ranier_...@hotmail.com) wrote:
> De: Stephen Frost
> Enviadas: Quarta-feira, 11 de Dezembro de 2019 15:34
>
> >I agree with not breaking things but that doesn't mean the only
> >reasonable approach is to do the absolute minimum- you might not be
> >breaking something t
On Wed, Dec 11, 2019 at 9:46 AM Ranier Vilela
wrote:
> I am the author of the patch.
> I'm repeating myself, but come on, I don't have confidence in proposing
> logic-altering changes for now.
>
>
Then you need to stop and patch the holes and not just throw paint on the
wall to cover things up.
De: Stephen Frost
Enviadas: Quarta-feira, 11 de Dezembro de 2019 15:34
>I agree with not breaking things but that doesn't mean the only
>reasonable approach is to do the absolute minimum- you might not be
>breaking something today, but it's going to confuse people later on down
>the road and may l
Greetings,
Didn't see this previously (it's our typical approach to 'reply-all' to
people), though I don't think it changes my feelings about the latest
proposed patch.
* Ranier Vilela (ranier_...@hotmail.com) wrote:
> De: Stephen Frost
> Enviadas: Terça-feira, 10 de Dezembro de 2019 17:52
>
> >
Greetings,
* Ranier Vilela (ranier_...@hotmail.com) wrote:
> New version the global patch unshadow.
> * names more consistent and readable.
> * without big changes.
> * goal,, unshadow all global variables, only, even the simplest.
This didn't address any of the comments that I raised elsewhere o
New version the global patch unshadow.
* names more consistent and readable.
* without big changes.
* goal,, unshadow all global variables, only, even the simplest.
regards,
Ranier Vilela
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6bc1a6b46d..a496727e3
De: Stephen Frost
Enviadas: Terça-feira, 10 de Dezembro de 2019 17:52
>There's multiple ways to get there though and I think what you're seeing
>is that the "just change it to something else" answer isn't necessairly
>going to be viewed as an improvement (or, at least, not enough of an
>improvemen
Greetings,
* Ranier Vilela (ranier_...@hotmail.com) wrote:
> >For someone that expounds consistency - this patch is the furthest thing
> >from it.
> >In some places names are randomly changed to have an underscore
> >>(authmethodlocal to authmethod_local with the obvious inconsistency as
> >wel
De: John W Higgins
Enviado: terça-feira, 10 de dezembro de 2019 15:58
>For someone that expounds consistency - this patch is the furthest thing from
>it.
>In some places names are randomly changed to have an underscore
>>(authmethodlocal to authmethod_local with the obvious inconsistency as wel
On Tue, Dec 10, 2019 at 5:21 AM Ranier Vilela
wrote:
> New version the global patch, with the considerations.
> Unfortunately WalReceiverConn cannot be used because it is currently the
> typedef name for the structure.
> I switched to WalReceiverConnection, it was long but it looks good.
> RedoRe
New version the global patch, with the considerations.
Unfortunately WalReceiverConn cannot be used because it is currently the
typedef name for the structure.
I switched to WalReceiverConnection, it was long but it looks good.
RedoRecPtr proper name has no consensus yet, so it was still lastRedoR
De: Alvaro Herrera
Enviado: segunda-feira, 9 de dezembro de 2019 22:06
>I find this choice a bit ugly and even more confusing than the original.
>I'd change this to be just "segsize".
Ok.
>I would tend to name the GUC variable as if it were a global in the
>sense that I proposed in my previous r
Alvaro Herrera writes:
> We have a not-consistently-used convention that names in CamelCase are
> used for global variables. Naming a function parameter in that style
> seems pointlessly confusing. I would rather use redorecptr as Tom
> suggested, which fits with the style used for the other arg
On 2019-Dec-09, Ranier Vilela wrote:
> --- a/src/backend/access/transam/xlogreader.c
> +++ b/src/backend/access/transam/xlogreader.c
> @@ -70,7 +70,7 @@ report_invalid_record(XLogReaderState *state, const char
> *fmt,...)
> * Returns NULL if the xlogreader couldn't be allocated.
> */
> XLogR
On 2019-Dec-09, Kyotaro Horiguchi wrote:
> > >BTW, if we do go forward with changing the RedoRecPtr uses, I'm not
> > >in love with "XRedoRecPtr" as a replacement parameter name; it conveys
> > >nothing much, and the "X" prefix is already overused in that area of
> > >the code. Perhaps "pRedoRec
Andres Freund writes:
> On 2019-12-09 11:59:23 -0500, Robert Haas wrote:
>> On Mon, Dec 9, 2019 at 11:48 AM Tom Lane wrote:
>>> The only thing I think is really a substantial bug risk here is your
>>> point about our own macros referencing our own global variables.
>>> We might be better off fixi
Hi,
On 2019-12-09 11:59:23 -0500, Robert Haas wrote:
> On Mon, Dec 9, 2019 at 11:48 AM Tom Lane wrote:
> > I think it depends a lot on the particular identifiers in use. You
> > mentioned examples like "i" and "lc", and I'd add other obviously
> > nonce variable names like "oldcxt". I'm not part
On 12/9/19 9:28 AM, Ranier Vilela wrote:
I'll create the commitfest entry based on this email once
this has been sent.
Can you add the patch attached?
That showed up in the commitfest entry automatically when you
replied to this thread with the attachment.
You might consider signing up so
>I'll create the commitfest entry based on this email once
>this has been sent.
Can you add the patch attached?
regards,
Ranier Vilela
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 8b2a2be1c0..e12f41cea4 100644
--- a/src/backend/replication/walsender.
This the second version of the global unshadow patch.
Taking into consideration made. In case anyone else revises.
regards
Ranier Vileladiff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6bc1a6b46d..83be23d77b 100644
--- a/src/backend/access/transam/xlog.c
+++
On Mon, Dec 9, 2019 at 11:48 AM Tom Lane wrote:
> I think it depends a lot on the particular identifiers in use. You
> mentioned examples like "i" and "lc", and I'd add other obviously
> nonce variable names like "oldcxt". I'm not particularly concerned
> about shadowing arising from somebody wri
Robert Haas writes:
> On Mon, Dec 9, 2019 at 10:23 AM Tom Lane wrote:
>> Well, again, this is a case-by-case question. I tend to agree that
>> changing that usage in subscriptioncmds.c might be a good idea.
>> That doesn't mean I need to be on board with wholesale removal
>> of shadowing warning
On Mon, Dec 9, 2019 at 10:23 AM Tom Lane wrote:
> Robert Haas writes:
> > On Sun, Dec 8, 2019 at 1:52 PM Tom Lane wrote:
> >> I don't think I'm actually on board with the goal here.
> > I don't know what to do about the RedoRecPtr mess, but surely
> > subscriptioncmds.c's use of synchronous_comm
Robert Haas writes:
> On Sun, Dec 8, 2019 at 1:52 PM Tom Lane wrote:
>> I don't think I'm actually on board with the goal here.
> I don't know what to do about the RedoRecPtr mess, but surely
> subscriptioncmds.c's use of synchronous_commit as a char * when it's
> already exists as a global vari
On Sun, Dec 8, 2019 at 1:52 PM Tom Lane wrote:
> Ranier Vilela writes:
> > This is the first part of the variable shadow fixes.
> > Basically it consists of renaming the variables in collision with the
> > global ones, with the minimum change in the semantics.
>
> I don't think I'm actually on b
> On 9 Dec 2019, at 12:02, Ranier Vilela wrote:
> diff --git a / src / backend / access / transam / multixact.c b / src /
> backend / access / transam / multixact.c
> index 7b2448e05b..6364014fb3 100644
> --- a / src / backend / access / transam / multixact.c
> +++ b / src / backend / access / t
De: Kyotaro Horiguchi
Enviado: segunda-feira, 9 de dezembro de 2019 03:40
>The file-scoped variable is needed to be process-persistent in any
>way. If we inhibit them, the upper-modules need to create the
>persistent area instead, for example, by calling XLogInitGlobals() or
>such, which makes thi
On Sun, Dec 08, 2019 at 02:14:03PM -0500, Tom Lane wrote:
> That is, these arguments *used* to be a different LSN pointer, and that
> commit changed them to be mostly equal to RedoRecPtr, and made what
> seems like a not very well-advised renaming to go with that.
Indeed. That part was ill-though
On 12/8/19 8:50 PM, Mark Dilger wrote:
On 12/8/19 10:25 AM, Mark Dilger wrote:
I was
still expecting multiple patches, perhaps named along the
lines of:
unshadow.RedoRecPtr.patch.1
unshadow.wal_segment_size.patch.1
unshadow.synchronous_commit.patch.1
unshadow.wrconn.patch.1
On 12/8/19 10:25 AM, Mark Dilger wrote:
I was
still expecting multiple patches, perhaps named along the
lines of:
unshadow.RedoRecPtr.patch.1
unshadow.wal_segment_size.patch.1
unshadow.synchronous_commit.patch.1
unshadow.wrconn.patch.1
unshadow.progname.patch.1
unshadow.am_sy
Hello.
At Mon, 9 Dec 2019 01:30:33 +, Ranier Vilela wrote
in
> >I don't think I'm actually on board with the goal here.
> Ok, I understand.
>
> >Basically, if we take this seriously, we're throwing away the notion of
> >nested variable scopes and programming as if we had just a flat namesp
>I don't think I'm actually on board with the goal here.
Ok, I understand.
>Basically, if we take this seriously, we're throwing away the notion of
>nested variable scopes and programming as if we had just a flat namespace.
>That wasn't any fun when we had to do it back in assembly-code days, and
>I think it would be better to split this patch into separate files,
>one for each global variable that is being shadowed.
Ok, I agree.
> The reasonI say so is apparent looking at the first one in the patch,
>RedoRecPtr. This process global variable is defined in xlog.c:
> static XLogRecPtr Re
Mark Dilger writes:
> I think it would be better to split this patch into separate files,
> one for each global variable that is being shadowed. The reason
> I say so is apparent looking at the first one in the patch,
> RedoRecPtr. This process global variable is defined in xlog.c:
>static X
Ranier Vilela writes:
> This is the first part of the variable shadow fixes.
> Basically it consists of renaming the variables in collision with the global
> ones, with the minimum change in the semantics.
I don't think I'm actually on board with the goal here.
Basically, if we take this seriou
On 12/7/19 3:42 PM, Ranier Vilela wrote:
This is the first part of the variable shadow fixes.
Basically it consists of renaming the variables in collision with the global
ones, with the minimum change in the semantics.
make check pass all the tests.
I think it would be better to split this
This is the first part of the variable shadow fixes.
Basically it consists of renaming the variables in collision with the global
ones, with the minimum change in the semantics.
make check pass all the tests.
regards,
Ranier Vileladiff --git a/src/backend/access/transam/xlog.c b/src/backend/acce
On 12/6/19 3:18 PM, Ranier Vilela wrote:
Robert Haas wrote:
Maybe you want to post a few examples. It's hard to discuss in the abstract.
I am working on the patch.
I think this is a great example.
I do not know if it is better to rename the local parameter, or if it should be
renamed the gl
Robert Haas wrote:
>Maybe you want to post a few examples. It's hard to discuss in the abstract.
I am working on the patch.
I think this is a great example.
I do not know if it is better to rename the local parameter, or if it should be
renamed the global variable.
line: 68
var char **synchronous
On Fri, Dec 6, 2019 at 7:59 AM Ranier Vilela wrote:
> What did the original author want, use the global variable or not use it by
> overriding the name.
> If it was to use the global variable, it will affect the behavior of the
> function, I believe.
Well, you haven't provided any examples, so
De: Mark Dilger
Enviado: quinta-feira, 5 de dezembro de 2019 21:06
>I suggested increasing the default warnings in an email some time ago,
>to which Tom made reasonable objections. You might want to take a
>look at his comments, and consider if you can overcome the concerns
>he had:
I understand
On Thu, Dec 05, 2019 at 01:41:20PM -0500, Robert Haas wrote:
> If you do decide to work on this, I recommend against preparing a
> single giant patch that changes every single one blindly. Try to think
> about which cases are the most egregious/dangerous and propose patches
> for those first. If th
Mark Dilger writes:
> On 12/3/19 5:24 PM, Ranier Vilela wrote:
>> Any comments?
> I suggested increasing the default warnings in an email some time ago,
> to which Tom made reasonable objections.
Yes, that whole thread is worth a read in this context:
https://www.postgresql.org/message-id/flat/
On 12/3/19 5:24 PM, Ranier Vilela wrote:
Hi,
I believe PostgreSQL can benefit from changing the alert level of compilation
warnings.
The current Level3 level for windows does not show any alerts, but that does
not mean that there are no problems.
Changing the level to Level4 and its equivale
On Thu, Dec 5, 2019 at 11:26 AM Ranier Vilela wrote:
> Even so, there is some failure to review or compile in Unix environment,
> because there are so many cases.
> -Wshadow with GCC can show the alerts.
I mean, compiler warnings are not errors, and there's no requirement
that we fix every warni
Robert wrote:
>Most of us don't develop on Windows, so changing warning levels on
>Windows won't really affect what developers see on their own machines,
>and thus probably doesn't have much value.
Yes the report is from msvc 2017.
Even so, there is some failure to review or compile in Unix enviro
On Tue, Dec 3, 2019 at 8:24 PM Ranier Vilela wrote:
> I believe PostgreSQL can benefit from changing the alert level of compilation
> warnings.
> The current Level3 level for windows does not show any alerts, but that does
> not mean that there are no problems.
> Changing the level to Level4 and
Hi,
I believe PostgreSQL can benefit from changing the alert level of compilation
warnings.
The current Level3 level for windows does not show any alerts, but that does
not mean that there are no problems.
Changing the level to Level4 and its equivalent for GCC in Unix environments
will show man
57 matches
Mail list logo
