COPY enhancements

On Thu, Sep 10, 2009 at 10:33:48AM -0400, Emmanuel Cecchet wrote:

Hi all,

Finally the error logging and autopartitioning patches for COPY that
I presented at PGCon are here!

Yay!

Error logging is described here:
http://wiki.postgresql.org/wiki/Error_logging_in_COPY
Autopartitioning is described here:
http://wiki.postgresql.org/wiki/Auto-partitioning_in_COPY

The attached patch contains both contribs as well as unit tests. I
will submit shortly the new patch at commitfest.postgresql.org.

My C isn't all that great, to put it mildly, but I noticed C++-style
//comments like this, which we don't use.

Are these dependent on each other, or could they be reviewed, etc.
separately?

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Hi David,

My C isn't all that great, to put it mildly, but I noticed C++-style
//comments like this, which we don't use.

Ok, I will fix that.

Are these dependent on each other, or could they be reviewed, etc.
separately?

The code implementing the functionality are independent (separate
methods or files) but error logging also catches potential routing
problems and log faulty tuples (the call to routing is just basically in
the try/catch of error logging).
So, yes they can be reviewed separately, but if both are integrated it
makes sense to review them together.

Cheers
Emmanuel

Emmanuel, Hackers,

Thank you for tackling this very long-time TODO.

Error logging is described here:
http://wiki.postgresql.org/wiki/Error_logging_in_COPY

Questions & Comments:

A) Why would someone want to turn error_logging on, but leave
error_logging_skip_tuples off? The pg_log already logs errors which
copy throws by default.

B) As I mentioned earlier, we'll want to provide the option of logging
to a file instead of to a table. That's not a reason to reject this
patch, but probably a TODO for 8.5.

C) Are we sure we want to handle this via GUCs rather than extensions to
COPY syntax? It seems like fairly often users would want to log
different COPY sources to different tables/files.

D) These GUCs are userset, I hope? (haven't dug into the code far
enough to tell yet).

E) What is error_logging_tuple_label for? You don't explain/give
examples. And how is error_logging_tuple_partition_key used?

F) Rawdata for rejected tuples is presumably BYTEA?

G) We should probably have a default for error_logging_table_name, such
as pg_copy_errors. Does that table get automatically created if it
doesn't exist?

H) Finally, one request of the TODO is some way to halt import after a
specified number of bad tuples because it probably means you have the
wrong file or wrong table. Do we still want that?

Autopartitioning is described here:
http://wiki.postgresql.org/wiki/Auto-partitioning_in_COPY

M) tuple_routing_in_copy should take "on" or "off", not 0 or 1.

N) Have you measured the overhead & speed of this kind of COPY as
opposed to COPY into a single table? Have you checked the overhead if
tuple_routing_in_copy is on, but you are not loading into a partitioned
table?

O) Is this capable of dealing with partitioning by more than one column,
or by an expression?

Finally, I'm going to suggest different names for the GUCs, as the names
you've chosen don't group well and would likely cause confusion. Here
are my suggestions, which all begin with "copy_" for prefix matching:

error_logging --> probaby not needed, see able
error_logging_skip_tuples --> copy_skip_bad_rows
error_logging_schema_name --> copy_logging_schema_name
error_logging_relation_name --> copy_logging_table_name
error_logging_tuple_label --> don't know what this is for, see above
error_logging_tuple_partition_key --> don't know what this is for, see above

tuple_routing_in_copy --> copy_partitioning
tuple_routing_cache_size --> copy_partitioning_cache_size

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh,

See the answers inlined.

Thank you for tackling this very long-time TODO.

Error logging is described here:
http://wiki.postgresql.org/wiki/Error_logging_in_COPY

Questions & Comments:

A) Why would someone want to turn error_logging on, but leave
error_logging_skip_tuples off? The pg_log already logs errors which copy throws by default.

When error_logging is on and skip_tuples is off, errors are logged in
the error table. If skip_tuples is on, tuples are not logged in the
error table.

B) As I mentioned earlier, we'll want to provide the option of logging
to a file instead of to a table. That's not a reason to reject this
patch, but probably a TODO for 8.5.

Ok but what should be the format of that file?

C) Are we sure we want to handle this via GUCs rather than extensions to
COPY syntax? It seems like fairly often users would want to log
different COPY sources to different tables/files.

