Thread: PQfnumber and quoted identifiers
There was a discussion on -interfaces that might need more consideration. http://archives.postgresql.org/pgsql-interfaces/2003-09/msg00026.php Apparently, it has so far been an undocumented feature of libpq's function PGfnumber (return column number from column name) that the column name needs to be double-quoted if it contains upper-case letters. That, is you need to write PQfnumber(res, "\"Bar\"") I think this is completely bizarre and pointless. This is a C interface and not SQL. Other libpq functions that accept names of SQL objects don't do this. Also, PQfname and PQfnumber ought to be inverses. Since this behavior was undocumented and no one had noticed it in the last 10 years, I think we can away with removing it. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: >There was a discussion on -interfaces that might need more consideration. > >http://archives.postgresql.org/pgsql-interfaces/2003-09/msg00026.php > > >Apparently, it has so far been an undocumented feature of libpq's function >PGfnumber (return column number from column name) that the column name >needs to be double-quoted if it contains upper-case letters. That, is you >need to write > >PQfnumber(res, "\"Bar\"") > >I think this is completely bizarre and pointless. This is a C interface >and not SQL. Other libpq functions that accept names of SQL objects don't >do this. Also, PQfname and PQfnumber ought to be inverses. > >Since this behavior was undocumented and no one had noticed it in the last >10 years, I think we can away with removing it. > > I don't agree; you'll certainly break all libpq apps that contact databases with columns that have uppercase or special chars, and the failure might be very subtle because in most cases you wouldn't expect that this function call fails after you successfully created a rowset. There's no way how an app could determine which flavor of escaping is necessary for PQfnumber. I completely agree that PQfnumber should have been designed C-like right from the start, at least this is how C programmers would expect it. I had to learn the hard way that doesn't. While I don't have a problem with either version, IMHO now it's far too late to change the behaviour. As an alternative, a new function could be invented. BTW, I'd suggest that libpq gets a PQversion() function or macro, so that slight changes in behaviour could be taken in account on the app side if necessary. Regards, Andreas
[ pgsql-interfaces added to cc list ] Andreas Pflug <pgadmin@pse-consulting.de> writes: > Peter Eisentraut wrote: >> There was a discussion on -interfaces that might need more consideration. >> >> http://archives.postgresql.org/pgsql-interfaces/2003-09/msg00026.php >> >> Apparently, it has so far been an undocumented feature of libpq's function >> PGfnumber (return column number from column name) that the column name >> needs to be double-quoted if it contains upper-case letters. That, is you >> need to write >> >> PQfnumber(res, "\"Bar\"") >> >> I think this is completely bizarre and pointless. This is a C interface >> and not SQL. Other libpq functions that accept names of SQL objects don't >> do this. Also, PQfname and PQfnumber ought to be inverses. >> >> Since this behavior was undocumented and no one had noticed it in the last >> 10 years, I think we can away with removing it. > I don't agree; you'll certainly break all libpq apps that contact > databases with columns that have uppercase or special chars, and the > failure might be very subtle because in most cases you wouldn't expect > that this function call fails after you successfully created a rowset. That was pretty much the argument that carried the day in the earlier thread. However, I'm not sure how many people really use PQfnumber (as opposed to hard-wiring assumptions about returned column numbers), and it would seem that the intersection of those people with people who use mixed-case column names may be nearly the empty set. If a lot of people did this, the behavior would have been discussed and documented (or changed) long ago. So I'm not convinced that we'd really break very many apps by changing to the behavior that everyone seems to agree is more sensible. A data point is that we did make comparable changes to the handling of database names a couple releases ago, and we got few if any gripes. Another data point is that the original Berkeley coding of PQfnumber did not have the case folding/dequoting behavior. The history seems to be: Original code: straight strcmp() of argument against returned column name 1997-05-19 23:38: replace strcmp() with strcasecmp() (no dequoting logic, pretty obviously a broken idea in hindsight) 1997-11-10 00:10: attempted to implement the current behavior of dequoting+downcasing, but due to a typo, the actual effect was to revert the behavior to exact match 1999-02-03 15:19: fix typo, installing the current behavior So other than the shortlived 6.2 release, releases before 6.5 had the behavior Peter wants. I find it interesting that it took more than a year for anyone to notice that the putative dequoting+downcasing logic installed for 6.3 didn't work. regards, tom lane
Tom Lane wrote: >That was pretty much the argument that carried the day in the earlier >thread. However, I'm not sure how many people really use PQfnumber >(as opposed to hard-wiring assumptions about returned column numbers), >and it would seem that the intersection of those people with people who >use mixed-case column names may be nearly the empty set. If a lot of >people did this, the behavior would have been discussed and documented >(or changed) long ago. So I'm not convinced that we'd really break >very many apps by changing to the behavior that everyone seems to agree >is more sensible. > > ... >So other than the shortlived 6.2 release, releases before 6.5 had the >behavior Peter wants. I find it interesting that it took more than a >year for anyone to notice that the putative dequoting+downcasing logic >installed for 6.3 didn't work. > pgAdmin3 beta testers found it... If you change it, please give me a chance to code it version-aware so quoting/non-quoting can be performed dependent on libpq in use. Regards, Andreas
On Sun, 2003-10-05 at 13:43, Andreas Pflug wrote: > Tom Lane wrote: > >So other than the shortlived 6.2 release, releases before 6.5 had the > >behavior Peter wants. I find it interesting that it took more than a > >year for anyone to notice that the putative dequoting+downcasing logic > >installed for 6.3 didn't work. > > > pgAdmin3 beta testers found it... > > If you change it, please give me a chance to code it version-aware so > quoting/non-quoting can be performed dependent on libpq in use. Is there a reason why it can't accecpt both formats? That way PQfname and PQfnumber could be inverses of each other. The only wart is that PQfnumber would also accecpt the "\"Bar\"" format also.
Peter Eisentraut wrote: > There was a discussion on -interfaces that might need more consideration. > > http://archives.postgresql.org/pgsql-interfaces/2003-09/msg00026.php > > > Apparently, it has so far been an undocumented feature of libpq's function > PGfnumber (return column number from column name) that the column name > needs to be double-quoted if it contains upper-case letters. That, is you > need to write > > PQfnumber(res, "\"Bar\"") > > I think this is completely bizarre and pointless. This is a C interface > and not SQL. Other libpq functions that accept names of SQL objects don't > do this. Also, PQfname and PQfnumber ought to be inverses. > > Since this behavior was undocumented and no one had noticed it in the last > 10 years, I think we can away with removing it. I agree. I would never expect to add quotes to a C string to preserve case (outside SQL), and the fact that PQfname doesn't return the string in quotes is another inconsistency. It is sort of like case coming out is significant, but case going in has to be quoted --- am I getting this right? Do we do this in other areas? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Andreas Pflug <pgadmin@pse-consulting.de> writes:
> If you change it, please give me a chance to code it version-aware so
> quoting/non-quoting can be performed dependent on libpq in use.
If you want a run-time test, the most reliable way would be to directly
test what PQfnumber does --- for instance, make a query known to
contain the column name "foo", and see what PQfnumber(res, "\"foo\"")
returns. I'm not sure there is any other solution, since we can't
retroactively install a version identifier in old libpq releases.
For a compile-time test, you could perhaps look to see if
PG_DIAG_SQLSTATE or one of the other new macros in postgres_ext.h
is defined. Not sure you really want a compile-time test though;
it'd break very easily if you get linked against some other version
of the library.
As for the more general question of whether to offer libpq version
identification going forward, I have no strong opinion on whether
it's really useful or not. If it's wanted, I'm tempted to suggest
that PQparameterStatus() could be extended to recognize "libpq_version"
paralleling "server_version". Not sure about a clean way to expose
the version at compile time.
regards, tom lane
"Matthew T. O'Connor" <matthew@zeut.net> writes:
> Is there a reason why it can't accecpt both formats?
Does it downcase FOO or not? You can't have it both ways.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Peter Eisentraut wrote: >> Since this behavior was undocumented and no one had noticed it in the last >> 10 years, I think we can away with removing it. > I agree. I would never expect to add quotes to a C string to preserve > case (outside SQL), and the fact that PQfname doesn't return the string > in quotes is another inconsistency. Actually I was planning to quiz you about the history. I can see from the CVS logs that you installed all the patches that added the quoting/downcasing behavior. Was there any discussion about it? I dug through the mail archives and found http://archives.postgresql.org/pgsql-ports/1997-05/msg00081.php http://archives.postgresql.org/pgsql-bugs/1997-05/msg00023.php http://archives.postgresql.org/pgsql-hackers/1997-11/msg00170.php but I could not find any actual discussion about whether there was a real bug or whether the complainants should be told to fix their code. regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Peter Eisentraut wrote: > >> Since this behavior was undocumented and no one had noticed it in the last > >> 10 years, I think we can away with removing it. > > > I agree. I would never expect to add quotes to a C string to preserve > > case (outside SQL), and the fact that PQfname doesn't return the string > > in quotes is another inconsistency. > > Actually I was planning to quiz you about the history. I can see from > the CVS logs that you installed all the patches that added the > quoting/downcasing behavior. Was there any discussion about it? > I dug through the mail archives and found > > http://archives.postgresql.org/pgsql-ports/1997-05/msg00081.php > http://archives.postgresql.org/pgsql-bugs/1997-05/msg00023.php > http://archives.postgresql.org/pgsql-hackers/1997-11/msg00170.php > > but I could not find any actual discussion about whether there was a > real bug or whether the complainants should be told to fix their code. Wow, 1997 --- seems like a time long ago. As I remember, I crudely coded up a fix for the complaint --- that's the way we did it back then. :-) There probably wasn't a lot of discussion, if any --- people complained, and we coded to fix the complaint. In hindsight, I should have told them the API was working properly, but the idea of telling a user they were wrong wasn't something we did back then --- we needed every user we could get. My guess is that the email reports from May, 1997 were in my mailbox, and in Novemeber I saw it again (I was a volunteer then) and I coded up the fix, posted it to the lists, then applied it. No one said anything about it, so it stayed in. Strange no one complained about it until now. I suppose that is because few folks use that function _and_ upper-case identifiers, as you mentioned. We could code the proper behavior, post a mention on general, and document it in the release notes, or wait for 7.5, or do nothing. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
> -----Original Message----- > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > Sent: 05 October 2003 20:41 > To: Tom Lane > Cc: Peter Eisentraut; PostgreSQL Development > Subject: Re: [HACKERS] PQfnumber and quoted identifiers > > > Strange no one complained about it until now. I suppose that > is because few folks use that function _and_ upper-case > identifiers, as you mentioned. We could code the proper > behavior, post a mention on general, and document it in the > release notes, or wait for 7.5, or do nothing. Can it wait for 7.5 (and some method of checking the libpq versions such as Tom's PQparameterStatus() suggestion) please? We already released pgAdmin III taking into account this behaviour in 7.4 and a change now would not be so good for us. Regards, Dave.
Dave Page wrote: > > > > -----Original Message----- > > From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > > Sent: 05 October 2003 20:41 > > To: Tom Lane > > Cc: Peter Eisentraut; PostgreSQL Development > > Subject: Re: [HACKERS] PQfnumber and quoted identifiers > > > > > > Strange no one complained about it until now. I suppose that > > is because few folks use that function _and_ upper-case > > identifiers, as you mentioned. We could code the proper > > behavior, post a mention on general, and document it in the > > release notes, or wait for 7.5, or do nothing. > > Can it wait for 7.5 (and some method of checking the libpq versions such > as Tom's PQparameterStatus() suggestion) please? We already released > pgAdmin III taking into account this behaviour in 7.4 and a change now > would not be so good for us. It is something we could clearly advertise as changing in 7.5. Wasn't there something else we planned to change in 7.5. I forgot it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> It is something we could clearly advertise as changing in 7.5.
I think assuming that advertising a planned change will accomplish much
is just fooling ourselves :-(. There'll be approximately the same
number of complaints either way, because people won't fix their code
in advance (indeed can't, unless we provide a version inquiry method
now).
I'm willing to bow to Dave's schedule-based concern about not doing
it in 7.4, though.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > It is something we could clearly advertise as changing in 7.5. > > I think assuming that advertising a planned change will accomplish much > is just fooling ourselves :-(. There'll be approximately the same > number of complaints either way, because people won't fix their code > in advance (indeed can't, unless we provide a version inquiry method > now). > > I'm willing to bow to Dave's schedule-based concern about not doing > it in 7.4, though. Agreed. We can add it to TODO. That is advertising the change. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Agreed. We can add it to TODO. That is advertising the change.
I'd be inclined to put something in the SGML docs describing
PQfnumber(), also.
regards, tom lane
Added to TODO: * Prevent libpq's PQfnumber() from lowercasing the column name --------------------------------------------------------------------------- Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Agreed. We can add it to TODO. That is advertising the change. > > I'd be inclined to put something in the SGML docs describing > PQfnumber(), also. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073