Re: [HACKERS] If subscription to foreign table valid ?

[Peter Eisentraut]

On 5/14/17 23:55, Neha Khatri wrote:
> With this patch the error will be like this:
> 
>   logical replication target relation public.t is not a table
> 
> But it is possible that the referred table is Foreign Table of
> Partitioned table (so actually the referred object is indeed a table).
> Would it make sense to specify in the message that the table is not a
> normal table or something in that line? 

This is how we phrase the message in other places where we check the
relkind.  We should keep that consistent.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Peter Eisentraut]

On 5/12/17 09:58, Petr Jelinek wrote:
> So I moved the relkind check to single function and call it from all the
> necessary places. See the attached

committed

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Neha Khatri]

On Fri, May 12, 2017 at 11:58 PM, Petr Jelinek  wrote:

> On 11/05/17 15:43, Petr Jelinek wrote:
> > Hi,
>
> >
> > We do check for this, but only during replication which we have to do
> > because the fact that relation 't' was foreign table during ALTER
> > SUBSCRIPTION does not mean that it won't be something else half hour
> later.
> >
> > I think it does make sense to add check for this into CREATE/ALTER
> > SUBSCRIBER though so that user is informed immediately about the mistake
> > rather than by errors in the logs later.
> >
> > I'll look into writing patch for this. I don't think it's beta blocker
> > though.
> >
>
> So I moved the relkind check to single function and call it from all the
> necessary places. See the attached
>
>
With this patch the error will be like this:

  logical replication target relation public.t is not a table

But it is possible that the referred table is Foreign Table of Partitioned
table (so actually the referred object is indeed a table).
Would it make sense to specify in the message that the table is not a
normal table or something in that line?

Regards,
Neha

Re: [HACKERS] If subscription to foreign table valid ?

[Noah Misch]

On Thu, May 11, 2017 at 05:55:02PM +0530, tushar wrote:
> I observed that -we cannot publish "foreign table" in Publication
> 
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't1');
> CREATE FOREIGN TABLE
> 
> postgres=# create publication pub for table t;
> ERROR:  "t" is not a table
> DETAIL:  Only tables can be added to publications.
> postgres=#
> 
> but same thing is not true for Subscription
> 
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't');
> CREATE FOREIGN TABLE
> postgres=# alter subscription sub refresh publication ;
> NOTICE:  added subscription for table public.t
> ALTER SUBSCRIPTION
> 
> Is this an expected behavior ?   if we cannot publish then how  can we  add
> subscription for it.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Neha Khatri]

>
>
>
> I sent my version of patch in parallel. I think we don't need to do the
> relation open like you did, all the info is in syscache.
>

That's right.

>
Regards,
Neha

Re: [HACKERS] If subscription to foreign table valid ?

[Petr Jelinek]

On 12/05/17 15:55, Neha Khatri wrote:
> On 5/11/17, Petr Jelinek  wrote:
>> Hi,
>>
>> On 11/05/17 14:25, tushar wrote:
>>> Hi,
>>>
>>> I observed that -we cannot publish "foreign table" in Publication
>>>
>>> but same thing is not true for Subscription
>>>
>>> postgres=# create foreign table t (n int) server db1_server options
>>> (table_name 't');
>>> CREATE FOREIGN TABLE
>>> postgres=# alter subscription sub refresh publication ;
>>> NOTICE:  added subscription for table public.t
>>> ALTER SUBSCRIPTION
>>>
>>> Is this an expected behavior ?   if we cannot publish then how  can we
>>> add subscription for it.
> 
> The above foreign table subscription succeeds only if the publisher
> has published a same named normal table i.e.
>   create table t (n int);
>   CREATE PUBLICATION mypub FOR TABLE t;
> 
> I think in current implemetation of LogicalRep. it is users
> responsibility to match the table definition at publisher and
> subscriber. Subscriber can not determine by itself what publisher has
> defined. This usecase does not align with this assumption.
> 
> 
>> However, the foreign tables indeed can't be subscribed.
> 
> I suspect that a user would want to subcribe a foreign table in real world.
> 
>> I think it does make sense to add check for this into CREATE/ALTER
>> SUBSCRIBER though so that user is informed immediately about the mistake
>> rather than by errors in the logs later.
> 
> Yes, system should prohibit such operation though.
> I tried to write to a patch to achieve this. It disalows subscription
> of relation other than RELKIND_RELATION.
> Attached is the patch. Comments?
> 

I sent my version of patch in parallel. I think we don't need to do the
relation open like you did, all the info is in syscache.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Petr Jelinek]