I agree that new COPY options could be easier to use, the implementation
is just more complex. However, the labels allows you to select the
tuples related to specific COPY commands.

D) These GUCs are userset, I hope? (haven't dug into the code far
enough to tell yet).

Yes.

E) What is error_logging_tuple_label for? You don't explain/give
examples. And how is error_logging_tuple_partition_key used?

We use the label and partition key in Aster products to easily retrieve
which COPY command on which partition did generate the bad tuples. By
default, the tuple_label contains the COPY command that was executed
(see example on Wiki) and the key contains the index of the tuple in the
source file (see example on Wiki).

F) Rawdata for rejected tuples is presumably BYTEA?

Yes. I forgot to put back the table description that can be seen in the
unit tests. I have updated the Wiki with the table definition.

G) We should probably have a default for error_logging_table_name, such
as pg_copy_errors. Does that table get automatically created if it
doesn't exist?

Yes, as indicated on the wiki the table is created automatically (see
config variable section).

H) Finally, one request of the TODO is some way to halt import after a
specified number of bad tuples because it probably means you have the
wrong file or wrong table. Do we still want that?

We can still do that. It can be another GUC variable or an option to
COPY. If the COPY command fails, everything gets rolled back (data in
the destination table and error table). That would be harder to
implement with a file (the rollback part).

Autopartitioning is described here:
http://wiki.postgresql.org/wiki/Auto-partitioning_in_COPY

M) tuple_routing_in_copy should take "on" or "off", not 0 or 1.

Ok.

N) Have you measured the overhead & speed of this kind of COPY as
opposed to COPY into a single table? Have you checked the overhead if
tuple_routing_in_copy is on, but you are not loading into a partitioned
table?

Yes. There is no noticeable overhead if there is no routing to do (but
routing is on).
If routing is involved, the overhead depends on how sorted your input
data is. If it all goes to the same partition, the caching effect works
well and there is no noticeable overhead. The cost is in the constraint
check and it depends on the complexity of the constraint. The more
constraints you have to check and the more complex they are, the more
overhead on each tuple routing.

O) Is this capable of dealing with partitioning by more than one column,
or by an expression?

Yes, we just use a brute force technique where we try all child tables
1-by-1 and rely on the existing Postgres constraint checking mechanism
(no new or duplicated code there).

Finally, I'm going to suggest different names for the GUCs, as the names
you've chosen don't group well and would likely cause confusion. Here
are my suggestions, which all begin with "copy_" for prefix matching:

error_logging --> probaby not needed, see able
error_logging_skip_tuples --> copy_skip_bad_rows
error_logging_schema_name --> copy_logging_schema_name
error_logging_relation_name --> copy_logging_table_name
error_logging_tuple_label --> don't know what this is for, see above
error_logging_tuple_partition_key --> don't know what this is for, see above

tuple_routing_in_copy --> copy_partitioning
tuple_routing_cache_size --> copy_partitioning_cache_size

This makes sense. I'll add that on my todo list.

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

Emmanuel,

BTW, some of the questions were for -hackers in general to give
feedback. Don't take just my responses as final "what you have to do";
other contributors will have opinions, some of which will be more
informed than mine.

A) Why would someone want to turn error_logging on, but leave
error_logging_skip_tuples off? The pg_log already logs errors which
copy throws by default.

When error_logging is on and skip_tuples is off, errors are logged in
the error table. If skip_tuples is on, tuples are not logged in the
error table.

Although if you're not skipping tuples, presumably only the first error
is logged, yes? At that point, COPY would stop.

B) As I mentioned earlier, we'll want to provide the option of logging
to a file instead of to a table. That's not a reason to reject this
patch, but probably a TODO for 8.5.

Ok but what should be the format of that file?

I'd say a CSV version of the table you have is the simplest way to go.

C) Are we sure we want to handle this via GUCs rather than extensions to
COPY syntax? It seems like fairly often users would want to log
different COPY sources to different tables/files.

I agree that new COPY options could be easier to use, the implementation
is just more complex. However, the labels allows you to select the
tuples related to specific COPY commands.

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands. So there's
advantages and disadvantages. My question was really for the -hackers
at large: is this the design we want? Or, more directly, is the GUC
approach anathema to anyone?

E) What is error_logging_tuple_label for? You don't explain/give
examples. And how is error_logging_tuple_partition_key used?

