Replace GroupIdBlock with int[] #17688
Conversation
dain
force-pushed
the
remove-group-id-block
branch
from
796cf6c to
a730466
Compare
| RunLengthEncodedBlock.create( | ||
| BIGINT.createFixedSizeBlockBuilder(1).writeLong(groupId).build(), | ||
| block.getPositionCount())); | ||
| int[] result = new int[block.getPositionCount()]; |
There was a problem hiding this comment.
Would it be beneficial to handle this case better?
We could still have GroupIdBLock that has two states, int[] and single value and count similar to SelectedPositions and then do
if (groupIdsBlock.isArray()) {
// handle array
} else {
// handle rle
}
``` in the `Accumulator`
There was a problem hiding this comment.
Maybe. I'm not sure the complexity will result in better performance in the 99% case where we don't have RLEs. My thought is we switch to int[] and establish a new base line. Then in follow up work we can investigate if adding wrapper with RLE support helps.
| Variable groupIdsBlock = scope.getVariable("groupIdsBlock"); | ||
| loopBody.append(thisVariable.getField(stateField).invoke("setGroupId", void.class, groupIdsBlock.invoke("getGroupId", long.class, position))); | ||
| Variable groupIds = scope.getVariable("groupIds"); | ||
| loopBody.append(thisVariable.getField(stateField).invoke("setGroupId", void.class, groupIds.getElement(position).cast(long.class))); |
There was a problem hiding this comment.
Does it make sense to also make io.trino.spi.function.GroupedAccumulatorState#setGroupId accept int instead of long?
There was a problem hiding this comment.
Yes! That will be in some follow up work.
dain
force-pushed
the
remove-group-id-block
branch
from
a730466 to
1fc7432
Compare
Group id block was used to carry the max group id to the grouped accumulator, but the max group id is now set directly with the setGroupCount method. An int[] is used because the GroupByHash implementations only create int group ids.
dain
force-pushed
the
remove-group-id-block
branch
from
1fc7432 to
ca61901
Compare
Group id block was used to carry the max group id to the grouped accumulator, but the max group id is now set directly with the setGroupCount method.
An int[] is used because the GroupByHash implementations only create int group ids.
Release notes
(x) This is not user-visible or docs only and no release notes are required.