Re: sketchy partcollation handling

On Sun, Jun 4, 2017 at 10:18 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> I think we can call it a bug of StorePartitionKey().  I looked at the
> similar code in index_create() (which actually I had originally looked at
> for reference when writing the partitioning code in question) and looks
> like it doesn't store the dependency for collation 0 and for the default
> collation of the database.  I think the partitioning code should do the
> same.  Attached find a patch for the same (which also updates the
> documentation as mentioned above); with the patch:

Thanks.  Committed.

> BTW, the places which check whether the collation to store a dependency
> for is the database default collation don't need to do that.  I mean the
> following code block in all of these places:
>
>        /* The default collation is pinned, so don't bother recording it */
>        if (OidIsValid(attr->attcollation) &&
>            attr->attcollation != DEFAULT_COLLATION_OID)
>        {
>            referenced.classId = CollationRelationId;
>            referenced.objectId = attr->attcollation;
>            referenced.objectSubId = 0;
>            recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
>        }
>
> That's because the default collation is pinned and the dependency code
> checks isObjectPinned() and does not create pg_depend entries if so.
> Those places are:
>
> AddNewAttributeTuples
> StorePartitionKey
> index_create
> GenerateTypeDependencies
> add_column_collation_dependency

We could go change them all, but I guess I don't particularly see the point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company