We use the label and partition key in Aster products to easily retrieve
which COPY command on which partition did generate the bad tuples. By
default, the tuple_label contains the COPY command that was executed
(see example on Wiki) and the key contains the index of the tuple in the
source file (see example on Wiki).

Ah, ok, let me suggest a modified table format then (btw, the table
format you give doesn't match your examples):

CREATE TABLE pg_copy_errors(
session_id TEXT -- session id from PostgreSQL
tupletimestamp TIMESTAMP WITH TIME ZONE,
targettable TEXT, -- table being copied to
errmessage TEXT, -- full error message encountered
sqlerrcode CHAR(5), -- sql error code
statement TEXT, -- the full copy statement which failed
label TEXT, -- optional user-supplied label
rawdata BYTEA, -- the failed data
constraint pg_copy_errors_pk primary key (session_id, tupletimestamp,
targettable)
);

.. the label would be supplied as copy_logging_label.

This would require you to collect the session_id, of course. Or the
pid, or something else. But, the table as you laid it out has no
natural key, which is a problem for debugging. Also, see discussion below.

I still don't understand how error_logging_tuple_partition_key is used.
Please give more explicit examples.

G) We should probably have a default for error_logging_table_name, such
as pg_copy_errors. Does that table get automatically created if it
doesn't exist?

Yes, as indicated on the wiki the table is created automatically (see
config variable section).

As long as you use CREATE IF NOT EXISTS, that should work ok.

H) Finally, one request of the TODO is some way to halt import after a
specified number of bad tuples because it probably means you have the
wrong file or wrong table. Do we still want that?

We can still do that. It can be another GUC variable or an option to
COPY. If the COPY command fails, everything gets rolled back (data in
the destination table and error table). That would be harder to
implement with a file (the rollback part).

With a logging file, you wouldn't worry about rollback. You'd just log
a statement to the file that it was rolled back.

I) Aster's current implementation has the advantage of being able to log
to any user-defined table, giving users the flexibility to log different
COPYs to different tables, or even put them on various tablespaces. Is
that what we want, though? Clearly it would make things simpler for
most (but not all) users to have just a canonical pg_copy_errors table
which was in pg_catalog. It would also presumably remove some code from
the patch (usually a good thing) What do people think?

J) One option I think we need if we're going to support logging to "any
defined logging table" is the option to make that table a temporary table.

O) Is this capable of dealing with partitioning by more than one column,
or by an expression?

Yes, we just use a brute force technique where we try all child tables
1-by-1 and rely on the existing Postgres constraint checking mechanism
(no new or duplicated code there).

Sounds disastrous performance-wise. There's no easy way to test the
expression without attempting an actual insert?

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus <josh@agliodbs.com> writes:

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands. So there's
advantages and disadvantages. My question was really for the -hackers
at large: is this the design we want? Or, more directly, is the GUC
approach anathema to anyone?

Half a dozen interrelated GUCs to control a single command fairly
screams "bad design" to me; especially the ones that specifically bear
on the command semantics, rather than being performance settings that
you could reasonably have system-wide defaults for. Could we please
look at doing it via COPY options instead?

It might be time to switch COPY over to a more easily extensible
option syntax, such as we just adopted for EXPLAIN.

regards, tom lane

On Thu, Sep 10, 2009 at 06:34:36PM -0400, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands. So there's
advantages and disadvantages. My question was really for the -hackers
at large: is this the design we want? Or, more directly, is the GUC
approach anathema to anyone?

Half a dozen interrelated GUCs to control a single command fairly
screams "bad design" to me; especially the ones that specifically bear
on the command semantics, rather than being performance settings that
you could reasonably have system-wide defaults for. Could we please
look at doing it via COPY options instead?

It might be time to switch COPY over to a more easily extensible
option syntax, such as we just adopted for EXPLAIN.

+1 :)

Cheers,
David (still working on that windowing bug)
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Tom,

It might be time to switch COPY over to a more easily extensible
option syntax, such as we just adopted for EXPLAIN.

+1. And we could maybe have a *single* GUC which was "copy_options" as
a session default.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh,

BTW, some of the questions were for -hackers in general to give
feedback. Don't take just my responses as final "what you have to do";
other contributors will have opinions, some of which will be more
informed than mine.

Understood.

A) Why would someone want to turn error_logging on, but leave
error_logging_skip_tuples off? The pg_log already logs errors which
copy throws by default.

