hapostgres / pg_auto_failover

Postgres extension and service for automated failover and high-availability
Other
1.13k stars 115 forks source link

group_id conflicts with keyword #509

Open gfphoenix78 opened 4 years ago

gfphoenix78 commented 4 years ago

Hi, hackers, I'm a database(postgresql variant) developer and we're considering to port pg_auto_failover for our database. The group_id is defined to be a keyword for a long time, which conflicts with the parameter in some functions. We can't change the keyword in our database. Besides we maintain a new variant of pg_auto_failover, is it possible to change the parameter name group_id to another name? Like, groupid?

DimCitus commented 3 years ago

Hi @gfphoenix78 ; we would consider such a PR for compatibility purposes yes. What I think should be done then is to change all the *_id names in the functions signatures in src/monitor/pgautofailover.sql to maintain consistency. Then we can disambiguate between column name and function argument name by prefixing with the function name where necessary, as in the existing following code:

CREATE FUNCTION pgautofailover.set_group_system_identifier
 (
    IN group_id            bigint,
    IN node_sysidentifier  bigint,
   OUT node_id          bigint,
   OUT node_name        text,
   OUT node_host        text,
   OUT node_port        int
 )
RETURNS setof record LANGUAGE SQL STRICT SECURITY DEFINER
AS $$
      update pgautofailover.node
         set sysidentifier = node_sysidentifier
       where groupid = set_group_system_identifier.group_id
         and sysidentifier = 0
   returning nodeid, nodename, nodehost, nodeport;
$$;

We will need to also prepare an upgrade SQL script with the renaming. We used to prepare such a script at upgrade time, but now that we have automated upgrades for the monitor extension version and schema, I think we should include that during development.

So in your PR, please also change the version number of the extension to 1.5.0.1 (Emacs style version number where X.Y.0 means development version for X.Y, and X.Y.1 would be the first stable release, X.Y.2 the first stable patch release, etc -- though we would just use X.Y as the stable release version number for the pgautofailover extension/schema).

Other PRs that will edit the extension schema will be able to use 1.5.0.2 and 1.5.0.3 etc at merge time. I think tracking schema changes with proper versioning during development is an improvement to what we've been doing up to now.