Re: ECPG - Some errno definitions don't match to the manual

Hi all,

I'm looking into some ecpg part of the official manual,
and I have found some strange things.

I'm now investigating SQLCODE and SQLSTATE, and I have found
that some of the errno definitions don't match to the manual.

For example, the manual says that ECPG_CONVERT_BOOL could be `-207'.
However, ECPG_CONVERT_BOOL is defined as `-211' in ecpgerrno.h.

> -207 (ECPG_CONVERT_BOOL)
>
> This means the host variable is of type bool and the datum in the database is neither 't' nor 'f'. (SQLSTATE 42804)

http://www.postgresql.org/docs/9.0/static/ecpg-errors.html

> #define ECPG_NUMERIC_FORMAT -207
> #define ECPG_CONVERT_BOOL -211

http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/include/ecpgerrno.h?rev=1.27;content-type=text%2Fx-cvsweb-markup;only_with_tag=REL9_0_STABLE

What does it mean? The manual is not up to date?

Any suggestions?

Regards,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

2010/7/22 Satoshi Nagayasu <satoshi(dot)nagayasu(at)gmail(dot)com>:
> Hi all,
>
> I'm looking into some ecpg part of the official manual,
> and I have found some strange things.
>
> I'm now investigating SQLCODE and SQLSTATE, and I have found
> that some of the errno definitions don't match to the manual.
>
> For example, the manual says that ECPG_CONVERT_BOOL could be `-207'.
> However, ECPG_CONVERT_BOOL is defined as `-211' in ecpgerrno.h.
>
>> -207 (ECPG_CONVERT_BOOL)
>>
>>     This means the host variable is of type bool and the datum in the database is neither 't' nor 'f'. (SQLSTATE 42804)
>
> http://www.postgresql.org/docs/9.0/static/ecpg-errors.html
>
>> #define ECPG_NUMERIC_FORMAT           -207
>> #define ECPG_CONVERT_BOOL             -211
>
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/ecpg/include/ecpgerrno.h?rev=1.27;content-type=text%2Fx-cvsweb-markup;only_with_tag=REL9_0_STABLE
>
> What does it mean? The manual is not up to date?

Yeah, I think that's what it means. Perhaps you could provide a patch
for the docs?

Another question is whether we oughn't to remove all mention of the
specific values of these constants from the documentation. That
wouldn't prevent all problems in this area, because constants can
still be added and removed, but ISTM that including the specific
numerical values here is encouraging people to depend on those values
when they probably shouldn't.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On 2010/08/09 21:27, Robert Haas wrote:
>> I'm now investigating SQLCODE and SQLSTATE, and I have found
>> that some of the errno definitions don't match to the manual.

>> What does it mean? The manual is not up to date?
>
> Yeah, I think that's what it means. Perhaps you could provide a patch
> for the docs?

Sure, I will make a patch for this. Please wait a moment.

> Another question is whether we oughn't to remove all mention of the
> specific values of these constants from the documentation. That
> wouldn't prevent all problems in this area, because constants can
> still be added and removed, but ISTM that including the specific
> numerical values here is encouraging people to depend on those values
> when they probably shouldn't.

Hmm, it's a difficult point. From the application programmers' viewpoint, error codes and error numbers would be an interface between their application and PostgreSQL internals. So I think (and I hope) it should be in the programmer's manual. On the other hand, from the PostgreSQL developer's viewpoint, it is hard to maintain all of documents up to date. So removing them from the manual will make maintenance easier.

I found that this difference was generated by the very very old change in the code repository. And I think it would never be happen anymore, because nowadays PostgreSQL developers, especially code reviewers, always require "document updates/patches" for any patches before applying. (as all you know :)

I agree with that taking care about both code and documents is not so easy, but the current review/development process seems to be appropriate.

Any comments or suggestions?

--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

On 2010/08/10 1:36, Satoshi Nagayasu wrote:
> On 2010/08/09 21:27, Robert Haas wrote:
>>> I'm now investigating SQLCODE and SQLSTATE, and I have found
>>> that some of the errno definitions don't match to the manual.
>
>>> What does it mean? The manual is not up to date?
>>
>> Yeah, I think that's what it means. Perhaps you could provide a patch
>> for the docs?
>
> Sure, I will make a patch for this. Please wait a moment.

A patch to fix this problem is attached.
Please take a look.

Regards,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

On Mon, Aug 9, 2010 at 11:29 PM, Satoshi Nagayasu
<satoshi(dot)nagayasu(at)gmail(dot)com> wrote:
> On 2010/08/10 1:36, Satoshi Nagayasu wrote:
>>
>> On 2010/08/09 21:27, Robert Haas wrote:
>>>>
>>>> I'm now investigating SQLCODE and SQLSTATE, and I have found
>>>> that some of the errno definitions don't match to the manual.
>>
>>>> What does it mean? The manual is not up to date?
>>>
>>> Yeah, I think that's what it means. Perhaps you could provide a patch
>>> for the docs?
>>
>> Sure, I will make a patch for this. Please wait a moment.
>
> A patch to fix this problem is attached.
> Please take a look.

I've applied the portion of this patch that just fixes the existing
entries to match ecpgerrno.h, and have back-patched it all the way
back to 7.4, since the contents of ecpgerrno.h haven't changed in the
interim. The remainder of this will need more review than I can give
it right now: since I'm not familiar with ECPG, I can't easily verify
that the new entries you've added are correct. I'd suggest posting a
new version and adding it here:

https://commitfest.postgresql.org/action/commitfest_view/open

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On 2010/08/12 4:08, Robert Haas wrote:
> On Mon, Aug 9, 2010 at 11:29 PM, Satoshi Nagayasu
> <satoshi(dot)nagayasu(at)gmail(dot)com> wrote:
>> On 2010/08/10 1:36, Satoshi Nagayasu wrote:
>>>
>>> On 2010/08/09 21:27, Robert Haas wrote:
>>>>>
>>>>> I'm now investigating SQLCODE and SQLSTATE, and I have found
>>>>> that some of the errno definitions don't match to the manual.

>> A patch to fix this problem is attached.
>> Please take a look.
>
> I've applied the portion of this patch that just fixes the existing
> entries to match ecpgerrno.h, and have back-patched it all the way
> back to 7.4, since the contents of ecpgerrno.h haven't changed in the
> interim. The remainder of this will need more review than I can give
> it right now: since I'm not familiar with ECPG, I can't easily verify
> that the new entries you've added are correct. I'd suggest posting a
> new version and adding it here:
>
> https://commitfest.postgresql.org/action/commitfest_view/open

Thank you for the commit, and I'll put the remainder
to the commitfest page.

Regards,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

On tor, 2010-08-12 at 09:51 +0900, Satoshi Nagayasu wrote:
> Thank you for the commit, and I'll put the remainder
> to the commitfest page.

Please submit a new patch that applies on top of what was already
committed.

Hi Peter,

Thanks for your reviewing.

On 2010/09/26 20:57, Peter Eisentraut wrote:
> On tor, 2010-08-12 at 09:51 +0900, Satoshi Nagayasu wrote:
>> Thank you for the commit, and I'll put the remainder
>> to the commitfest page.
>
> Please submit a new patch that applies on top of what was already
> committed.

Here is a new patch applicable to the cvs head.
Please check it out.

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

As a comment from Álvaro Herrera, I have made some changes around ENOMEM
from quotes to tags.

