Hi,

(btw, I vote against tarballing patches)

+   <tgroup cols="4">
+    <thead>
+     <row>
+      <entry>Name</entry>
+      <entry>Type</entry>
+      <entry>References</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+
+    <tbody>
+     <row>
+      <entry><structfield>oid</structfield></entry>
+      <entry><type>oid</type></entry>
+      <entry></entry>
+      <entry>Row identifier (hidden attribute; must be explicitly 
selected)</entry>
+     </row>
+

+     <row>
+      <entry><structfield>subpublications</structfield></entry>
+      <entry><type>name[]</type></entry>
+      <entry></entry>
+      <entry>Array of subscribed publication names. These reference the
+       publications on the publisher server.
+      </entry>

Why is this names and not oids? So you can see it across databases?

I think this again should have an owner.


 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/commands/event_trigger.c 
b/src/backend/commands/event_trigger.c
index 68d7e46..523008d 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -112,6 +112,7 @@ static event_trigger_support_data event_trigger_support[] = 
{
        {"SCHEMA", true},
        {"SEQUENCE", true},
        {"SERVER", true},
+       {"SUBSCRIPTION", true},

Hm, is that ok? Subscriptions are shared, so ...?


+               /*
+                * If requested, create the replication slot on remote side for 
our
+                * newly created subscription.
+                *
+                * Note, we can't cleanup slot in case of failure as reason for
+                * failure might be already existing slot of the same name and 
we
+                * don't want to drop somebody else's slot by mistake.
+                */
+               if (create_slot)
+               {
+                       XLogRecPtr                      lsn;
+
+                       /*
+                        * Create the replication slot on remote side for our 
newly created
+                        * subscription.
+                        *
+                        * Note, we can't cleanup slot in case of failure as 
reason for
+                        * failure might be already existing slot of the same 
name and we
+                        * don't want to drop somebody else's slot by mistake.
+                        */

We should really be able to recognize that based on the error code...

+/*
+ * Drop subscription by OID
+ */
+void
+DropSubscriptionById(Oid subid)
+{

+       /*
+        * We must ignore errors here as that would make it impossible to drop
+        * subscription when publisher is down.
+        */

I'm not convinced.  Leaving a slot around without a "record" of it on
the creating side isn't nice either. Maybe a FORCE flag or something?

+subscription_create_opt_item:
+                       subscription_opt_item
+                       | INITIALLY IDENT
+                               {
+                                       if (strcmp($2, "enabled") == 0)
+                                               $$ = makeDefElem("enabled",
+                                                                               
 (Node *)makeInteger(TRUE), @1);
+                                       else if (strcmp($2, "disabled") == 0)
+                                               $$ = makeDefElem("enabled",
+                                                                               
 (Node *)makeInteger(FALSE), @1);
+                                       else
+                                               ereport(ERROR,
+                                                               
(errcode(ERRCODE_SYNTAX_ERROR),
+                                                                
errmsg("unrecognized subscription option \"%s\"", $1),
+                                                                        
parser_errposition(@2)));
+                               }
+                       | IDENT
+                               {
+                                       if (strcmp($1, "create_slot") == 0)
+                                               $$ = makeDefElem("create_slot",
+                                                                               
 (Node *)makeInteger(TRUE), @1);
+                                       else if (strcmp($1, "nocreate_slot") == 
0)
+                                               $$ = makeDefElem("create_slot",
+                                                                               
 (Node *)makeInteger(FALSE), @1);
+                               }
+               ;

Hm, the IDENT case ignores $1 if it's not create_slot/nocreate_slot and
thus leaves $$ uninitialized?

I again really would like to have the error checking elsewhere.



- Andres


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

Reply via email to