Re: [PATCH] ltree, lquery, and ltxtquery binary protocol support

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Nino Floris <mail(at)ninofloris(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] ltree, lquery, and ltxtquery binary protocol support
Date: 2019-09-12 14:00:02
Message-ID: CAPpHfds1LO5nACigN4LJDTVx_3wFiUJa2kH57H+2PGYNvicnDg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 11, 2019 at 9:45 PM Nino Floris <mail(at)ninofloris(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-09-12 14:00:31 Re: Misleading comment in tuplesort_set_bound
Previous Message Alexander Korotkov 2019-09-12 13:55:04 Re: Write visibility map during CLUSTER/VACUUM FULL