mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-31 08:46:58 +02:00
fix(Supabase Node): Fix getAll returning duplicate or missing records during pagination (#30020)
Co-authored-by: Alexander Gekov <40495748+alexander-gekov@users.noreply.github.com>
This commit is contained in:
parent
c3cf5c7057
commit
6362afe477
|
|
@ -268,5 +268,25 @@ export const rowFields: INodeProperties[] = [
|
|||
default: 50,
|
||||
description: 'Max number of results to return',
|
||||
},
|
||||
// Supabase REST API uses PostgREST under the hood. Without a stable ORDER BY, offset-based
|
||||
// pagination returns non-deterministic pages — the database may return the same row in both
|
||||
// page 1 and page 2, or skip rows entirely. Adding ?order=column ensures consistent page
|
||||
// boundaries and prevents duplicates when using Return All or Limit >= 1000.
|
||||
// See https://supabase.com/docs/guides/api/sql-to-rest for the order parameter syntax.
|
||||
{
|
||||
displayName: 'Order By',
|
||||
name: 'orderBy',
|
||||
type: 'string',
|
||||
displayOptions: {
|
||||
show: {
|
||||
resource: ['row'],
|
||||
operation: ['getAll'],
|
||||
},
|
||||
},
|
||||
default: '',
|
||||
placeholder: 'e.g. ID or created_at.desc',
|
||||
description:
|
||||
'Column(s) to order results by, e.g. <code>ID</code> or <code>created_at.desc</code>. Recommended when using Return All or Limit ≥ 1000 to avoid duplicate or missing records.',
|
||||
},
|
||||
...getFilters(['row'], ['getAll'], {}),
|
||||
];
|
||||
|
|
|
|||
|
|
@ -382,15 +382,23 @@ export class Supabase implements INodeType {
|
|||
endpoint = `${endpoint}?${encodeURI(filterString)}`;
|
||||
}
|
||||
|
||||
if (!returnAll) {
|
||||
qs.limit = this.getNodeParameter('limit', 0);
|
||||
}
|
||||
const requestedLimit = !returnAll
|
||||
? (this.getNodeParameter('limit', 0) as number)
|
||||
: undefined;
|
||||
|
||||
const orderBy = this.getNodeParameter('orderBy', i, '') as string;
|
||||
|
||||
let rows: IDataObject[] = [];
|
||||
|
||||
try {
|
||||
let responseLength = 0;
|
||||
do {
|
||||
if (requestedLimit !== undefined) {
|
||||
qs.limit = Math.min(requestedLimit - rows.length, 1000);
|
||||
}
|
||||
if (orderBy) {
|
||||
qs.order = orderBy;
|
||||
}
|
||||
const newRows = await supabaseApiRequest.call(
|
||||
this,
|
||||
'GET',
|
||||
|
|
@ -403,7 +411,10 @@ export class Supabase implements INodeType {
|
|||
responseLength = newRows.length;
|
||||
rows = rows.concat(newRows);
|
||||
qs.offset = rows.length;
|
||||
} while (responseLength >= 1000);
|
||||
} while (
|
||||
responseLength >= 1000 &&
|
||||
(requestedLimit === undefined || rows.length < requestedLimit)
|
||||
);
|
||||
const executionData = this.helpers.constructExecutionMetaData(
|
||||
this.helpers.returnJsonArray(rows),
|
||||
{ itemData: { item: i } },
|
||||
|
|
|
|||
|
|
@ -62,6 +62,109 @@ describe('Test Supabase Node', () => {
|
|||
return fakeExecuteFunction;
|
||||
};
|
||||
|
||||
describe('getAll pagination', () => {
|
||||
it('should make exactly one request when limit is less than 1000', async () => {
|
||||
const supabaseApiRequest = jest
|
||||
.spyOn(utils, 'supabaseApiRequest')
|
||||
.mockResolvedValueOnce(Array.from({ length: 50 }, (_, i) => ({ id: i })));
|
||||
|
||||
const fakeExecuteFunction = createMockExecuteFunction({
|
||||
resource: 'row',
|
||||
operation: 'getAll',
|
||||
returnAll: false,
|
||||
limit: 50,
|
||||
tableId: 'my_table',
|
||||
filterType: 'none',
|
||||
orderBy: '',
|
||||
});
|
||||
|
||||
await node.execute.call(fakeExecuteFunction);
|
||||
|
||||
expect(supabaseApiRequest).toHaveBeenCalledTimes(1);
|
||||
|
||||
supabaseApiRequest.mockRestore();
|
||||
});
|
||||
|
||||
it('should make exactly one request when limit equals 1000', async () => {
|
||||
const supabaseApiRequest = jest
|
||||
.spyOn(utils, 'supabaseApiRequest')
|
||||
.mockResolvedValueOnce(Array.from({ length: 1000 }, (_, i) => ({ id: i })));
|
||||
|
||||
const fakeExecuteFunction = createMockExecuteFunction({
|
||||
resource: 'row',
|
||||
operation: 'getAll',
|
||||
returnAll: false,
|
||||
limit: 1000,
|
||||
tableId: 'my_table',
|
||||
filterType: 'none',
|
||||
orderBy: '',
|
||||
});
|
||||
|
||||
await node.execute.call(fakeExecuteFunction);
|
||||
|
||||
expect(supabaseApiRequest).toHaveBeenCalledTimes(1);
|
||||
|
||||
supabaseApiRequest.mockRestore();
|
||||
});
|
||||
|
||||
it('should paginate and request only remaining rows on the last page when limit > 1000', async () => {
|
||||
const capturedQs: IDataObject[] = [];
|
||||
const supabaseApiRequest = jest
|
||||
.spyOn(utils, 'supabaseApiRequest')
|
||||
.mockImplementation(async (_method, _endpoint, _body, qs) => {
|
||||
capturedQs.push({ ...qs });
|
||||
return capturedQs.length === 1
|
||||
? Array.from({ length: 1000 }, (_, i) => ({ id: i }))
|
||||
: Array.from({ length: 500 }, (_, i) => ({ id: i + 1000 }));
|
||||
});
|
||||
|
||||
const fakeExecuteFunction = createMockExecuteFunction({
|
||||
resource: 'row',
|
||||
operation: 'getAll',
|
||||
returnAll: false,
|
||||
limit: 1500,
|
||||
tableId: 'my_table',
|
||||
filterType: 'none',
|
||||
orderBy: '',
|
||||
});
|
||||
|
||||
await node.execute.call(fakeExecuteFunction);
|
||||
|
||||
expect(supabaseApiRequest).toHaveBeenCalledTimes(2);
|
||||
expect(capturedQs[0]).toMatchObject({ limit: 1000 });
|
||||
expect(capturedQs[0]).not.toHaveProperty('offset');
|
||||
expect(capturedQs[1]).toMatchObject({ limit: 500, offset: 1000 });
|
||||
|
||||
supabaseApiRequest.mockRestore();
|
||||
});
|
||||
|
||||
it('should include order parameter in the request when orderBy is set', async () => {
|
||||
const supabaseApiRequest = jest.spyOn(utils, 'supabaseApiRequest').mockResolvedValueOnce([]);
|
||||
|
||||
const fakeExecuteFunction = createMockExecuteFunction({
|
||||
resource: 'row',
|
||||
operation: 'getAll',
|
||||
returnAll: true,
|
||||
tableId: 'my_table',
|
||||
filterType: 'none',
|
||||
orderBy: 'id',
|
||||
});
|
||||
|
||||
await node.execute.call(fakeExecuteFunction);
|
||||
|
||||
expect(supabaseApiRequest).toHaveBeenCalledWith(
|
||||
'GET',
|
||||
'/my_table',
|
||||
{},
|
||||
expect.objectContaining({ order: 'id' }),
|
||||
undefined,
|
||||
{},
|
||||
);
|
||||
|
||||
supabaseApiRequest.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
it('should allow filtering on the same field multiple times', async () => {
|
||||
const supabaseApiRequest = jest
|
||||
.spyOn(utils, 'supabaseApiRequest')
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user