Re: [HACKERS] thread-safe libpq and DBD::Pg - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [HACKERS] thread-safe libpq and DBD::Pg
Date
Msg-id 199808091734.NAA12951@candle.pha.pa.us
Whole thread Raw
In response to Re: [HACKERS] thread-safe libpq and DBD::Pg  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] thread-safe libpq and DBD::Pg
List pgsql-hackers
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > Yes, it is very silly that PQoidStatus() returns a char*.  I believe it
> > was only done that way because it was pulling the oid out of the
> > cmdStatus field.  We really should return an int.  Perhaps we don't
> > because we would be hardcoding the oid type on the client side.
>
> Type Oid is already visible to libpq clients: it's returned by PQftype()
> as the type code for attributes.  I think encouraging clients to store
> oids as type Oid rather than some-int-type-chosen-at-random would be a
> good thing, not a bad thing.
>
> However, improving the cleanliness of the API might not be worth the
> cost of breaking application code, even though this routine is probably
> not very widely used.  Does anyone else have an opinion pro or con?
>
> > Perhaps we could double-use the cmdStatus field, perhaps when they ask
> > for the oid, we put a null after the oid value, and return a pointer to
> > the inside of the cmdStatus field.  If the call for PQcmdStatus(), we
> > remove the null and return the string.  Strange, I admit.
>
> I had a variant of that idea: what's stored in cmdStatus looks like
>         INSERT oidvalue count
> but there's a fair amount of room left over (cmdStatus is 40 bytes ...
> and we could make it bigger if we wanted).  We can make PQoidStatus
> copy the oidvalue into the free space:
>         cmdStatus = "INSERT oidvalue count\0oidvalue\0"
> and return a pointer to here ...............^
> That way PQcmdStatus and PQoidStatus don't interfere with each other.
> The result of PQoidStatus will still get zapped by the next interaction
> with the backend, but at least it's not as transient as it would be in
> the errorMessage field.
>
> But, still, converting to an integer return value would be a *lot*
> cleaner.
>
> If we were going to do that, I'd be strongly inclined to change
> PQcmdTuples to return an int (or more likely a long) as well.
> I can't envision any situation where people aren't writing
> atoi(PQcmdTuples(conn)) so why not save them the trouble...

Another idea I had was to a char* field to the result structure.
Initialize it to NULL, then when they want oidStatus, we malloc() some
memory, assign it to our char*, and copy the string into there, and
return that to the user.  If we get called later, we can re-use the
memory, and free() it when we free the PQresult.

It was added in 6.2.

If we change it, we will have to require people using those calls to
modify their apps, rather than just relink with the new libpq.  However,
old binaries will run fine because they would be linked to the old
libpq.

php3 uses PQoidStatus, so I can imagine great problems if we change the
libpq interface.  I am afraid we are stuck with char*.


--
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] thread-safe libpq and DBD::Pg
Next
From: "Oliver Elphick"
Date:
Subject: Re: [HACKERS] How do I construct a varlena?