Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of non-deterministic filters in SelectiveStreamReaders #13278

Merged
merged 1 commit into from
Aug 28, 2019

Conversation

bhhari
Copy link
Contributor

@bhhari bhhari commented Aug 23, 2019

No description provided.

@bhhari bhhari requested a review from mbasmanova August 23, 2019 04:54
@mbasmanova mbasmanova requested a review from a team August 23, 2019 14:13
@mbasmanova mbasmanova self-assigned this Aug 23, 2019
@mbasmanova mbasmanova added the aria Presto Aria performance improvements label Aug 23, 2019
@mbasmanova
Copy link
Contributor

Would you add a test that fails without this change? You can make an array of timestamps from shipdate, commitdate and receiptdate: array[cast(shipdate as timestamp), cast(commitdate as timestamp), cast(receiptdate as timestamp)] as timestamps.

@bhhari bhhari force-pushed the timestamp_selective_reader branch 2 times, most recently from 47df2de to 6accbb7 Compare August 24, 2019 01:39
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@bhhari bhhari force-pushed the timestamp_selective_reader branch from 6accbb7 to 5c1187b Compare August 27, 2019 03:29
@bhhari bhhari force-pushed the timestamp_selective_reader branch from 5c1187b to ad15392 Compare August 27, 2019 03:32
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhhari Looks good % one question below

@bhhari bhhari force-pushed the timestamp_selective_reader branch 2 times, most recently from fc9292a to 7eaf05a Compare August 28, 2019 03:35
@bhhari bhhari force-pushed the timestamp_selective_reader branch from 7eaf05a to 0f23f31 Compare August 28, 2019 06:43
@bhhari bhhari changed the title Add non-deterministic filter support for TimestampSelectiveStreamReader Fix non-deterministic filter usecase for SelectiveStreamReaders Aug 28, 2019
@mbasmanova mbasmanova changed the title Fix non-deterministic filter usecase for SelectiveStreamReaders Fix handling of non-deterministic filters in SelectiveStreamReaders Aug 28, 2019
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhhari Looks good to me. Thanks for all these fixes.

I'd update commit message to Fix handling of non-deterministic filters in SelectiveStreamReaders for clarity.

@bhhari bhhari force-pushed the timestamp_selective_reader branch from 0f23f31 to 99b722a Compare August 28, 2019 16:39
@mbasmanova mbasmanova merged commit 0ea8208 into prestodb:master Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aria Presto Aria performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants