Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-03-18 14:36:23 +0300, Yury Zhuravlev wrote: > Robert Haas wrote: > >On Wed, Mar 2, 2016 at 9:48 PM, Peter Eisentraut wrote: > >>On 2/11/16 9:30 PM, Michael Paquier wrote: ... > > > >We need to decide what to do about this. I disagree with Peter: I > >think that

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Fri, Mar 18, 2016 at 8:36 PM, Yury Zhuravlev wrote: > Robert Haas wrote: >> >> On Wed, Mar 2, 2016 at 9:48 PM, Peter Eisentraut wrote: >>> >>> On 2/11/16 9:30 PM, Michael Paquier wrote: ... >> >> >> We need to decide what to do about this. I

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Sat, Mar 19, 2016 at 12:08 AM, Yury Zhuravlev wrote: > Michael Paquier wrote: >> >> FWIW, when compiling with MS 2015 using the set of perl scripts I am >> not seeing this compilation error... > > This error not in build stage but in GIN regresion tests. CMake

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Thu, Mar 10, 2016 at 11:00 PM, Andres Freund wrote: > On 2016-03-11 04:50:45 +0100, Michael Paquier wrote: >> On Thu, Mar 10, 2016 at 11:40 PM, Robert Haas wrote: >> > We need to decide what to do about this. I disagree with Peter: I >> > think that

Re: [HACKERS] GinPageIs* don't actually return a boolean

Robert Haas wrote: On Wed, Mar 2, 2016 at 9:48 PM, Peter Eisentraut wrote: On 2/11/16 9:30 PM, Michael Paquier wrote: ... We need to decide what to do about this. I disagree with Peter: I think that regardless of stdbool, what we've got right now is sloppy coding - bad

Re: [HACKERS] GinPageIs* don't actually return a boolean

Michael Paquier writes: > FWIW, when compiling with MS 2015 using the set of perl scripts I am > not seeing this compilation error... We may want to understand first > what kind of dependency is involved when doing the cmake build > compared to what is done with

Re: [HACKERS] GinPageIs* don't actually return a boolean

Michael Paquier wrote: FWIW, when compiling with MS 2015 using the set of perl scripts I am not seeing this compilation error... This error not in build stage but in GIN regresion tests. CMake nothing to do with. -- Yury Zhuravlev Postgres Professional: http://www.postgrespro.com The

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund wrote: I plan to commit something like this, unless there's very loud protest from Peter's side. I agree. Peter proposal can be considered in a separate thread. Thanks. -- Yury Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-03-11 04:50:45 +0100, Michael Paquier wrote: > On Thu, Mar 10, 2016 at 11:40 PM, Robert Haas wrote: > > We need to decide what to do about this. I disagree with Peter: I > > think that regardless of stdbool, what we've got right now is sloppy > > coding - bad style

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-03-02 21:48:16 -0500, Peter Eisentraut wrote: > After reviewing this thread and relevant internet lore, I think this > might be the wrong way to address this problem. It is in general not > guaranteed in C that a Boolean-sounding function or macro returns 0 or > 1. Prime examples are

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Thu, Mar 10, 2016 at 11:40 PM, Robert Haas wrote: > We need to decide what to do about this. I disagree with Peter: I > think that regardless of stdbool, what we've got right now is sloppy > coding - bad style if nothing else. Furthermore, I think that while C > lets

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Wed, Mar 2, 2016 at 9:48 PM, Peter Eisentraut wrote: > On 2/11/16 9:30 PM, Michael Paquier wrote: >>> Well, Yury was saying upthread that some MSVC versions have a problem >>> with the existing coding, which would be a reason to back-patch ... >>> but I'd like to see a failing

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2/11/16 9:30 PM, Michael Paquier wrote: >> Well, Yury was saying upthread that some MSVC versions have a problem >> with the existing coding, which would be a reason to back-patch ... >> but I'd like to see a failing buildfarm member first. Don't particularly >> want to promise to support

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Sat, Feb 13, 2016 at 1:48 AM, Tom Lane wrote: > Andres Freund writes: >> On February 12, 2016 5:29:44 PM GMT+01:00, Tom Lane >> wrote: >>> We should standardize on the "((var & FLAG) != 0)" >>> pattern, which works reliably in all

Re: [HACKERS] GinPageIs* don't actually return a boolean

