Add release notes for 0.216#12194
Conversation
There was a problem hiding this comment.
what used to happen and what happens now?
There was a problem hiding this comment.
@sopel39 Karol, could you help answer Rebecca's question?
There was a problem hiding this comment.
I don't think it's super clear what "predicate pushdown" means. I think it would be clearer to say Push filters on window function partition symbols below window operators. (I assume that's what this is about)
There was a problem hiding this comment.
We shouldn't mention "partition symbols" at all since that is an internal detail of the planner. This should be explained in end-user terms.
There was a problem hiding this comment.
maybe add that SHOW STATS FOR should be used instead
There was a problem hiding this comment.
also, it looks like we put SQL syntax in code backticks. so that should be done here too (or removed at show create table. whichever is our standard practice)
There was a problem hiding this comment.
Yes please put code, SQL stuff in double backticks. There are other places that need to be updated.
There was a problem hiding this comment.
Also need a . at the end.
There was a problem hiding this comment.
clarify that it's pushed down to the connector. We sometimes use "pushed down" to mean pushed down within the plan and sometimes pushed down to the connector.
There was a problem hiding this comment.
Need a . at the end.
There was a problem hiding this comment.
set the configuration property (add "the")
There was a problem hiding this comment.
surround "run-teardown-on-result-mismatch" with double backticks like the other configuration properties
There was a problem hiding this comment.
Add ``run-teardown-on-result-mismatch`` config property. ...
There was a problem hiding this comment.
temporary tables (plural)
There was a problem hiding this comment.
"rare (but practical)" doesn't mean a whole lot to me. Get rid of those words.
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
a9f3c5c to
2762328
Compare
There was a problem hiding this comment.
We should put parquet related docs under Hive Changes.
There was a problem hiding this comment.
Put optimize_mixed_distinct_aggregation in double backticks.
There was a problem hiding this comment.
Yes please put code, SQL stuff in double backticks. There are other places that need to be updated.
There was a problem hiding this comment.
Need a . at the end.
There was a problem hiding this comment.
Also need a . at the end.
There was a problem hiding this comment.
Add support for predicate pushdown for columns with ``char(x)`` type.
There was a problem hiding this comment.
Add ``run-teardown-on-result-mismatch`` config property. ...
There was a problem hiding this comment.
Is there a doc that we can link to here for reduce_agg?
There was a problem hiding this comment.
@nezihyigitbasi @wenleix Currently, there is not. Wenlei will be adding one.
There was a problem hiding this comment.
We should add the documentation before releasing. The code should not have been merged without documentation.
There was a problem hiding this comment.
Fix the rendering of live plan view for ...
There was a problem hiding this comment.
@nezihyigitbasi This way is it not clear what was wrong? Whether the UI didn't look nice or it failed to render at all.
There was a problem hiding this comment.
The ... in the end means the rest is the same, sorry for the confusion.
Fix the rendering of live plan view for queries involving index joins (:issue:`12054 `)
There was a problem hiding this comment.
If we still want to go one level deeper in terms of detail, we can also add what exactly is broken with rendering.
There was a problem hiding this comment.
Return final results to clients immediately for failed queries.
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
2762328 to
844c200
Compare
|
@nezihyigitbasi @rschlussel Nezih, Rebecca, thank you for reviews. I made the changes you suggested. Would you take another look? |
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
844c200 to
96e2f50
Compare
rschlussel
left a comment
rschlussel
left a comment
There was a problem hiding this comment.
A couple additional comments.
nezihyigitbasi
left a comment
nezihyigitbasi
left a comment
There was a problem hiding this comment.
Adding some more comments.
There was a problem hiding this comment.
We need double backticks around count(*) as * will make the rest of the sentence italic.
``count(*)``
There was a problem hiding this comment.
What does "fix" mean? Did it fail during planning? Return an incorrect result?
There was a problem hiding this comment.
@sopel39 Karol, could you answer David's question?
There was a problem hiding this comment.
Maybe:
Do not return ``NULL`` as ``count(*)`` aggregation result when ``optimize_mixed_distinct_aggregation`` is enabled.
?
There was a problem hiding this comment.
This release note item is talking in terms of the plan tree. Can we make it more user friendly?
We should say something like Add an optimizer rule to filter the window partitions before executing the window operators. Please check with the author of this change if this statement is correct.
There was a problem hiding this comment.
@sopel39 Karol, could you review the proposed wording?
electrum
left a comment
electrum
left a comment
There was a problem hiding this comment.
In addition to comments, this needs the additional release notes that were recently added to the release notes issue
There was a problem hiding this comment.
What does "fix" mean? Did it fail during planning? Return an incorrect result?
There was a problem hiding this comment.
We should add the documentation before releasing. The code should not have been merged without documentation.
There was a problem hiding this comment.
This is an implementation detail and should not be a release note item. If there is a user visible change, such as allowing connecting to Kudu servers newer than version X, then mention that directly.
There was a problem hiding this comment.
@kokosing Grzegorz, is there any user-visible change?
There was a problem hiding this comment.
@electrum David, would you update https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines with the new guidelines?
mbasmanova
force-pushed
the
release-notes-0.216
branch
2 times, most recently
from
16c2284 to
1a3c056
Compare
|
@rschlussel @nezihyigitbasi @electrum Comments addressed. Please take another look. |
There was a problem hiding this comment.
@mbasmanova please fix the issue link as commented above.
There was a problem hiding this comment.
Add ``number_of_replicas`` table property to ...
There was a problem hiding this comment.
This is the same as another item above.
* Improve performance for :func:`array_intersect`.
There was a problem hiding this comment.
When I build the release notes this item gets rendered like below, which is not what we want.
Add support for Amazon S3 Configuration, which enables pushing down projections and predicates into S3 for text files.
To fix this we can provide a custom text for the link:
:ref:`S3 select pushdown <s3selectpushdown>`
There was a problem hiding this comment.
This needs to be removed since we are reverting the change
There was a problem hiding this comment.
remove "the" and change to "structured" to "structural"
Is this an engine change that applies to all connectors?
There was a problem hiding this comment.
CC: @yingsu00 Ying, "Is this an engine change that applies to all connectors?"
There was a problem hiding this comment.
@mbasmanova These are array function changes in the engine, and it should apply to all connectors.
There was a problem hiding this comment.
Is this an engine change that applies to all connectors?
I think this could be mine. Originally it was: Improve performance of table scan on structured types by removing unnecessary memory accounting.
Yes. this applies to all connectors
There was a problem hiding this comment.
Link to the function and move above to be with the other optimizations
There was a problem hiding this comment.
Change SHOW STATS here to be a link so that it is consistent with the one above. Or change the one above to be text.
There was a problem hiding this comment.
This needs to be removed as we are reverting this change
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
1a3c056 to
c9e2940
Compare
|
@nezihyigitbasi @electrum Nezih, David, thank you for reviewing. Comments addressed. |
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
c9e2940 to
30aaa39
Compare
mbasmanova
force-pushed
the
release-notes-0.216
branch
from
30aaa39 to
902c429
Compare
|
@nezihyigitbasi Thank you, Nezih. Addressed all comments. |
7 participants
#12081