Message-ID: <95a01e66-0c92-2ff6-b243-849ddd11685d@gmail.com>
Date: Thu, 2 May 2024 07:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #18449: Altering column type fails when an SQL routine
 depends on the column
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org,
 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
References: <18449-f8248467aaa294d5@postgresql.org>
 <403599.1714268901@sss.pgh.pa.us>
 <548a47bc-87ae-b3df-c6a2-60b9966f808b@gmail.com>
 <1944626.1714572523@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <1944626.1714572523@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/95a01e66-0c92-2ff6-b243-849ddd11685d%40gmail.com>
Precedence: bulk

01.05.2024 17:08, Tom Lane wrote:
> Hm.  We could fix this by introducing another single-purpose error
> report, but I'm starting to think that that's failing to learn from
> experience.  Who's to say that other column dependencies aren't
> possible, now or in the future?  The only thing stopping us from
> treating the default: case as a normal ERRCODE_FEATURE_NOT_SUPPORTED
> error is that it might be hard to phrase the error message in a nice
> way.  We already have a precedent for this being an acceptable
> errdetail:
>
>                      ereport(ERROR,
>                              (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>                               errmsg("cannot alter type of a column used in a policy definition"),
>                               errdetail("%s depends on column \"%s\"",
>                                         getObjectDescription(&foundObject, false),
>                                         colName)));
>
> It doesn't seem too awful to me to write the errmsg as
>
>                               errmsg("cannot alter type of a column used in a %s",
>                                      get_object_class_descr(foundObject.classid)),

I agree, though by doing that we'll loose the ability to detect obviously
wrong dependencies, say, when a role depends on a table column (if
INTERNAL_ERRORs expected to be handled as something abnormal),
but ATExecAlterColumnType() is hardly an appropriate place for such
detection anyway. So I would sacrifice this ability to make this function
simpler now and in the future.

Best regards,
Alexander