Re: Patch: Add parse_type Function - Mailing list pgsql-hackers

From Erik Wienhold
Subject Re: Patch: Add parse_type Function
Date
Msg-id h7loa2dkoqkloga4vqa6ir5gjq5xqnk5rf5mmjznrclztrkh5f@ef6u65vxtgqy
Whole thread Raw
In response to Re: Patch: Add parse_type Function  ("David E. Wheeler" <david@justatheory.com>)
Responses Re: Patch: Add parse_type Function
List pgsql-hackers
On 2024-02-19 23:59 +0100, David E. Wheeler wrote:
> On Feb 19, 2024, at 15:47, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> >> 1. Add a to_regtypmod() for those who just want the typemod.
> > 
> > Seems like there's a good case for doing that.
> 
> I’ll work on that.

See the patch I wrote for my benchmarks.  But it's pretty easy anyway to
cut down parse_type() ;)

> > I'm less thrilled about that, mainly because I can't think of a good
> > name for it ("format_type_string" is certainly not that).  Is the
> > use-case for this functionality really strong enough that we need to
> > provide it as a single function rather than something assembled out
> > of spare parts?
> 
> Not essential for pgTAP, no, as we can just use the parts. It was the
> typmod bit that was missing.

But you don't actually need reformat_type() in pgTAP.  You can just get
the type OID and modifier of the want_type and have_type and compare
those.  Then use format_type() for the error message.  Looks a bit
cleaner to me than doing the string comparison.

On second thought, I guess comparing the reformatted type names is
necessary in order to have a uniform API on older Postgres releases
where pgTAP has to provide its own to_regtypmod() based on typmodin
functions.

> On Feb 19, 2024, at 17:13, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> > After some time ruminating, a couple of possibilities occurred to
> > me: reformat_type(text) canonical_type_name(text)
> 
> I was just thinking about hitting the thesaurus entry for “canonical”,
> but I quite like reformat_type. It’s super clear and draws the
> parallel to format_type() more clearly. Will likely steal the name for
> pgTAP if we don’t add it to core. :-)
> 
> > I'm still unconvinced about that, though.
> 
> I guess the questions are:
> 
> * What are the other use cases for it?

Can't think of any right now other than this are-type-names-the-same
check.  Perhaps also for pretty-printing user-provided type names where
we don't take the type info from e.g. pg_attribute.

> * How obvious is it how to do it?
> 
> For the latter, it could easily be an example in the docs.

Can be mentioned right under format_type().

-- 
Erik



pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Synchronizing slots from primary to standby
Next
From: "David E. Wheeler"
Date:
Subject: Re: Patch: Add parse_type Function