From f1ff07e9f5fe2294995ac0d71e931ed235cb4a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Flar=20Eker?= Date: Thu, 26 Feb 2026 20:57:40 +0300 Subject: [PATCH 1/2] Fix aggregate queries without explicit grouping When continuing an aggregate query without an explicit GROUP BY clause, the code attempted to deserialize a group key even when groupingKeyValue was null, causing incorrect results. Add null checks to skip group key handling when there is no explicit grouping, ensuring continued queries work correctly. --- .../cursors/aggregate/StreamGrouping.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java index a9fb1f242d..815093ff2a 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java @@ -116,10 +116,12 @@ public StreamGrouping(@Nullable final Value groupingKeyValue, this.accumulator = aggregateValue.createAccumulatorWithInitialState(context.getTypeRepository(), null); } else { this.accumulator = aggregateValue.createAccumulatorWithInitialState(context.getTypeRepository(), partialAggregationResult.getAccumulatorStatesList()); - try { - this.currentGroup = DynamicMessage.parseFrom(context.getTypeRepository().newMessageBuilder(groupingKeyValue.getResultType()).getDescriptorForType(), partialAggregationResult.getGroupKey().toByteArray()); - } catch (InvalidProtocolBufferException e) { - throw new RuntimeException(e); + if (groupingKeyValue != null) { + try { + this.currentGroup = DynamicMessage.parseFrom(context.getTypeRepository().newMessageBuilder(groupingKeyValue.getResultType()).getDescriptorForType(), partialAggregationResult.getGroupKey().toByteArray()); + } catch (InvalidProtocolBufferException e) { + throw new RuntimeException(e); + } } } this.store = store; @@ -212,16 +214,18 @@ private Object evalGroupingKey(@Nullable final Object currentObject) { @Nullable public RecordCursorProto.PartialAggregationResult getPartialAggregationResult() { - if (currentGroup == null) { + if (groupingKeyValue != null && currentGroup == null) { return null; } List accumulatorStates = accumulator.getAccumulatorStates(); if (accumulatorStates.isEmpty()) { return null; } - return RecordCursorProto.PartialAggregationResult.newBuilder() - .setGroupKey(Objects.requireNonNull((Message)currentGroup).toByteString()) - .addAllAccumulatorStates(accumulatorStates) - .build(); + final RecordCursorProto.PartialAggregationResult.Builder builder = RecordCursorProto.PartialAggregationResult.newBuilder() + .addAllAccumulatorStates(accumulatorStates); + if (groupingKeyValue != null) { + builder.setGroupKey(Objects.requireNonNull((Message)currentGroup).toByteString()); + } + return builder.build(); } } From 40fc163d5acf29531b2d19aa477c3b3fadde85fc Mon Sep 17 00:00:00 2001 From: caglareker Date: Sat, 7 Mar 2026 00:05:04 +0300 Subject: [PATCH 2/2] Add regression test for ungrouped aggregate continuations --- .../aggregate-ungrouped-continuation.yamsql | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 yaml-tests/src/test/resources/aggregate-ungrouped-continuation.yamsql diff --git a/yaml-tests/src/test/resources/aggregate-ungrouped-continuation.yamsql b/yaml-tests/src/test/resources/aggregate-ungrouped-continuation.yamsql new file mode 100644 index 0000000000..1486bf3ba7 --- /dev/null +++ b/yaml-tests/src/test/resources/aggregate-ungrouped-continuation.yamsql @@ -0,0 +1,62 @@ +# +# aggregate-ungrouped-continuation.yamsql +# +# This source file is part of the FoundationDB open source project +# +# Copyright 2021-2024 Apple Inc. and the FoundationDB project authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Regression test for aggregate queries without explicit GROUP BY +# that use continuations (maxRows). Previously, resuming from a +# continuation with no grouping key caused a NullPointerException +# in StreamGrouping when groupingKeyValue was null. + +--- +schema_template: + create table t1(id bigint, col1 bigint, col2 bigint, primary key(id)) + create index mv1 as select sum(col2) from t1 group by col1 +--- +setup: + steps: + - query: INSERT INTO T1 + VALUES (1, 10, 1), + (2, 10, 2), + (3, 10, 3), + (4, 20, 4), + (5, 20, 5), + (6, 20, 6) +--- +test_block: + name: aggregate-ungrouped-continuation + tests: + - + # Aggregate without GROUP BY, no continuation needed (single result row) + - query: select sum(col2) from t1 + - result: [{!l 21}] + - + # Aggregate without GROUP BY with maxRows=1 to force continuation. + # This triggered a NullPointerException before the fix because + # getPartialAggregationResult tried to serialize a null currentGroup. + - query: select sum(col2) from t1 + - maxRows: 1 + - result: [{!l 21}] + - result: [] + - + # Aggregate with GROUP BY and maxRows=1 to verify continuations + # still work correctly with grouping keys present. + - query: select col1, sum(col2) from t1 group by col1 + - maxRows: 1 + - result: [{!l 10, !l 6}] + - result: [{!l 20, !l 15}] + - result: [] \ No newline at end of file