On Fri, Feb 5, 2021, at 4:01 AM, Amit Kapila wrote: > I am not completely whether we should retire replorigin_drop or just > keep it for backward compatibility? What do you think? Anybody else > has any opinion? We could certainly keep some code for backward compatibility, however, we have to consider if it is (a) an exposed API and/or (b) a critical path. We break several extensions every release due to Postgres extensibility. For (a), it is not an exposed function, I mean, we are not changing `pg_replication_origin_drop`. Hence, there is no need to keep it. In (b), we could risk slowing down some critical paths that we decide to keep the old function and create a new one that contains additional features. It is not the case for this function. It is rare that an extension does not have a few #ifdef if it supports multiple Postgres versions. IMO we should keep as little code as possible into the core in favor of maintainability.
- replorigin_drop(roident, true); + replorigin_drop_by_name(name, false /* missing_ok */ , true /* nowait */ ); A modern IDE would certainly show you the function definition that allows you to check what each parameter value is without having to go back and forth. I saw a few occurrences of this pattern in the source code and IMO it could be used when it is not obvious what that value means. Booleans are easier to figure out, however, sometimes integer and text are not. -- Euler Taveira EDB https://www.enterprisedb.com/