Yury Zhuravlev wrote: Robert Haas wrote: So, is it being pulled in indirectly by some other #include? I can double-check it tomorrow. I've found who include stdbool.h and think it is inevitable for MSVC2013 and MSVC2015. In port/win32.h we inlcude process.h. In process.h included

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Thu, Feb 11, 2016 at 9:30 PM, Michael Paquier wrote: > On Fri, Feb 12, 2016 at 3:45 AM, Tom Lane wrote: >> Andres Freund writes: >>> On 2016-02-11 13:37:17 -0500, Tom Lane wrote: Absolutely; they don't work safely for

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Fri, Feb 12, 2016 at 9:39 AM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Feb 12, 2016 at 8:48 AM, Andres Freund wrote: >>> E.g. if you include stdbool.h [ ginStepRight breaks ] > >> Ah-ha. OK, now I get it. So then I

Re: [HACKERS] GinPageIs* don't actually return a boolean

> OK, that seems reasonable from here. What I'm still fuzzy about is > why including stdbool.h causes a failure. Is it because it defines a > type called "bool" that clashes with ours? That seems like the > obvious explanation, but then how does that not cause the compiler to > just straight-up

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-12 07:59:06 -0500, Robert Haas wrote: > OK, that seems reasonable from here. What I'm still fuzzy about is > why including stdbool.h causes a failure. Is it because it defines a > type called "bool" that clashes with ours? That seems like the > obvious explanation, but then how does

Re: [HACKERS] GinPageIs* don't actually return a boolean

Robert Haas writes: > On Fri, Feb 12, 2016 at 8:48 AM, Andres Freund wrote: >> E.g. if you include stdbool.h [ ginStepRight breaks ] > Ah-ha. OK, now I get it. So then I agree we should back-patch this > at least as far as 9.3 where MSVC 2013 became

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Fri, Feb 12, 2016 at 8:48 AM, Andres Freund wrote: > On 2016-02-12 07:59:06 -0500, Robert Haas wrote: >> OK, that seems reasonable from here. What I'm still fuzzy about is >> why including stdbool.h causes a failure. Is it because it defines a >> type called "bool" that

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-12 09:39:20 -0500, Tom Lane wrote: > Robert Haas writes: > > On Fri, Feb 12, 2016 at 8:48 AM, Andres Freund wrote: > >> E.g. if you include stdbool.h [ ginStepRight breaks ] > > > Ah-ha. OK, now I get it. So then I agree we should

Re: [HACKERS] GinPageIs* don't actually return a boolean

Robert Haas writes: > On Fri, Feb 12, 2016 at 9:39 AM, Tom Lane wrote: >> Um, no, that does not follow. The unanswered question here is why, >> when we *have not* included stdbool.h and *have* typedef'd bool as >> just plain "char", we would get C99

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund writes: > On February 12, 2016 5:29:44 PM GMT+01:00, Tom Lane > wrote: >> We should standardize on the "((var & FLAG) != 0)" >> pattern, which works reliably in all cases. > That's what the second version of my patch, and I presume

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund wrote: Did you read what I wrote? Read. That's not correct for char booleans, because the can have different bits set. Current code support this behavior. But to shoot his leg becomes easier. != 0 much better of course. Thanks. -- Yury Zhuravlev Postgres Professional:

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund wrote: Unless I am missing something major, that doesn't seem to achieve all that much. A cast to a char based bool wouldn't normalize this to 0 or 1. So you're still not guaranteed to be able to do somebool == anotherbool when either are set based on such a macro. In C99

Re: [HACKERS] GinPageIs* don't actually return a boolean

On February 12, 2016 5:40:29 PM GMT+01:00, Yury Zhuravlev wrote: >Andres Freund wrote: >> Unless I am missing something major, that doesn't seem to >> achieve all that much. A cast to a char based bool wouldn't >> normalize this to 0 or 1. So you're still not

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Fri, Feb 12, 2016 at 9:47 AM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Feb 12, 2016 at 9:39 AM, Tom Lane wrote: >>> Um, no, that does not follow. The unanswered question here is why, >>> when we *have not* included

Re: [HACKERS] GinPageIs* don't actually return a boolean

Teodor Sigaev writes: > One more option for patch: > #define GinPageIsLeaf(page)((bool)(GinPageGetOpaque(page)->flags & > GIN_LEAF)) I think that's a seriously bad coding pattern to adopt, because it would work for some people but not others if the flag bit is to the left

Re: [HACKERS] GinPageIs* don't actually return a boolean

On February 12, 2016 5:15:59 PM GMT+01:00, Teodor Sigaev wrote: >One more option for patch: > >#define GinPageIsLeaf(page)((bool)(GinPageGetOpaque(page)->flags & >GIN_LEAF)) > >Seems it will work on any platform with built-in bool. But I don't know >will it >work with

