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(at)gmx(dot)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(at)pse-consulting(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Andreas Pflug <pgadmin(at)pse-consulting(dot)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(at)zeut(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073