Thread: pgsql: Allow on-the-fly capture of DDL event details

pgsql: Allow on-the-fly capture of DDL event details

From
Alvaro Herrera
Date:
Allow on-the-fly capture of DDL event details

This feature lets user code inspect and take action on DDL events.
Whenever a ddl_command_end event trigger is installed, DDL actions
executed are saved to a list which can be inspected during execution of
a function attached to ddl_command_end.

The set-returning function pg_event_trigger_ddl_commands can be used to
list actions so captured; it returns data about the type of command
executed, as well as the affected object.  This is sufficient for many
uses of this feature.  For the cases where it is not, we also provide a
"command" column of a new pseudo-type pg_ddl_command, which is a
pointer to a C structure that can be accessed by C code.  The struct
contains all the info necessary to completely inspect and even
reconstruct the executed command.

There is no actual deparse code here; that's expected to come later.
What we have is enough infrastructure that the deparsing can be done in
an external extension.  The intention is that we will add some deparsing
code in a later release, as an in-core extension.

A new test module is included.  It's probably insufficient as is, but it
should be sufficient as a starting point for a more complete and
future-proof approach.

Authors: Álvaro Herrera, with some help from Andres Freund, Ian Barwick,
Abhijit Menon-Sen.

Reviews by Andres Freund, Robert Haas, Amit Kapila, Michael Paquier,
Craig Ringer, David Steele.
Additional input from Chris Browne, Dimitri Fontaine, Stephen Frost,
Petr Jelínek, Tom Lane, Jim Nasby, Steven Singer, Pavel Stěhule.

Based on original work by Dimitri Fontaine, though I didn't use his
code.

Discussion:
  https://www.postgresql.org/message-id/m2txrsdzxa.fsf@2ndQuadrant.fr
  https://www.postgresql.org/message-id/20131108153322.GU5809@eldon.alvh.no-ip.org
  https://www.postgresql.org/message-id/20150215044814.GL3391@alvh.no-ip.org

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/b488c580aef4e05f39be5daaab6464da5b22a494

