The following bug has been logged online:

Bug reference: 1643
Logged by: Boris van Schooten
Email address: schooten@cs.utwente.nl
PostgreSQL version: 7.4.5, 8.0.2
Operating system: FreeBSD
Description: dbf2pg broken + quick fix
Details:

As several people on the mailing lists have already noted, dbf2pg is broken.
It usually fails to enter the numeric values from the dbf file in the sql
tables. If you enable verbose output (-vv) you see "Illegal numeric value
found" errors.

As it turns out, the integer checking code (contrib/dbase/dbf2pg.c, function
isinteger) is broken. It appears the function reports 0 in case it finds a
space in the dbf rather than a digit. I disabled it (always made it return
1). Now, my dbf files import fine.

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

"Boris van Schooten" <schooten@cs.utwente.nl> writes:
> As it turns out, the integer checking code (contrib/dbase/dbf2pg.c, function
> isinteger) is broken. It appears the function reports 0 in case it finds a
> space in the dbf rather than a digit. I disabled it (always made it return
> 1). Now, my dbf files import fine.

Hmm. I know nothing about dbase ... but if the test has any use at all,
I guess it must be to handle NULL values. How does dbase represent a
NULL? Why is this code only checking this for integer columns?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

It only does the integer check for numbers of type integer (when # of
decimals = 0), noninteger numbers are not checked in any way. If
isinteger returns 0, the program does give a warning, suggesting it thinks
it detected an illegal field rather than a null field. Contrary to the
isinteger check on dates, which does not give a warning, though it does
enter a null. On dates the check makes sense because dates are always 8
digits long and have no blank padding like numbers. See the spec.

http://www.dbase.com/KnowledgeBase/int/db7_file_fmt.htm

Don't know anything about nulls in dbf though. I am not a dbase expert, I
just run into dbfs often when trying to enter gis data into postgis.

Kind regards,
Boris van Schooten

On Tue, 3 May 2005, Tom Lane wrote:

> "Boris van Schooten" <schooten@cs.utwente.nl> writes:
> > As it turns out, the integer checking code (contrib/dbase/dbf2pg.c, function
> > isinteger) is broken. It appears the function reports 0 in case it finds a
> > space in the dbf rather than a digit. I disabled it (always made it return
> > 1). Now, my dbf files import fine.
>
> Hmm. I know nothing about dbase ... but if the test has any use at all,
> I guess it must be to handle NULL values. How does dbase represent a
> NULL? Why is this code only checking this for integer columns?
>
> regards, tom lane
>



---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

Boris van Schooten <schooten@cs.utwente.nl> writes:
> Don't know anything about nulls in dbf though. I am not a dbase expert, I
> just run into dbfs often when trying to enter gis data into postgis.

I'm considering the following patch, which turns around the test: check
for an empty string and if so believe it's a null, otherwise just insert
the value as-is. I dunno if the check for null is actually meaningful,
but I doubt this will break any cases that worked before. Comments anyone?

regards, tom lane


Index: contrib/dbase/dbf2pg.c
================================================== =================
RCS file: /cvsroot/pgsql/contrib/dbase/dbf2pg.c,v
retrieving revision 1.21
diff -c -r1.21 dbf2pg.c
*** contrib/dbase/dbf2pg.c 14 Sep 2004 03:28:28 -0000 1.21
--- contrib/dbase/dbf2pg.c 4 May 2005 17:55:29 -0000
***************
*** 63,93 ****
char *convert_charset(char *string);
#endif
void usage(void);
- unsigned int isinteger(char *);


