Обсуждение: [HACKERS] compiler warning with VS 2017

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> I am getting a compiler warning when I build the latest HEAD PostgreSQL with
> visual studio 2017.
> The code at the line is,
> tuple->values[i] = (char *) (Size)0xdeadbeef; /* make bad usage more obvious */

Yeah, you're not the first to complain about this.  To my mind that
coding is not pretty, not cute, and not portable: there's not even
a good reason to believe that dereferencing the pointer would result
in a crash.  Perhaps the author can explain to us why this is better
than just assigning NULL.

Actually, looking around a bit there, it's not even clear why
we should be booby-trapping the value of an unchanged column in
the first place.  So I'd say that not only is the code dubious
but the comment is inadequate too.
        regards, tom lane

On 05/05/17 06:50, Tom Lane wrote:
> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>> I am getting a compiler warning when I build the latest HEAD PostgreSQL with
>> visual studio 2017.
>> The code at the line is,
>> tuple->values[i] = (char *) (Size)0xdeadbeef; /* make bad usage more obvious */
> 
> Yeah, you're not the first to complain about this.  To my mind that
> coding is not pretty, not cute, and not portable: there's not even
> a good reason to believe that dereferencing the pointer would result
> in a crash.  Perhaps the author can explain to us why this is better
> than just assigning NULL.
> 
> Actually, looking around a bit there, it's not even clear why
> we should be booby-trapping the value of an unchanged column in
> the first place.  So I'd say that not only is the code dubious
> but the comment is inadequate too.

Hmm, as far as I can recollect this is just leftover debugging code that
was intended to help ensure that we are checking the "changed"
everywhere we are supposed to (since I changed handling of these
structured quite a bit during development). Should be changed to NULL,
that's what we usually do in this type of situation.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
> On 05/05/17 06:50, Tom Lane wrote:
>> Actually, looking around a bit there, it's not even clear why
>> we should be booby-trapping the value of an unchanged column in
>> the first place.  So I'd say that not only is the code dubious
>> but the comment is inadequate too.

> Hmm, as far as I can recollect this is just leftover debugging code that
> was intended to help ensure that we are checking the "changed"
> everywhere we are supposed to (since I changed handling of these
> structured quite a bit during development). Should be changed to NULL,
> that's what we usually do in this type of situation.

So the comment should be something like "if the column is unchanged,
we should not attempt to access its value beyond this point.  To
help catch any such attempts, set the string to NULL" ?
        regards, tom lane

On 05/05/17 16:56, Tom Lane wrote:
> Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
>> On 05/05/17 06:50, Tom Lane wrote:
>>> Actually, looking around a bit there, it's not even clear why
>>> we should be booby-trapping the value of an unchanged column in
>>> the first place.  So I'd say that not only is the code dubious
>>> but the comment is inadequate too.
> 
>> Hmm, as far as I can recollect this is just leftover debugging code that
>> was intended to help ensure that we are checking the "changed"
>> everywhere we are supposed to (since I changed handling of these
>> structured quite a bit during development). Should be changed to NULL,
>> that's what we usually do in this type of situation.
> 
> So the comment should be something like "if the column is unchanged,
> we should not attempt to access its value beyond this point.  To
> help catch any such attempts, set the string to NULL" ?
> 

Yes that sounds about right. We don't get any data for unchanged TOAST
columns (that's limitation of logical decoding) so we better not touch them.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services

Petr Jelinek <petr.jelinek@2ndquadrant.com> writes:
> On 05/05/17 16:56, Tom Lane wrote:
>> So the comment should be something like "if the column is unchanged,
>> we should not attempt to access its value beyond this point.  To
>> help catch any such attempts, set the string to NULL" ?

> Yes that sounds about right. We don't get any data for unchanged TOAST
> columns (that's limitation of logical decoding) so we better not touch them.

OK.  I just made it say "we don't get the value of unchanged columns".
        regards, tom lane