On 11/05/17 15:43, Petr Jelinek wrote:
> Hi,
> 
> On 11/05/17 14:25, tushar wrote:
>> Hi,
>>
>> I observed that -we cannot publish "foreign table" in Publication
>>
>> postgres=# create foreign table t (n int) server db1_server options
>> (table_name 't1');
>> CREATE FOREIGN TABLE
>>
>> postgres=# create publication pub for table t;
>> ERROR:  "t" is not a table
>> DETAIL:  Only tables can be added to publications.
>> postgres=#
>>
>> but same thing is not true for Subscription
>>
>> postgres=# create foreign table t (n int) server db1_server options
>> (table_name 't');
>> CREATE FOREIGN TABLE
>> postgres=# alter subscription sub refresh publication ;
>> NOTICE:  added subscription for table public.t
>> ALTER SUBSCRIPTION
>>
>> Is this an expected behavior ?   if we cannot publish then how  can we 
>> add subscription for it.
>>
> 
> Thanks for report. What you can publish and what you can subscribe is
> not necessarily same (we can write to relations which we can't capture
> from wal, for example unlogged table can't be published but can be
> subscribed).
> 
> However, the foreign tables indeed can't be subscribed. I originally
> planned to have foreign tables allowed on subscriber but it turned out
> to be more complex to implement than I had anticipated do I ripped the
> code for that from the original patch.
> 
> We do check for this, but only during replication which we have to do
> because the fact that relation 't' was foreign table during ALTER
> SUBSCRIPTION does not mean that it won't be something else half hour later.
> 
> I think it does make sense to add check for this into CREATE/ALTER
> SUBSCRIBER though so that user is informed immediately about the mistake
> rather than by errors in the logs later.
> 
> I'll look into writing patch for this. I don't think it's beta blocker
> though.
> 

So I moved the relkind check to single function and call it from all the
necessary places. See the attached

I now wonder if we should do some other checks as well (columns etc).

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


Check-relkind-of-tables-in-CREATE-ALTER-SUBSCRIPTION.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Neha Khatri]

[Correction below]

On 5/12/17, Neha Khatri  wrote:
> On 5/11/17, Petr Jelinek  wrote
>
>> However, the foreign tables indeed can't be subscribed.

Yes, I suspect that a user would _not_ want to subcribe a foreign
table in real world.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Neha Khatri]

On 5/11/17, Petr Jelinek  wrote:
> Hi,
>
> On 11/05/17 14:25, tushar wrote:
>> Hi,
>>
>> I observed that -we cannot publish "foreign table" in Publication
>>
>> but same thing is not true for Subscription
>>
>> postgres=# create foreign table t (n int) server db1_server options
>> (table_name 't');
>> CREATE FOREIGN TABLE
>> postgres=# alter subscription sub refresh publication ;
>> NOTICE:  added subscription for table public.t
>> ALTER SUBSCRIPTION
>>
>> Is this an expected behavior ?   if we cannot publish then how  can we
>> add subscription for it.

The above foreign table subscription succeeds only if the publisher
has published a same named normal table i.e.
  create table t (n int);
  CREATE PUBLICATION mypub FOR TABLE t;

I think in current implemetation of LogicalRep. it is users
responsibility to match the table definition at publisher and
subscriber. Subscriber can not determine by itself what publisher has
defined. This usecase does not align with this assumption.


> However, the foreign tables indeed can't be subscribed.

I suspect that a user would want to subcribe a foreign table in real world.

> I think it does make sense to add check for this into CREATE/ALTER
> SUBSCRIBER though so that user is informed immediately about the mistake
> rather than by errors in the logs later.

Yes, system should prohibit such operation though.
I tried to write to a patch to achieve this. It disalows subscription
of relation other than RELKIND_RELATION.
Attached is the patch. Comments?

Regards,
Neha
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b76cdc5..f6013da 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -440,9 +440,20 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
 			{
 RangeVar   *rv = (RangeVar *) lfirst(lc);
 Oid			relid;
+Relation		relation;
 
 relid = RangeVarGetRelid(rv, AccessShareLock, false);
 
+relation = RelationIdGetRelation(relid);
+
+if (RelationGetForm(relation)->relkind != RELKIND_RELATION)
+	ereport(ERROR,
+			(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			 errmsg("\"%s\" is not a normal table",
+	RelationGetRelationName(relation)),
+			 errdetail("Only normal tables can be subscribed.")));
+RelationClose(relation);
+
 SetSubscriptionRelState(subid, relid, table_state,
 		InvalidXLogRecPtr);
 			}
