Re: sketchy partcollation handling

On Sun, Jun 4, 2017 at 10:18 PM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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