When error_logging is on and skip_tuples is off, errors are logged in
the error table. If skip_tuples is on, tuples are not logged in the
error table.

Although if you're not skipping tuples, presumably only the first error
is logged, yes? At that point, COPY would stop.

No, when error_logging is on, COPY never stops. If skip_tuples is on,
the faulty tuples are just ignored and not logged, if it is off (it does
not skip) it logs every faulty tuple in the error table and continues
until the end. If error_logging is off, COPY stops on the first error
and aborts.

B) As I mentioned earlier, we'll want to provide the option of logging
to a file instead of to a table. That's not a reason to reject this
patch, but probably a TODO for 8.5.

Ok but what should be the format of that file?

I'd say a CSV version of the table you have is the simplest way to go.

Ok.

C) Are we sure we want to handle this via GUCs rather than extensions to
COPY syntax? It seems like fairly often users would want to log
different COPY sources to different tables/files.

I agree that new COPY options could be easier to use, the implementation
is just more complex. However, the labels allows you to select the
tuples related to specific COPY commands.

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands. So there's
advantages and disadvantages. My question was really for the -hackers
at large: is this the design we want? Or, more directly, is the GUC
approach anathema to anyone?

As it is a new feature, users will either have to add the proper set
commands or modify their COPY commands. Both ways look good to me.

E) What is error_logging_tuple_label for? You don't explain/give
examples. And how is error_logging_tuple_partition_key used?

We use the label and partition key in Aster products to easily retrieve
which COPY command on which partition did generate the bad tuples. By
default, the tuple_label contains the COPY command that was executed
(see example on Wiki) and the key contains the index of the tuple in the
source file (see example on Wiki).

Ah, ok, let me suggest a modified table format then (btw, the table
format you give doesn't match your examples):

The order of the columns does not matter, just the name.

CREATE TABLE pg_copy_errors(
session_id TEXT -- session id from PostgreSQL
tupletimestamp TIMESTAMP WITH TIME ZONE,
targettable TEXT, -- table being copied to
errmessage TEXT, -- full error message encountered
sqlerrcode CHAR(5), -- sql error code
statement TEXT, -- the full copy statement which failed
label TEXT, -- optional user-supplied label
rawdata BYTEA, -- the failed data
constraint pg_copy_errors_pk primary key (session_id, tupletimestamp,
targettable)
);

.. the label would be supplied as copy_logging_label.

This would require you to collect the session_id, of course. Or the
pid, or something else. But, the table as you laid it out has no
natural key, which is a problem for debugging. Also, see discussion below.

I still don't understand how error_logging_tuple_partition_key is used.
Please give more explicit examples.

I am not really sure why you need a natural key.
By default, the partition_key contains the index of the faulty entry and
label the copy command. This could be your key.
If you look at the example at
http://wiki.postgresql.org/wiki/Error_logging_in_COPY, input_file.txt
has 5 rows out of which only 1 and 5 are correct (2, 3 and 4 are bad).
The key indicates the row that caused the problem (2, 3, 4) and label
contains the full COPY statement. I am not sure to understand what the
pid or session_id would bring in that scenario.

G) We should probably have a default for error_logging_table_name, such
as pg_copy_errors. Does that table get automatically created if it
doesn't exist?

Yes, as indicated on the wiki the table is created automatically (see
config variable section).

As long as you use CREATE IF NOT EXISTS, that should work ok.

Actually the code checks if the table exists with a try_heap_openrv and
creates the table if it fails.

H) Finally, one request of the TODO is some way to halt import after a
specified number of bad tuples because it probably means you have the
wrong file or wrong table. Do we still want that?

We can still do that. It can be another GUC variable or an option to
COPY. If the COPY command fails, everything gets rolled back (data in
the destination table and error table). That would be harder to
implement with a file (the rollback part).

With a logging file, you wouldn't worry about rollback. You'd just log
a statement to the file that it was rolled back.

Ok.

I) Aster's current implementation has the advantage of being able to log
to any user-defined table, giving users the flexibility to log different
COPYs to different tables, or even put them on various tablespaces. Is
that what we want, though? Clearly it would make things simpler for
most (but not all) users to have just a canonical pg_copy_errors table
which was in pg_catalog. It would also presumably remove some code from
the patch (usually a good thing) What do people think?

