Thanks. On Wed, Dec 23, 2015 at 12:24 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Mon, Dec 21, 2015 at 6:34 AM, Ashutosh Bapat > <ashutosh.ba...@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. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company