Re: Refactor textToQualifiedNameList()

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()
Date: 2018-10-09 08:47:48
Message-ID: CAFj8pRDA1047vHF0yJN34=Hyatb-C8sQZSewpV2uYn6R-pFNNg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi

Ășt 9. 10. 2018 v 5:28 odesĂ­latel Michael Paquier <michael(at)paquier(dot)xyz>
napsal:

> 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
1000;

And I tested

explain analyze select
nextval('pretty_long_schema_name.pretty_long_sequence_name') from
generate_series(1,10000000);

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
> either.
>

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.

Regards

Pavel

> --
> Michael
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
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