Thread: PQfnumber and quoted identifiers

PQfnumber and quoted identifiers

From
Peter Eisentraut
Date:
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



Re: PQfnumber and quoted identifiers

From
Andreas Pflug
Date:
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




Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
[ 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


Re: PQfnumber and quoted identifiers

From
Andreas Pflug
Date:
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





Re: PQfnumber and quoted identifiers

From
"Matthew T. O'Connor"
Date:
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.



Re: PQfnumber and quoted identifiers

From
Bruce Momjian
Date:
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
 


Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
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


Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
"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


Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
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


Re: PQfnumber and quoted identifiers

From
Bruce Momjian
Date:
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
 


Re: PQfnumber and quoted identifiers

From
"Dave Page"
Date:

> -----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.


Re: PQfnumber and quoted identifiers

From
Bruce Momjian
Date:
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
 


Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
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


Re: PQfnumber and quoted identifiers

From
Bruce Momjian
Date:
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
 


Re: PQfnumber and quoted identifiers

From
Tom Lane
Date:
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


Re: PQfnumber and quoted identifiers

From
Bruce Momjian
Date:
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