The code that would be removed would just be the table creation and the
GUC variable for the table name. That would not remove much code.

J) One option I think we need if we're going to support logging to "any
defined logging table" is the option to make that table a temporary table.

Actually you can create the table beforehand (check unit tests for an
example) and so it is possible to make it a temporary table if you want
(I would just have to test it, I did not try it yet).

O) Is this capable of dealing with partitioning by more than one column,
or by an expression?

Yes, we just use a brute force technique where we try all child tables
1-by-1 and rely on the existing Postgres constraint checking mechanism
(no new or duplicated code there).

Sounds disastrous performance-wise. There's no easy way to test the
expression without attempting an actual insert?

An option that was proposed at PGCon by someone in the audience (sorry I
don't remember who), was to build a query plan and find out from there
which child table should get the tuple. It will be disastrous
performance-wise if you always have misses and need to check a lot of
constraints (we can always build a worst case scenario test). However,
in my tests, even with completely random data, routing is faster than a
script doing sorting in multiple files + manual inserts in each child
table. In practice, even a small cache size usually works very well as
soon as you have enough locality in your source data.
The problem is that faulty tuples will have to check all constraints to
figure out that there is no child table for them. Note that if you have
a deep tree hierarchy, you don't necessarily have to insert at the top
of the tree, you can start anywhere down the tree.
Once we have the full partitioning support (in 8.5?), we can probably
re-use some of their mechanism to route directly the tuple to the right
child.

Thanks for your great feedback,
Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

I am not really sure why you need a natural key.

a) because we shouldn't be building any features which teach people bad
db design, and

b) because I will presumably want to purge records from this table
periodically and doing so without a key is likely to result in purging
the wrong records.

By default, the partition_key contains the index of the faulty entry and
label the copy command. This could be your key.

Well, you still haven't explained the partition_key to me, so I'm not
quite clear on that. Help?

The reason why I'd like to have a session_id or pid or similar is so
that I can link the copy errors to which backend is erroring in the
other system views or in the pg_log.

Imagine a system where you have multiple network clients doing COPYs; if
one of them starts bugging out and all I have is a tablename, filename
and time, I'm not going to be able to figure out which client is causing
the problems. The reason I mention this case is that I have a client
who has a production application like this right now.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus wrote:

I am not really sure why you need a natural key.

a) because we shouldn't be building any features which teach people bad
db design, and

b) because I will presumably want to purge records from this table
periodically and doing so without a key is likely to result in purging
the wrong records.

Agreed, but I am not sure that imposing unilaterally a key is going to
suit everyone.

By default, the partition_key contains the index of the faulty entry and
label the copy command. This could be your key.

Well, you still haven't explained the partition_key to me, so I'm not
quite clear on that. Help?

Please re-read my previous message for the default behavior. If you look
at the example at http://wiki.postgresql.org/wiki/Error_logging_in_COPY,
input_file.txt has 5 rows out of which only 1 and 5 are correct (2, 3
and 4 are bad). The partition key indicates the row that caused the
problem (2 for row 2, 3 for row 3, ...) and label contains the full COPY
statement.
If you want to know how it is used in the Aster product, we will have to
ask Alex who did implement the feature in the product.

The reason why I'd like to have a session_id or pid or similar is so
that I can link the copy errors to which backend is erroring in the
other system views or in the pg_log.

Imagine a system where you have multiple network clients doing COPYs; if
one of them starts bugging out and all I have is a tablename, filename
and time, I'm not going to be able to figure out which client is causing
the problems. The reason I mention this case is that I have a client
who has a production application like this right now

All the clients are copying the same file to the same table?
I would imagine that every client processes different files and from the
file names it would be easy to identify the faulty client. I am not sure
how you would use the pid to identify the faulty client more easily?

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

Hackers,

Let me try to give more context on some of the things discussed.
Feedback is appreciated.

Thanks

- A

Emmanuel Cecchet wrote:

Josh,

BTW, some of the questions were for -hackers in general to give
feedback. Don't take just my responses as final "what you have to do";
other contributors will have opinions, some of which will be more
informed than mine.

Understood.

A) Why would someone want to turn error_logging on, but leave
error_logging_skip_tuples off? The pg_log already logs errors which
copy throws by default.

When error_logging is on and skip_tuples is off, errors are logged in
the error table. If skip_tuples is on, tuples are not logged in the
error table.