Modified Files
--------------
doc/src/sgml/event-trigger.sgml                    |   11 +-
doc/src/sgml/func.sgml                             |   93 ++-
src/backend/catalog/aclchk.c                       |   37 +-
src/backend/commands/event_trigger.c               |  706 +++++++++++++++++++-
src/backend/commands/opclasscmds.c                 |   63 +-
src/backend/commands/schemacmds.c                  |   12 +
src/backend/commands/tablecmds.c                   |   11 +-
src/backend/commands/tsearchcmds.c                 |    5 +
src/backend/nodes/copyfuncs.c                      |    1 +
src/backend/nodes/equalfuncs.c                     |    1 +
src/backend/parser/gram.y                          |    6 +
src/backend/tcop/utility.c                         |  274 +++++---
src/backend/utils/adt/format_type.c                |    3 +
src/backend/utils/adt/pseudotypes.c                |   65 +-
src/include/catalog/catversion.h                   |    2 +-
src/include/catalog/opfam_internal.h               |   28 +
src/include/catalog/pg_proc.h                      |   11 +
src/include/catalog/pg_type.h                      |    4 +
src/include/commands/defrem.h                      |    1 +
src/include/commands/event_trigger.h               |   26 +
src/include/commands/extension.h                   |    2 +-
src/include/nodes/parsenodes.h                     |   10 +
src/include/tcop/deparse_utility.h                 |  105 +++
src/include/utils/aclchk_internal.h                |   45 ++
src/include/utils/builtins.h                       |    5 +
src/test/modules/Makefile                          |    5 +-
src/test/modules/test_ddl_deparse/Makefile         |   19 +
.../test_ddl_deparse/expected/alter_function.out   |   15 +
.../test_ddl_deparse/expected/alter_sequence.out   |   15 +
.../test_ddl_deparse/expected/alter_table.out      |   18 +
.../test_ddl_deparse/expected/alter_type_enum.out  |    7 +
.../test_ddl_deparse/expected/comment_on.out       |   25 +
.../expected/create_conversion.out                 |    6 +
.../test_ddl_deparse/expected/create_domain.out    |   11 +
.../test_ddl_deparse/expected/create_extension.out |    5 +
.../test_ddl_deparse/expected/create_rule.out      |   30 +
.../test_ddl_deparse/expected/create_schema.out    |   19 +
.../expected/create_sequence_1.out                 |   11 +
.../test_ddl_deparse/expected/create_table.out     |  160 +++++
.../test_ddl_deparse/expected/create_trigger.out   |   18 +
.../test_ddl_deparse/expected/create_type.out      |   24 +
.../test_ddl_deparse/expected/create_view.out      |   19 +
.../modules/test_ddl_deparse/expected/defprivs.out |    6 +
.../modules/test_ddl_deparse/expected/matviews.out |    8 +
.../modules/test_ddl_deparse/expected/opfamily.out |   67 ++
.../test_ddl_deparse/expected/test_ddl_deparse.out |   40 ++
src/test/modules/test_ddl_deparse/regress_schedule |   21 +
.../test_ddl_deparse/sql/alter_function.sql        |   17 +
.../test_ddl_deparse/sql/alter_sequence.sql        |   17 +
.../modules/test_ddl_deparse/sql/alter_table.sql   |   13 +
.../test_ddl_deparse/sql/alter_type_enum.sql       |    6 +
.../modules/test_ddl_deparse/sql/comment_on.sql    |   17 +
.../test_ddl_deparse/sql/create_conversion.sql     |    6 +
.../modules/test_ddl_deparse/sql/create_domain.sql |   10 +
.../test_ddl_deparse/sql/create_extension.sql      |    6 +
.../modules/test_ddl_deparse/sql/create_rule.sql   |   31 +
.../modules/test_ddl_deparse/sql/create_schema.sql |   17 +
.../test_ddl_deparse/sql/create_sequence_1.sql     |   12 +
.../modules/test_ddl_deparse/sql/create_table.sql  |  142 ++++
.../test_ddl_deparse/sql/create_trigger.sql        |   18 +
.../modules/test_ddl_deparse/sql/create_type.sql   |   21 +
.../modules/test_ddl_deparse/sql/create_view.sql   |   17 +
src/test/modules/test_ddl_deparse/sql/defprivs.sql |    6 +
src/test/modules/test_ddl_deparse/sql/matviews.sql |    8 +
src/test/modules/test_ddl_deparse/sql/opfamily.sql |   53 ++
.../test_ddl_deparse/sql/test_ddl_deparse.sql      |   42 ++
.../test_ddl_deparse/test_ddl_deparse--1.0.sql     |   16 +
.../modules/test_ddl_deparse/test_ddl_deparse.c    |  267 ++++++++
.../test_ddl_deparse/test_ddl_deparse.control      |    4 +
69 files changed, 2667 insertions(+), 155 deletions(-)


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Allow on-the-fly capture of DDL event details
>
> This feature lets user code inspect and take action on DDL events.
> Whenever a ddl_command_end event trigger is installed, DDL actions
> executed are saved to a list which can be inspected during execution of
> a function attached to ddl_command_end.

Buildfarm machines on Windows are complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodon&dt=2015-05-11%2023%3A00%3A01

Visibly REGRESS and REGRESS_OPTS are messed up in the Makefile of
test_ddl_deparsing. The patch attached make tests pass correctly.
--
Michael

Attachment

Re: pgsql: Allow on-the-fly capture of DDL event details

From
Alvaro Herrera
Date:
Michael Paquier wrote:
> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Allow on-the-fly capture of DDL event details
> >
> > This feature lets user code inspect and take action on DDL events.
> > Whenever a ddl_command_end event trigger is installed, DDL actions
> > executed are saved to a list which can be inspected during execution of
> > a function attached to ddl_command_end.
>
> Buildfarm machines on Windows are complaining:
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodon&dt=2015-05-11%2023%3A00%3A01
>
> Visibly REGRESS and REGRESS_OPTS are messed up in the Makefile of
> test_ddl_deparsing. The patch attached make tests pass correctly.

Um.  In my system, if I set REGRESS to test_ddl_deparse per your patch,
the test_ddl_deparse test is run twice, at the beginning (per the
schedule file) and at the end, and it fails on the second run.

I tried setting REGRESS to empty, and to leave it unset, but that only
causes make to say "nothing to be done for installcheck" which isn't
good either.  I suppose I could delete the last line from the schedule
file and put it in the REGRESS line in the Makefile, but that seems
wrong too.

