Re: 64-bit pgbench V2

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Greg Smith <greg(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: 64-bit pgbench V2
Date: 2011-02-06 16:09:42
Message-ID: 201102061609.p16G9gT17831@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


What happened to this idea/patch?

---------------------------------------------------------------------------

Greg Smith wrote:
> Tom Lane wrote:
> > Please choose a way that doesn't introduce new portability assumptions.
> > The backend gets along fine without strtoll, and I don't see why pgbench
> > should have to require it.
> >
>
> Funny you should mention this...it turns out there is some code already
> there, I just didn't notice it before because it's only the unsigned
> 64-bit strtoul used, not the signed one I was looking for, and it's only
> called in one place I didn't previously check.
> src/interfaces/ecpg/ecpglib/data.c does this:
>
> *((unsigned long long int *) (var + offset * act_tuple)) =
> strtoull(pval, &scan_length, 10);
>
> The appropriate autoconf magic was in the code all along for both
> versions, so my bad not noticing it until now. It even transparently
> remaps the BSD-ism of calling it strtoq.
>
> I suspect that this alone isn't sufficient to make the code I'm trying
> to wedge into pgbench to always work on the platforms I consider must
> haves, because of the weird names like _strtoi64 that Windows uses:
> http://msdn.microsoft.com/en-us/library/h80404d3(v=VS.80).aspx In fact,
> I wouldn't be surprised to discover the ECPG code above doesn't do the
> right thing if compiled with a 64-bit MSVC version. Don't expect that's
> a popular combination to explicitly test in a way that hits the code
> path where this line is at.
>
> The untested (I need to setup for building Windows to really confirm
> this works) next patch attempt I've attached does what I think is the
> right general sort of thing here. It extends the autoconf remapping
> that was already being done to include the second variation on how the
> function needed can be named in a MSVC build. This might improve the
> ECPG compatibility issue I theorize could be there on that platform.
> Given the autoconf stuff and use of the unsigned version was already a
> dependency, I'd rather improve that code (so it's more obvious when it
> is broken) than do the refactoring work suggested to re-use the server's
> internal 64-bit parsing method instead. I could split this into two
> patches instead--"add 64-bit strtoull/strtoll support for MSVC" on the
> presumption it's actually broken now (possibly wrong on my part) and
> "make pgbench use 64-bit values"--but it's not so complicated as one.
>
> I expect there is almost zero overlap between "needs pgbench setshell to
> return >32 bit return values" and "not on a platform with a working
> 64-bit strtoull variation". What I did to hedge against that was add a
> little check to pgbench that lets you confirm whether setshell lines are
> limited to 32 bits or not, depending on whether the appropriate function
> was found. It tries to fall back to the existing strtol in that case,
> and I've put a note when that happens (and matching documentation to
> look for it) into the debug output of the program.
>
> I'll continue with testing work here, but what's attached is now the
> first form I think this could potentially be committed in once it's
> known to be free of obvious bugs (testing at this database scale takes
> forever). I can revisit not using the library function instead if Tom
> or someone else really opposes this new approach. Given most of the
> autoconf bits are already there and the limited number of platforms
> where this is a problem, I think there's little gain for doing that work
> though.
>
> Style/functional suggestions appreciated.
>
> --
> Greg Smith 2ndQuadrant US Baltimore, MD
> PostgreSQL Training, Services and Support
> greg(at)2ndQuadrant(dot)com www.2ndQuadrant.us
>

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

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jan Urbański 2011-02-06 16:14:10 Re: REVIEW: PL/Python table functions
Previous Message Itagaki Takahiro 2011-02-06 15:13:53 Re: Add support for logging the current role