Re: Adding a pg_get_owned_sequence function?

From: Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Nathan Bossart <nathandbossart(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Adding a pg_get_owned_sequence function?
Date: 2024-01-08 16:58:02
Message-ID: 87a5pfn4o5.fsf@wibble.ilmari.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

vignesh C <vignesh21(at)gmail(dot)com> writes:

> On Tue, 24 Oct 2023 at 22:00, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>>
>> On Tue, Sep 12, 2023 at 03:53:28PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> > Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> >> It's possible that we could get away with just summarily changing
>> >> the argument type from text to regclass. ISTR that we did exactly
>> >> that with nextval() years ago, and didn't get too much push-back.
>> >> But we couldn't do the same for the return type. Also, this
>> >> approach does nothing for the concern about the name being
>> >> misleading.
>> >
>> > Maybe we should go all the way the other way, and call it
>> > pg_get_identity_sequence() and claim that "serial" is a legacy form of
>> > identity columns?
>>
>> Hm. Could we split it into two functions, pg_get_owned_sequence() and
>> pg_get_identity_sequence()? I see that commit 3012061 [0] added support
>> for identity columns to pg_get_serial_sequence() because folks expected
>> that to work, so maybe that's a good reason to keep them together. If we
>> do elect to keep them combined, I'd be okay with renaming it to
>> pg_get_identity_sequence() along with your other proposed changes.

We can't make pg_get_serial_sequence(text, text) not work on identity
columns any more, that would break existing users, and making the new
function not work on serial columns would make it harder for people to
migrate to it. If you're suggesting adding two new functions,
pg_get_identity_sequence(regclass, name) and
pg_get_serial_sequence(regclass, name)¹, which only work on the type of
column corresponding to the name, I don't think that's great for
usability or migratability either.

> I have changed the status of the patch to "Waiting on Author" as
> Nathan's comments have not yet been followed up.

Thanks for the nudge, here's an updated patch that together with the
above addresses them. I've changed the commitfest entry back to "Needs
review".

> Regards,
> Vignesh

- ilmari

Attachment Content-Type Size
v2-0001-Add-pg_get_identity_sequence-function.patch text/x-diff 10.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2024-01-08 17:00:00 Re: Add a perl function in Cluster.pm to generate WAL
Previous Message Tom Lane 2024-01-08 16:51:22 Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500