Not sure what's the real fix here ..

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> > Allow on-the-fly capture of DDL event details
>> >
>> > This feature lets user code inspect and take action on DDL events.
>> > Whenever a ddl_command_end event trigger is installed, DDL actions
>> > executed are saved to a list which can be inspected during execution of
>> > a function attached to ddl_command_end.
>>
>> Buildfarm machines on Windows are complaining:
>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodon&dt=2015-05-11%2023%3A00%3A01
>>
>> Visibly REGRESS and REGRESS_OPTS are messed up in the Makefile of
>> test_ddl_deparsing. The patch attached make tests pass correctly.
>
> Um.  In my system, if I set REGRESS to test_ddl_deparse per your patch,
> the test_ddl_deparse test is run twice, at the beginning (per the
> schedule file) and at the end, and it fails on the second run.
>
> I tried setting REGRESS to empty, and to leave it unset, but that only
> causes make to say "nothing to be done for installcheck" which isn't
> good either.  I suppose I could delete the last line from the schedule
> file and put it in the REGRESS line in the Makefile, but that seems
> wrong too.
>
> Not sure what's the real fix here ..

I think that you should then use MODULE_big instead of MODULES, and
set OBJS as "test_dll_parser.o $(WIN32RES)".
--
Michael


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Michael Paquier wrote:
>>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Not sure what's the real fix here ..
>
> I think that you should then use MODULE_big instead of MODULES, and
> set OBJS as "test_dll_parser.o $(WIN32RES)".

Taking it back, listing explicitly the list of tests in the Makefile's
REGRESS works just fine. Patch attached.
--
Michael

Attachment

Re: pgsql: Allow on-the-fly capture of DDL event details

From
Alvaro Herrera
Date:
Michael Paquier wrote:
> On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
> > <alvherre@2ndquadrant.com> wrote:
> >> Michael Paquier wrote:
> >>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >> Not sure what's the real fix here ..
> >
> > I think that you should then use MODULE_big instead of MODULES, and
> > set OBJS as "test_dll_parser.o $(WIN32RES)".
>
> Taking it back, listing explicitly the list of tests in the Makefile's
> REGRESS works just fine. Patch attached.

Sure.  I want to avoid doing that, though: we may want to generate a
schedule based on src/test/regress/serial_schedule, so that newly added
tests to the regular suite are automatically considered by this module.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Tue, May 12, 2015 at 1:05 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>> On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
>> > <alvherre@2ndquadrant.com> wrote:
>> >> Michael Paquier wrote:
>> >>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> >> Not sure what's the real fix here ..
>> >
>> > I think that you should then use MODULE_big instead of MODULES, and
>> > set OBJS as "test_dll_parser.o $(WIN32RES)".
>>
>> Taking it back, listing explicitly the list of tests in the Makefile's
>> REGRESS works just fine. Patch attached.
>
> Sure.  I want to avoid doing that, though: we may want to generate a
> schedule based on src/test/regress/serial_schedule, so that newly added
> tests to the regular suite are automatically considered by this module.

Mind share more details about that? How would you detect that a given
test in serial_schedule needs to be considered by test_ddl_deparse
automatically? WIth a new type of keyword in a schedule file? This
looks like a different feature to me that's going to need more
infrastructure...

If you want to continue on this way though, I imagine that you will
need to add a special case in fetchTests of vcregress.pl. In any case,
it is not good to keep the buildfarm machines broken for too long, and
personally I would rather avoid adding one more hack in the MSVC build
scripts.
--
Michael


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Tue, May 12, 2015 at 1:18 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, May 12, 2015 at 1:05 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Michael Paquier wrote:
>>> On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
>>> <michael.paquier@gmail.com> wrote:
>>> > On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
>>> > <alvherre@2ndquadrant.com> wrote:
>>> >> Michael Paquier wrote:
>>> >>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>>> >> Not sure what's the real fix here ..
>>> >
>>> > I think that you should then use MODULE_big instead of MODULES, and
>>> > set OBJS as "test_dll_parser.o $(WIN32RES)".
>>>
>>> Taking it back, listing explicitly the list of tests in the Makefile's
>>> REGRESS works just fine. Patch attached.
>>
>> Sure.  I want to avoid doing that, though: we may want to generate a
>> schedule based on src/test/regress/serial_schedule, so that newly added
>> tests to the regular suite are automatically considered by this module.
>
> Mind share more details about that? How would you detect that a given
> test in serial_schedule needs to be considered by test_ddl_deparse
> automatically? WIth a new type of keyword in a schedule file? This
> looks like a different feature to me that's going to need more
> infrastructure...
>
> If you want to continue on this way though, I imagine that you will
> need to add a special case in fetchTests of vcregress.pl. In any case,
> it is not good to keep the buildfarm machines broken for too long, and
> personally I would rather avoid adding one more hack in the MSVC build
> scripts.

