public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 2/2] Include the leader PID in logfile 3+ messages / 3 participants [nested] [flat]
* [PATCH v5 2/2] Include the leader PID in logfile @ 2020-03-14 03:03 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-03-14 03:03 UTC (permalink / raw) See also: b025f32e0b, which added the leader PID to pg_stat_activity --- doc/src/sgml/config.sgml | 10 ++++++- src/backend/utils/error/elog.c | 29 +++++++++++++++++++ src/backend/utils/misc/postgresql.conf.sample | 1 + 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 994155ca00..e1da2e12e8 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6694,6 +6694,13 @@ local0.* /var/log/postgresql <entry>Process ID</entry> <entry>no</entry> </row> + <row> + <entry><literal>%P</literal></entry> + <entry>Process ID of the parallel group leader, if this process is + a parallel query worker. + </entry> + <entry>no</entry> + </row> <row> <entry><literal>%t</literal></entry> <entry>Time stamp without milliseconds</entry> @@ -7026,7 +7033,7 @@ log_line_prefix = '%m [%p] %q%u@%d/%a ' character count of the error position therein, location of the error in the PostgreSQL source code (if <varname>log_error_verbosity</varname> is set to <literal>verbose</literal>), - application name, and backend type. + application name, backend type, and leader PID. Here is a sample table definition for storing CSV-format log output: <programlisting> @@ -7056,6 +7063,7 @@ CREATE TABLE postgres_log location text, application_name text, backend_type text, + leader_pid integer, PRIMARY KEY (session_id, session_line_num) ); </programlisting> diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index b0b1a09ab1..60e2f524a7 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -77,6 +77,7 @@ #include "postmaster/syslogger.h" #include "storage/ipc.h" #include "storage/proc.h" +#include "storage/procarray.h" #include "tcop/tcopprot.h" #include "utils/guc.h" #include "utils/memutils.h" @@ -2425,6 +2426,23 @@ log_line_prefix(StringInfo buf, ErrorData *edata) case 'p': appendStringInfo(buf, "%*d", padding, MyProcPid); break; + + case 'P': + if (MyProc) + { + PGPROC *leader = MyProc->lockGroupLeader; + if (leader != NULL && leader->pid != MyProcPid) + /* leader_pid is blank for the leader itself */ + appendStringInfo(buf, "%*d", padding, leader->pid); + else + appendStringInfoSpaces(buf, + padding > 0 ? padding : -padding); + } + else if (padding != 0) + appendStringInfoSpaces(buf, + padding > 0 ? padding : -padding); + break; + case 'l': appendStringInfo(buf, "%*ld", padding, log_line_number); break; @@ -2766,6 +2784,17 @@ write_csvlog(ErrorData *edata) else appendCSVLiteral(&buf, GetBackendTypeDesc(MyBackendType)); + appendStringInfoChar(&buf, ','); + + /* leader PID */ + if (MyProc) + { + PGPROC *leader = MyProc->lockGroupLeader; + /* leader_pid is blank for leader itself */ + if (leader && leader->pid != MyProcPid) + appendStringInfo(&buf, "%d", leader->pid); + } + appendStringInfoChar(&buf, '\n'); /* If in the syslogger process, try to write messages direct to file */ diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index b0715ae188..1ecdfc51a6 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -537,6 +537,7 @@ # %h = remote host # %b = backend type # %p = process ID + # %P = leader PID # %t = timestamp without milliseconds # %m = timestamp with milliseconds # %n = timestamp with milliseconds (as a Unix epoch) -- 2.17.0 --fDERRRNgB4on1jOB-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Add reject_limit option to file_fdw @ 2024-10-17 13:45 torikoshia <[email protected]> 2024-10-17 16:51 ` Re: Add reject_limit option to file_fdw Fujii Masao <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: torikoshia @ 2024-10-17 13:45 UTC (permalink / raw) To: pgsql-hackers Hi, 4ac2a9bec introduced reject_limit option to the COPY command, and I was wondering if it might be beneficial to add the same option to file_fdw. Although there may be fewer practical use cases compared to COPY, it could still be useful in situations where the file being read via file_fdw is subject to modifications and there is a need to tolerate a limited number of errors. What do you think? I've attached a patch. Based on the synopsis of the CREATE/ALTER FOREIGN TABLE commands, the value for the foreign table's option must be single-quoted. I’m not entirely sure if this is the correct approach, but in order to accommodate this, the patch modifies the value of reject_limit option to accept not only numeric values but also strings. -- Regards, -- Atsushi Torikoshi NTT DATA Group Corporation Attachments: [text/x-diff] v1-0001-file_fdw-Add-reject_limit-option-to-file_fdw.patch (4.5K, ../../[email protected]/2-v1-0001-file_fdw-Add-reject_limit-option-to-file_fdw.patch) download | inline diff: From 8d969465257ebc511a0d1b1520f29095103ae4af Mon Sep 17 00:00:00 2001 From: Atsushi Torikoshi <[email protected]> Date: Thu, 17 Oct 2024 22:19:06 +0900 Subject: [PATCH v1] file_fdw: Add reject_limit option to file_fdw --- contrib/file_fdw/expected/file_fdw.out | 10 +++++++++- contrib/file_fdw/file_fdw.c | 8 ++++++++ contrib/file_fdw/sql/file_fdw.sql | 4 +++- doc/src/sgml/file-fdw.sgml | 12 ++++++++++++ src/backend/commands/copy.c | 12 +++++++++++- 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out index 593fdc782e..1accc36d68 100644 --- a/contrib/file_fdw/expected/file_fdw.out +++ b/contrib/file_fdw/expected/file_fdw.out @@ -206,7 +206,7 @@ SELECT * FROM agg_csv c JOIN agg_text t ON (t.a = c.a) ORDER BY c.a; SELECT * FROM agg_bad; -- ERROR ERROR: invalid input syntax for type real: "aaa" CONTEXT: COPY agg_bad, line 3, column b: "aaa" --- on_error and log_verbosity tests +-- on_error, log_verbosity and reject_limit tests ALTER FOREIGN TABLE agg_bad OPTIONS (ADD on_error 'ignore'); SELECT * FROM agg_bad; NOTICE: 1 row was skipped due to data type incompatibility @@ -224,6 +224,14 @@ SELECT * FROM agg_bad; 42 | 324.78 (2 rows) +ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1'); +SELECT * FROM agg_bad; + a | b +-----+-------- + 100 | 99.097 + 42 | 324.78 +(2 rows) + ANALYZE agg_bad; -- misc query tests \t on diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 043204c3e7..2ae83ba09b 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -77,6 +77,7 @@ static const struct FileFdwOption valid_options[] = { {"encoding", ForeignTableRelationId}, {"on_error", ForeignTableRelationId}, {"log_verbosity", ForeignTableRelationId}, + {"reject_limit", ForeignTableRelationId}, {"force_not_null", AttributeRelationId}, {"force_null", AttributeRelationId}, @@ -788,6 +789,13 @@ retry: */ ResetPerTupleExprContext(estate); + if (cstate->opts.reject_limit > 0 && \ + cstate->num_errors > cstate->opts.reject_limit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg("skipped more than REJECT_LIMIT (%lld) rows due to data type incompatibility", + (long long) cstate->opts.reject_limit))); + /* Repeat NextCopyFrom() until no soft error occurs */ goto retry; } diff --git a/contrib/file_fdw/sql/file_fdw.sql b/contrib/file_fdw/sql/file_fdw.sql index edd77c5cd2..ee66d7d831 100644 --- a/contrib/file_fdw/sql/file_fdw.sql +++ b/contrib/file_fdw/sql/file_fdw.sql @@ -150,11 +150,13 @@ SELECT * FROM agg_csv c JOIN agg_text t ON (t.a = c.a) ORDER BY c.a; -- error context report tests SELECT * FROM agg_bad; -- ERROR --- on_error and log_verbosity tests +-- on_error, log_verbosity and reject_limit tests ALTER FOREIGN TABLE agg_bad OPTIONS (ADD on_error 'ignore'); SELECT * FROM agg_bad; ALTER FOREIGN TABLE agg_bad OPTIONS (ADD log_verbosity 'silent'); SELECT * FROM agg_bad; +ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1'); +SELECT * FROM agg_bad; ANALYZE agg_bad; -- misc query tests diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml index bb3579b077..b05085fd7d 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -149,6 +149,18 @@ </listitem> </varlistentry> + <varlistentry> + <term><literal>reject_limit</literal></term> + + <listitem> + <para> + Specifies the maximum number of errors tolerated while converting a column's + input value to its data type, the same as <command>COPY</command>'s + <literal>REJECT_LIMIT</literal> option. + </para> + </listitem> + </varlistentry> + </variablelist> <para> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 3485ba8663..c6c8bc10b1 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -424,7 +424,17 @@ defGetCopyOnErrorChoice(DefElem *def, ParseState *pstate, bool is_from) static int64 defGetCopyRejectLimitOption(DefElem *def) { - int64 reject_limit = defGetInt64(def); + int64 reject_limit; + + if (def->arg == NULL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("%s requires a numeric value", + def->defname))); + else if (nodeTag(def->arg) == T_String) + reject_limit = pg_strtoint64(strVal(def->arg)); + else + reject_limit = defGetInt64(def); if (reject_limit <= 0) ereport(ERROR, -- 2.39.2 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add reject_limit option to file_fdw 2024-10-17 13:45 Add reject_limit option to file_fdw torikoshia <[email protected]> @ 2024-10-17 16:51 ` Fujii Masao <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Fujii Masao @ 2024-10-17 16:51 UTC (permalink / raw) To: torikoshia <[email protected]>; pgsql-hackers On 2024/10/17 22:45, torikoshia wrote: > Hi, > > 4ac2a9bec introduced reject_limit option to the COPY command, and I was wondering if it might be beneficial to add the same option to file_fdw. > > Although there may be fewer practical use cases compared to COPY, it could still be useful in situations where the file being read via file_fdw is subject to modifications and there is a need to tolerate a limited number of errors. Agreed. > > What do you think? > > I've attached a patch. Thanks for the patch! Could you add it to the next CommitFest? +ALTER FOREIGN TABLE agg_bad OPTIONS (reject_limit '1'); +SELECT * FROM agg_bad; + a | b +-----+-------- + 100 | 99.097 + 42 | 324.78 +(2 rows) Wouldn't it be better to include a test where a SELECT query fails, even with on_error set to "ignore," because the number of errors exceeds reject_limit? + if (cstate->opts.reject_limit > 0 && \ The trailing backslash isn't needed here. + <varlistentry> + <term><literal>reject_limit</literal></term> This entry should be placed right after the on_error option, following the same order as in the COPY command documentation. > Based on the synopsis of the CREATE/ALTER FOREIGN TABLE commands, the value for the foreign table's option must be single-quoted. I’m not entirely sure if this is the correct approach, but in order to accommodate this, the patch modifies the value of reject_limit option to accept not only numeric values but also strings. > I don't have a better approach for this, so I'm okay with your solution. Just one note: it would be helpful to explain and comment why defGetCopyRejectLimitOption() accepts and parses both int64 and string values. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-10-17 16:51 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-14 03:03 [PATCH v5 2/2] Include the leader PID in logfile Justin Pryzby <[email protected]> 2024-10-17 13:45 Add reject_limit option to file_fdw torikoshia <[email protected]> 2024-10-17 16:51 ` Re: Add reject_limit option to file_fdw Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox