Hi Zhihong, On Sun, Apr 3, 2022 at 11:38 PM Zhihong Yu <z...@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
