search

ansible-collections / community.postgresql

Manage PostgreSQL with Ansible
https://galaxy.ansible.com/ui/repo/published/community/postgresql/
Other
108 stars 88 forks source link

Info, user, ... modules fail when `datestyle = 'redwood,show_time'` is set in postgresql.conf #711

Closed jbisabel closed 3 months ago

jbisabel commented 3 months ago
SUMMARY

When trying to fetch roles info, create users,... the module chokes on timestamps when datestyle = 'redwood,show_time' in the postgresql.conf of the database

ISSUE TYPE
COMPONENT NAME

postgresql_info

ANSIBLE VERSION
ansible [core 2.17.0]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/***/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/***/.venv/lib64/python3.11/site-packages/ansible
  ansible collection location = /home/***/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/***/.venv/bin/ansible
  python version = 3.11.7 (main, Jan 26 2024, 15:26:41) [GCC 8.5.0 20210514 (Red Hat 8.5.0-21)] (/home/***/.venv/bin/python3.11)
  jinja version = 3.1.4
  libyaml = True
COLLECTION VERSION
Collection        Version
----------------- -------
community.general 9.0.1
CONFIGURATION
CONFIG_FILE() = /etc/ansible/ansible.cfg
DEFAULT_HASH_BEHAVIOUR(/etc/ansible/ansible.cfg) = merge
DEFAULT_HOST_LIST(/etc/ansible/ansible.cfg) = ['/etc/ansible/dynamic_inventory']
DEFAULT_LOAD_CALLBACK_PLUGINS(/etc/ansible/ansible.cfg) = True
DEFAULT_MANAGED_STR(/etc/ansible/ansible.cfg) = This file managed by Ansible. Manual changes will be overwritten on future deployments.
DEFAULT_STDOUT_CALLBACK(/etc/ansible/ansible.cfg) = default
DEPRECATION_WARNINGS(/etc/ansible/ansible.cfg) = False
DISPLAY_SKIPPED_HOSTS(/etc/ansible/ansible.cfg) = True
RETRY_FILES_ENABLED(/etc/ansible/ansible.cfg) = True
OS / ENVIRONMENT
STEPS TO REPRODUCE
    - name: Gather info
      community.postgresql.postgresql_info:
        port: 5001
        filter:
          - roles
      register: info
EXPECTED RESULTS
ACTUAL RESULTS
The full traceback is:
  File "/tmp/ansible_community.postgresql.postgresql_info_payload_tkwdwg8r/ansible_community.postgresql.postgresql_info_payload.zip/ansible_collections/community/postgresql/plugins/modules/postgresql_info.py", line 1077, in __exec_sql
  File "/tmp/.venv/lib64/python3.11/site-packages/psycopg/cursor.py", line 859, in fetchall
    records = self._tx.load_rows(self._pos, self.pgresult.ntuples, self._make_row)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/.venv/lib64/python3.11/site-packages/psycopg/_transform.py", line 309, in load_rows
    record[col] = self._row_loaders[col](val)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/.venv/lib64/python3.11/site-packages/psycopg/types/datetime.py", line 507, in load
    return self._load_method(self, data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/.venv/lib64/python3.11/site-packages/psycopg/types/datetime.py", line 560, in _load_notimpl
    raise NotImplementedError(
fatal: [**********************]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "ca_cert": null,
            "connect_params": {},
            "db": null,
            "filter": [
                "roles"
            ],
            "login_host": "",
            "login_password": "",
            "login_unix_socket": "",
            "login_user": "postgres",
            "port": 5001,
            "session_role": null,
            "ssl_cert": null,
            "ssl_key": null,
            "ssl_mode": "prefer",
            "trust_input": true
        }
    },
    "msg": "Cannot execute SQL 'SELECT r.rolname, r.rolsuper, r.rolcanlogin, r.rolvaliduntil, ARRAY(SELECT b.rolname FROM pg_catalog.pg_auth_members AS m JOIN pg_catalog.pg_roles AS b ON (m.roleid = b.oid) WHERE m.member = r.oid) AS memberof FROM pg_catalog.pg_roles AS r WHERE r.rolname !~ '^pg_'': can't parse timestamptz with DateStyle 'Redwood, SHOW_TIME': 'infinity'"
}
hunleyd commented 3 months ago

datestyle = 'redwood' is only a thing in EDB's fork of Postgres, and is meant to show dates in an Oracle-compatible format. I defer to @Andersson007 , but I don't see us supporting fork changes like this.

jbisabel commented 3 months ago

Wouldn't it be best practice to set a datestyle on the connection? For example patching in the following snippet in the connect_to_db(...) function in postgres.py bypasses the issue:

cursor = db_connection.cursor(row_factory=psycopg.rows.dict_row)
try:
  cursor.execute('SET datestyle TO iso')
except Exception as e:
  module.fail_json(msg="Could not set date style: %s" % to_native(e))
finally:
  cursor.close()

Though that does seem to subsequently land me in a different can of worms with the python conversion of infinity

"Cannot execute SQL 'SELECT r.rolname, r.rolsuper, r.rolcanlogin, r.rolvaliduntil, ARRAY(SELECT b.rolname FROM pg_catalog.pg_auth_members AS m JOIN pg_catalog.pg_roles AS b ON (m.roleid = b.oid) WHERE m.member = r.oid) AS memberof FROM pg_catalog.pg_roles AS r WHERE r.rolname !~ '^pg_'': timestamp too large (after year 10K): 'infinity'"
jbisabel commented 3 months ago

The latter seems to be a known psycopg thing, going by the docs here. Inserting the following snippet in postgres.py fixes it at least for the info module...

class InfTimestamptzLoader(TimestamptzLoader):
    def load(self, data):
        if data == b"infinity":
            return datetime.max
        elif data == b"-infinity":
            return datetime.min
        else:
            return super().load(data)

psycopg.adapters.register_loader("timestamptz", InfTimestamptzLoader)
Andersson007 commented 3 months ago

@jbisabel hello, thanks for reporting the issue @hunleyd i incline to agree that we shouldn't support forks in general. Though, if this is the only thing preventing EDB users to use the collection, maybe we should allow it provided that it does not break things for postgres users? This is the first report specific to a fork i've ever seen here (i.e. in the last 5 years), so not sure if there'll be an influx of them if we make a precedent. I'm not trying to persuade to do it, really don't know. I would recommend interested parties to create a fork of this collection for EDB things but wouldn't it be an overkill.

What do you think @hunleyd @jbisabel @aleksvagachev @betanummeric @jchancojr ?

UPDATE: i think we should run any fork specific code only when that fork runs (to identify a server implementation and use if srv_implementation == 'EDB' to run that code and we definitely shouldn't run EDB or whatever non postgres in our CI. Considering this, it'd be impossible to test such changes..

jbisabel commented 3 months ago

First off I propose to split off the infinity issue into a separate issue, since from my testing it seems to be independent of this one, I can reproduce it using the default postgres datestyle.

Regarding datestyle, I'm kind of the opinion that setting it on the connection is a good idea in general, regardless of whether you're talking to a standard postgres or whatever fork, just to make sure you're getting a consistent representation across your playbook run. If not you run the risk of encountering different datestyles depending on whatever is set in the postgres.conf you're currently talking to.