From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Jacob Champion <jchampion(at)timescale(dot)com> |
Cc: | "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Joe Conway <mail(at)joeconway(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: SYSTEM_USER reserved word implementation |
Date: | 2022-09-28 03:28:25 |
Message-ID: | YzO/PawKtWSvLGFC@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Sep 27, 2022 at 03:38:49PM -0700, Jacob Champion wrote:
> On 9/26/22 06:29, Drouvot, Bertrand wrote:
> Since there are only internal clients to the API, I'd argue this makes
> more sense as an Assert(authn_id != NULL), but I don't think it's a
> dealbreaker.
Using an assert() looks like a good idea from here. If this is called
with a NULL authn, this could reflect a problem in the authentication
logic.
>> As far the assertion failure mentioned by Michael when moving the
>> SVFOP_SYSTEM_USER from NAMEOID to TEXTOID: V4 is assuming that it is
>> safe to force the collation to C_COLLATION_OID for SQLValueFunction
>> having a TEXT type, but I would be happy to also hear your thoughts
>> about it.
>
> Unfortunately I don't have much to add here; I don't know enough about
> the underlying problems.
I have been looking at that, and after putting my hands on that this
comes down to the facility introduced in 40c24bf. So, I think that
we'd better use COERCE_SQL_SYNTAX so as there is no need to worry
about the shortcuts this patch is trying to use with the collation
setup. And there are a few tests for get_func_sql_syntax() in
create_view.sql. Note that this makes the patch slightly shorter, and
simpler.
The docs still mentioned "name", and not "text".
This brings in a second point. 40c24bf has refrained from removing
SQLValueFunction, but based the experience on this thread I see a
pretty good argument in doing the jump once and for all. This
deserves a separate discussion, though. I'll do that and create a new
thread.
--
Michael
Attachment | Content-Type | Size |
---|---|---|
v5-0001-system_user-implementation.patch | text/x-diff | 14.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2022-09-28 03:28:56 | Re: A doubt about a newly added errdetail |
Previous Message | Andres Freund | 2022-09-28 02:52:42 | Re: Strip -mmacosx-version-min options from plperl build |