Hi Tom,
Thanks a lot for pushing this through.
In complete agreement on fixing mbstrlen, it would clearly have lead to cut off string sends, or worse (does the binary protocol use null terminated strings, or are they length prefixed?). Apologies anyways, it's been a while so I don't know how it may have ended up there, thanks for catching it.
Even though it was a bit bumpy, and vastly different from my usual github contribution flow, I've had a good time contributing my first patch!
Best regards,
Nino Floris
I wrote:
> Fortunately for the odds of getting this patch accepted, we just
> pushed an ALTER TYPE improvement that will solve your problem [1].
> Please replace ltree--1.2.sql with an upgrade script that uses
> that, and resubmit.
I decided it would be a shame for this to miss v13, seeing that
(a) it'd provide real-world use of that ALTER TYPE code, and
(b) we already bumped ltree's extension version for v13.
So I went ahead and rebased this, reviewed and pushed it. The
main non-cosmetic thing I changed, other than using ALTER TYPE,
was that you had the output functions looking like this:
pq_sendtext(&buf, res, pg_mbstrlen(res));
That's just wrong; it should be plain strlen(). Did you copy
that coding from someplace? I could not find any similar
occurrences in our code tree.
regards, tom lane
