Обсуждение: Patch: Allow formatting in log_line_prefix
Heikki wrote that it might be useful to allow formatting in the log_line_prefix here http://www.postgresql.org/message-id/5187CADB.50306@vmware.com
I thought I'd take a bash at implementing space padding part on log_line_prefix options.David Rowley
Вложения
On 08/27/2013 05:06 AM, David Rowley wrote: > Heikki wrote that it might be useful to allow formatting in the > log_line_prefix here > http://www.postgresql.org/message-id/5187CADB.50306@vmware.com > > I thought I'd take a bash at implementing space padding part on > log_line_prefix options. > > It's been a while since I worked with the PostgreSQL source, so please > forgive me if my coding style is a bit off or the patch is not quite > in the correct format. > > It's a little rough around the edges at the moment and likely the > documentation needs more work, but I'm at the stage of wanting to know > if this is even wanted or needed by people. > > If you apply this patch you can do things like have %10u %-10d in your > log_line_prefix which will include spaces to give the option the > number you specify as the minimum width. Left and right aligning is > supported. > > Let this be the thread to collect consensus to whether this needed and > useful. I was just working on this last week! I didn't get a chance to post it because I was still polishing it. I first took the same approach as you but decided it reduced the clarity of the code so I re-did it a different way. I'll attach it here along with yours to see which way others like it. Did you test performance? You should add your patch to the next commitfest. -- Vik
Вложения
On Tue, Aug 27, 2013 at 7:55 PM, Vik Fearing <vik.fearing@dalibo.com> wrote:
I was just working on this last week! I didn't get a chance to post itOn 08/27/2013 05:06 AM, David Rowley wrote:
> Heikki wrote that it might be useful to allow formatting in the
> log_line_prefix here
> http://www.postgresql.org/message-id/5187CADB.50306@vmware.com
>
> I thought I'd take a bash at implementing space padding part on
> log_line_prefix options.
>
> It's been a while since I worked with the PostgreSQL source, so please
> forgive me if my coding style is a bit off or the patch is not quite
> in the correct format.
>
> It's a little rough around the edges at the moment and likely the
> documentation needs more work, but I'm at the stage of wanting to know
> if this is even wanted or needed by people.
>
> If you apply this patch you can do things like have %10u %-10d in your
> log_line_prefix which will include spaces to give the option the
> number you specify as the minimum width. Left and right aligning is
> supported.
>
> Let this be the thread to collect consensus to whether this needed and
> useful.
because I was still polishing it. I first took the same approach as you
but decided it reduced the clarity of the code so I re-did it a
different way. I'll attach it here along with yours to see which way
others like it.
I guess it's not too surprising someone else took this up too.
I was not too happy either at having to edit code for each option, even more so with the temp buffers I had to create in 2 places where 2 variables we used in the formatting.
I did not do any performance testing yet. I didn't think the regression would be very big with my patch as I'm only doing extra copying for %c and %r. I had also removed the strlen() before the loop starts, which I thought might actually speed the whole thing up a bit.
I've not tested applying your patch yet and I'm not the best at reading context diffs to tell, but I get the impression that we both decided to pad even when the variable is not valid, e.g. the host and database being empty during start-up.
Did you test performance? You should add your patch to the next commitfest.
--
Vik
I could add it, but I'm just thinking it's a bit strange to have 2 patches in a commitfest which do the same thing, but saying that we both went about it quite differently, I don't see a way to take the best out of both and make one. For me it looks like a bit of a choice between performance and more compact and cleaner code. I didn't think processing the log_line_prefix would be too big a hotspot, but I know performance regression is normally avoided at great cost, if necessary. Perhaps someone else can chime in here with some advice about preferences.
I could perhaps run a few benchmarks to test performance of the 2 of someone thought it was a good idea.
Regards
David