Re: WIP patch for Oid formatting in printf/elog strings

> On Dec 18, 2014, at 5:23 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> Mark Dilger wrote:
>
>> I've been going through a copy of the code and replacing int32 and uint32
>> with the appropriate type such as Oid, TransactionId, and such, and have
>> created my own typedef for TypeModType, in order to help chase through
>> the code and figure out what functions handle what kind of data. By
>> defining TypeModType to int64, for example, I get lots of compiler errors
>> when passing a TypeModType * into a function that takes int32 *, and that
>> lets me know that the callers of that function think of it as a typemod
>> argument,
>> even if it does not have any clear documentation to that effect.
>>
>> The exercise is a bit tedious, as I have to change lots of strings like
>> "the value %d is out of bounds" to something like
>> "the value " TYPEMODTYPE_FORMAT " is out of bounds", but the clarity
>> I get from it helps enough that it is useful to me.
>
> If it weren't for the format string issue, which makes translations
> impossible, I'd say let's go ahead with these ideas. For all the
> drawbacks that a 64-bit TransactionId has, I'm sure there's people who
> would prefer that to the constant vacuuming the 32-bit system causes.

I have created #defines from configure which can be used in the code:

#if USE_BIGVARLENA

“the value %ld is out of bounds”

#else

“the value %d is out of bounds”

#endif

(Note the %ld vs %d)

I have not used these defines specifically to switch between string constants in my
working copy, but it could be done, with the obvious reduction in brevity.

> But the int32/TypModType thing looks like we could carry without causing
> any trouble at all. Not the format string part of it, though. How
> large is a patch that changes typmod from int32 to TypModType?
>
> I can see value in having 64-bit OIDs, for databases with large number
> of toasted items. Not in the default build, mind you.
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I am attaching a patch that includes my current changes. This compiles
for me, at least on mac, and runs the regression tests (see below), so long as
I do not use the new configure options —enable-bigoid or —enable-bigvarlena.
I do use those from time to time, to get the code configured that way, but
only so I can see what breaks and work to fix it.

—enable-bigxid compiles and runs the regression tests with only one failure,
at that seems to just be a difference in the query plan that it is choosing.

—enable-bigcid is probably a complete waste of time as a feature that anybody
would want, but it helps me find places in the code where uint32 needs to be
changed to CommandId, and it passes all regression tests, at least on my mac.

Mark Dilger