Re: [HACKERS] GinPageIs* don't actually return a boolean

One more option for patch: #define GinPageIsLeaf(page)((bool)(GinPageGetOpaque(page)->flags & GIN_LEAF)) Seems it will work on any platform with built-in bool. But I don't know will it work with 'typedef char bool' if high bit will be set. That's true, but it doesn't really seem like a

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-12 09:47:47 -0500, Tom Lane wrote: > Robert Haas writes: > > On Fri, Feb 12, 2016 at 9:39 AM, Tom Lane wrote: > >> Um, no, that does not follow. The unanswered question here is why, > >> when we *have not* included stdbool.h and *have*

Re: [HACKERS] GinPageIs* don't actually return a boolean

On February 12, 2016 5:29:44 PM GMT+01:00, Tom Lane wrote: > We should standardize on the "((var & FLAG) != 0)" >pattern, which works reliably in all cases. That's what the second version of my patch, and I presume Michael's updated one as well, does. I think the only open

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Thu, Feb 11, 2016 at 9:15 AM, Andres Freund wrote: > On 2016-02-11 08:50:41 -0500, Robert Haas wrote: >> Are we thinking to back-patch this? I would be disinclined to >> back-patch widespread changes like this. If there's a specific >> instance related to Gin where this

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-11 09:51:26 -0500, Robert Haas wrote: > I have never been quite clear on what you think is going to cause > stdbool.h inclusions to become more common. Primarily because it's finally starting to be supported across the board, thanks to MS finally catching up. Then there's uglyness

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Tue, Feb 9, 2016 at 11:53 PM, Michael Paquier wrote: > On Wed, Feb 10, 2016 at 1:17 AM, Yury Zhuravlev > wrote: >> I've just run into a problem with these macro. Function ginStepRight breaks >> completely when compiled using the MSVC2013

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-11 08:50:41 -0500, Robert Haas wrote: > Are we thinking to back-patch this? I would be disinclined to > back-patch widespread changes like this. If there's a specific > instance related to Gin where this is causing a tangible problem, we > could back-patch just that part, with a clear

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Fri, Feb 12, 2016 at 3:45 AM, Tom Lane wrote: > Andres Freund writes: >> On 2016-02-11 13:37:17 -0500, Tom Lane wrote: >>> Absolutely; they don't work safely for testing bits that aren't in the >>> rightmost byte of a flag word, for instance. I'm on

Re: [HACKERS] GinPageIs* don't actually return a boolean

Robert Haas wrote: So, is it being pulled in indirectly by some other #include? I can double-check it tomorrow. -- Yury Zhuravlev Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund writes: > On 2016-02-11 09:51:26 -0500, Robert Haas wrote: >> I have never been quite clear on what you think is going to cause >> stdbool.h inclusions to become more common. > Primarily because it's finally starting to be supported across the > board, thanks to

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-11 13:06:14 -0500, Tom Lane wrote: > But I'm unconvinced that we need to make our .c files prepared for > stdbool.h to be #included in them. The fixes, besides two stylistic edits around !! use, are all in headers. The issue is that we return things meant to be used in a boolean

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Thu, Feb 11, 2016 at 9:54 AM, Andres Freund wrote: > On 2016-02-11 09:51:26 -0500, Robert Haas wrote: >> I have never been quite clear on what you think is going to cause >> stdbool.h inclusions to become more common. > > Primarily because it's finally starting to be

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund writes: > And anyway, these macros are a potential issue even without stdbool.h > style booleans. Absolutely; they don't work safely for testing bits that aren't in the rightmost byte of a flag word, for instance. I'm on board with making these fixes, I'm just

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2016-02-11 13:37:17 -0500, Tom Lane wrote: > Andres Freund writes: > > And anyway, these macros are a potential issue even without stdbool.h > > style booleans. > > Absolutely; they don't work safely for testing bits that aren't in the > rightmost byte of a flag word, for

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund writes: > On 2016-02-11 13:37:17 -0500, Tom Lane wrote: >> Absolutely; they don't work safely for testing bits that aren't in the >> rightmost byte of a flag word, for instance. I'm on board with making >> these fixes, I'm just unconvinced that stdbool is a good

Re: [HACKERS] GinPageIs* don't actually return a boolean

