A few of the recent function name change suggestions have originated from my internal review comments. But, these changes are not very effective unless they are done across existing functions as well to keep everything consistent.
Background ---------- TBH, I always found many of the current HEAD names to be confusing. I am often having to double-check back with the function comment to find what was the intention. e.g. Here is a list of some existing function names of pg_publication.c. GetPubPartitionOptionRelations GetRelationPublications GetPublicationRelations GetAllTablesPublications GetAllPublicationRelations GetPublicationSchemas GetSchemaPublications GetSchemaPublicationRelations GetAllSchemaPublicationRelations GetPublication GetPublicationByName At the core of my confusion is that the pattern GetYyyyXxxx means 2 different things: - Sometimes the Yyyy is a search condition or "criteria" for what you are getting. e.g. GetRelationPublications means "Get publications that the specified relid is a member of" - sometimes the Yyyy is more like an "attribute" for what you are getting. e.g. GetAllTablesPublications means "Get all of the FOR ALL TABLES publications" Often there is some ambiguity: e.g GetAllPublicationRelations - does it mean "Get relations list equivalent to a FOR ALL TABLES publication"? - does it mean "Get relations that are members of all the specified publications"? e.g. GetSchemaPublications - does it mean "Get publications marked as FOR TABLES IN SCHEMA"? - does it mean "Get publications for the specified schemaid"? etc. ~~~ This thread's EXCEPT patch is not causing new problems, it's just adding yet more functions to the long list of those that I felt are ambiguous. IMO, when a function returns XXX, then the function name should be prefixed GetXXX. So, I would advocate for existing function names to change; something like below. GetPubPartitionOptionRelations -> GetRelsByRelidAndPartitionOpt GetRelationPublications -> GetPubsByRelid GetPublicationRelations -> GetRelsByPubidMarkedForTable GetAllTablesPublications -> GetPubsMarkedForAllTables GetAllPublicationRelations -> GetRelsOfPubsMarkedForAll GetPublicationSchemas -> GetSchemasByPubidMarkedForTablesInSchema GetSchemaPublications -> GetPubsMarkedForTablesInSchemaBySchemaid GetSchemaPublicationRelations -> GetPublishedRelsBySchemaid GetAllSchemaPublicationRelations -> GetRelsOfPubsMarkedForTablesInSchema GetPublication -> GetPubByPubid GetPublicationByName -> GetPubByName Then, any new (non-static) functions for this EXCEPT patch would be named similarly. ~ I understand that maybe everyone else feels the current names are fine as-is and not confusing at all. Or, maybe the confusion is recognised, but now is just not a good time of year to change everything. Anyway, I just wanted to give the background, and explain reasons for those function name suggestions. ====== Kind Regards, Peter Smith. Fujitsu Australia.
