Re: [PATCH] ltree, lquery, and ltxtquery binary protocol support - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: [PATCH] ltree, lquery, and ltxtquery binary protocol support
Date
Msg-id CAPpHfds1LO5nACigN4LJDTVx_3wFiUJa2kH57H+2PGYNvicnDg@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] ltree, lquery, and ltxtquery binary protocol support  (Nino Floris <mail@ninofloris.com>)
List pgsql-hackers
On Wed, Sep 11, 2019 at 9:45 PM Nino Floris <mail@ninofloris.com> wrote:
>  > * We currently don't add new extension SQL-script for new extension
> > version (i.e. ltree--1.2.sql).  Instead, we provide just a migration
> > script (i.e. ltree--1.1--1.2.sql).  This simplifies testing of
> > extension migration – plain extension creation implies migration.
>
> I wasn't sure how to add new methods to the type without doing a full
> CREATE TYPE, as ALTER TYPE doesn't allow updates to functions. At that
> point wouldn't it be better as a new version?
> I checked some other extensions like hstore to find reference material
> how to do a new CREATE TYPE, all did a full version bump.
> Should I just do a DROP and CREATE instead in a migration?

Oh, I appears we didn't add send/recv to any pluggable datatype one we
get extension system.

Ideally we need to adjust ALTER TYPE command so that it could add
send/recv functions to an existing type.

I see couple other options:
1) Write migration script, which directly updates pg_type.
2) Add ltree--1.2.sql and advise users to recreate extension to get
binary protocol support.
But both these options are cumbersome.

What do you think about writing patch for ALTER TYPE?

> >  * What is motivation for binary format for lquery and ltxtquery?  One
> could transfer large datasets of ltree's.  But is it so for lquery and
> ltxtquery, which are used just for querying.
>
> Completeness, Npgsql (and other drivers like Ecto from Elixir and
> probably many others as well) can't do any text fallback in the binary
> protocol without manual configuration.
> This is because these drivers don't know much (or anything) about the
> SQL they send so it wouldn't know to apply it for which columns.
> I believe there has been a proposal at some point to enhance the
> binary protocol to additionally allow clients to specify text/binary
> per data type instead of per column.
> That would allow all these drivers to automate this, but I think it
> never went anywhere.
> As it stands it's not ergonomic to ask developers to setup this
> metadata per query they write.
>
>  * Just send binary representation of datatype is not OK.  PostgreSQL
> supports a lot of platforms with different byte ordering, alignment
> and so on.  You basically need to send each particular field using
> pq_send*() function.
>
> Oh my, I don't think I did? I copied what jsonb is doing,
> specifically, sending the textual representation of the data type with
> a version field prefixed.
> (It is why I introduced deparse_ltree/lquery, to take the respective
> structure and create a null terminated string of its textual form)
> So there are no other fields besides version and the string
> representation of the structure.
> ltree, lquery, and ltxtquery all seem to have a lossless and compact
> textual interpretation.
> My motivation here has been "if it's good enough for jsonb it should
> be good enough for a niche extension like ltree" especially as having
> any binary support is better than not having it at all.
> I can change it to anything you'd like to see instead but I would need
> some pointers as to what that would be.

Oh, sorry.  I didn't notice you send textual representation for ltree,
lquery, and ltxtquery.  And the point is not performance for these
datatypes, but completeness.  Now I got it, then it looks OK.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Write visibility map during CLUSTER/VACUUM FULL
Next
From: Tom Lane
Date:
Subject: Re: Misleading comment in tuplesort_set_bound