On вторник, 29 сентября 2015 г. 19:02:59 MSK, Alvaro Herrera wrote: Andres Freund wrote: I went through all headers in src/include and checked for macros containing [^&]&[^&] and checked whether they have this hazard. Found a fair number. That patch also changes !! tests into != 0 style. I

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Wed, Feb 10, 2016 at 1:17 AM, Yury Zhuravlev wrote: > I've just run into a problem with these macro. Function ginStepRight breaks > completely when compiled using the MSVC2013 and MSVC2015 (since these > releases use C99's bools but without stdbool.h like C++). > I

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund wrote: > I went through all headers in src/include and checked for macros > containing [^&]&[^&] and checked whether they have this hazard. Found a > fair number. > > That patch also changes !! tests into != 0 style. I don't think you pushed this, did you? -- Álvaro Herrera

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-12 18:52:59 -0400, Tom Lane wrote: Andres Freund and...@anarazel.de writes: I went through all headers in src/include and checked for macros containing [^][^] and checked whether they have this hazard. Found a fair number. That patch also changes !! tests into != 0 style.

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund and...@anarazel.de writes: On 2015-08-12 18:52:59 -0400, Tom Lane wrote: Looks OK to me, except I wonder why you did this #define TRIGGER_FIRED_FOR_ROW(event) \ -((event) TRIGGER_EVENT_ROW) +(((event) TRIGGER_EVENT_ROW) == TRIGGER_EVENT_ROW) rather than != 0.

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-12 19:03:50 -0400, Tom Lane wrote: The adjacent code is doing something different than a bit-test, though: it's checking whether multibit fields have particular values. Yea, I know, that's why I was on the fence about it. Since you have an opinion and I couldn't really decide it's

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund and...@anarazel.de writes: I went through all headers in src/include and checked for macros containing [^][^] and checked whether they have this hazard. Found a fair number. That patch also changes !! tests into != 0 style. Looks OK to me, except I wonder why you did this

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-11 13:49:19 -0300, Alvaro Herrera wrote: Andres Freund wrote: On 2015-08-11 12:43:03 -0400, Robert Haas wrote: On Tue, Aug 11, 2015 at 12:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: We do not use !! elsewhere for this purpose, and I for one find it a pretty ugly locution.

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Tue, Aug 11, 2015 at 12:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: We do not use !! elsewhere for this purpose, and I for one find it a pretty ugly locution. We do, actually, now, in contrib/pg_xlogdump/pg_xlogdump.c. I'd be in favor of getting rid of those. -- Robert Haas EnterpriseDB:

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund wrote: On 2015-08-11 12:43:03 -0400, Robert Haas wrote: On Tue, Aug 11, 2015 at 12:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: We do not use !! elsewhere for this purpose, and I for one find it a pretty ugly locution. We do, actually, now, in

Re: [HACKERS] GinPageIs* don't actually return a boolean

On Tue, Aug 11, 2015 at 11:42 AM, Andres Freund and...@anarazel.de wrote: #define GinPageIsLeaf(page)( GinPageGetOpaque(page)-flags GIN_LEAF ) #define GinPageIsData(page)( GinPageGetOpaque(page)-flags GIN_DATA ) #define GinPageIsList(page)( GinPageGetOpaque(page)-flags GIN_LIST )

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-11 12:04:48 -0400, Robert Haas wrote: On Tue, Aug 11, 2015 at 11:42 AM, Andres Freund and...@anarazel.de wrote: #define GinPageIsLeaf(page)( GinPageGetOpaque(page)-flags GIN_LEAF ) #define GinPageIsData(page)( GinPageGetOpaque(page)-flags GIN_DATA ) #define

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-11 12:43:03 -0400, Robert Haas wrote: On Tue, Aug 11, 2015 at 12:40 PM, Tom Lane t...@sss.pgh.pa.us wrote: We do not use !! elsewhere for this purpose, and I for one find it a pretty ugly locution. We do, actually, now, in contrib/pg_xlogdump/pg_xlogdump.c. I'd be in favor of

Re: [HACKERS] GinPageIs* don't actually return a boolean

On 2015-08-11 17:42:37 +0200, Andres Freund wrote: Hi, #define GinPageIsLeaf(page)( GinPageGetOpaque(page)-flags GIN_LEAF ) #define GinPageIsData(page)( GinPageGetOpaque(page)-flags GIN_DATA ) #define GinPageIsList(page)( GinPageGetOpaque(page)-flags GIN_LIST ) ... These

Re: [HACKERS] GinPageIs* don't actually return a boolean

Andres Freund and...@anarazel.de writes: #define GinPageIsLeaf(page)( GinPageGetOpaque(page)-flags GIN_LEAF ) #define GinPageIsData(page)( GinPageGetOpaque(page)-flags GIN_DATA ) #define GinPageIsList(page)( GinPageGetOpaque(page)-flags GIN_LIST ) These macros don't actually