On Mon, Jul 4, 2022 at 12:59 AM vignesh C <vignes...@gmail.com> wrote: ... > > 2. > > /* ALTER SUBSCRIPTION <name> SET ( */ > > else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && > > TailMatches("SET", "(")) > > - COMPLETE_WITH("binary", "slot_name", "streaming", > > "synchronous_commit", "disable_on_error"); > > + COMPLETE_WITH("binary", "origin", "slot_name", "streaming", > > "synchronous_commit", "disable_on_error"); > > /* ALTER SUBSCRIPTION <name> SKIP ( */ > > else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && > > TailMatches("SKIP", "(")) > > COMPLETE_WITH("lsn"); > > @@ -3152,7 +3152,7 @@ psql_completion(const char *text, int start, int end) > > /* Complete "CREATE SUBSCRIPTION <name> ... WITH ( <opt>" */ > > else if (HeadMatches("CREATE", "SUBSCRIPTION") && TailMatches("WITH", > > "(")) > > COMPLETE_WITH("binary", "connect", "copy_data", "create_slot", > > - "enabled", "slot_name", "streaming", > > + "enabled", "origin", "slot_name", "streaming", > > "synchronous_commit", "two_phase", "disable_on_error"); > > > > Why do you choose to add a new option in-between other parameters > > instead of at the end which we normally do? The one possible reason I > > can think of is that all the parameters at the end are boolean so you > > want to add this before those but then why before slot_name, and again > > I don't see such a rule being followed for other parameters. > > I was not sure if it should be maintained in alphabetical order, > anyway since the last option "disable_on_error" is at the end, I have > changed it to the end. >
Although it seems it is not a hard rule, mostly the COMPLETE_WITH are coded using alphabetical order. Anyway, I think that was a clear intention here too since 13 of 14 parameters were already in alphabetical order; it is actually only that "disable_on_error" parameter that was misplaced; not the new "origin" parameter. Also, in practice, on <tab> those completions will get output in alphabetical order, so IMO it makes more sense for the code to be consistent with the output: e.g. test_sub=# create subscription sub xxx connection '' publication pub WITH ( BINARY DISABLE_ON_ERROR STREAMING CONNECT ENABLED SYNCHRONOUS_COMMIT COPY_DATA ORIGIN TWO_PHASE CREATE_SLOT SLOT_NAME test_sub=# ------ Kind Regards, Peter Smith. Fujitsu Australia