You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/20 18:53:02 UTC
[GitHub] [pinot] klsince opened a new pull request, #8571: get range index conditionally as before
klsince opened a new pull request, #8571:
URL: https://github.com/apache/pinot/pull/8571
This PR reverts a few recent changes to get range index conditionally as before, in particular, sorted single-value column doesn't need to get range index
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a diff in pull request #8571: get range index conditionally as before
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8571:
URL: https://github.com/apache/pinot/pull/8571#discussion_r854462632
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java:
##########
@@ -123,13 +123,6 @@ public PhysicalColumnIndexContainer(SegmentDirectory.Reader segmentReader, Colum
_bloomFilter = null;
}
- if (loadRangeIndex) {
Review Comment:
Can’t you just add a !isSorted check here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #8571: range index shouldn’t be loaded when the column is sorted
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #8571:
URL: https://github.com/apache/pinot/pull/8571
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8571: get range index conditionally as before
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #8571:
URL: https://github.com/apache/pinot/pull/8571#issuecomment-1104352652
> Suggest changing line 80 to `boolean loadRangeIndex = !metadata.isSorted() && indexLoadingConfig.getRangeIndexColumns().contains(columnName);`
loadRangeIndex already had the second check baked into it. The only thing which needs addressing here is whether the column is sorted, so this should be a one line change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8571: get range index conditionally as before
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #8571:
URL: https://github.com/apache/pinot/pull/8571#issuecomment-1104359338
I’d suggest changing the PR title and description - the range index _is_ loaded conditionally, it just shouldn’t be loaded when the column is sorted. _Before_ the range index just wasn’t loaded when there wasn’t a dictionary, regardless of configuration. This PR fixes a bug where an attempt is made to load a range index for sorted columns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org