Thread: doc patch: clarify the naming rule for injection_points
Dear hackers, This is a forked thread from [1]. Naming rule of points is not determined yet, but most of them have lower cases and each term are divided by dash "-". I think this is a good chance to formally clarify it. PSA adding the description. I was confused the correct place for the description. I added at the end of first paragraph, because it describes how we add and use it. Suggestions are very welcomed. [1]: https://www.postgresql.org/message-id/OSCPR01MB14966B78F3AF15C252EB9E02FF5B32%40OSCPR01MB14966.jpnprd01.prod.outlook.com Best regards, Hayato Kuroda FUJITSU LIMITED
Attachment
Hi, > Naming rule of points is not determined yet, but most of them have lower cases > and each term are divided by dash "-". I think this is a good chance to formally > clarify it. PSA adding the description. > > I was confused the correct place for the description. I added at the end of first > paragraph, because it describes how we add and use it. Suggestions are very > welcomed. ``` - their own code using the same macro. + their own code using the same macro. The name of injection points must be + lower characters, and dashes must separate its terms. ``` Perhaps "must" is a too strong statement. I suggest something like: """ By convention, the name of injection points ... """ I have mixed feelings about the patch overall though. This would mean that we need to rename injection points like this: `` AtEOXact_Inval-with-transInvalInfo heap_update-before-pin ``` Otherwise we would contradict our own documentation. I don't mind heap-update-before-pin, but not 100% sure if: at-eo-xact-inval-with-trans-inval-info ... would be a better name than the current one. -- Best regards, Aleksander Alekseev
Dear Aleksander, > ``` > - their own code using the same macro. > + their own code using the same macro. The name of injection points must > be > + lower characters, and dashes must separate its terms. > ``` > > Perhaps "must" is a too strong statement. I suggest something like: > > """ > By convention, the name of injection points ... > """ It is always difficult for me to distinguish them, thanks for comments. PSA updated. > > I have mixed feelings about the patch overall though. This would mean > that we need to rename injection points like this: > > `` > AtEOXact_Inval-with-transInvalInfo > heap_update-before-pin > ``` > > Otherwise we would contradict our own documentation. Thanks for telling these counterexamples. I grepped with "INJECTION_POINTS" and "IS_INJECTION_POINT_ATTACHED", and no others are found. > I don't mind heap-update-before-pin, but not 100% sure if: > > at-eo-xact-inval-with-trans-inval-info > > ... would be a better name than the current one. I also feel just converting lower case is not good. The point seems to locate in the end-of-transaction callback and it accepts invalidation messages. Based on the fact, how about "inval-process-invalidation-messages"? 0002 did simple replacements of these words. Best regards, Hayato Kuroda FUJITSU LIMITED