search

aws / postgresql-logfdw

Apache License 2.0
22 stars 6 forks source link

Tidy up log fdw #6

Closed BRupireddy2 closed 1 year ago

BRupireddy2 commented 1 year ago

Issue #, if available: NA

Description of changes:

Tidy up log_fdw

This commit basically cleans up the log_fdw extension with the following notable changes:

  1. Wraps list_postgres_log_files() with pg_ls_logdir() to not break compatibility.
  2. Allows pg_read_server_files roles to execute list_postgres_log_files() and create_foreign_table_for_log_file(). Additionally, allows pg_monitor role to execute list_postgres_log_files() similar to pg_ls_logdir().
  3. Adds tests to cover the extension code.
  4. Run pgindent on log_fdw.c

Note that the log_fdw extension is verified on master, REL_15_STABLE and REL_14_STABLE. In the PG versions REL_13_STABLE and prior, the COPY...FROM APIs were a bit different hence the log_fdw extension don't get compiled. To make the log_fdw compatible until all supported PG version (REL_11_STABLE), we might have to use version macro PG_VERSION_NUM to differentiate the code.

PS: It will be good to add the log_fdw tests to CI. However, the CI pipeline isn't able to run 'make check', it fails with "make check" is not supported. The CI pipeline currently runs 'make installcheck'. Note that when a test uses custom conf file for specific GUC settings, 'make installcheck' isn't supported for such tests. And log_fdw tests use custom conf file for enabling syslogger and other GUC settings because of which 'make installcheck' had to be disabled.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

davecramer commented 1 year ago

I find it very difficult to separate the pg_indent changes from anything else. My preference would be to have that as a separate PR after all the non-style changes go in.

davecramer commented 1 year ago

looks good to me. Thanks!