Prev: mapping object names to role IDs
Next: [HACKERS] Exposing the Xact commit order to the user
From: Tom Lane on 23 May 2010 11:24
Stephen Frost <sfrost(a)snowman.net> writes:
> While reviewing bfba40e2c7b3909d3de13bd1b83b7e85fa8dfec2 (mmm, we like
> git diff -p), I noted that c.h is already included by both extern.h
> and ecpg.header through postgres_fe.h. Given this and that we're
> already doing alot of similar #define's there (unlike in those other
> files), I felt c.h was a more appropriate place. Putting it in c.h
> also means we don't have to duplicate that code.

Ugh. Moving that to c.h doesn't render it not junk code. (For one
thing, it will not operate as intended if you haven't previously
#included <limits.h>, which in fact is not included in c.h.)

If we need this we should do it properly with autoconf.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Stephen Frost on 23 May 2010 11:50
* Tom Lane (tgl(a)sss.pgh.pa.us) wrote:
> Ugh. Moving that to c.h doesn't render it not junk code. (For one
> thing, it will not operate as intended if you haven't previously
> #included <limits.h>, which in fact is not included in c.h.)

Doh.

> If we need this we should do it properly with autoconf.

My autoconf foo is not very good, but once I finish a couple of other
things I'll take a shot at doing it that way.

Thanks!

Stephen
From: Michael Meskes on 24 May 2010 12:13
On Sat, May 22, 2010 at 11:20:50PM -0400, Stephen Frost wrote:
> git diff -p), I noted that c.h is already included by both extern.h
> and ecpg.header through postgres_fe.h. Given this and that we're
> already doing alot of similar #define's there (unlike in those other
> files), I felt c.h was a more appropriate place. Putting it in c.h
> also means we don't have to duplicate that code.

But do other parts of PG also need it? Keep in mind that this works for ecpg
because it needs LLONG_MIN or LONGLONG_MIN anyway. I'm not sure if there are
compilers that have long long without those defines, but I'd guess there
aren't.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber meskes(a)jabber.org
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Michael Meskes on 24 May 2010 12:15
On Sun, May 23, 2010 at 11:50:00AM -0400, Stephen Frost wrote:
> > If we need this we should do it properly with autoconf.

I absolutely agree and planed to do that *after* the release if it makes sense
for the rest of PG, but wouldn't want to mess with it in the current
situtation. On the other hand I didn't want to release with that bug in there.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
ICQ 179140304, AIM/Yahoo/Skype michaelmeskes, Jabber meskes(a)jabber.org
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on 24 May 2010 13:12
Michael Meskes <meskes(a)postgresql.org> writes:
> On Sat, May 22, 2010 at 11:20:50PM -0400, Stephen Frost wrote:
>> git diff -p), I noted that c.h is already included by both extern.h
>> and ecpg.header through postgres_fe.h. Given this and that we're
>> already doing alot of similar #define's there (unlike in those other
>> files), I felt c.h was a more appropriate place. Putting it in c.h
>> also means we don't have to duplicate that code.

> But do other parts of PG also need it? Keep in mind that this works for ecpg
> because it needs LLONG_MIN or LONGLONG_MIN anyway. I'm not sure if there are
> compilers that have long long without those defines, but I'd guess there
> aren't.

I think the current coding is extremely fragile (if it indeed works at
all) because of its assumption that <limits.h> has been included
already. In any case, we have configure tests that exist only for the
benefit of contrib modules, so it's hard to argue that we shouldn't have
one that exists only for ecpg.

I think we should fix this (properly) for 9.0.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 |  Next  |  Last
Pages: 1 2
Prev: mapping object names to role IDs
Next: [HACKERS] Exposing the Xact commit order to the user