|From:||Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>|
|To:||Michael Paquier <michael(at)paquier(dot)xyz>|
|Cc:||PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>|
|Subject:||Re: Refactor textToQualifiedNameList()|
|Views:||Raw Message | Whole Thread | Download mbox|
út 9. 10. 2018 v 5:28 odesílatel Michael Paquier <michael(at)paquier(dot)xyz>
> On Mon, Oct 08, 2018 at 03:22:55PM +0000, Pavel Stehule wrote:
> > I tested this patch. This patch removes some duplicate rows, what is
> > good - on second hand, after this patch, the textToQualifiedNameList
> > does one more copy of input string more. I looked where this function
> > is used, and I don't think so it is a issue.
> > This patch is trivial - all tests passed and I don't see any
> > problem. I'll mark as ready for commit.
> > The new status of this patch is: Ready for Committer
> Are you sure that there is no performance impact? Say implement a
> simple SQL-callable function which does textToQualifiedNameList on the
> same string N times, and then compare the time it takes to run HEAD and
> the refactored implementation. I may buy that an extra palloc call is
> not something to worry about, but in my experience it can be a problem.
I try to find some worst case where this function is used and I expect so
most critical usage is in "nextval" function.
I disabled fsync and debug assertions and created sequence by command
create sequence pretty_long_schema_name.pretty_long_sequence_name cache
And I tested
explain analyze select
The difference on 10M calls is about 300ms - it is about 6%.
It is probably the cost of one palloc and one free more.
I am not able to decide if it is too much. If it is too much, then probably
is better do nothing. Introducing another third function we will have more
lines than before.
> This patch also changes stringToQualifiedNameList from regproc.h to
> varlena.h, which would break extensions calling it. That's not nice
It is not nice, but this is not serious problem for extensions's authors. I
remember more similar issues when I worked on Orafce extension.
My opinion is neutral - this patch reduces some lines, but not too much,
and has some overhead, but not too significant. Maybe more rich comment or
notes can be best solution.
|Next Message||Michael Paquier||2018-10-09 09:10:00||Re: partition tree inspection functions|
|Previous Message||'Christoph Moench-Tegeder'||2018-10-09 08:12:17||Re: Function for listing archive_status directory|