Btw, just adding that a .gitignore file with /results/ is missing in
src/test/modules/test_ddl_deparse.
--
Michael


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Andrew Dunstan
Date:
On 05/12/2015 12:05 AM, Alvaro Herrera wrote:
> Michael Paquier wrote:
>> On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
>>> <alvherre@2ndquadrant.com> wrote:
>>>> Michael Paquier wrote:
>>>>> On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>>>> Not sure what's the real fix here ..
>>> I think that you should then use MODULE_big instead of MODULES, and
>>> set OBJS as "test_dll_parser.o $(WIN32RES)".
>> Taking it back, listing explicitly the list of tests in the Makefile's
>> REGRESS works just fine. Patch attached.
> Sure.  I want to avoid doing that, though: we may want to generate a
> schedule based on src/test/regress/serial_schedule, so that newly added
> tests to the regular suite are automatically considered by this module.
>


If you want to do that you will need to make some special adjustment in
the MSVC build system. You can't use $(srcdir) in REGRESS and expect the
MSVC builds not to break otherwise. We don't do that anywhere else AFAICT.

cheers

andrew



Re: pgsql: Allow on-the-fly capture of DDL event details

From
Alvaro Herrera
Date:
Michael Paquier wrote:
> On Tue, May 12, 2015 at 1:05 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:

> > Sure.  I want to avoid doing that, though: we may want to generate a
> > schedule based on src/test/regress/serial_schedule, so that newly added
> > tests to the regular suite are automatically considered by this module.
>
> Mind share more details about that? How would you detect that a given
> test in serial_schedule needs to be considered by test_ddl_deparse
> automatically? WIth a new type of keyword in a schedule file? This
> looks like a different feature to me that's going to need more
> infrastructure...

I was thinking in something simple such as
  cat $(srcdir)/src/test/regress/serial_schedule | grep -v tablespace >> the_schedule

> If you want to continue on this way though, I imagine that you will
> need to add a special case in fetchTests of vcregress.pl.

Hrm.

> In any case, it is not good to keep the buildfarm machines broken for
> too long, and personally I would rather avoid adding one more hack in
> the MSVC build scripts.

The MSVC build scripts *are* hacks themselves.  Anyway, I'll look into
it.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Andrew Dunstan
Date:
On 05/12/2015 10:03 AM, Alvaro Herrera wrote:
>> In any case, it is not good to keep the buildfarm machines broken for
>> too long, and personally I would rather avoid adding one more hack in
>> the MSVC build scripts.
> The MSVC build scripts *are* hacks themselves.  Anyway, I'll look into
> it.
>

Nobody has come up with a workable alternative. Nobody will be happier
than me if they do.

cheers

andrew


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Alvaro Herrera
Date:
Michael Paquier wrote:

> Mind share more details about that? How would you detect that a given
> test in serial_schedule needs to be considered by test_ddl_deparse
> automatically? WIth a new type of keyword in a schedule file? This
> looks like a different feature to me that's going to need more
> infrastructure...

Well, I remembered that the list of tests to run can be passed as a
variable to the make run, so the schedule file is not the best way to go
about this anyway.  Therefore I have pushed your patch (including the
.gitignore file)

Thanks

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: pgsql: Allow on-the-fly capture of DDL event details

From
Michael Paquier
Date:
On Wed, May 13, 2015 at 12:31 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Michael Paquier wrote:
>
>> Mind share more details about that? How would you detect that a given
>> test in serial_schedule needs to be considered by test_ddl_deparse
>> automatically? WIth a new type of keyword in a schedule file? This
>> looks like a different feature to me that's going to need more
>> infrastructure...
>
> Well, I remembered that the list of tests to run can be passed as a
> variable to the make run, so the schedule file is not the best way to go
> about this anyway.  Therefore I have pushed your patch (including the
> .gitignore file)

I would find better the idea to add a new variable, let's say
REGRESS_SCHEDULE to pass a list of schedule paths that will be taken
into account by PGXS and declare them with --schedule (pg_regress can
take multiple schedules), and add logic to parse it in the MSVC files.
No need to add a special case in the MSVC scripts this way.
--
Michael