Thread: Add a test for "cannot truncate foreign table"

Add a test for "cannot truncate foreign table"

From
Yugo NAGATA
Date:
Hello,

During checking regression tests of TRUNCATE on foreign
tables for other patch [1], I found that there is no test
for foreign tables that don't support TRUNCATE. 

When a foreign table has handler but doesn't support TRUNCATE,
an error "cannot truncate foreign table xxx" occurs. So, what
about adding a test this message output? We can add this test
for file_fdw because it is one of the such foreign data wrappers.

I attached a patch.

[1] https://postgr.es/m/20220527172543.0a2fdb469cf048b81c0967d3@sraoss.co.jp

-- 
Yugo NAGATA <nagata@sraoss.co.jp>

Attachment

Re: Add a test for "cannot truncate foreign table"

From
Fujii Masao
Date:

On 2022/06/30 10:48, Yugo NAGATA wrote:
> Hello,
> 
> During checking regression tests of TRUNCATE on foreign
> tables for other patch [1], I found that there is no test
> for foreign tables that don't support TRUNCATE.
> 
> When a foreign table has handler but doesn't support TRUNCATE,
> an error "cannot truncate foreign table xxx" occurs. So, what
> about adding a test this message output? We can add this test
> for file_fdw because it is one of the such foreign data wrappers.
> 
> I attached a patch.

Thanks for the patch! It looks good to me.
I changed the status of this patch to ready-for-committer,
and will commit it barring any objeciton.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Re: Add a test for "cannot truncate foreign table"

From
Tom Lane
Date:
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> On 2022/06/30 10:48, Yugo NAGATA wrote:
>> When a foreign table has handler but doesn't support TRUNCATE,
>> an error "cannot truncate foreign table xxx" occurs. So, what
>> about adding a test this message output? We can add this test
>> for file_fdw because it is one of the such foreign data wrappers.

> Thanks for the patch! It looks good to me.
> I changed the status of this patch to ready-for-committer,
> and will commit it barring any objeciton.

This seems like a fairly pointless expenditure of test cycles
to me.  Perhaps more importantly, what will you do when
somebody adds truncate support to that FDW?  I don't think
there's an inherent reason for it to be read-only.

            regards, tom lane



Re: Add a test for "cannot truncate foreign table"

From
Fujii Masao
Date:

On 2022/07/08 0:33, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> On 2022/06/30 10:48, Yugo NAGATA wrote:
>>> When a foreign table has handler but doesn't support TRUNCATE,
>>> an error "cannot truncate foreign table xxx" occurs. So, what
>>> about adding a test this message output? We can add this test
>>> for file_fdw because it is one of the such foreign data wrappers.
> 
>> Thanks for the patch! It looks good to me.
>> I changed the status of this patch to ready-for-committer,
>> and will commit it barring any objeciton.
> 
> This seems like a fairly pointless expenditure of test cycles
> to me.  Perhaps more importantly, what will you do when
> somebody adds truncate support to that FDW?

One idea is to create dummy FDW (like foreign_data.sql regression test does) not supporting TRUNCATE and use it for the
test.

BTW, file_fdw already has the similar test cases for INSERT, UPDATE and DELETE, as follows.

-- updates aren't supported
INSERT INTO agg_csv VALUES(1,2.0);
ERROR:  cannot insert into foreign table "agg_csv"
UPDATE agg_csv SET a = 1;
ERROR:  cannot update foreign table "agg_csv"
DELETE FROM agg_csv WHERE a = 100;
ERROR:  cannot delete from foreign table "agg_csv"

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Re: Add a test for "cannot truncate foreign table"

From
Kyotaro Horiguchi
Date:
At Fri, 8 Jul 2022 01:06:18 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> 
> 
> On 2022/07/08 0:33, Tom Lane wrote:
> > Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> >> On 2022/06/30 10:48, Yugo NAGATA wrote:
> >>> When a foreign table has handler but doesn't support TRUNCATE,
> >>> an error "cannot truncate foreign table xxx" occurs. So, what
> >>> about adding a test this message output? We can add this test
> >>> for file_fdw because it is one of the such foreign data wrappers.
> > 
> >> Thanks for the patch! It looks good to me.
> >> I changed the status of this patch to ready-for-committer,
> >> and will commit it barring any objeciton.
> > This seems like a fairly pointless expenditure of test cycles
> > to me.  Perhaps more importantly, what will you do when
> > somebody adds truncate support to that FDW?

As Fujii-san mentioned below, file_fdw has tests for INSERT/UPDATE and
DELETE.  If somebody added DELETE to file_fdw, the test for DELETE
rejection would be turned into a normal test of the DELETE function.
I don't see a difference between TRUNCATE and other updating commands
from this point of view.

> One idea is to create dummy FDW (like foreign_data.sql regression test
> does) not supporting TRUNCATE and use it for the test.

I think the proposed test is not that for FDW framework, but for a
specific FDW module, file_fdw.

> BTW, file_fdw already has the similar test cases for INSERT, UPDATE
> and DELETE, as follows.
> 
> -- updates aren't supported
> INSERT INTO agg_csv VALUES(1,2.0);
> ERROR:  cannot insert into foreign table "agg_csv"
> UPDATE agg_csv SET a = 1;
> ERROR:  cannot update foreign table "agg_csv"
> DELETE FROM agg_csv WHERE a = 100;
> ERROR:  cannot delete from foreign table "agg_csv"

Agreed.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Re: Add a test for "cannot truncate foreign table"

From
Yugo NAGATA
Date:
On Fri, 08 Jul 2022 09:44:10 +0900 (JST)
Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:

> At Fri, 8 Jul 2022 01:06:18 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> > 
> > 
> > On 2022/07/08 0:33, Tom Lane wrote:
> > > Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> > >> On 2022/06/30 10:48, Yugo NAGATA wrote:
> > >>> When a foreign table has handler but doesn't support TRUNCATE,
> > >>> an error "cannot truncate foreign table xxx" occurs. So, what
> > >>> about adding a test this message output? We can add this test
> > >>> for file_fdw because it is one of the such foreign data wrappers.
> > > 
> > >> Thanks for the patch! It looks good to me.
> > >> I changed the status of this patch to ready-for-committer,
> > >> and will commit it barring any objeciton.
> > > This seems like a fairly pointless expenditure of test cycles
> > > to me.  Perhaps more importantly, what will you do when
> > > somebody adds truncate support to that FDW?
> 
> As Fujii-san mentioned below, file_fdw has tests for INSERT/UPDATE and
> DELETE.  If somebody added DELETE to file_fdw, the test for DELETE
> rejection would be turned into a normal test of the DELETE function.
> I don't see a difference between TRUNCATE and other updating commands
> from this point of view.
> 
> > One idea is to create dummy FDW (like foreign_data.sql regression test
> > does) not supporting TRUNCATE and use it for the test.
> 
> I think the proposed test is not that for FDW framework, but for a
> specific FDW module, file_fdw.

Yes, the patch is an improvement for the test of flie_fdw. 

If we want to test foreign table modifications for the FDW framework, 
we will have to add such tests in foreign_data.sql, because foreign
table modifications are tested only for postgres_fdw and file_fdw. 

> > BTW, file_fdw already has the similar test cases for INSERT, UPDATE
> > and DELETE, as follows.
> > 
> > -- updates aren't supported
> > INSERT INTO agg_csv VALUES(1,2.0);
> > ERROR:  cannot insert into foreign table "agg_csv"
> > UPDATE agg_csv SET a = 1;
> > ERROR:  cannot update foreign table "agg_csv"
> > DELETE FROM agg_csv WHERE a = 100;
> > ERROR:  cannot delete from foreign table "agg_csv"
> 
> Agreed.
> 
> regards.
> 
> -- 
> Kyotaro Horiguchi
> NTT Open Source Software Center

Regards,
Yugo Nagata

-- 
Yugo NAGATA <nagata@sraoss.co.jp>



Re: Add a test for "cannot truncate foreign table"

From
Etsuro Fujita
Date:
On Fri, Jul 8, 2022 at 11:07 AM Yugo NAGATA <nagata@sraoss.co.jp> wrote:
> On Fri, 08 Jul 2022 09:44:10 +0900 (JST)
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> > At Fri, 8 Jul 2022 01:06:18 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
> > > On 2022/07/08 0:33, Tom Lane wrote:

> > > >> On 2022/06/30 10:48, Yugo NAGATA wrote:
> > > >>> When a foreign table has handler but doesn't support TRUNCATE,
> > > >>> an error "cannot truncate foreign table xxx" occurs. So, what
> > > >>> about adding a test this message output? We can add this test
> > > >>> for file_fdw because it is one of the such foreign data wrappers.

> > > > This seems like a fairly pointless expenditure of test cycles
> > > > to me.  Perhaps more importantly, what will you do when
> > > > somebody adds truncate support to that FDW?

> > As Fujii-san mentioned below, file_fdw has tests for INSERT/UPDATE and
> > DELETE.  If somebody added DELETE to file_fdw, the test for DELETE
> > rejection would be turned into a normal test of the DELETE function.
> > I don't see a difference between TRUNCATE and other updating commands
> > from this point of view.

I agree on this point.

> > > One idea is to create dummy FDW (like foreign_data.sql regression test
> > > does) not supporting TRUNCATE and use it for the test.

> If we want to test foreign table modifications for the FDW framework,
> we will have to add such tests in foreign_data.sql, because foreign
> table modifications are tested only for postgres_fdw and file_fdw.

Rather than doing so, I'd vote for adding a test case to file_fdw, as
proposed in the patch, because that would be much simpler and much
less expensive.

Best regards,
Etsuro Fujita



Re: Add a test for "cannot truncate foreign table"

From
Fujii Masao
Date:

On 2022/07/08 17:03, Etsuro Fujita wrote:
> Rather than doing so, I'd vote for adding a test case to file_fdw, as
> proposed in the patch, because that would be much simpler and much
> less expensive.

So ISTM that most agreed to push Nagata-san's patch adding the test for TRUNCATE on foreign table with file_fdw. So
barringany objection, I will commit the patch.
 

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Re: Add a test for "cannot truncate foreign table"

From
Fujii Masao
Date:

On 2022/07/15 16:52, Fujii Masao wrote:
> 
> 
> On 2022/07/08 17:03, Etsuro Fujita wrote:
>> Rather than doing so, I'd vote for adding a test case to file_fdw, as
>> proposed in the patch, because that would be much simpler and much
>> less expensive.
> 
> So ISTM that most agreed to push Nagata-san's patch adding the test for TRUNCATE on foreign table with file_fdw. So
barringany objection, I will commit the patch.
 

Pushed. Thanks!

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Re: Add a test for "cannot truncate foreign table"

From
Yugo NAGATA
Date:
On Wed, 20 Jul 2022 09:38:17 +0900
Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

> 
> 
> On 2022/07/15 16:52, Fujii Masao wrote:
> > 
> > 
> > On 2022/07/08 17:03, Etsuro Fujita wrote:
> >> Rather than doing so, I'd vote for adding a test case to file_fdw, as
> >> proposed in the patch, because that would be much simpler and much
> >> less expensive.
> > 
> > So ISTM that most agreed to push Nagata-san's patch adding the test for TRUNCATE on foreign table with file_fdw. So
barringany objection, I will commit the patch.
 
> 
> Pushed. Thanks!

Thanks!

> 
> Regards,
> 
> -- 
> Fujii Masao
> Advanced Computing Technology Center
> Research and Development Headquarters
> NTT DATA CORPORATION


-- 
Yugo NAGATA <nagata@sraoss.co.jp>