On Thu, Dec 12, 2019 at 9:45 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Wed, Dec 11, 2019 at 5:22 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Dec 9, 2019 at 1:27 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > > I have review the patch set and here are few comments/questions > > > > > > 1. > > > +static void > > > +pg_decode_stream_change(LogicalDecodingContext *ctx, > > > + ReorderBufferTXN *txn, > > > + Relation relation, > > > + ReorderBufferChange *change) > > > +{ > > > + OutputPluginPrepareWrite(ctx, true); > > > + appendStringInfo(ctx->out, "streaming change for TXN %u", txn->xid); > > > + OutputPluginWrite(ctx, true); > > > +} > > > > > > Should we show the tuple in the streamed change like we do for the > > > pg_decode_change? > > > > > > > I think so. The patch shows the message in > > pg_decode_stream_message(), so why to prohibit showing tuple here? > > > > > 2. pg_logical_slot_get_changes_guts > > > It recreate the decoding slot [ctx = > > > CreateDecodingContext(InvalidXLogRecPtr] but doesn't set the streaming > > > to false, should we pass a parameter to > > > pg_logical_slot_get_changes_guts saying whether we want streamed results > > > or not > > > > > > > CreateDecodingContext internally calls StartupDecodingContext which > > sets the value of streaming based on if the plugin has provided > > callbacks for streaming functions. Isn't that sufficient? Why do we > > need additional parameters here? > > I don't think that if plugin provides streaming function then we > should stream. Like pgoutput plugin provides streaming function but > we only stream if streaming is on in create subscription command. So > I feel that should be true with any plugin. >
How about adding a new boolean parameter (streaming) in pg_create_logical_replication_slot()? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com