On Tue, Jul 29, 2025 at 8:46 AM David G. Johnston < david.g.johns...@gmail.com> wrote:
> On Monday, July 28, 2025, Daniel Bauman <danielban...@gmail.com> wrote: > >> The doc fragment you shared is explaining to customers that basic syntax >> checks are done before postgres gets to logging the transaction. >> I don't think that is the same as clearly explaining to users statement >> logging happens before execution. >> > > He quoted the wrong sentence. This is the definitive one: > > For clients using extended query protocol, logging occurs when an Execute > message is received, and values of the Bind parameters are included (with > any embedded single-quote marks doubled). > David J. > > > > >> At least for me, as a user and reader of the docs but not someone >> familiar with the code, the docs didn't make it clear to me how statement >> logging corresponded to query execution. >> >> Do you think there's room to document something like >> "Statements are logged before they are executed. It's not guaranteed that >> logged statements have been successfully executed." >> I'd be happy to submit and iterate on a pull request if you do. >> > > The first sentence maybe. If you find this important enough to submit a > patch using our submission process go ahead. On that note, please observe > that we in-line reply and trim here, not top-post. > Thanks for the callout, in-line replying here :) I think the first sentence would be an improvement. However, based on the fact that errors are ignored and the fact that there is no fsync (have looked, haven't found an fsync and don't think doing so would make sense) on the ereport path I think there's room for making it clear that logging of all transactions is not guaranteed. Do you disagree with making this explicit? The tradeoff to ignore errors and not fsync every log absolutely makes sense to me. It's just something it would be beneficial for users to be aware of. Particularly those concerned with auditing. > > >> >> I'd also like to understand what happens if there are errors writing the >> log - like the disk where the log directory is configured being full. >> My understanding is the following. ereport ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/include/utils/elog.h#L163) >> will end up calling errfinish and errfinish will call EmitErrorReport ( >> https://github.com/postgres/postgres/blob/master/src/backend/utils/error/elog.c#L543) >> which will call send_message_to_server_log ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/utils/error/elog.c#L1733) >> and that will call write_syslogger_file ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/utils/error/elog.c#L3443C2-L3443C16 >> ). >> write_syslogger_file looks like it handles errors by logging to stderr >> but not raising an error condition that would cancel the transaction ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/postmaster/syslogger.c#L1126 >> ) >> >> Do I understand correctly or am I on the wrong codepath? >> > > That sounds right. It’s deemed overly harsh to crash the server just > because some logging doesn’t happen. > I agree. I'm not suggesting a change, just making the docs explicit. On Tue, Jul 29, 2025 at 8:46 AM David G. Johnston < david.g.johns...@gmail.com> wrote: > On Monday, July 28, 2025, Daniel Bauman <danielban...@gmail.com> wrote: > >> The doc fragment you shared is explaining to customers that basic syntax >> checks are done before postgres gets to logging the transaction. >> I don't think that is the same as clearly explaining to users statement >> logging happens before execution. >> > > He quoted the wrong sentence. This is the definitive one: > > For clients using extended query protocol, logging occurs when an Execute > message is received, and values of the Bind parameters are included (with > any embedded single-quote marks doubled). > David J. > > > > >> At least for me, as a user and reader of the docs but not someone >> familiar with the code, the docs didn't make it clear to me how statement >> logging corresponded to query execution. >> >> Do you think there's room to document something like >> "Statements are logged before they are executed. It's not guaranteed that >> logged statements have been successfully executed." >> I'd be happy to submit and iterate on a pull request if you do. >> > > The first sentence maybe. If you find this important enough to submit a > patch using our submission process go ahead. On that note, please observe > that we in-line reply and trim here, not top-post. > > >> >> I'd also like to understand what happens if there are errors writing the >> log - like the disk where the log directory is configured being full. >> My understanding is the following. ereport ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/include/utils/elog.h#L163) >> will end up calling errfinish and errfinish will call EmitErrorReport ( >> https://github.com/postgres/postgres/blob/master/src/backend/utils/error/elog.c#L543) >> which will call send_message_to_server_log ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/utils/error/elog.c#L1733) >> and that will call write_syslogger_file ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/utils/error/elog.c#L3443C2-L3443C16 >> ). >> write_syslogger_file looks like it handles errors by logging to stderr >> but not raising an error condition that would cancel the transaction ( >> https://github.com/postgres/postgres/blob/71c0921b649d7a800eb2d6f93539890eaa14d979/src/backend/postmaster/syslogger.c#L1126 >> ) >> >> Do I understand correctly or am I on the wrong codepath? >> > > That sounds right. It’s deemed overly harsh to crash the server just > because some logging doesn’t happen. > > David J. > >