- Indicates that your virtual memory is exhausted, defined as `-ENOMEM'.

+ Indicates that your virtual memory is exhausted, defined as <literal>-ENOMEM</literal>.

Thanks,

On 2010/09/28 7:51, Satoshi Nagayasu wrote:
> Hi Peter,
>
> Thanks for your reviewing.
>
> On 2010/09/26 20:57, Peter Eisentraut wrote:
>> On tor, 2010-08-12 at 09:51 +0900, Satoshi Nagayasu wrote:
>>> Thank you for the commit, and I'll put the remainder
>>> to the commitfest page.
>>
>> Please submit a new patch that applies on top of what was already
>> committed.
>
> Here is a new patch applicable to the cvs head.
> Please check it out.
>
> Thanks,

--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

On Mon, Sep 27, 2010 at 6:51 PM, Satoshi Nagayasu
<satoshi(dot)nagayasu(at)gmail(dot)com> wrote:
> Hi Peter,
>
> Thanks for your reviewing.
>
> On 2010/09/26 20:57, Peter Eisentraut wrote:
>>
>> On tor, 2010-08-12 at 09:51 +0900, Satoshi Nagayasu wrote:
>>>
>>> Thank you for the commit, and I'll put the remainder
>>> to the commitfest page.
>>
>> Please submit a new patch that applies on top of what was already
>> committed.
>
> Here is a new patch applicable to the cvs head.
> Please check it out.

Are you aware that we have switched to git?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On 2010/09/28 9:40, Robert Haas wrote:
>> Here is a new patch applicable to the cvs head.
>> Please check it out.
>
> Are you aware that we have switched to git?

Oops, I visited a page describing about the coderepository
from here:

http://www.postgresql.org/developer/coding

Yeah, I know the code repository has been moved to Git,
but I believed that Git and anon-cvs are available
since the doc mentions with them both.

anon-cvs is not available for now?
If so, I have to learn Git at first.

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

Satoshi Nagayasu wrote:
> Oops, I visited a page describing about the coderepository
> from here:
>
> http://www.postgresql.org/developer/coding

I just made a pass through the wiki cleaning up all of the outdated CVS
related stuff I knew of there that this page was pointing to. It's a
sequential group of 9 changes to revert if we're forsaken and somehow
this conversion stops. Main thing left with obsolete info is
http://developer.postgresql.org/pgdocs/postgres/cvs.html

> Yeah, I know the code repository has been moved to Git,
> but I believed that Git and anon-cvs are available
> since the doc mentions with them both.

If the patch you generated applies to the git version of the repo
cleanly, you may be fine for now. The anon-cvs version is mainly
intended as a source to get a copy of the source code just to compile or
read, not to develop patches against. For that it would be better to
switch to git when you get some time to spend on that.

--
Greg Smith, 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us
Author, "PostgreSQL 9.0 High Performance" Pre-ordering at:
https://www.packtpub.com/postgresql-9-0-high-performance/book

On 2010/09/28 10:31, Greg Smith wrote:
> If the patch you generated applies to the git version of the repo cleanly, you may be fine for now. The anon-cvs version is mainly intended as a source to get a copy of the source code just to compile or read, not to develop patches against. For that it would be better to switch to git when you get some time to spend on that.

So, I have created another patch from git repository.
The attached one could be applied to the latest code.

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

Excerpts from Satoshi Nagayasu's message of lun sep 27 22:38:44 -0400 2010:
> On 2010/09/28 10:31, Greg Smith wrote:
> > If the patch you generated applies to the git version of the repo cleanly, you may be fine for now. The anon-cvs version is mainly intended as a source to get a copy of the source code just to compile or read, not to develop patches against. For that it would be better to switch to git when you get some time to spend on that.
>
> So, I have created another patch from git repository.
> The attached one could be applied to the latest code.

Looking at this patch I cannot but wonder if this should be a table
instead of a plain list.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On 2010/09/28 12:40, Alvaro Herrera wrote:
> Excerpts from Satoshi Nagayasu's message of lun sep 27 22:38:44 -0400 2010:
>> On 2010/09/28 10:31, Greg Smith wrote:
>>> If the patch you generated applies to the git version of the repo cleanly, you may be fine for now. The anon-cvs version is mainly intended as a source to get a copy of the source code just to compile or read, not to develop patches against. For that it would be better to switch to git when you get some time to spend on that.
>>
>> So, I have created another patch from git repository.
>> The attached one could be applied to the latest code.
>
> Looking at this patch I cannot but wonder if this should be a table
> instead of a plain list.

What's happen if you run the patch command at top of the repository?

[snaga(at)devwa02 postgresql]$ pwd
/home/snaga/pgsql/repo/postgresql
[snaga(at)devwa02 postgresql]$ patch -p1 < ../ecpg-git.sgml.diff
patching file doc/src/sgml/ecpg.sgml
[snaga(at)devwa02 postgresql]$

or

[snaga(at)devwa02 postgresql]$ git apply --reject ../ecpg-git.sgml.diff
../ecpg-git.sgml.diff:123: trailing whitespace.

Checking patch doc/src/sgml/ecpg.sgml...
Applied patch doc/src/sgml/ecpg.sgml cleanly.
warning: 1 line adds whitespace errors.
[snaga(at)devwa02 postgresql]$

Sorry I'm not familiar with Git yet, but I think above command can
apply the patch.
(I created the patch with the command `git diff'. Is this correct?)

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

Excerpts from Satoshi Nagayasu's message of mar sep 28 00:06:19 -0400 2010:
> On 2010/09/28 12:40, Alvaro Herrera wrote:

> > Looking at this patch I cannot but wonder if this should be a table
> > instead of a plain list.
>
> What's happen if you run the patch command at top of the repository?