Although if you're not skipping tuples, presumably only the first error
is logged, yes? At that point, COPY would stop.

No, when error_logging is on, COPY never stops. If skip_tuples is on,
the faulty tuples are just ignored and not logged, if it is off (it does
not skip) it logs every faulty tuple in the error table and continues
until the end. If error_logging is off, COPY stops on the first error
and aborts.

B) As I mentioned earlier, we'll want to provide the option of logging
to a file instead of to a table. That's not a reason to reject this
patch, but probably a TODO for 8.5.

Ok but what should be the format of that file?

I'd say a CSV version of the table you have is the simplest way to go.

Ok.

C) Are we sure we want to handle this via GUCs rather than extensions to
COPY syntax? It seems like fairly often users would want to log
different COPY sources to different tables/files.

I agree that new COPY options could be easier to use, the implementation
is just more complex. However, the labels allows you to select the
tuples related to specific COPY commands.

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands. So there's
advantages and disadvantages. My question was really for the -hackers
at large: is this the design we want? Or, more directly, is the GUC
approach anathema to anyone?

As it is a new feature, users will either have to add the proper set
commands or modify their COPY commands. Both ways look good to me.

The implementation of this feature at Aster required us to set options
at the PostgreSQL backend according to the user input, which comes from
an enhanced COPY command (just as suggested above). I agree that from a
user's perspective it makes more sense to do something like this:

COPY foo from '/tmp/foo.txt' with csv LOG ERRORS INTO my_error_table;

E) What is error_logging_tuple_label for? You don't explain/give
examples. And how is error_logging_tuple_partition_key used?

We populate this field with a session id coming from our system. This
will allow to identify malformed tuples coming from different sessions.
In the case of PostgreSQL we can use a similar field (e.g. transaction
id or session id).

We use the label and partition key in Aster products to easily retrieve
which COPY command on which partition did generate the bad tuples. By
default, the tuple_label contains the COPY command that was executed
(see example on Wiki) and the key contains the index of the tuple in the
source file (see example on Wiki).

Ah, ok, let me suggest a modified table format then (btw, the table
format you give doesn't match your examples):

The order of the columns does not matter, just the name.

CREATE TABLE pg_copy_errors(
session_id TEXT -- session id from PostgreSQL
tupletimestamp TIMESTAMP WITH TIME ZONE,
targettable TEXT, -- table being copied to
errmessage TEXT, -- full error message encountered
sqlerrcode CHAR(5), -- sql error code
statement TEXT, -- the full copy statement which failed
label TEXT, -- optional user-supplied label
rawdata BYTEA, -- the failed data
constraint pg_copy_errors_pk primary key (session_id, tupletimestamp,
targettable)
);

.. the label would be supplied as copy_logging_label.

This would require you to collect the session_id, of course. Or the
pid, or something else. But, the table as you laid it out has no
natural key, which is a problem for debugging. Also, see discussion below.

I still don't understand how error_logging_tuple_partition_key is used.
Please give more explicit examples.

I am not really sure why you need a natural key.
By default, the partition_key contains the index of the faulty entry and
label the copy command. This could be your key.
If you look at the example at
http://wiki.postgresql.org/wiki/Error_logging_in_COPY, input_file.txt
has 5 rows out of which only 1 and 5 are correct (2, 3 and 4 are bad).
The key indicates the row that caused the problem (2, 3, 4) and label
contains the full COPY statement. I am not sure to understand what the
pid or session_id would bring in that scenario.

Again, the key field makes a lot of sense in the case where you have
multiple PostgreSQL instances acting as different "shards" in a cluster
setup. The key column would then contain which shard logged the error to
it's local table. We could just drop this column in the single
PostgreSQL case.

G) We should probably have a default for error_logging_table_name, such
as pg_copy_errors. Does that table get automatically created if it
doesn't exist?

Yes, as indicated on the wiki the table is created automatically (see
config variable section).

As long as you use CREATE IF NOT EXISTS, that should work ok.

Actually the code checks if the table exists with a try_heap_openrv and
creates the table if it fails.

H) Finally, one request of the TODO is some way to halt import after a
specified number of bad tuples because it probably means you have the
wrong file or wrong table. Do we still want that?

We can still do that. It can be another GUC variable or an option to
COPY. If the COPY command fails, everything gets rolled back (data in
the destination table and error table). That would be harder to
implement with a file (the rollback part).

With a logging file, you wouldn't worry about rollback. You'd just log
a statement to the file that it was rolled back.

Ok.

Where would this file live then? Since you are doing all the error
logging at the backend, I am assuming it would need to live "on the
server node". As opposed to a file which lies in the same directory from
where the original file was loaded via, let's say, psql.

Tom Lane wrote:

It might be time to switch COPY over to a more easily extensible
option syntax, such as we just adopted for EXPLAIN.

Could you point me to the thread detailing the extensible option syntax
for EXPLAIN? Is that new to 8.5?

Thanks
Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

On Thu, Sep 10, 2009 at 6:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Josh Berkus <josh@agliodbs.com> writes:

Yes, and GUCs allow users to retrofit this approach onto existing
infrastructure without changing their COPY commands.  So there's
advantages and disadvantages.  My question was really for the -hackers
at large: is this the design we want?  Or, more directly, is the GUC
approach anathema to anyone?

Half a dozen interrelated GUCs to control a single command fairly
screams "bad design" to me; especially the ones that specifically bear
on the command semantics, rather than being performance settings that
you could reasonably have system-wide defaults for.  Could we please
look at doing it via COPY options instead?

It might be time to switch COPY over to a more easily extensible
option syntax, such as we just adopted for EXPLAIN.

I like this idea, perhaps not surprisingly (for those not following at
home: that was my patch). Unfortunately, it looks to me like there is
no way to do this without overhauling the syntax. If the existing
syntax required a comma between options (i.e. "copy blah to stdout
binary, oids" rather than "copy to stdout binary oids", this would be
pretty straightforward; but it doesn't even allow one).

I wonder if we should consider allowing COPY options to be
comma-separated beginning with 8.5, and then require it in 8.6. Other
options include continuing to support the old syntax for the existing
options, but allowing some new syntax as well and requiring its use
for all new options (this is what we did with EXPLAIN, but there were
only two pre-existing options there), and just changing the syntax
incompatibly and telling users to suck it up. But I'm not sure I like
either of those choices.

...Robert

Hi Robert,

I like this idea, perhaps not surprisingly (for those not following at
home: that was my patch). Unfortunately, it looks to me like there is
no way to do this without overhauling the syntax. If the existing
syntax required a comma between options (i.e. "copy blah to stdout
binary, oids" rather than "copy to stdout binary oids", this would be
pretty straightforward; but it doesn't even allow one).

Well some options like CSV FORCE ... take a comma separated list of
columns. This would require all options to become reserved keywords or
force parenthesis around option parameters.

I wonder if we should consider allowing COPY options to be
comma-separated beginning with 8.5, and then require it in 8.6. Other
options include continuing to support the old syntax for the existing
options, but allowing some new syntax as well and requiring its use
for all new options (this is what we did with EXPLAIN, but there were
only two pre-existing options there), and just changing the syntax
incompatibly and telling users to suck it up. But I'm not sure I like
either of those choices.

We could keep the current syntax for backward compatibility only (can be
dropped in a future release) and have a new syntax (starting in 8.5). To
avoid confusion between both, we could just replace WITH with something
else (or just drop it) to indicate that this is the new syntax.

The new syntax could look like:

COPY /tablename/ [ ( /column/ [, ...] ) ]
FROM { '/filename/' | STDIN }
[ [, BINARY ]
[, OIDS ]
[, DELIMITER [ AS ] '/delimiter/' ]
[, NULL [ AS ] '/null string/' ]
[, CSV [ HEADER ]
[ QUOTE [ AS ] '/quote/' ]
[ ESCAPE [ AS ] '/escape/' ]
[ FORCE NOT NULL (/column/ [, ...]) ]
[, ERRORS { SKIP |
LOG INTO { tablename | 'filename' }
[ LABEL label_name ]
[ KEY key_name ]
[ MAX ERRORS /count/ ] } ]

Is this what you had in mind?

Emmanuel

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

Emmanuel Cecchet <manu@asterdata.com> writes:

The new syntax could look like:

COPY /tablename/ [ ( /column/ [, ...] ) ]
FROM { '/filename/' | STDIN }
[ [, BINARY ]
[, OIDS ]
[, DELIMITER [ AS ] '/delimiter/' ]
[, NULL [ AS ] '/null string/' ]
[, CSV [ HEADER ]
[ QUOTE [ AS ] '/quote/' ]
[ ESCAPE [ AS ] '/escape/' ]
[ FORCE NOT NULL (/column/ [, ...]) ]
[, ERRORS { SKIP |
LOG INTO { tablename | 'filename' }
[ LABEL label_name ]
[ KEY key_name ]
[ MAX ERRORS /count/ ] } ]

Is this what you had in mind?

No. because that doesn't do a darn thing to make the option set less
hard-wired into the syntax. I was thinking of a strict keyword/value
format with non-wired-in keywords ... and only *one* keyword per value.
See EXPLAIN.

regards, tom lane

Tom,

I looked at EXPLAIN
(http://www.postgresql.org/docs/current/interactive/sql-explain.html)
and there is not a single line of what you are talking about.
And the current syntax is just EXPLAIN [ ANALYZE ] [ VERBOSE ] /statement
/
If I try to decrypt what you said, you are looking at something like

COPY /tablename/ [ ( /column/ [, ...] ) ]
FROM { '/filename/' | STDIN }
[option1=value[,...]]

That would give something like:
COPY foo FROM 'input.txt' binary=on, oids=on, errors=skip, max_errors=10

If this is not what you are thinking, please provide an example.

Emmanuel
/

/

Emmanuel Cecchet <manu@asterdata.com> writes:

The new syntax could look like:

COPY /tablename/ [ ( /column/ [, ...] ) ]
FROM { '/filename/' | STDIN }
[ [, BINARY ]
[, OIDS ]
[, DELIMITER [ AS ] '/delimiter/' ]
[, NULL [ AS ] '/null string/' ]
[, CSV [ HEADER ]
[ QUOTE [ AS ] '/quote/' ]
[ ESCAPE [ AS ] '/escape/' ]
[ FORCE NOT NULL (/column/ [, ...]) ]
[, ERRORS { SKIP |
LOG INTO { tablename | 'filename' }
[ LABEL label_name ]
[ KEY key_name ]
[ MAX ERRORS /count/ ] } ]

Is this what you had in mind?

No. because that doesn't do a darn thing to make the option set less
hard-wired into the syntax. I was thinking of a strict keyword/value
format with non-wired-in keywords ... and only *one* keyword per value.
See EXPLAIN.

regards, tom lane

--
Emmanuel Cecchet
Aster Data Systems
Web: http://www.asterdata.com

On Fri, Sep 11, 2009 at 10:53 AM, Emmanuel Cecchet <manu@asterdata.com> wrote:

Tom,

I looked at EXPLAIN
(http://www.postgresql.org/docs/current/interactive/sql-explain.html) and
there is not a single line of what you are talking about.
And the current syntax is just EXPLAIN [ ANALYZE ] [ VERBOSE ] /statement
/

http://developer.postgresql.org/pgdocs/postgres/sql-explain.html

Or look at your CVS/git checkout.

...Robert

On Fri, Sep 11, 2009 at 10:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Emmanuel Cecchet <manu@asterdata.com> writes:

The new syntax could look like:

COPY /tablename/ [ ( /column/ [, ...] ) ]
    FROM { '/filename/' | STDIN }
    [ [, BINARY ]
      [, OIDS ]
      [, DELIMITER [ AS ] '/delimiter/' ]
      [, NULL [ AS ] '/null string/' ]
      [, CSV [ HEADER ]
             [ QUOTE [ AS ] '/quote/' ]
             [ ESCAPE [ AS ] '/escape/' ]
             [ FORCE NOT NULL (/column/ [, ...]) ]
      [, ERRORS { SKIP |
                  LOG INTO { tablename | 'filename' }
                    [ LABEL label_name ]
                    [ KEY key_name ]
                    [ MAX ERRORS /count/ ] } ]

Is this what you had in mind?

No. because that doesn't do a darn thing to make the option set less
hard-wired into the syntax. I was thinking of a strict keyword/value
format with non-wired-in keywords ... and only *one* keyword per value.
See EXPLAIN.

I was thinking something like:

COPY tablename [ ( column [, ...] ) ] FROM { 'filename' | STDIN }
[WITH] [option [, ...]]

Where:

option := ColId [Sconst] | FORCE NOT NULL (column [,...])

I don't see any reasonable way to sandwhich the FORCE NOT NULL syntax
into a keyword/value notation.

...Robert