Re: BUG #17056: Segmentation fault on altering the type of the foreign table column with a default

On 6/10/21 7:11 PM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On 6/10/21 6:10 PM, Tom Lane wrote:
>>> Hmm. The equivalent DDL on a plain table works fine, but this is
>>> crashing in the code that manipulates attmissingval. I suspect some
>>> confusion about whether a foreign table column should even *have*
>>> attmissingval. Andrew, any thoughts?
>> My initial thought would be that it should not. If the foreign table has
>> rows with missing columns then it should be up to the foreign server to
>> supply them transparently. We have no notion what the foreign semantics
>> of missing columns are.
> Yeah, that was kind of what I thought. Probably only RELKIND_RELATION
> rels should ever have attmissingval; but certainly, anything without
> local storage should not.
>
>> I can take a look at a fix tomorrow. My inclination would be simply to
>> skip setting attmissingval for foreign tables.
> Seems like in addition to that, we'll need a defense in this specific
> code to cope with the case where the foreign column already has an
> attmissingval. Or maybe, the logic to not store a new one will be enough
> to keep us from reaching this crash; but we need to be sure it is enough.

The first piece could be fairly simply accomplished by something like this

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index afa830d924..ac89efefe8 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2287,7 +2287,8 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
        valuesAtt[Anum_pg_attribute_atthasdef - 1] = true;
        replacesAtt[Anum_pg_attribute_atthasdef - 1] = true;
 
-       if (add_column_mode && !attgenerated)
+       if (rel->rd_rel->relkind == RELKIND_RELATION  && add_column_mode &&
+           !attgenerated)
        {
            expr2 = expression_planner(expr2);
            estate = CreateExecutorState();

I'm guessing we want to exclude materialized views and partitioned
tables as well as things without local storage.

How to ignore something that's got into the catalog that shouldn't be
there is less clear. At the point where we fetch missing values all we
have access to is a TupleDesc.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com