Re: On login trigger: take three

Hi Ted,

> bq. in to the system
> 'in to' -> into
> bq. Any bugs in a trigger procedure
> Any bugs -> Any bug

Thanks! Fixed typos in the attached v35.

> + if (event == EVT_Login)
> + dbgtag = CMDTAG_LOGIN;
> + else
> + dbgtag = CreateCommandTag(parsetree);
> The same snippet appears more than once. It seems CMDTAG_LOGIN handling
can be moved into `CreateCommandTag`.

It appears twice in fact, both cases are nearby: in the main code and under
the assert-checking ifdef.
Moving CMDTAG_LOGIN to CreateCommandTag would change the CreateCommandTag
function signature and ideology. CreateCommandTag finds a tag based on the
command parse tree, while login event is a specific case when we do not
have any command and the parse tree is NULL. Changing CreateCommandTag
signature for these two calls looks a little bit overkill as it would lead
to changing lots of other places the function is called from.
We could move this snippet to a separate function, but here are the similar
concerns I think: it would make sense for a more common or a more complex
snippet, but not for two cases of if-else one-liners.

--
best regards,
Mikhail A. Gribkov

e-mail: youzhick(at)gmail(dot)com
*http://www.flickr.com/photos/youzhick/albums
<http://www.flickr.com/photos/youzhick/albums>*
http://www.strava.com/athletes/5085772
phone: +7(916)604-71-12
Telegram: @youzhick

On Sat, Dec 17, 2022 at 3:29 PM Ted Yu <yuzhihong(at)gmail(dot)com> wrote:

>
>
> On Sat, Dec 17, 2022 at 3:46 AM Mikhail Gribkov <youzhick(at)gmail(dot)com>
> wrote:
>
>> Hi Nikita,
>>
>> > Mikhail, I've checked the patch and previous discussion,
>> > the condition mentioned earlier is still actual:
>>
>> Thanks for pointing this out! My bad, I forgot to fix the documentation
>> example.
>> Attached v34 has this issue fixed, as well as a couple other problems
>> with the example code.
>>
>> --
>> best regards,
>> Mikhail A. Gribkov
>>
> Hi,
>
> bq. in to the system
>
> 'in to' -> into
>
> bq. Any bugs in a trigger procedure
>
> Any bugs -> Any bug
>
> + if (event == EVT_Login)
> + dbgtag = CMDTAG_LOGIN;
> + else
> + dbgtag = CreateCommandTag(parsetree);
>
> The same snippet appears more than once. It seems CMDTAG_LOGIN handling
> can be moved into `CreateCommandTag`.
>
> Cheers
>