csvlog_fields review
От | Alex Hunsaker |
---|---|
Тема | csvlog_fields review |
Дата | |
Msg-id | CAFaPBrR3yN7qVmvdwZvC9fUyugD77VJ+TpCnMTuAgdjWwMs-pg@mail.gmail.com обсуждение исходный текст |
Список | pgsql-hackers |
It bit rotted a bit find a new version attached that includes the following fixes: - show_session_authorization() no longer exists, instead access the session_authorization_guc directly (like we do for show_role in commands/variable.c). I find it quite ugly tho... - it changed %u to %U and %U to be %u... flipped it back so %u remains unchanged - num_fields in write_csvlog was unused, removed it - new_csv_fields would leak in an error path of assign_csvlog_fields (which probably matters as we are in TopMemoryContext) All of Itagaki-san's comments still stand: > * csvlog_fields and csvlog_header won't work with non-default log_filename when it doesn't contain seconds in the format. They expect they can always open empty log files. I think at the very least we should document this? Or maybe only write out the header when its a zero length file? > * The long default value for csvlog_fields leads long text line in postgresql.conf, SHOW ALL, pg_settings view, but there were no better alternative solutions in the past discussion. I think it might be worth revisiting using the %X syntax log_line_prefix uses instead of inventing our own long form names. > * csvlog_fields is marked as PGC_POSTMASTER. It can protect mixed formats in a csv file on default log_filename, but other similar GUC variables are usually marked AS PGC_SIGHUP. I don't really see this as a problem... As it stands I think we still need to address the first two questions before its ready for a -commiter.
Вложения
В списке pgsql-hackers по дате отправления: