Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 12/20/14 12:11 PM, Steve Singer wrote: On 12/19/2014 10:41 AM, Alex Shulgin wrote: I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong, and in your case there would be a message like the following: WARNING: pg_hba.conf not reloaded So an extra hint about file timestamp is unneeded. That makes sense to me. I haven't found any new issues with this patch. I think it is ready for a committer. There were later comments in this thread that disagreed with the extra logging infrastructure, and there were some questions about whether it should only log on failed authentication attempts. Altogether, still some open questions about behavior and implementation approach. So I'm marking this as returned with feedback for now. Personally, I think this could be a useful feature, but it needs more fine-tuning. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 2014-12-15 19:38:16 +0300, Alex Shulgin wrote: Attached is the modified version of the original patch by Craig, addressing the handling of the new hint_log error data field and removing the client-side HINT. I'm not a big fan of this implementation. We're adding a fair bit of infrastructure (i.e. server-only hints) where in other places we use NOTICE for similar hints. Why don't we just add emit a NOTICE or WARNING in the relevant place saying that pg_hba.conf is outdated? Then the server won't log those if configured appropriately, which doesn't seem like a bad thing. Note that = ERROR messages aren't sent to the client during authentication. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 2015-01-16 18:01:24 +0100, Andres Freund wrote: Why don't we just add emit a NOTICE or WARNING in the relevant place saying that pg_hba.conf is outdated? Then the server won't log those if configured appropriately, which doesn't seem like a bad thing. Note that = ERROR messages aren't sent to the client during authentication. I think a 'WARNING: client authentication failed/succeeded with a outdated pg_hba.conf in effect' would actually be a good way to present this. It's not only failed logins where this is relevant. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
Andres Freund and...@2ndquadrant.com writes: Why don't we just add emit a NOTICE or WARNING in the relevant place saying that pg_hba.conf is outdated? Then the server won't log those if configured appropriately, which doesn't seem like a bad thing. Note that = ERROR messages aren't sent to the client during authentication. I think people felt that sending that information to the client wouldn't be a good idea security-wise. But I'd phrase it as why not just emit a LOG message?. I agree that new logging infrastructure seems like overkill. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 2015-01-16 12:21:13 -0500, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Why don't we just add emit a NOTICE or WARNING in the relevant place saying that pg_hba.conf is outdated? Then the server won't log those if configured appropriately, which doesn't seem like a bad thing. Note that = ERROR messages aren't sent to the client during authentication. I think people felt that sending that information to the client wouldn't be a good idea security-wise. It won't if issued during the right phase of the authentication: /* * client_min_messages is honored only after we complete the * authentication handshake. This is required both for security * reasons and because many clients can't handle NOTICE messages * during authentication. */ if (ClientAuthInProgress) output_to_client = (elevel = ERROR); else output_to_client = (elevel = client_min_messages || elevel == INFO); } Surely deserves a comment on the emitting site. But I'd phrase it as why not just emit a LOG message?. Well, LOGs can be sent to the client just the same, no? Just requires a nondefault client_min_messages. But as I don't think sending logs to the client is a unsurmountable problem (due to the above) I don't really care if we use WARNING or LOG. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
Andres Freund and...@2ndquadrant.com writes: On 2015-01-16 12:21:13 -0500, Tom Lane wrote: I think people felt that sending that information to the client wouldn't be a good idea security-wise. It won't if issued during the right phase of the authentication: Good point. But as I don't think sending logs to the client is a unsurmountable problem (due to the above) I don't really care if we use WARNING or LOG. If we're expecting the DBA to need to read it in the postmaster log, remember that LOG is actually *more* likely to get to the log than WARNING is. Choosing WARNING because you think it sounds more significant is a mistake. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 12/19/2014 10:41 AM, Alex Shulgin wrote: I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong, and in your case there would be a message like the following: WARNING: pg_hba.conf not reloaded So an extra hint about file timestamp is unneeded. That makes sense to me. I haven't found any new issues with this patch. I think it is ready for a committer. -- Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 12/15/2014 11:38 AM, Alex Shulgin wrote: These are all valid concerns IMHO. Attached is the modified version of the original patch by Craig, addressing the handling of the new hint_log error data field and removing the client-side HINT. I'm also moving this to the current CF. -- Alex The updated patch removes the client message. I feel this addresses Peter's concern. The updated patch also addresses the missing free I mentioned in my original review. The patch applies cleanly to head, One thing I'm noticed while testing is that if you do the following 1. Edit pg_hba.conf to allow a connection from somewhere 2. Attempt to connect, you get an error + the hind in the server log 3. Make another change to pg_hba.conf and introduce an error in the file 4. Attempt to reload the config files, pg_hba.conf the reload fails because of the above error 5. Attempt to connect you still can't connect since we have the original pg_hba.conf loaded You don't get the warning in step 5 since we update PgReloadTime even if the reload didn't work. Is this enough of a concern to justify the extra complexity in tracking the reload time of the pg_hba and pg_ident times directly? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
Steve Singer st...@ssinger.info writes: On 12/15/2014 11:38 AM, Alex Shulgin wrote: These are all valid concerns IMHO. Attached is the modified version of the original patch by Craig, addressing the handling of the new hint_log error data field and removing the client-side HINT. I'm also moving this to the current CF. -- Alex The updated patch removes the client message. I feel this addresses Peter's concern. The updated patch also addresses the missing free I mentioned in my original review. The patch applies cleanly to head, One thing I'm noticed while testing is that if you do the following 1. Edit pg_hba.conf to allow a connection from somewhere 2. Attempt to connect, you get an error + the hind in the server log 3. Make another change to pg_hba.conf and introduce an error in the file 4. Attempt to reload the config files, pg_hba.conf the reload fails because of the above error 5. Attempt to connect you still can't connect since we have the original pg_hba.conf loaded You don't get the warning in step 5 since we update PgReloadTime even if the reload didn't work. Is this enough of a concern to justify the extra complexity in tracking the reload time of the pg_hba and pg_ident times directly? I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong, and in your case there would be a message like the following: WARNING: pg_hba.conf not reloaded So an extra hint about file timestamp is unneeded. -- Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 12/19/2014 11:41 PM, Alex Shulgin wrote: I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong Well, actually, the whole point was that the user who's connecting (likely also the DBA) will see a HINT telling them that there's more in the logs. But that got removed. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
Craig Ringer cr...@2ndquadrant.com writes: On 12/19/2014 11:41 PM, Alex Shulgin wrote: I don't think so. The scenario this patch relies on assumes that the DBA will remember to look in the log if something goes wrong Well, actually, the whole point was that the user who's connecting (likely also the DBA) will see a HINT telling them that there's more in the logs. But that got removed. While it sounds useful at first glance, I believe Peter's arguments upthread provide enough justification to avoid sending the hint to the client. -- Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
Peter Eisentraut pete...@gmx.net writes: On 10/16/14 11:34 PM, Craig Ringer wrote: psql: FATAL: Peer authentication failed for user fred HINT: See the server error log for additional information. I think this is wrong for many reasons. I have never seen an authentication system that responds with, hey, what you just did didn't get you in, but the administrators are currently in the process of making a configuration change, so why don't you check that out. We don't know whether the user has access to the server log. They probably don't. Also, it is vastly more likely that the user really doesn't have access in the way they chose, so throwing in irrelevant hints will be distracting. Moreover, it will be confusing to regular users if this message sometimes shows up and sometimes doesn't, independent of their own state and actions. Finally, the fact that a configuration change is in progress is privileged information. Unprivileged users can deduct from the presence of this message that administrators are doing something, and possibly that they have done something wrong. I think it's fine to log a message in the server log if the pg_hba.conf file needs reloading. But the client shouldn't know about this at all. These are all valid concerns IMHO. Attached is the modified version of the original patch by Craig, addressing the handling of the new hint_log error data field and removing the client-side HINT. I'm also moving this to the current CF. -- Alex From 702e0ac31f3d8023ad8c064d90bdf5a8441fddea Mon Sep 17 00:00:00 2001 From: Craig Ringer cr...@2ndquadrant.com Date: Fri, 17 Oct 2014 11:18:18 +0800 Subject: [PATCH 1/2] Add an errhint_log, akin to errdetail_log This allows a different HINT to be sent to the server error log and to the client, which will be useful where there's security sensitive information that's more appropriate for a HINT than a DETAIL message. --- src/backend/utils/error/elog.c | 59 -- src/include/utils/elog.h | 7 + 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c new file mode 100644 index 2316464..da55207 *** a/src/backend/utils/error/elog.c --- b/src/backend/utils/error/elog.c *** errfinish(int dummy,...) *** 503,508 --- 503,510 pfree(edata-detail_log); if (edata-hint) pfree(edata-hint); + if (edata-hint_log) + pfree(edata-hint_log); if (edata-context) pfree(edata-context); if (edata-schema_name) *** errhint(const char *fmt,...) *** 1015,1020 --- 1017,1042 return 0; /* return value does not matter */ } + /* + * errhint_log --- add a hint_log error message text to the current error + */ + int + errhint_log(const char *fmt,...) + { + ErrorData *edata = errordata[errordata_stack_depth]; + MemoryContext oldcontext; + + recursion_depth++; + CHECK_STACK_DEPTH(); + oldcontext = MemoryContextSwitchTo(edata-assoc_context); + + EVALUATE_MESSAGE(edata-domain, hint_log, false, true); + + MemoryContextSwitchTo(oldcontext); + recursion_depth--; + return 0; /* return value does not matter */ + } + /* * errcontext_msg --- add a context error message text to the current error *** CopyErrorData(void) *** 1498,1503 --- 1520,1527 newedata-detail_log = pstrdup(newedata-detail_log); if (newedata-hint) newedata-hint = pstrdup(newedata-hint); + if (newedata-hint_log) + newedata-hint_log = pstrdup(newedata-hint_log); if (newedata-context) newedata-context = pstrdup(newedata-context); if (newedata-schema_name) *** FreeErrorData(ErrorData *edata) *** 1536,1541 --- 1560,1567 pfree(edata-detail_log); if (edata-hint) pfree(edata-hint); + if (edata-hint_log) + pfree(edata-hint_log); if (edata-context) pfree(edata-context); if (edata-schema_name) *** ThrowErrorData(ErrorData *edata) *** 1607,1612 --- 1633,1640 newedata-detail_log = pstrdup(edata-detail_log); if (edata-hint) newedata-hint = pstrdup(edata-hint); + if (edata-hint_log) + newedata-hint_log = pstrdup(edata-hint_log); if (edata-context) newedata-context = pstrdup(edata-context); if (edata-schema_name) *** ReThrowError(ErrorData *edata) *** 1669,1674 --- 1697,1704 newedata-detail_log = pstrdup(newedata-detail_log); if (newedata-hint) newedata-hint = pstrdup(newedata-hint); + if (newedata-hint_log) + newedata-hint_log = pstrdup(newedata-hint_log); if (newedata-context) newedata-context = pstrdup(newedata-context); if (newedata-schema_name) *** write_csvlog(ErrorData *edata) *** 2710,2717 appendCSVLiteral(buf, edata-detail); appendStringInfoChar(buf, ','); ! /* errhint */ ! appendCSVLiteral(buf, edata-hint); appendStringInfoChar(buf, ','); /* internal query */ ---
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On Thu, Nov 27, 2014 at 8:49 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Nov 6, 2014 at 05:46:42PM -0500, Peter Eisentraut wrote: Finally, the fact that a configuration change is in progress is privileged information. Unprivileged users can deduct from the presence of this message that administrators are doing something, and possibly that they have done something wrong. I think it's fine to log a message in the server log if the pg_hba.conf file needs reloading. But the client shouldn't know about this at all. Should we do this for postgresql.conf too? It doesn't really make sense; or at least, the exact same thing doesn't make any sense. If an authentication attempt fails unexpectedly, it might be because of a pg_hba.conf change that wasn't reloaded, so it makes sense to try to tip off the DBA. But it can't really be because of a pending postgresql.conf change that hasn't been reloaded, because those don't generally affect authentication. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On Thu, Nov 6, 2014 at 05:46:42PM -0500, Peter Eisentraut wrote: Finally, the fact that a configuration change is in progress is privileged information. Unprivileged users can deduct from the presence of this message that administrators are doing something, and possibly that they have done something wrong. I think it's fine to log a message in the server log if the pg_hba.conf file needs reloading. But the client shouldn't know about this at all. Should we do this for postgresql.conf too? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On Thu, Nov 6, 2014 at 5:46 PM, Peter Eisentraut pete...@gmx.net wrote: I think it's fine to log a message in the server log if the pg_hba.conf file needs reloading. But the client shouldn't know about this at all. I agree. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 10/16/14 11:34 PM, Craig Ringer wrote: psql: FATAL: Peer authentication failed for user fred HINT: See the server error log for additional information. I think this is wrong for many reasons. I have never seen an authentication system that responds with, hey, what you just did didn't get you in, but the administrators are currently in the process of making a configuration change, so why don't you check that out. We don't know whether the user has access to the server log. They probably don't. Also, it is vastly more likely that the user really doesn't have access in the way they chose, so throwing in irrelevant hints will be distracting. Moreover, it will be confusing to regular users if this message sometimes shows up and sometimes doesn't, independent of their own state and actions. Finally, the fact that a configuration change is in progress is privileged information. Unprivileged users can deduct from the presence of this message that administrators are doing something, and possibly that they have done something wrong. I think it's fine to log a message in the server log if the pg_hba.conf file needs reloading. But the client shouldn't know about this at all. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 10/16/2014 11:34 PM, Craig Ringer wrote: Given the generally positive reception to this, here's a patch. The first patch adds an errhint_log , akin to the current errdetail_log, so we can send a different HINT to the server log than we do to the client. The patch behaves as you describe.I feel that this feature would be useful , and you implemented the suggestions given that requested the reload notice but be sent to the client but instead just a hint about checking the server log. You follow the pattern set with detail_log which makes sense. The variable name hint_log doesn't make it obvious to me that the hint goes to the server log, but not the client. The comment for errhint_log should maybe explicitly say that. One question about the code: Does errfinish (elog.c at around line 505) need to free hint_log ? (I would assume it does) Other than that the patch looks good to me. - Something else I noticed while testing. This isn't introduced by your patch but I am wondering if it an existing bug if I setup my configuration like this: #data_directory = 'ConfigDir' # use data in another directory # (change requires restart) hba_file = 'ConfigDir/pg_hba2.conf' # host-based authentication file and start postgres like ./postgres -D ../data it looks for pg2hba2.conf at bin/ConfigDir/pg_hba2.conf (relative to the bin directory I started it from) Then if I change my pg_hba.conf and do a reload I get the following in the log LOG: parameter hba_file cannot be changed without restarting the server LOG: configuration file /usr/local/pgsql95git/bin/../data/postgresql.conf contains errors; unaffected changes were applied set_config_option is comparing the relative path with the absolute path. Steve (Even if DETAIL was appropriate for this info, which it isn't, I can't use errdetail_log because it's already used for other information in some of the same error sites.) The second patch adds a test during errors to report if pg_hba.conf is stale, or if pg_ident.conf is stale. Typical output, client: psql: FATAL: Peer authentication failed for user fred HINT: See the server error log for additional information. Typical output, server: LOG: provided user name (fred) and authenticated user name (craig) do not match FATAL: Peer authentication failed for user fred DETAIL: Connection matched pg_hba.conf line 84: local all all peer HINT: pg_hba.conf has been changed since last server configuration reload. Reload the server configuration to apply the changes. I've added this to the next CF. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] HINT: pg_hba.conf changed since last config reload
On 08/10/2014 07:48 PM, Craig Ringer wrote: Hi all I just had an idea I wanted to run by you all before turning it into a patch. People seem to get confused when they get auth errors because they changed pg_hba.conf but didn't reload. Should we emit a HINT alongside the main auth error in that case? Given the amount of confusion that I see around pg_hba.conf from new users, I figure anything that makes it less confusing might be a good thing if there aren't other consequences. Interested in a patch? Given the generally positive reception to this, here's a patch. The first patch adds an errhint_log , akin to the current errdetail_log, so we can send a different HINT to the server log than we do to the client. (Even if DETAIL was appropriate for this info, which it isn't, I can't use errdetail_log because it's already used for other information in some of the same error sites.) The second patch adds a test during errors to report if pg_hba.conf is stale, or if pg_ident.conf is stale. Typical output, client: psql: FATAL: Peer authentication failed for user fred HINT: See the server error log for additional information. Typical output, server: LOG: provided user name (fred) and authenticated user name (craig) do not match FATAL: Peer authentication failed for user fred DETAIL: Connection matched pg_hba.conf line 84: local all all peer HINT: pg_hba.conf has been changed since last server configuration reload. Reload the server configuration to apply the changes. I've added this to the next CF. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From 8b2bf6ae615d716ca9857017fd862386c6bdcd1f Mon Sep 17 00:00:00 2001 From: Craig Ringer cr...@2ndquadrant.com Date: Fri, 17 Oct 2014 11:18:18 +0800 Subject: [PATCH 1/2] Add an errhint_log, akin to errdetail_log This allows a different HINT to be sent to the server error log and to the client, which will be useful where there's security sensitive information that's more appropriate for a HINT than a DETAIL message. --- src/backend/utils/error/elog.c | 54 -- src/include/utils/elog.h | 7 ++ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 32a9663..59be8a6 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -1015,6 +1015,26 @@ errhint(const char *fmt,...) return 0; /* return value does not matter */ } +/* + * errhint_log --- add a hint_log error message text to the current error + */ +int +errhint_log(const char *fmt,...) +{ + ErrorData *edata = errordata[errordata_stack_depth]; + MemoryContext oldcontext; + + recursion_depth++; + CHECK_STACK_DEPTH(); + oldcontext = MemoryContextSwitchTo(edata-assoc_context); + + EVALUATE_MESSAGE(edata-domain, hint_log, false, true); + + MemoryContextSwitchTo(oldcontext); + recursion_depth--; + return 0; /* return value does not matter */ +} + /* * errcontext_msg --- add a context error message text to the current error @@ -1498,6 +1518,8 @@ CopyErrorData(void) newedata-detail_log = pstrdup(newedata-detail_log); if (newedata-hint) newedata-hint = pstrdup(newedata-hint); + if (newedata-hint_log) + newedata-hint_log = pstrdup(newedata-hint_log); if (newedata-context) newedata-context = pstrdup(newedata-context); if (newedata-schema_name) @@ -1536,6 +1558,8 @@ FreeErrorData(ErrorData *edata) pfree(edata-detail_log); if (edata-hint) pfree(edata-hint); + if (edata-hint_log) + pfree(edata-hint_log); if (edata-context) pfree(edata-context); if (edata-schema_name) @@ -1618,6 +1642,8 @@ ReThrowError(ErrorData *edata) newedata-detail_log = pstrdup(newedata-detail_log); if (newedata-hint) newedata-hint = pstrdup(newedata-hint); + if (newedata-hint_log) + newedata-hint_log = pstrdup(newedata-hint_log); if (newedata-context) newedata-context = pstrdup(newedata-context); if (newedata-schema_name) @@ -2659,8 +2685,11 @@ write_csvlog(ErrorData *edata) appendCSVLiteral(buf, edata-detail); appendStringInfoChar(buf, ','); - /* errhint */ - appendCSVLiteral(buf, edata-hint); + /* errhint or errhint_log */ + if (edata-hint_log) + appendCSVLiteral(buf, edata-hint_log); + else + appendCSVLiteral(buf, edata-hint); appendStringInfoChar(buf, ','); /* internal query */ @@ -2777,25 +2806,22 @@ send_message_to_server_log(ErrorData *edata) if (Log_error_verbosity = PGERROR_DEFAULT) { - if (edata-detail_log) - { - log_line_prefix(buf, edata); - appendStringInfoString(buf, _(DETAIL: )); - append_with_tabs(buf, edata-detail_log); - appendStringInfoChar(buf, '\n'); - } - else if (edata-detail) + const char * const detail + = edata-detail_log != NULL ? edata-detail_log : edata-detail; + const char * const hint + = edata-hint_log != NULL ?