@@ -553,8 +564,21 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data)
 	{
 		RangeVar   *rv = (RangeVar *) lfirst(lc);
 		Oid			relid;
+		Relation		relation;
 
 		relid = RangeVarGetRelid(rv, AccessShareLock, false);
+
+		relation = RelationIdGetRelation(relid);
+
+		if (RelationGetForm(relation)->relkind != RELKIND_RELATION)
+			ereport(NOTICE,
+	(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+	 errmsg("\"%s\" is not a normal table",
+		RelationGetRelationName(relation)),
+	 errdetail("Only normal tables can be subscribed.")));
+
+		RelationClose(relation);
+
 		pubrel_local_oids[off++] = relid;
 
 		if (!bsearch(, subrel_local_oids,

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[tushar]

On 05/11/2017 07:13 PM, Petr Jelinek wrote:

I think it does make sense to add check for this into CREATE/ALTER
SUBSCRIBER though so that user is informed immediately about the mistake
rather than by errors in the logs later.
+1 , there are  few similar cases   - where user does not  get error at 
prompt , for instance

--when publication doesn't not exist
postgres=# create subscription sub connection 'dbname=postgres port=5000 
user=centos password=a' publication nowhere;

NOTICE:  synchronized table states
NOTICE:  created replication slot "sub" on publisher
CREATE SUBSCRIPTION
--No check validation for Publication name in ALTER
postgres=# alter subscription sub set publication _ refresh;
ALTER SUBSCRIPTION
--slot name given in ALTER
postgres=# alter subscription sub with ( slot name='nowhere');
ALTER SUBSCRIPTION
--and due to that , we are not able to drop it later.
postgres=# drop subscription sub;
ERROR:  could not drop the replication slot "nowhere" on publisher
DETAIL:  The error was: ERROR:  replication slot "nowhere" does not exist
postgres=#

--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Petr Jelinek]

Hi,

On 11/05/17 14:25, tushar wrote:
> Hi,
> 
> I observed that -we cannot publish "foreign table" in Publication
> 
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't1');
> CREATE FOREIGN TABLE
> 
> postgres=# create publication pub for table t;
> ERROR:  "t" is not a table
> DETAIL:  Only tables can be added to publications.
> postgres=#
> 
> but same thing is not true for Subscription
> 
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't');
> CREATE FOREIGN TABLE
> postgres=# alter subscription sub refresh publication ;
> NOTICE:  added subscription for table public.t
> ALTER SUBSCRIPTION
> 
> Is this an expected behavior ?   if we cannot publish then how  can we 
> add subscription for it.
> 

Thanks for report. What you can publish and what you can subscribe is
not necessarily same (we can write to relations which we can't capture
from wal, for example unlogged table can't be published but can be
subscribed).

However, the foreign tables indeed can't be subscribed. I originally
planned to have foreign tables allowed on subscriber but it turned out
to be more complex to implement than I had anticipated do I ripped the
code for that from the original patch.

We do check for this, but only during replication which we have to do
because the fact that relation 't' was foreign table during ALTER
SUBSCRIPTION does not mean that it won't be something else half hour later.

I think it does make sense to add check for this into CREATE/ALTER
SUBSCRIBER though so that user is informed immediately about the mistake
rather than by errors in the logs later.

I'll look into writing patch for this. I don't think it's beta blocker
though.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] If subscription to foreign table valid ?

[Robert Haas]

On Thu, May 11, 2017 at 8:25 AM, tushar  wrote:
> I observed that -we cannot publish "foreign table" in Publication
>
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't1');
> CREATE FOREIGN TABLE
>
> postgres=# create publication pub for table t;
> ERROR:  "t" is not a table
> DETAIL:  Only tables can be added to publications.
> postgres=#
>
> but same thing is not true for Subscription
>
> postgres=# create foreign table t (n int) server db1_server options
> (table_name 't');
> CREATE FOREIGN TABLE
> postgres=# alter subscription sub refresh publication ;
> NOTICE:  added subscription for table public.t
> ALTER SUBSCRIPTION
>
> Is this an expected behavior ?   if we cannot publish then how  can we  add
> subscription for it.

This is not a complete test case, but it does appear to be odd behavior.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] If subscription to foreign table valid ?

[tushar]

Hi,

I observed that -we cannot publish "foreign table" in Publication

postgres=# create foreign table t (n int) server db1_server options 
(table_name 't1');

CREATE FOREIGN TABLE

postgres=# create publication pub for table t;
ERROR:  "t" is not a table
DETAIL:  Only tables can be added to publications.
postgres=#

but same thing is not true for Subscription

postgres=# create foreign table t (n int) server db1_server options 
(table_name 't');

CREATE FOREIGN TABLE
postgres=# alter subscription sub refresh publication ;
NOTICE:  added subscription for table public.t
ALTER SUBSCRIPTION

Is this an expected behavior ?   if we cannot publish then how  can we  
add subscription for it.


--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers