On Mon, Dec 21, 2015 at 6:34 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: >> I went over this patch in some detail today and did a lot of cosmetic >> cleanup. The results are attached. I'm fairly happy with this >> version, but let me know what you think. Of course, feedback from >> others is more than welcome also. >> > > Attached patch with some cosmetic changes (listed here for your quick > reference) > 1. , was replaced with ; in comment "inner join, expressions in the " at one > place, which is correct, but missed other place. > 2. The comment "First, consider whether any each active EC is potentially" > should use either "any" or "each". I have reworded it as "First, consider > whether any of the active ECs is potentially ...". Or we can use "First, > find all of the active ECs which are potentially ....". > 3. "having the remote side due the sort generally won't be any worse ..." - > instead of "due" we should use "do"? > 4. Added static prototype of function get_useful_ecs_for_relation(). > 5. The comment "Extract unique EC for query, if any, so we don't consider it > again." is too crisp. Phrase "Unique EC for query" is confusing; EC can not > be associated with a query per say and EC's are always unique because of > canonicalisation. May be we should reword it as "Extract single EC for > ordering of query, if any, so we don't consider it again." Is that cryptic > as well?
Thanks. I committed this version with one small tweak.
