Horton

Re: New Object Access Type hooks

Andrew Dunstan <andrew@dunslane.net>

View thread View on postgr.es
From: Andrew Dunstan <andrew@dunslane.net>
To: Mark Dilger <mark.dilger@enterprisedb.com>, Joshua Brindle <joshua.brindle@crunchydata.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Jeff Davis <pgsql@j-davis.com>, Joe Conway <joe@crunchydata.com>, Stephen Frost <sfrost@snowman.net>
Date: 2022-03-18T22:04:11Z
Lists: pgsql-hackers 
On 3/18/22 11:15, Mark Dilger wrote:
>
>> On Mar 18, 2022, at 7:16 AM, Joshua Brindle <joshua.brindle@crunchydata.com> wrote:
>>
>> This is great, thank you for doing this. I didn't even realize the OAT
>> hooks had no regression tests.
>>
>> It looks good to me, I reviewed both and tested the module. I wonder
>> if the slight abuse of subid is warranted with brand new hooks going
>> in but not enough to object, I just hope this doesn't rise to the too
>> large to merge this late level.


The core code is extracted from a current CF patch, so I think in
principle it's OK.


I haven't looked at it in detail, but regarding the test code I'm not
sure why there's a .control file, since this isn't a loadable extension,
not why there's a test_oat_hooks.h file.


> The majority of the patch is regression testing code, stuff which doesn't get installed.  It's even marked as NO_INSTALLCHECK, so it won't get installed even as part of "make installcheck".  That seems safe enough to me.
>
> Not including tests of OAT seems worse, as it leaves us open to breaking the behavior without realizing we've done so.  A refactoring of the core code might cause hooks to be called in a different order, something which isn't necessarily wrong, but should not be done unknowingly.
>

Yes, and in any case we've added test code after feature freeze in the past.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Commits

  1. Update some tests in 013_crash_restart.pl.

  2. Avoid freeing objects during json aggregate finalization

  3. SQL/JSON constructors

  4. Force NO_LOCALE / UTF8 for test_oat_hooks tests

  5. Temporarily disable installcheck for test_oat_hooks module

  6. Fix new Object Access hooks test

  7. Add String object access hooks

  8. Add a test module for Object Access hooks