-
- unsigned int
- isinteger(char *buff)
- {
- char *i = buff;
-
- while (*i != '\0')
- {
- if (i == buff)
- if ((*i == '-') ||
- (*i == '+'))
- {
- i++;
- continue;
- }
- if (!isdigit((unsigned char) *i))
- return 0;
- i++;
- }
- return 1;
- }
-
static inline void
strtoupper(char *string)
{
--- 63,70 ----
***************
*** 471,478 ****
/* handle the date first - liuk */
if (fields[h].db_type == 'D')
{
! if ((strlen(foo) == 8) && isinteger(foo))
{
snprintf(pgdate, 11, "%c%c%c%c-%c%c-%c%c",
foo[0], foo[1], foo[2], foo[3],
foo[4], foo[5], foo[6], foo[7]);
--- 448,462 ----
/* handle the date first - liuk */
if (fields[h].db_type == 'D')
{
! if (strlen(foo) == 0)
{
+ /* assume empty string means a NULL */
+ strcat(query, "\\N");
+ }
+ else if (strlen(foo) == 8 &&
+ strspn(foo, "0123456789") == 8)
+ {
+ /* transform YYYYMMDD to Postgres style */
snprintf(pgdate, 11, "%c%c%c%c-%c%c-%c%c",
foo[0], foo[1], foo[2], foo[3],
foo[4], foo[5], foo[6], foo[7]);
***************
*** 480,505 ****
}
else
{
! /*
! * empty field must be inserted as NULL value in
! * this way
! */
! strcat(query, "\\N");
}
}
! else if ((fields[h].db_type == 'N') &&
! (fields[h].db_dec == 0))
{
! if (isinteger(foo))
! strcat(query, foo);
! else
{
strcat(query, "\\N");
- if (verbose)
- fprintf(stderr, "Illegal numeric value found "
- "in record %d, field \"%s\"\n",
- i, fields[h].db_name);
}
}
else
{
--- 464,482 ----
}
else
{
! /* try to insert it as-is */
! strcat(query, foo);
}
}
! else if (fields[h].db_type == 'N')
{
! if (strlen(foo) == 0)
{
+ /* assume empty string means a NULL */
strcat(query, "\\N");
}
+ else
+ strcat(query, foo);
}
else
{

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

Looks good to me. I'd prefer to have a warning message (if (verbose)
fprintf stderr) for each of the exceptional conditions though. I'm even
so paranoid I prefer to have the verbose switch on by default.

Kinds regards,
Boris van Schooten

On Wed, 4 May 2005, Tom Lane wrote:

> Boris van Schooten <schooten@cs.utwente.nl> writes:
> > Don't know anything about nulls in dbf though. I am not a dbase expert, I
> > just run into dbfs often when trying to enter gis data into postgis.
>
> I'm considering the following patch, which turns around the test: check
> for an empty string and if so believe it's a null, otherwise just insert
> the value as-is. I dunno if the check for null is actually meaningful,
> but I doubt this will break any cases that worked before. Comments anyone?
>
> regards, tom lane


---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

Tom, where are we on this patch? Seems we need to do something.

Here is the thread:

http://archives.postgresql.org/pgsql...5/msg00008.php

---------------------------------------------------------------------------

Tom Lane wrote:
> Boris van Schooten <schooten@cs.utwente.nl> writes:
> > Don't know anything about nulls in dbf though. I am not a dbase expert, I
> > just run into dbfs often when trying to enter gis data into postgis.
>
> I'm considering the following patch, which turns around the test: check
> for an empty string and if so believe it's a null, otherwise just insert
> the value as-is. I dunno if the check for null is actually meaningful,
> but I doubt this will break any cases that worked before. Comments anyone?
>
> regards, tom lane
>
>
> Index: contrib/dbase/dbf2pg.c
> ================================================== =================
> RCS file: /cvsroot/pgsql/contrib/dbase/dbf2pg.c,v
> retrieving revision 1.21
> diff -c -r1.21 dbf2pg.c
> *** contrib/dbase/dbf2pg.c 14 Sep 2004 03:28:28 -0000 1.21
> --- contrib/dbase/dbf2pg.c 4 May 2005 17:55:29 -0000
> ***************
> *** 63,93 ****
> char *convert_charset(char *string);
> #endif
> void usage(void);
> - unsigned int isinteger(char *);
>
>
> -
> - unsigned int
> - isinteger(char *buff)
> - {
> - char *i = buff;
> -
> - while (*i != '\0')
> - {
> - if (i == buff)
> - if ((*i == '-') ||
> - (*i == '+'))
> - {
> - i++;
> - continue;
> - }
> - if (!isdigit((unsigned char) *i))
> - return 0;
> - i++;
> - }
> - return 1;
> - }
> -
> static inline void
> strtoupper(char *string)
> {
> --- 63,70 ----
> ***************
> *** 471,478 ****
> /* handle the date first - liuk */
> if (fields[h].db_type == 'D')
> {
> ! if ((strlen(foo) == 8) && isinteger(foo))
> {
> snprintf(pgdate, 11, "%c%c%c%c-%c%c-%c%c",
> foo[0], foo[1], foo[2], foo[3],
> foo[4], foo[5], foo[6], foo[7]);
> --- 448,462 ----
> /* handle the date first - liuk */
> if (fields[h].db_type == 'D')
> {
> ! if (strlen(foo) == 0)
> {
> + /* assume empty string means a NULL */
> + strcat(query, "\\N");
> + }
> + else if (strlen(foo) == 8 &&
> + strspn(foo, "0123456789") == 8)
> + {
> + /* transform YYYYMMDD to Postgres style */
> snprintf(pgdate, 11, "%c%c%c%c-%c%c-%c%c",
> foo[0], foo[1], foo[2], foo[3],
> foo[4], foo[5], foo[6], foo[7]);
> ***************
> *** 480,505 ****
> }
> else
> {
> ! /*
> ! * empty field must be inserted as NULL value in
> ! * this way
> ! */
> ! strcat(query, "\\N");
> }
> }
> ! else if ((fields[h].db_type == 'N') &&
> ! (fields[h].db_dec == 0))
> {
> ! if (isinteger(foo))
> ! strcat(query, foo);
> ! else
> {
> strcat(query, "\\N");
> - if (verbose)
> - fprintf(stderr, "Illegal numeric value found "
> - "in record %d, field \"%s\"\n",
> - i, fields[h].db_name);
> }
> }
> else
> {
> --- 464,482 ----
> }
> else
> {
> ! /* try to insert it as-is */
> ! strcat(query, foo);
> }
> }
> ! else if (fields[h].db_type == 'N')
> {
> ! if (strlen(foo) == 0)
> {
> + /* assume empty string means a NULL */
> strcat(query, "\\N");
> }
> + else
> + strcat(query, foo);
> }
> else
> {
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>

--
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, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom, where are we on this patch? Seems we need to do something.

It's in my to-do pile. I was debating whether it's OK to commit to
8.0 branch, or should I just put it in HEAD.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom, where are we on this patch? Seems we need to do something.
>
> It's in my to-do pile. I was debating whether it's OK to commit to
> 8.0 branch, or should I just put it in HEAD.

I am thinking just HEAD. If someone really needs it we can point them
to CVS HEAD and they can test.

--
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, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq