On 2/15/25 12:14, Robins Tharakan wrote:
> Hi Tomas,
>
> Thanks for taking a look - apologies for the delay here.
>
> On Tue, 10 Dec 2024 at 09:09, Tomas Vondra <tomas@vondra.me
> <mailto:tomas@vondra.me>> wrote:
>>
>> 1) Does it make sense to still show "ALL" when the query returns
>> nothing? Not sure if we already have a way to handle this.
>>
>
> +1 - "ALL" is pointless when there is nothing to RESET, but I didn't
> find an easy way to make it conditional (which was partly the
> cause of delay here). For this, I tried (and not yet successful) to
> create a new set of macros - something along the lines of
> COMPLETE_WITH_QUERY_IFNOTEMPTY_PLUS() - where
> the second part is conditional on whether the query list returns
> a non-empty set.
>
> If this is a blocker, I'll continue working on a macro that allows that
> more easily (although it'd be great if you could point me to a better
> way to implement that). For now, reattached v1 to this email for
> convenience.
>
Thanks. I don't think it's a blocker - in fact, after thinking about it
a bit more, I believe showing the "ALL" is consistent with what we do in
other places when the query returns nothing. Consider for example CLOSE:
/* CLOSE */
else if (Matches("CLOSE"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_cursors,
"ALL");
Other places do similar stuff. It's just a noop. So I don't think you
need to waste time inventing something for this one place.
>
>> 2) Should we do the same thing for ALTER DATABASE? That also allows
>> setting options.
>>
> +1. Attached a separate patch for ALTER DATABASE RESET in case
> you think it is good to go alongside (albeit again, this still does show
> ALL even if there's no vars to RESET).
>
Thanks. These patches look fine to me. I'll get them committed.
regards
--
Tomas Vondra