Sorry, I didn't try applying it. I don't think there's anything wrong
with the patch per se. I'm just saying that maybe this page needs more
significant SGML markup hacking.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Greg Smith <greg(at)2ndquadrant(dot)com> writes:
> I just made a pass through the wiki cleaning up all of the outdated CVS
> related stuff I knew of there that this page was pointing to. It's a
> sequential group of 9 changes to revert if we're forsaken and somehow
> this conversion stops. Main thing left with obsolete info is
> http://developer.postgresql.org/pgdocs/postgres/cvs.html

Hrm, that page shouldn't even exist anymore. Peter?

regards, tom lane

On 2010/09/28 14:08, Alvaro Herrera wrote:
> Excerpts from Satoshi Nagayasu's message of mar sep 28 00:06:19 -0400 2010:
>> On 2010/09/28 12:40, Alvaro Herrera wrote:
>
>>> Looking at this patch I cannot but wonder if this should be a table
>>> instead of a plain list.
>>
>> What's happen if you run the patch command at top of the repository?
>
> Sorry, I didn't try applying it. I don't think there's anything wrong
> with the patch per se. I'm just saying that maybe this page needs more
> significant SGML markup hacking.

I see. Yes, at first, I thought it, too.

However, at this time, I think SGML hack is not necessary right now,
because the important thing is to fix several missing definitions,

So, I want to hear from others on this issue before starting SGML hack
(such as "create a table instead of a list").
If markup hack is necessary for this time, I'm ready to go.

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

On tis, 2010-09-28 at 02:46 -0400, Tom Lane wrote:
> Greg Smith <greg(at)2ndquadrant(dot)com> writes:
> > I just made a pass through the wiki cleaning up all of the outdated CVS
> > related stuff I knew of there that this page was pointing to. It's a
> > sequential group of 9 changes to revert if we're forsaken and somehow
> > this conversion stops. Main thing left with obsolete info is
> > http://developer.postgresql.org/pgdocs/postgres/cvs.html
>
> Hrm, that page shouldn't even exist anymore. Peter?

I've changed the doc build to use rsync so that orphaned files get
deleted.

On tis, 2010-09-28 at 11:38 +0900, Satoshi Nagayasu wrote:
> On 2010/09/28 10:31, Greg Smith wrote:
> > If the patch you generated applies to the git version of the repo cleanly, you may be fine for now. The anon-cvs version is mainly intended as a source to get a copy of the source code just to compile or read, not to develop patches against. For that it would be better to switch to git when you get some time to spend on that.
>
> So, I have created another patch from git repository.
> The attached one could be applied to the latest code.

Committed, with some adjustments. Notably, you probably misinterpreted
the meaning of the -600 error code from the comment in header file.
Also, the -600 and -601 codes are not generated by the current code at
all, so I have commented them out in the documentation.

On 2010/09/29 20:04, Peter Eisentraut wrote:
> On tis, 2010-09-28 at 11:38 +0900, Satoshi Nagayasu wrote:
>> On 2010/09/28 10:31, Greg Smith wrote:
>>> If the patch you generated applies to the git version of the repo cleanly, you may be fine for now. The anon-cvs version is mainly intended as a source to get a copy of the source code just to compile or read, not to develop patches against. For that it would be better to switch to git when you get some time to spend on that.
>>
>> So, I have created another patch from git repository.
>> The attached one could be applied to the latest code.
>
> Committed, with some adjustments. Notably, you probably misinterpreted
> the meaning of the -600 error code from the comment in header file.
> Also, the -600 and -601 codes are not generated by the current code at
> all, so I have commented them out in the documentation.

Thank you for the commit.

Yes, I have checked only the header file (ecpgerrno.h), and not checked
several caller functions. So, your commit seems to be an appropriate way.

Thanks,
--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

Peter Eisentraut wrote:
> On tis, 2010-09-28 at 02:46 -0400, Tom Lane wrote:
>
>> Greg Smith <greg(at)2ndquadrant(dot)com> writes:
>>
>>> I just made a pass through the wiki cleaning up all of the outdated CVS
>>> related stuff I knew of there that this page was pointing to. It's a
>>> sequential group of 9 changes to revert if we're forsaken and somehow
>>> this conversion stops. Main thing left with obsolete info is
>>> http://developer.postgresql.org/pgdocs/postgres/cvs.html
>>>
>> Hrm, that page shouldn't even exist anymore. Peter?
>>
>
> I've changed the doc build to use rsync so that orphaned files get
> deleted.
>

