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.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
