Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote: > I don't believe '' should be special, any more than 'fred' should be. As > it stands now, NULL 'fred' does say that ,, and '"", are empty strings. > > >Again, I can assist in making these modifications to the patch. > > > > > > I appreciate your efforts. But as indicated elsewhere, right now I'm > leaning towards reworking this into a client, because the road seems to > be blocked on doing what I regard as necessary in the backend. I don't agree about moving this to a client. It is too important for that. Many admin apps and psql need this capability and having it in a client just requires everyone to reinvent the wheel. Let me think over you issues and see what I can come up with. The patch isn't that big and already does 90% of what we need. We just need ot get that extra 10%. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Basic subtransaction facility
Hackers,
Here is a very preliminar patch that allows the user to say "BEGIN"
inside a transaction and have the system react accordingly. This is
only a modification to xact.c (and slightly to other places to allow it
to work); the important functions are empty.
It compiles fine for me with both SUBTRANSACTIONS defined and not
defined; when not defined, the behavior is the same as the current code.
Please note that I have made some errors more fatal than they are now,
as bugs in this code will have much worse effects than a flaw in the
current transaction system.
One quick note: there are two ENDABORT states for a subtransaction,
SUBENDABORT_OK and SUBENDABORT_ERROR. They signal whether the parent
transaction should be aborted after the child transaction finishes or
not: an aborted subtransaction where the user issues COMMIT should
abort the parent transaction; if the user issues ROLLBACK, the parent
can be allowed to continue.
Please have a look and comment. This file does not move a lot so I
don't think it will suffer from a lot of code drift.
--
Alvaro Herrera ()
"I think my standards have lowered enough that now I think 'good design'
is when the page doesn't irritate the living f*ck out of me." (JWZ)
Index: src/backend/access/transam/xact.c
===
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.165
diff -c -r1.165 xact.c
*** src/backend/access/transam/xact.c 5 Apr 2004 03:11:39 - 1.165
--- src/backend/access/transam/xact.c 14 Apr 2004 02:50:02 -
***
*** 189,194
--- 189,219
static void RecordTransactionAbort(void);
static void StartTransaction(void);
+ #ifdef SUBTRANSACTIONS
+ static void StartSubTransaction(void);
+ static void CommitSubTransaction(void);
+ static void AbortSubTransaction(void);
+ static void CleanupSubTransaction(void);
+ static void PushCurrentTransaction(void);
+ static void PopTransaction(void);
+ static bool IsSubTransaction(void);
+ static void ShowTransactionState(void);
+ static void ShowTransactionStateRec(TransactionState, int);
+ #else /* SUBTRANSACTIONS */
+ #define StartSubTransaction()
+ #define CommitSubTransaction()
+ #define AbortSubTransaction()
+ #define CleanupSubTransaction()
+ #define PushCurrentTransaction()
+ #define PopTransaction()
+ #define IsSubTransaction() (false)
+ #define ShowTransactionState()
+ #define ShowTransactionStateRec()
+ #endif /* SUBTRANSACTIONS */
+
+ static char *BlockStateAsString(TBlockState);
+ static char *TransStateAsString(TransState);
+
/*
*global variables holding the current transaction state.
*/
***
*** 200,205
--- 225,237
TRANS_DEFAULT, /* transaction state */
TBLOCK_DEFAULT /* transaction block state from the
client
* perspective */
+ #ifdef SUBTRANSACTIONS
+ ,
+ 0, /* nesting level */
+ NULL, /* top transaction memory
context */
+ NULL, /* commit memory context */
+ NULL/* parent transaction */
+ #endif
};
static TransactionState CurrentTransactionState = &CurrentTransactionStateData;
***
*** 281,287
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_ABORT)
return true;
return false;
--- 313,320
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_ABORT ||
! s->blockState == TBLOCK_SUBABORT)
return true;
return false;
***
*** 1145,1189
break;
/*
-* We should never experience this -- it means the STARTED
state
-* was not changed in the previous CommitTransactionCommand.
-*/
- case TBLOCK_STARTED:
- elog(WARNING, "StartTransactionCommand: unexpected
TBLOCK_STARTED");
- break;
-
- /*
-* We should never experience this -- if we do it means the
-* BEGIN state was not changed in the previous
-* CommitTransactionCommand(). If we get it, we print a
-* warning and change to the in-progress state.
-*/
- case TBLOCK_BEGIN:
- elog(WARNING, "StartTransactionCommand: unexpected
TBLOCK_BEGIN");
- s->blockState = TBLOCK_INPROGRESS;
- break;
-
- /*
Re: [PATCHES] Update french translation of the 7.4 branch
LELARGE Guillaume wrote: Here are some updates of the french .po files. ## pg_dump-fr.po 391 messages traduits. ## pg_resetxlog-fr.po 57 messages traduits. ## pgscripts-fr.po 112 messages traduits. ## psql-fr.po 455 messages traduits. pg_resetxlog, pg_dump and pgscripts .po files are new ones. They need to be apply with the 7.4 branch. Thanks. I didn't see them applied in the 7.4 branch. Were they wrong ? -- Guillaume. signature.asc Description: OpenPGP digital signature
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian wrote: I see that the default NULL for CSV mode is ''. I was hoping the default was something more special. Right now, by default, comma-comma is a null and comma-double-quote-double-quote-comma is a zero-length string. I am thinking there should be a way to set NULL to be either of those, or neither of those, in which case comma-comma is a zero-length string too. To me, these characteristics are a property of the file, not of the individual fields. For example, WITH NULL BOTH would allow ,, and ,"", to both be null, I can't see a real world use for this setting. And I think it would break the property of the patch as it currently stands, that we can unambiguously import what we exported, no matter what the settings. I don't think we should abandon that lightly. Quite apart from any other reason because it makes testing easier (just compare what you wrote with what you read back). while using WITH NULL NONE, both ,, and ,"", are zero-length strings. Again, I think this will break that property. But if that's what it takes to be able to import to a table with NOT NULL in at least some cases I could live with it. Just. But in the general case it won't work. Say you are importing into a table with the following defn: (a text, b text not null, c int). then the line 'x,,' will fail on b if '' is null, and will fail on c if '' is empty string. And yet this sort of output is exactly what is to be expected from a spreadsheet. And, finally, the default is WITH NULL STRICT (or SOME) where ,, is NULL and ,"", is the zero-length string. That's what happens now with the default. Those are all existing keywords, and those special NULL values would only be available in CSV mode. I am not sure what NULL '' should so in these cases. I am thinking we would actually disable it for CSV mode because you would need to define which '' you are talking about. If you specify an actual string for NULL like WITH NULL 'fred', then both ,, and ,"", are zero-length strings, I think. I don't believe '' should be special, any more than 'fred' should be. As it stands now, NULL 'fred' does say that ,, and '"", are empty strings. Again, I can assist in making these modifications to the patch. I appreciate your efforts. But as indicated elsewhere, right now I'm leaning towards reworking this into a client, because the road seems to be blocked on doing what I regard as necessary in the backend. cheers andrew ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian <[EMAIL PROTECTED]> writes: > Andrew Dunstan wrote: >> For all those reasons I disallowed use of WITH OIDS for CSV mode. > While doing OIDs seems atypical, it seems like a reasonable thing that > CSV should be able to do. Basically, I see no reason to disable it. I agree. It's an orthogonal thing, and we shouldn't make CSV mode work differently from normal mode where we don't have to. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote:
> Yes. What *I* meant was that allowing it was OK with me, and not worth
> arguing over.
OK, thanks.
> Incidentally, the patch looks OK at first glance, and seems to work
> fine, modulo today's little controversies, with this exception:
>
> if (csv_mode)
> {
> if (!quote)
> quote = "\"";
> if (!escape)
> escape = "\""; /* should be "escape = quote;" */
> }
Oh, excellent point.
On my other WITH NULL ALL|STRICT|NONE email, we could have '' be ALL and
have it be WITH NULL ''|STRICT|NONE.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (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/faqs/FAQ.html
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian wrote:
Andrew Dunstan wrote:
Bruce Momjian wrote:
While doing OIDs seems atypical, it seems like a reasonable thing that
CSV should be able to do. Basically, I see no reason to disable it.
OK. We have bigger fish to fry ;-)
Uh, sorry, what I meant was that we should have it working in this
patch. No reason to leave it for later. We have to get this 100% right
the first time because people will start relying on it.
Yes. What *I* meant was that allowing it was OK with me, and not worth
arguing over.
Incidentally, the patch looks OK at first glance, and seems to work
fine, modulo today's little controversies, with this exception:
if (csv_mode)
{
if (!quote)
quote = "\"";
if (!escape)
escape = "\""; /* should be "escape = quote;" */
}
cheers
andrew
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote: > >As for setting default values, I think that is a good idea. I suggested > >a while back. There could be another keyword, DEFAULT, on the COPY FROM > >command that is used to define a code that will be replaced by the > >default value (or NULL if there is no default for a column) similar to > >how the NULL code is replaced by NULL. > > > > > > Well, as I indicated we can deal with this in a subsequent round, I > think. However, here's an idea. We know (or can easily discover) if > there is a NOT NULL constraint that can apply to the attribute (or > domain if it is a domain type). If isnull is set on the read-in value in > such a case, instead of trying to insert null, and knowing we would > fail, try to insert the value we actually read (usually ''), even though > we think we found a null. This will succeed with text fields, and fail > with, for example, int fields. That strikes me as quite reasonable > behavior, although perhaps qualifying for a warning. Or perhaps we > could enable such behavior with a flag. > > Of course, this would be for CSV mode only - standard TEXT mode should > work as now. I see that the default NULL for CSV mode is ''. I was hoping the default was something more special. Right now, by default, comma-comma is a null and comma-double-quote-double-quote-comma is a zero-length string. I am thinking there should be a way to set NULL to be either of those, or neither of those, in which case comma-comma is a zero-length string too. To me, these characteristics are a property of the file, not of the individual fields. For example, WITH NULL BOTH would allow ,, and ,"", to both be null, while using WITH NULL NONE, both ,, and ,"", are zero-length strings. And, finally, the default is WITH NULL STRICT (or SOME) where ,, is NULL and ,"", is the zero-length string. Those are all existing keywords, and those special NULL values would only be available in CSV mode. I am not sure what NULL '' should so in these cases. I am thinking we would actually disable it for CSV mode because you would need to define which '' you are talking about. If you specify an actual string for NULL like WITH NULL 'fred', then both ,, and ,"", are zero-length strings, I think. Again, I can assist in making these modifications to the patch. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Updated COPY CSV patch
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: Tom Lane wrote: And how would you define "numeric"? At least the following: int8 int2 int4 float4 float8 numeric money and domains based on them. Wrong answer, as this excludes user-defined types. COPY should not discriminate on the basis of recognizing particular data types. I'm trying to keep this as simple as possible. But we have to be a bit smart if we want to be able to export nicely. Here's the problem: say you have a text field that stores something that has numeric form (phone number, SSN, whatever). You want that exported as text (i.e. quoted). Otherwise, things like leading zeros will get lost by the importing program. However, you *must* not quote genuine number values, or they will not be imported correctly either. Again, you are trying to make COPY into something it isn't and shouldn't be. Exporting nicely has a lot more wrinkles than importing nicely, because predicting the behaviour of the program we might be exporting to is difficult. s/difficult/impossible/. I might be willing to accept this sort of cruft if it were well-defined cruft, but in point of fact you are trying to set up expectations that will be impossible to satisfy. We will be forever making more little tweaks to COPY that render its behavior ever less predictable, in the vain hope of reclosing the can of worms you want to open. It would be a lot wiser to implement this sort of behavior outside the backend, in code that is easily hacked by users. I have neither the time nor the inclination for a fight over this. I hope I never have to use this anyway. Plan B would be to provide a libpq client for importing/exporting CSVs. This is such a basic function that I'd like to see it in the core distribution. Most of the required logic is in what has already been done. It would mean that it had to be done via a client connection, which might have speed implications for very large imports that could run faster direct from the server's file system. I thought there was an agreement on how to do this, but the straight jacket into which it is being forced will significantly limit its utility, IMNSHO. cheers andrew ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote: > Bruce Momjian wrote: > > >While doing OIDs seems atypical, it seems like a reasonable thing that > >CSV should be able to do. Basically, I see no reason to disable it. > > > > > > > > OK. We have bigger fish to fry ;-) Uh, sorry, what I meant was that we should have it working in this patch. No reason to leave it for later. We have to get this 100% right the first time because people will start relying on it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> And how would you define "numeric"? > At least the following: > int8 > int2 > int4 > float4 > float8 > numeric > money > and domains based on them. Wrong answer, as this excludes user-defined types. COPY should not discriminate on the basis of recognizing particular data types. > I'm trying to keep this as simple as possible. But we have to be a bit > smart if we want to be able to export nicely. Here's the problem: say > you have a text field that stores something that has numeric form (phone > number, SSN, whatever). You want that exported as text (i.e. quoted). > Otherwise, things like leading zeros will get lost by the importing > program. However, you *must* not quote genuine number values, or they > will not be imported correctly either. Again, you are trying to make COPY into something it isn't and shouldn't be. > Exporting nicely has a lot more wrinkles than importing nicely, because > predicting the behaviour of the program we might be exporting to is > difficult. s/difficult/impossible/. I might be willing to accept this sort of cruft if it were well-defined cruft, but in point of fact you are trying to set up expectations that will be impossible to satisfy. We will be forever making more little tweaks to COPY that render its behavior ever less predictable, in the vain hope of reclosing the can of worms you want to open. It would be a lot wiser to implement this sort of behavior outside the backend, in code that is easily hacked by users. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Well, as I indicated we can deal with this in a subsequent round, I > think. However, here's an idea. We know (or can easily discover) if > there is a NOT NULL constraint that can apply to the attribute (or > domain if it is a domain type). If isnull is set on the read-in value in > such a case, instead of trying to insert null, and knowing we would > fail, try to insert the value we actually read (usually ''), even though > we think we found a null. This will succeed with text fields, and fail > with, for example, int fields. That strikes me as quite reasonable > behaviour, although perhaps qualifying for a warning. Or perhaps we > could enable such behaviour with a flag. I think this is all a *really really* bad idea. COPY is not supposed to be an "intuit what the user meant" facility. It is supposed to give reliable, predictable results, and in particular it *is* supposed to raise errors if the data is wrong. If you want intuition in interpreting bogus CSV files then an external data conversion program is the place to do it. > Of course, this would be for CSV mode only - standard TEXT mode should > work as now. The CSV flag should not produce any fundamental changes in COPY's behavior, only change the external format it handles. Guessing has no place here. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian wrote: While doing OIDs seems atypical, it seems like a reasonable thing that CSV should be able to do. Basically, I see no reason to disable it. OK. We have bigger fish to fry ;-) cheers andrew ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote: > > > > I have two open issues. First, CSV should support WITH OIDS, no? > > > > Why on earth would you want to? OIDs only have emaning to postgresql. > Dumping to/from CSVs should not be seen as an alternative to Postgresql's > normal text or binary file formats. Rather, it is a way of exchanging > data with other programs that can't conveniently read or write those > formats, but can read/write CSVs. I expect the data to be making a one > way trip. OIDs make no sense to those other programs. > > > For all those reasons I disallowed use of WITH OIDS for CSV mode. > > If you think that we should have it, it will be simple enough to do. It > just strikes me as a very odd thing to do. While doing OIDs seems atypical, it seems like a reasonable thing that CSV should be able to do. Basically, I see no reason to disable it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Updated COPY CSV patch
Bruno Wolff III wrote: On Tue, Apr 13, 2004 at 06:58:24 -0400, Andrew Dunstan <[EMAIL PROTECTED]> wrote: One area that we should think about as an enhancement is NOT NULL fields. As it stands now, we will get what we normally get when we try to insert a null into a NOT NULL field, namely an error. If the field has a simple literal default we could force that. And in the special case of text/varchar fields, it would be reasonable to force an empty string even if no default is set. There isn't a nice easy answer, I'm afraid. We shouldn't hold up putting this in on that account, but handling this better is certainly a TODO. If you try to insert NULLs into a nonnull field you should get an error. If you have unquoted empty strings, and are not using the empty string as the NULL marker, then you can just not set the NULL code to be the empty string. If you need to turn this on and off by column, then perhaps it would be better to do that externally. As for setting default values, I think that is a good idea. I suggested a while back. There could be another keyword, DEFAULT, on the COPY FROM command that is used to define a code that will be replaced by the default value (or NULL if there is no default for a column) similar to how the NULL code is replaced by NULL. Well, as I indicated we can deal with this in a subsequent round, I think. However, here's an idea. We know (or can easily discover) if there is a NOT NULL constraint that can apply to the attribute (or domain if it is a domain type). If isnull is set on the read-in value in such a case, instead of trying to insert null, and knowing we would fail, try to insert the value we actually read (usually ''), even though we think we found a null. This will succeed with text fields, and fail with, for example, int fields. That strikes me as quite reasonable behaviour, although perhaps qualifying for a warning. Or perhaps we could enable such behaviour with a flag. Of course, this would be for CSV mode only - standard TEXT mode should work as now. cheers andrew (trying to be creative here) ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Updated COPY CSV patch
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: Thinking about this some more maybe the right rule would be "quote all non-numeric non-null values". And how would you define "numeric"? At least the following: int8 int2 int4 float4 float8 numeric money and domains based on them. I do *not* like putting data-type-specific knowledge into COPY. I'm trying to keep this as simple as possible. But we have to be a bit smart if we want to be able to export nicely. Here's the problem: say you have a text field that stores something that has numeric form (phone number, SSN, whatever). You want that exported as text (i.e. quoted). Otherwise, things like leading zeros will get lost by the importing program. However, you *must* not quote genuine number values, or they will not be imported correctly either. So, either we get a little bit smart about the types we are exporting, or our export is going to be broken in some cases. We need to be able to use more information than is contained in the text representation of the value. I would certainly hope to keep all this confined to the CSV code path, and not intrude on any existing functionality in any way. Exporting nicely has a lot more wrinkles than importing nicely, because predicting the behaviour of the program we might be exporting to is difficult. If you can suggest a better way I'm all ears. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Thinking about this some more maybe the right rule would be "quote > all non-numeric non-null values". And how would you define "numeric"? I do *not* like putting data-type-specific knowledge into COPY. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Updated COPY CSV patch
Andrew Dunstan wrote:
Bruce Momjian said:
Second, I found a problem with NULLs. If I do:
.
test=> create table test (x text, y text);
CREATE TABLE
test=> insert into test values ('', NULL);
INSERT 17221 1
test=>
then this:
test=> copy test to '/tmp/b' with csv;
creates:
"",
and this:
test=> copy test to '/tmp/b' with csv NULL 'fred';
creates:
,fred
Is that logical? A non-null field went from "" to nothing.
One more point about this - we can't force quoting of every non-null
value, which would remove the "inconsistency" you see here, because
spreadsheets especially infer information from whether or not a CSV value
is quoted. In particular, they will not usually treat a quoted numeric
value as numeric, which would be a very undesirable effect.
Thinking about this some more maybe the right rule would be "quote
all non-numeric non-null values". That would fix the odd effect that
Bruce saw, and it would also stop a spreadsheet from interpreting a date
like 2002-03-04 as an arithmetic expression.
Note that we don't care if a value is quoted or not - the only inference
we draw from it is that if it is quoted it can't be null. We don't need
to draw type inferences from it because we know the type we are trying
to import into from the table defn. This fits nicely with the rule about
being liberal with what you accept.
Thoughts?
cheers
andrew
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Updated COPY CSV patch
On Tue, Apr 13, 2004 at 06:58:24 -0400, Andrew Dunstan <[EMAIL PROTECTED]> wrote: > > One area that we should think about as an enhancement is NOT NULL fields. > As it stands now, we will get what we normally get when we try to insert > a null into a NOT NULL field, namely an error. If the field has a simple > literal default we could force that. And in the special case of > text/varchar fields, it would be reasonable to force an empty string even > if no default is set. There isn't a nice easy answer, I'm afraid. We > shouldn't hold up putting this in on that account, but handling this > better is certainly a TODO. If you try to insert NULLs into a nonnull field you should get an error. If you have unquoted empty strings, and are not using the empty string as the NULL marker, then you can just not set the NULL code to be the empty string. If you need to turn this on and off by column, then perhaps it would be better to do that externally. As for setting default values, I think that is a good idea. I suggested a while back. There could be another keyword, DEFAULT, on the COPY FROM command that is used to define a code that will be replaced by the default value (or NULL if there is no default for a column) similar to how the NULL code is replaced by NULL. ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian said:
> Second, I found a problem with NULLs. If I do:
> .
>test=> create table test (x text, y text);
>CREATE TABLE
>test=> insert into test values ('', NULL);
>INSERT 17221 1
>test=>
>
> then this:
>
>test=> copy test to '/tmp/b' with csv;
>
> creates:
>
>"",
>
> and this:
>
>test=> copy test to '/tmp/b' with csv NULL 'fred';
>
> creates:
>
>,fred
>
> Is that logical? A non-null field went from "" to nothing.
>
One more point about this - we can't force quoting of every non-null
value, which would remove the "inconsistency" you see here, because
spreadsheets especially infer information from whether or not a CSV value
is quoted. In particular, they will not usually treat a quoted numeric
value as numeric, which would be a very undesirable effect.
cheers
andrew
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Updated COPY CSV patch
Bruce Momjian said:
>
> OK, here is a new version of the patch that includes the grammar changes
we agreed upon, SGML changes, and \copy support. I will not make any more
changes without contacting you so feel free to make adjustments and repost.
Excellent. Quick work :-) I will test later today.
Meanwhile, quick response to your comments.
>
> I have two open issues. First, CSV should support WITH OIDS, no?
>
Why on earth would you want to? OIDs only have emaning to postgresql.
Dumping to/from CSVs should not be seen as an alternative to Postgresql's
normal text or binary file formats. Rather, it is a way of exchanging
data with other programs that can't conveniently read or write those
formats, but can read/write CSVs. I expect the data to be making a one
way trip. OIDs make no sense to those other programs.
For all those reasons I disallowed use of WITH OIDS for CSV mode.
If you think that we should have it, it will be simple enough to do. It
just strikes me as a very odd thing to do.
> Second, I found a problem with NULLs. If I do:
> .
>test=> create table test (x text, y text);
>CREATE TABLE
>test=> insert into test values ('', NULL);
>INSERT 17221 1
>test=>
>
> then this:
>
>test=> copy test to '/tmp/b' with csv;
>
> creates:
>
>"",
>
> and this:
>
>test=> copy test to '/tmp/b' with csv NULL 'fred';
>
> creates:
>
>,fred
>
> Is that logical? A non-null field went from "" to nothing.
>
Yes, I think it is logical, although it is strange, I agree. See below.
> I think it is caused by this code:
>
> bool force_quote = (strcmp(string, null_print) == 0);
> CopyAttributeOutCSV(string, delim, quote, escape,
> force_quote);
>
> The reason it happens is that when the null string is '', it matches a
zero-length string, so the value is quoted. When the null stirng isn't
blank, a zero-length string doesn't match the null string so it isn't
quoted.I think we need to add special logic for zero-length strings so
they are always quoted, even if there is a special null string. This will
make our dumps more consistent, I think, or maybe the current behavior is
OK. It just struck me as strange.
>
I agree it seems strange, but that's because you probably aren't that
used to dealing with CSVs.
The code you quote is there so that if *we* reload a CSV *we* created the
null property is preserved. That's actually quite a neat property, and
one I would have said is "beyond contract requirements" :-). The effect
you have seen seems perverse because you have chosen a perverse null
marker - I expect everyone (well, nearly everyone) who uses this will do
what seems to me natural, and use '' as the null marker, and then, as you
saw, the results look quite intuitive.
The real test of this is how well it plays with other programs, rather
than how well data makes a round trip from postgresql to postgresql. In
that sense, we need to let it out into the wild a bit and see what happens.
There is a bit of an impedance mismatch between the way databases look at
data and the way spreadsheets look at data. And CSV is a horribly brain-
dead data format. Its one saving grace is that it is a lowest common
denominator format that practically every data handling proggram under
the sun understands. So there is going to be a bit of oddness. And we may
have to make the odd tweak here and there.
One area that we should think about as an enhancement is NOT NULL fields.
As it stands now, we will get what we normally get when we try to insert
a null into a NOT NULL field, namely an error. If the field has a simple
literal default we could force that. And in the special case of
text/varchar fields, it would be reasonable to force an empty string even
if no default is set. There isn't a nice easy answer, I'm afraid. We
shouldn't hold up putting this in on that account, but handling this
better is certainly a TODO.
(MySQL solves this problem by requiring every NOT NULL field to have a
default, and silently supplying one if it is not specified, and the
inserting a null is the sane as inserting DEFAULT. I don't think we want
to go down that road this bit me badly a few weeks back when I
applied some schema changes someone had carelessly designed relying on
this behaviour. Of course I was running PostgrSQL and the whole thing
blew up on me.)
> I did a dump/reload test with a null string and null, and it worked fine.
>
> Is there any data that can not be dumped/reloaded via CSV?
>
I know it works fine for geometric types. I have seen it dump ACLs quite
happily, although I didn't try to reload them. I will do some testing
soon with arrays and byteas.
cheers
andrew
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] aclitem accessor functions
Dear Peter, > > I needed these functions to browse aclitems from user land. I can > > load them when necessary, but it seems to me that these accessors for > > a backend type belong to the backend, so I submit them. > > Can you explain what you want to do from the user level? Sure. Before developing that point, I want to underline that it should be a general principle to avoid "opaque" datatypes in pg that stores structured information without being able to access them directly. Either you trust the database as a general tool, and it can manipulate its own data, or you do no trust it and you want any special system thing to be "programmed" in C or whatever instead of "queried" from SQL. As for the specifics: I'm developing some "pg_advisor" schema that was discussed earlier, so as to check the design, performance, system... coherency of a database. This is developed in userland with simple relational queries on pg_catalog, and some help by plpgsql if I cannot do without it. Among many other things, I would like to check that granted rights are granted by grantors who have a with grant option, for all possible objects and users. > We already have a bunch of functions for analyzing privileges. Sure, but these has_privilege functions answer to specific questions. I could do that with these functions, but they hide queries, and I think that some joins would be enough to get all the answers directly without sub-quering for all possible object and user. I order to do so, I need to be able to read the "aclitem" type, so I added these accessors. The patch just adds 5 2-lines C functions. Have a nice day, -- Fabien. ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] win32 fixes
> * rationalized pipe read EINTR for win32 Should read: * rationalized pipe read EOF for win32 Corrected patch attached. --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html diff.patch Description: Binary data ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