That one is fixed. Doesn't look like the incoming path from the main
postgresql.org page is completely sorted out yet though now that I check
that too. If people start at http://www.postgresql.org/developer/coding
and click "Information about the Source Code Repository", they'll head
to http://www.postgresql.org/docs/current/static/git.html - good so far
- but clicking on "Up" from there goes to the 9.0
http://www.postgresql.org/docs/current/static/cvs.html still, suggesting
the official repo is the CVS one.

Not sure if that is just cached stuff on the static pages that Peter's
rsync change will clear eventually too, or if that's a final piece that
needs a backport to fix.

--
Greg Smith, 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us
Author, "PostgreSQL 9.0 High Performance" Pre-ordering at:
https://www.packtpub.com/postgresql-9-0-high-performance/book

On Thu, Sep 30, 2010 at 09:00, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
> Peter Eisentraut wrote:
>
> On tis, 2010-09-28 at 02:46 -0400, Tom Lane wrote:
>
>
> Greg Smith <greg(at)2ndquadrant(dot)com> writes:
>
>
> I just made a pass through the wiki cleaning up all of the outdated CVS
> related stuff I knew of there that this page was pointing to. It's a
> sequential group of 9 changes to revert if we're forsaken and somehow
> this conversion stops. Main thing left with obsolete info is
> http://developer.postgresql.org/pgdocs/postgres/cvs.html
>
>
> Hrm, that page shouldn't even exist anymore. Peter?
>
>
> I've changed the doc build to use rsync so that orphaned files get
> deleted.
>
>
> That one is fixed.  Doesn't look like the incoming path from the main
> postgresql.org page is completely sorted out yet though now that I check
> that too.  If people start at http://www.postgresql.org/developer/coding and
> click "Information about the Source Code Repository", they'll head to
> http://www.postgresql.org/docs/current/static/git.html - good so far - but
> clicking on "Up" from there goes to the 9.0
> http://www.postgresql.org/docs/current/static/cvs.html still, suggesting the
> official repo is the CVS one.
>
> Not sure if that is just cached stuff on the static pages that Peter's rsync
> change will clear eventually too, or if that's a final piece that needs a
> backport to fix.

No.

Peters rsync changes only affects the developer docs. The ones you are
looking at are the 9.0.0 docs. They are "stuck" at 9.0.0 until we
release 9.0.1, which is when they'll pick up the new text about the
git repo.

If we hadn't scheduled a 9.0.1 release fairly soon I'd look at somehow
getting that back in the old version, but given that we have, it's
just more work than it's worth (the docs handling on the website is
not very nice at this point - something that's being worked on)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On 2010/09/30 17:08, Magnus Hagander wrote:
> Peters rsync changes only affects the developer docs. The ones you are
> looking at are the 9.0.0 docs. They are "stuck" at 9.0.0 until we
> release 9.0.1, which is when they'll pick up the new text about the
> git repo.
>
> If we hadn't scheduled a 9.0.1 release fairly soon I'd look at somehow
> getting that back in the old version, but given that we have, it's
> just more work than it's worth (the docs handling on the website is
> not very nice at this point - something that's being worked on)

I think the easiest way to avoid(my) misunderstanding is
to put some note, like "Use Git to make your patch",
on the Coding page.

http://www.postgresql.org/developer/coding

Since it was my entry point, I guess it's enough for now,
even if 9.0.1 will take more time (or not). :)

--
NAGAYASU Satoshi <satoshi(dot)nagayasu(at)gmail(dot)com>

Satoshi Nagayasu wrote:
> I think the easiest way to avoid(my) misunderstanding is
> to put some note, like "Use Git to make your patch",
> on the Coding page.
>
> http://www.postgresql.org/developer/coding

If you visit that page again right now, you'll find it hard to reach a
destination that doesn't talk about git instead of CVS. "Information
about the Source Code Repository", the FAQ, "Developer Section on
wiki.postgresql.org", every one of those has now been corrected. The
one way left you can still get the impression the project uses CVS will
go away when 9.0.1 ships, but even that one is a secondary link--one you
only see after a visit to a page talking about git.

Thanks for the report and sorry about the confusion. I don't think
people who visit the "Coding" page are likely to run into the problem
you did at this point anymore. Everything that can be easily cleaned up
is now, and that one stray CVS reference will go away shortly.

--
Greg Smith, 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us
Author, "PostgreSQL 9.0 High Performance" Pre-ordering at:
https://www.packtpub.com/postgresql-9-0-high-performance/book