Hi Zhihong,
On Sun, Apr 3, 2022 at 11:38 PM Zhihong Yu <zyu@yugabyte.com> wrote:
> + WRITE_ENUM_FIELD(status, SubqueryScanStatus);
>
> Looks like the new field can be named subquerystatus - this way its purpose is clearer.
I agree that “status” is too general. “subquerystatus” might be good,
but I’d like to propose “scanstatus” instead, because I think this
would be consistent with the naming of the RowMarkType-enum member
“markType” in PlanRowMark defined in the same file.
> + * mark_async_capable_plan
> + * Check whether a given Path node is async-capable, and if so, mark the
> + * Plan node created from it as such.
>
> Please add comment explaining what the return value means.
Ok, how about something like this?
“Check whether a given Path node is async-capable, and if so, mark the
Plan node created from it as such and return true; otherwise, return
false.”
> + if (!IsA(plan, Result) &&
> + mark_async_capable_plan(plan,
> + ((ProjectionPath *) path)->subpath))
> + return true;
>
> by returning true, `plan->async_capable = true;` is skipped.
> Is that intentional ?
That is intentional; we don’t need to set the async_capable flag
because in that case the flag would already have been set by the above
mark_async_capable_plan(). Note that we pass “plan” to that function.
Thanks for reviewing!
Best regards,
Etsuro Fujita
