Обсуждение: pgsql: Add hooks for session start and session end
Add hooks for session start and session end These hooks can be used in loadable modules. A simple test module is included. Discussion: https://postgr.es/m/20170720204733.40f2b7eb.nagata@sraoss.co.jp Fabrízio de Royes Mello and Yugo Nagata Reviewed by Michael Paquier and Aleksandr Parfenov Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/cd8ce3a22c0b48d32ffe6543837ba3bb647ac2b2 Modified Files -------------- src/backend/tcop/postgres.c | 6 + src/backend/utils/init/postinit.c | 6 + src/include/tcop/tcopprot.h | 7 ++ src/test/modules/Makefile | 1 + src/test/modules/test_session_hooks/.gitignore | 4 + src/test/modules/test_session_hooks/Makefile | 21 ++++ src/test/modules/test_session_hooks/README | 2 + .../expected/test_session_hooks.out | 31 +++++ .../modules/test_session_hooks/session_hooks.conf | 2 + .../test_session_hooks/sql/test_session_hooks.sql | 12 ++ .../test_session_hooks/test_session_hooks--1.0.sql | 4 + .../test_session_hooks/test_session_hooks.c | 134 +++++++++++++++++++++ .../test_session_hooks/test_session_hooks.control | 3 + 13 files changed, 233 insertions(+)
Andrew Dunstan <andrew@dunslane.net> writes:
> Add hooks for session start and session end
Buildfarm doesn't like this one bit :-(
regards, tom lane
On 11/15/2017 11:50 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> Add hooks for session start and session end > Buildfarm doesn't like this one bit :-( > > Strange, it worked when I tested. I'll look into it. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 11/15/2017 11:50 AM, Tom Lane wrote:
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>> Add hooks for session start and session end
>> Buildfarm doesn't like this one bit :-(
> Strange, it worked when I tested. I'll look into it.
Looks to me like the problem is that the test can only work in "check"
mode, not "installcheck" mode, because in the latter case you don't
get to do preloaded libraries.
regards, tom lane
On 11/15/2017 01:56 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 11/15/2017 11:50 AM, Tom Lane wrote: >>> Andrew Dunstan <andrew@dunslane.net> writes: >>>> Add hooks for session start and session end >>> Buildfarm doesn't like this one bit :-( >> Strange, it worked when I tested. I'll look into it. > Looks to me like the problem is that the test can only work in "check" > mode, not "installcheck" mode, because in the latter case you don't > get to do preloaded libraries. > > Yeah, that was my first instinct. But then how do the test_rls_hooks tests succeed? They too have a shared_preload_library. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 11/15/2017 02:17 PM, Andrew Dunstan wrote: > > On 11/15/2017 01:56 PM, Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >>> On 11/15/2017 11:50 AM, Tom Lane wrote: >>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>> Add hooks for session start and session end >>>> Buildfarm doesn't like this one bit :-( >>> Strange, it worked when I tested. I'll look into it. >> Looks to me like the problem is that the test can only work in "check" >> mode, not "installcheck" mode, because in the latter case you don't >> get to do preloaded libraries. >> >> > > > Yeah, that was my first instinct. But then how do the test_rls_hooks > tests succeed? They too have a shared_preload_library. > Aha, it does LOAD 'test_rls_hooks'; at the start of its script. Of course we can't do that here. Not sure what the best way to resolve this is. For now I'm inclined just to override the installcheck target in the module's makefile and disable it in vcregress.pl. Maybe we need to shift from installcheck to check mode for these modules. That would involve a buildfarm script change. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 11/15/2017 02:17 PM, Andrew Dunstan wrote:
>> Yeah, that was my first instinct. But then how do the test_rls_hooks
>> tests succeed? They too have a shared_preload_library.
> Aha, it does
> LOAD 'test_rls_hooks';
> at the start of its script. Of course we can't do that here.
> Not sure what the best way to resolve this is. For now I'm inclined just
> to override the installcheck target in the module's makefile and disable
> it in vcregress.pl.
Yeah.
> Maybe we need to shift from installcheck to check mode for these
> modules. That would involve a buildfarm script change.
Seems like expending an awful lot of cycles in support of a rather trivial
feature.
regards, tom lane
On 11/15/2017 04:30 PM, Tom Lane wrote: > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >> On 11/15/2017 02:17 PM, Andrew Dunstan wrote: >>> Yeah, that was my first instinct. But then how do the test_rls_hooks >>> tests succeed? They too have a shared_preload_library. >> Aha, it does >> LOAD 'test_rls_hooks'; >> at the start of its script. Of course we can't do that here. >> Not sure what the best way to resolve this is. For now I'm inclined just >> to override the installcheck target in the module's makefile and disable >> it in vcregress.pl. > Yeah. > >> Maybe we need to shift from installcheck to check mode for these >> modules. That would involve a buildfarm script change. > Seems like expending an awful lot of cycles in support of a rather trivial > feature. Possibly. At any rate I'm not going to do it immediately. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Nov 16, 2017 at 6:53 AM, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote: > On 11/15/2017 04:30 PM, Tom Lane wrote: >> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >>> On 11/15/2017 02:17 PM, Andrew Dunstan wrote: >>>> Yeah, that was my first instinct. But then how do the test_rls_hooks >>>> tests succeed? They too have a shared_preload_library. >>> Aha, it does >>> LOAD 'test_rls_hooks'; >>> at the start of its script. Of course we can't do that here. >>> Not sure what the best way to resolve this is. For now I'm inclined just >>> to override the installcheck target in the module's makefile and disable >>> it in vcregress.pl. >> Yeah. >> >>> Maybe we need to shift from installcheck to check mode for these >>> modules. That would involve a buildfarm script change. >> Seems like expending an awful lot of cycles in support of a rather trivial >> feature. > > Possibly. At any rate I'm not going to do it immediately. So why not just nuking it from the core code? The test module is basically dead code now, and has been useful during review to check that the positions of the hooks are adapted to run SQL queries at session start and end. I don't mind to include it in my own set of plugins on github instead. -- Michael
On 11/15/2017 05:07 PM, Michael Paquier wrote: > On Thu, Nov 16, 2017 at 6:53 AM, Andrew Dunstan > <andrew.dunstan@2ndquadrant.com> wrote: >> On 11/15/2017 04:30 PM, Tom Lane wrote: >>> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: >>>> On 11/15/2017 02:17 PM, Andrew Dunstan wrote: >>>>> Yeah, that was my first instinct. But then how do the test_rls_hooks >>>>> tests succeed? They too have a shared_preload_library. >>>> Aha, it does >>>> LOAD 'test_rls_hooks'; >>>> at the start of its script. Of course we can't do that here. >>>> Not sure what the best way to resolve this is. For now I'm inclined just >>>> to override the installcheck target in the module's makefile and disable >>>> it in vcregress.pl. >>> Yeah. >>> >>>> Maybe we need to shift from installcheck to check mode for these >>>> modules. That would involve a buildfarm script change. >>> Seems like expending an awful lot of cycles in support of a rather trivial >>> feature. >> Possibly. At any rate I'm not going to do it immediately. > So why not just nuking it from the core code? The test module is > basically dead code now, and has been useful during review to check > that the positions of the hooks are adapted to run SQL queries at > session start and end. I don't mind to include it in my own set of > plugins on github instead. It's not dead at all. "make check" works just fine. It would be possible to create a trivial buildfarm module to run that if we wanted to. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Greetings, * Andrew Dunstan (andrew.dunstan@2ndquadrant.com) wrote: > On 11/15/2017 04:30 PM, Tom Lane wrote: > > Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > >> Maybe we need to shift from installcheck to check mode for these > >> modules. That would involve a buildfarm script change. > > Seems like expending an awful lot of cycles in support of a rather trivial > > feature. > > Possibly. At any rate I'm not going to do it immediately. This is something that's bitten us a few times with various test modules, so I don't think that effort would be wasted. Thanks! Stephen