Should we replace the checks for access method OID with handler OID?
Hi All,
While reviewing the patch for pg_surgery contrib module - [1]/messages/by-id/1D56CEFD-E195-4E6B-B870-3383E3E8C65E@vmware.com, Asim
Praveen suggested that it would be better to replace the check for
access method OID with handler OID. Otherwise, if someone creates a
new AM using the AM handler that is originally supported for e.g.
"heap_tableam_handler" and if this new AM is used to create a table,
then one cannot perform surgery on such tables because we have checks
for access method OID which would reject this new AM as we only allow
heap AM. For e.g. if we do this:
create access method myam type table handler heap_tableam_handler;
create table mytable (…) using myam;
And use an access method OID check, we won't be able to perform
surgery on mytable created above because it isn't the heap table
although its table structure is actually heap.
This problem won't be there if the check for access method OID is
replaced with handler OID. I liked this suggestion from Asim and did
the changes accordingly. However, while browsing the code for other
contrib modules, I could find such checks present in some of the
contrib modules like pgstattuple, pageinspect and pgrowlocks as well.
So, just wondering if we should be doing similar changes in these
contrib modules also.
Thoughts?
[1]: /messages/by-id/1D56CEFD-E195-4E6B-B870-3383E3E8C65E@vmware.com
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
On Thu, Aug 27, 2020 at 5:37 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
While reviewing the patch for pg_surgery contrib module - [1], Asim
Praveen suggested that it would be better to replace the check for
access method OID with handler OID. Otherwise, if someone creates a
new AM using the AM handler that is originally supported for e.g.
"heap_tableam_handler" and if this new AM is used to create a table,
then one cannot perform surgery on such tables because we have checks
for access method OID which would reject this new AM as we only allow
heap AM. For e.g. if we do this:create access method myam type table handler heap_tableam_handler;
create table mytable (…) using myam;And use an access method OID check, we won't be able to perform
surgery on mytable created above because it isn't the heap table
although its table structure is actually heap.
The only reason I can see why it would make sense to do this sort of
thing is if you wanted to create a new AM for testing purposes which
behaves like some existing AM but is technically a different AM. And
if you did that, then I guess the change you are proposing would make
it behave more like it's the same thing after all, which seems like it
might be missing the point.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Aug 27, 2020 at 9:21 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Aug 27, 2020 at 5:37 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
While reviewing the patch for pg_surgery contrib module - [1], Asim
Praveen suggested that it would be better to replace the check for
access method OID with handler OID. Otherwise, if someone creates a
new AM using the AM handler that is originally supported for e.g.
"heap_tableam_handler" and if this new AM is used to create a table,
then one cannot perform surgery on such tables because we have checks
for access method OID which would reject this new AM as we only allow
heap AM. For e.g. if we do this:create access method myam type table handler heap_tableam_handler;
create table mytable (…) using myam;And use an access method OID check, we won't be able to perform
surgery on mytable created above because it isn't the heap table
although its table structure is actually heap.The only reason I can see why it would make sense to do this sort of
thing is if you wanted to create a new AM for testing purposes which
behaves like some existing AM but is technically a different AM. And
if you did that, then I guess the change you are proposing would make
it behave more like it's the same thing after all, which seems like it
might be missing the point.
Okay, understood.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com