Skip to content

Keep sum stat for string when min/max stat is too long in ORC writer#13879

Merged
wenleix merged 1 commit intoprestodb:masterfrom
islamismailov:presto_fix_orc_sum_statt
Dec 26, 2019
Merged

Keep sum stat for string when min/max stat is too long in ORC writer#13879
wenleix merged 1 commit intoprestodb:masterfrom
islamismailov:presto_fix_orc_sum_statt

Conversation

@islamismailov
Copy link
Member

== RELEASE NOTES ==

General Changes
* Output (sum) string stat even if min/max values are too long. This is needed for the read-path to be able to better estimate the size of row.

@islamismailov
Copy link
Member Author

Might need to fully revert this diff, since we do not need to drop min/max to avoid OOMing the verifier anymore:

ea23a90

@islamismailov islamismailov force-pushed the presto_fix_orc_sum_statt branch from e456644 to 2e8272e Compare December 19, 2019 19:35
Copy link
Contributor

@wenleix wenleix left a comment

Choose a reason for hiding this comment

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

LGTM.

For the commit message, I would suggest

Keep sum stat for string when min/max stat is too long in ORC writer 

@islamismailov islamismailov force-pushed the presto_fix_orc_sum_statt branch from 2e8272e to caa5af8 Compare December 25, 2019 20:30
@islamismailov islamismailov changed the title StringStatisticsBuilder: do not drop sum if min/max stat too long Keep sum stat for string when min/max stat is too long in ORC writer Dec 25, 2019
@islamismailov
Copy link
Member Author

Only those with write access to this repository can merge pull requests.

  • Can please somebody with write access merge this whenever possible?

@highker
Copy link

highker commented Dec 26, 2019

Assign to @wenleix to merge

@wenleix wenleix merged commit 241a3f5 into prestodb:master Dec 26, 2019
@wenleix
Copy link
Contributor

wenleix commented Dec 26, 2019

Merged #13879. Thanks @islamismailov for the contribution!

@aweisberg aweisberg mentioned this pull request Jan 17, 2020
7 tasks
@caithagoras caithagoras mentioned this pull request Jan 22, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants