RegionedPageStore contexts must not unconditionally supportUnboundedF… #4261
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cpwright
commented
Aug 1, 2023
| @@ -133,11 +143,12 @@ private static long validateMask(final long mask, final String name) { | |||
| /** | |||
| * A regioned page store for use when the full set of regions and their sizes are known. | |||
| */ | |||
| abstract class Static<ATTR extends Any, INNER_ATTR extends ATTR, REGION_TYPE extends Page<INNER_ATTR>> | |||
| abstract class Static<ATTR extends Any, INNER_ATTR extends ATTR, REGION_TYPE extends ColumnRegion<INNER_ATTR>> | |||
There was a problem hiding this comment.
If the bound is Page instead of ColumnRegion, we can't interrogate the supportsUnboundedFill method.
rcaudy
added
bug
rcaudy
reviewed
Aug 2, 2023
Comment on lines
+23
to
+24
| * Do the fill contexts produced by this region support unbounded fill? | ||
| * @return true if the contexts support unbounded fill; false otherwise |
There was a problem hiding this comment.
Suggested change
| * Do the fill contexts produced by this region support unbounded fill? | |
| * @return true if the contexts support unbounded fill; false otherwise | |
| * Do the {@link FillContext fill contexts} produced by this region support unbounded fill? | |
| * | |
| * @return {@code true} if the contexts support unbounded fill; {@code false} otherwise |
| private Context innerContext; | ||
|
|
||
| public RegionContextHolder(final int chunkCapacity, @Nullable final SharedContext sharedContext) { | ||
|
|
||
| public RegionContextHolder(final int chunkCapacity, @Nullable final SharedContext sharedContext, boolean supportsUnboundedFill) { |
There was a problem hiding this comment.
Suggested change
| public RegionContextHolder(final int chunkCapacity, @Nullable final SharedContext sharedContext, boolean supportsUnboundedFill) { | |
| public RegionContextHolder( | |
| final int chunkCapacity, | |
| @Nullable final SharedContext sharedContext, | |
| final boolean supportsUnboundedFill) { |
| @@ -33,6 +33,8 @@ interface MakeDeferred<ATTR extends Values, REGION_TYPE extends ColumnRegion<ATT | |||
| private volatile int regionCount = 0; | |||
| private volatile REGION_TYPE[] regions; | |||
|
|
|||
| private volatile boolean allRegionsSupportUnbounded = true; | |||
There was a problem hiding this comment.
Suggested change
| private volatile boolean allRegionsSupportUnbounded = true; | |
| private volatile boolean allRegionsSupportUnboundedFill = true; |
| if (region == null) { | ||
| return regions[regionIndex] = nullRegion; | ||
| } | ||
| allRegionsSupportUnbounded &= region.supportsUnboundedFill(); |
There was a problem hiding this comment.
Suggested change
| allRegionsSupportUnbounded &= region.supportsUnboundedFill(); | |
| allRegionsSupportUnboundedFill &= region.supportsUnboundedFill(); |
|
|
||
| @Override | ||
| public boolean supportsUnboundedFill() { | ||
| return allRegionsSupportUnbounded; |
There was a problem hiding this comment.
Suggested change
| return allRegionsSupportUnbounded; | |
| return allRegionsSupportUnboundedFill; |
Comment on lines
+132
to
+133
|
|
||
|
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ill.