asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann" <>
Subject Re: index-only plans
Date Thu, 11 Aug 2016 22:08:29 GMT
Hi Taewoo,

I think that pulling the function rename out would be a great step 
Please do this.


P.S. Please also look at my other comments. E.g. there are whitespace 
formatting changes that increase the number of changes files and lines 
each reviewer needs to look at. While these don’t change the behavior, 
reviewers (I am one of those) see those multiple times as they go back 
forth though the change and need to convince themselves multiple times 
they are benign :)

On 11 Aug 2016, at 14:46, Taewoo Kim wrote:

> Thanks Till for reviewing this giant patch set.
> At this moment, what I can try to do is removing all necessary test 
> cases
> and changes that are related to full-text search preparation (changing 
> the
> function name of "contains" to "string-contains") since I thought this
> index-only plan branch could be merged first.
> I tried to separate logical LIMIT push-down to the index search and
> index-only plan. But, it turns out that it was hard. Other than this, 
> all
> changes are related to index-only plan part (most of them are 
> accessMethod
> related.) In addition, Young-Seok already had one round.
> Best,
> Taewoo
> On Thu, Aug 11, 2016 at 2:43 PM, Till Westmann <> 
> wrote:
>> Hi,
>> we still have the big change on index-only plans outstanding. I think 
>> that
>> it would be good to have that feature. However, at it’s current 
>> size (+45K
>> lines, -15K lines) it is very (!) difficult to review. So I think 
>> that one
>> approach to get there would be to break it down into smaller more
>> achievable
>> steps.
>> I’ve added a few comments to the review with thoughts I had to do 
>> that.
>> What do you think?
>> Is that a good approach? Is it feasible?
>> Are there other ways?
>> Thanks for your thoughts,
>> Till

View raw message