Thanks for all your comments!
Thankfully it seems that this feature is regarded as not
meaningless one, I'm going to do some improvements.
On Wed, Aug 19, 2020 at 10:56 PM Michael Paquier <michael@paquier.xyz>
wrote:
> On Wed, Aug 19, 2020 at 06:12:02PM +0900, Fujii Masao wrote:
>> On 2020/08/19 17:40, torikoshia wrote:
>>> Yes, I didn't add regression tests because of the unstability of the
>>> output.
>>> I thought it would be OK since other views like pg_stat_slru and
>>> pg_shmem_allocations
>>> didn't have tests for their outputs.
>>
>> You're right.
>
> If you can make a test with something minimal and with a stable
> output, adding a test is helpful IMO, or how can you make easily sure
> that this does not get broken, particularly in the event of future
> refactorings, or even with platform-dependent behaviors?
OK. Added a regression test on sysviews.sql.
(0001-Added-a-regression-test-for-pg_backend_memory_contex.patch)
Fujii-san gave us an example, but I added more simple one considering
the simplicity of other tests on that.
On Thu, Aug 20, 2020 at 12:02 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > By the way, I was looking at the code that has been committed, and I
> > think that it is awkward to have a SQL function in mcxt.c, which is a
> > rather low-level interface. I think that this new code should be
> > moved to its own file, one suggestion for a location I have being
> > src/backend/utils/adt/mcxtfuncs.c.
>
> I agree with that,
Thanks for pointing out.
Added a patch for relocating the codes to mcxtfuncs.c.
(patches/0001-Rellocated-the-codes-for-pg_backend_memory_contexts-.patch)
> On Thu, Aug 20, 2020 at 11:09 AM Fujii Masao
> <masao.fujii@oss.nttdata.com> wrote:
> On 2020/08/20 0:01, Tom Lane wrote:
>> Given the lack of clear use-case, and the possibility (admittedly
>> not strong) that this is still somehow a security hazard, I think
>> we should revert it. If it stays, I'd like to see restrictions
>> on who can read the view.
> For example, allowing only the role with pg_monitor to see this view?
Attached a patch adding that restriction.
(0001-Restrict-the-access-to-pg_backend_memory_contexts-to.patch)
Of course, this restriction makes pg_backend_memory_contexts hard to use
when the user of the target session is not granted pg_monitor because
the
scope of this view is session local.
In this case, I imagine additional operations something like temporarily
granting pg_monitor to that user.
Thoughts?
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
