Обсуждение: naming of async_mode parameter

Поиск
Список
Период
Сортировка

naming of async_mode parameter

От
Zhihong Yu
Дата:
Hi, Etsuro-san:
I was looking at
  Fix EXPLAIN ANALYZE for async-capable nodes.

which adds the following parameter / field:

+   bool        async_mode;     /* true if node is in async mode */

async_mode implies an enum: {sync, async}
Since there are only two values, the data type is bool. I think it should be named is_async.

What do you think?

Cheers

Re: naming of async_mode parameter

От
Etsuro Fujita
Дата:
Hi,

On Thu, May 13, 2021 at 2:23 AM Zhihong Yu <zyu@yugabyte.com> wrote:
> I was looking at
>   Fix EXPLAIN ANALYZE for async-capable nodes.

Thanks for that!

> which adds the following parameter / field:
>
> +   bool        async_mode;     /* true if node is in async mode */
>
> async_mode implies an enum: {sync, async}
> Since there are only two values, the data type is bool. I think it should be named is_async.

By async_mode, I mean "is in async mode?", as commented above.  I
thought the naming is_in_async_mode would be a bit long, so I
shortened it to async_mode.  IIUC, I think another example in our
codebase would be the hash_spill_mode parameter in the AggState
struct.  So I think async_mode would be acceptable IMO.

Best regards,
Etsuro Fujita



Re: naming of async_mode parameter

От
Zhihong Yu
Дата:


On Fri, May 14, 2021 at 1:05 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
Hi,

On Thu, May 13, 2021 at 2:23 AM Zhihong Yu <zyu@yugabyte.com> wrote:
> I was looking at
>   Fix EXPLAIN ANALYZE for async-capable nodes.

Thanks for that!

> which adds the following parameter / field:
>
> +   bool        async_mode;     /* true if node is in async mode */
>
> async_mode implies an enum: {sync, async}
> Since there are only two values, the data type is bool. I think it should be named is_async.

By async_mode, I mean "is in async mode?", as commented above.  I
thought the naming is_in_async_mode would be a bit long, so I
shortened it to async_mode.  IIUC, I think another example in our
codebase would be the hash_spill_mode parameter in the AggState
struct.  So I think async_mode would be acceptable IMO.

Best regards,
Etsuro Fujita

Hi,
Searching postgres codebase reveals the following (partial) examples:

bool is_varlena
bool is_leaf 

I think these are more intuitive.

If you think is_in_async_mode is too long, how about naming the parameter is_async ?

If you agree, I can send out a patch.

Cheers

Re: naming of async_mode parameter

От
Etsuro Fujita
Дата:
Hi,

On Fri, May 14, 2021 at 9:00 PM Zhihong Yu <zyu@yugabyte.com> wrote:
> Searching postgres codebase reveals the following (partial) examples:
>
> bool is_varlena
> bool is_leaf
>
> I think these are more intuitive.
>
> If you think is_in_async_mode is too long, how about naming the parameter is_async ?

Sorry, I don’t think we need to rename the parameter, because I think
async_mode is also a good name for it, and the naming follows that of
existing parameters in our codebase, such as hash_spill_mode and
csv_mode in the CopyFormatOptions struct.

I think this would be just a matter of preference.

Best regards,
Etsuro Fujita