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 > g...@2ndquadrant.com www.2ndQuadrant.us > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
