"Boris van Schooten" <schooten (AT) cs (DOT) utwente.nl> writes:
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

Boris van Schooten <schooten (AT) cs (DOT) utwente.nl> writes:
Thanks for the link. As far as can be told from this, dbase hasn't got
nulls at all -- is that correct, or are they just omitting a ton of
relevant information?

It looks to me like we should just remove the special case for integer
fields altogether. The special case for date fields is wrong in detail
as well: as coded it will accept "date" fields with a leading sign,
which surely is not intentional.

regards, tom lane

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

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:



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

I can't find anything about dbase nulls either. Never seen one. I'll
report it when I find nulls (or other bugs in the code). I'm using some
5-10 different db formats, and noticed most of the converters need some
work (as usual).

The isinteger seems to be made to do double duty, but it doesn't exactly
do it well. The code is dated +/-1995, and the version number is unclear,
but I noticed there have been recent changes. Maybe cvs will give more
info?

Tell me when you got your patch ready, then I'll test the newest version.

There was this funny thing with the bug report form on the postgres site.
I got mails which suggest that the message bounced. Later I sent a mail to
Bruce Momjian. I never knew if you replied to the bug report or to the
mail to Bruce. Do you know what happened here? Do bug reports get
bounced?

Kind regards,
Boris van Schooten

On Tue, 3 May 2005, Tom Lane wrote:





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

Boris van Schooten <schooten (AT) cs (DOT) utwente.nl> writes:
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 (AT) postgresql (DOT) 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:


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

Boris van Schooten <schooten (AT) cs (DOT) utwente.nl> writes:
Don't really see the need for it. What we are doing here is trusting to
the backend to error-check the input, rather than making a half-baked
attempt to do error checking in dbf2pg.

regards, tom lane

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

I don't see this as half baked error checking, but rather, as error
reporting during a "half baked conversion". I prefer to know when the
converter encounters something it doesn't understand, rather than having
it silently enter nulls into the db.

It was the original error reporting that helped me find this bug, I
suppose.

Kind regards,
Boris van Schooten

On Thu, 5 May 2005, Tom Lane wrote:



---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

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:
--
Bruce Momjian | http://candle.pha.pa.us
pgman (AT) candle (DOT) 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 (AT) candle (DOT) pha.pa.us> writes:
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