Skip to content

fix(search): allow LOAD * in FT.AGGREGATE #2726 #3241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nkaradzhov merged 1 commit into from
Apr 22, 2026
+28 −18
Merged

fix(search): allow LOAD * in FT.AGGREGATE #2726 #3241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/search/lib/commands/AGGREGATE.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ describe('AGGREGATE', () => {
});

describe('with LOAD', () => {
it('all attributes (*)', () => {
assert.deepEqual(
parseArgs(AGGREGATE, 'index', '*', {
LOAD: '*'
}),
['FT.AGGREGATE', 'index', '*', 'LOAD', '*', 'DIALECT', DEFAULT_DIALECT]
);
});
describe('single', () => {
describe('without alias', () => {
it('string', () => {
Expand Down
38 changes: 20 additions & 18 deletions packages/search/lib/commands/AGGREGATE.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ type LoadField = RediSearchProperty | {
export const FT_AGGREGATE_STEPS = {
GROUPBY: 'GROUPBY',
SORTBY: 'SORTBY',
APPLY: 'APPLY',
APPLY: 'APPLY',
LIMIT: 'LIMIT',
FILTER: 'FILTER'
} as const;

type FT_AGGREGATE_STEPS = typeof FT_AGGREGATE_STEPS;
type FT_AGGREGATE_STEPS = typeof FT_AGGREGATE_STEPS;

export type FtAggregateStep = FT_AGGREGATE_STEPS[keyof FT_AGGREGATE_STEPS];

Expand Down Expand Up @@ -121,7 +121,7 @@ interface FilterStep extends AggregateStep<FT_AGGREGATE_STEPS['FILTER']> {
export interface FtAggregateOptions {
VERBATIM?: boolean;
ADDSCORES?: boolean;
LOAD?: LoadField | Array<LoadField>;
LOAD?: '*' | LoadField | Array<LoadField>;
TIMEOUT?: number;
STEPS?: Array<GroupByStep | SortStep | ApplyStep | LimitStep | FilterStep>;
PARAMS?: FtSearchParams;
Expand Down Expand Up @@ -166,7 +166,7 @@ export default {
transformTuplesReply(rawReply[i] as ArrayReply<BlobStringReply>, preserve, typeMapping)
);
}

return {
// https://redis.io/docs/latest/commands/ft.aggregate/#return
// FT.AGGREGATE returns an array reply where each row is an array reply and represents a single aggregate result.
Expand All @@ -180,31 +180,33 @@ export default {
unstableResp3: true
} as const satisfies Command;

export function parseAggregateOptions(parser: CommandParser , options?: FtAggregateOptions) {
export function parseAggregateOptions(parser: CommandParser, options?: FtAggregateOptions) {
if (options?.VERBATIM) {
parser.push('VERBATIM');
}

if (options?.ADDSCORES) {
parser.push('ADDSCORES');
}
}

if (options?.LOAD) {
const args: Array<RedisArgument> = [];

if (Array.isArray(options.LOAD)) {
for (const load of options.LOAD) {
pushLoadField(args, load);
}
parser.push('LOAD');
if (options.LOAD === '*') {
parser.push('*');
} else {
pushLoadField(args, options.LOAD);
}
const args: Array<RedisArgument> = [];

parser.push('LOAD');
parser.pushVariadicWithLength(args);
}
if (Array.isArray(options?.LOAD)) {
for (const load of options.LOAD) {
pushLoadField(args, load);
}
} else {
pushLoadField(args, options?.LOAD);
}

if (options?.TIMEOUT !== undefined) {
parser.pushVariadicWithLength(args);
}
} if (options?.TIMEOUT !== undefined) {

@cursor cursor Bot Apr 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline causes } if on same line

Low Severity

The closing brace of the LOAD block and the TIMEOUT if statement are on the same line (} if (), making it visually ambiguous whether this is an independent if or was intended to be an else if. The original code had these as clearly separated independent if blocks with a blank line between them. A future maintainer could easily mistake this for a missing else and introduce a regression.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 095ae1c. Configure here.

parser.push('TIMEOUT', options.TIMEOUT.toString());
}

Expand Down
Loading