Skip to content

Commit 441e8dc

Browse files
committed
Fix: #54 Fix pagination in GetStreamsToSnapshot for large datasets
Address issue #54 in NEventStore.Persistence.Sql where GetStreamsToSnapshot failed when the number of streams exceeded PageSize. Updated SQL queries to include `C.StreamId > @StreamId` for proper pagination. Added `_lastStreamId` tracking in tests to validate the pagination logic. Introduced new test cases to ensure `GetFrom` and `GetFromAsync` methods handle large datasets without crashing. Updated SQL dialect resources and persistence engine methods to support the new pagination logic.
1 parent 02c4fa4 commit 441e8dc

8 files changed

Lines changed: 68 additions & 4 deletions

‎Changelog.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
- Per-stream operations and delete: (BucketId, StreamId)
88
- Snapshot join/aggregation: (StreamId, StreamRevision)
99

10+
### BugFix
11+
12+
- Fixed GetStreamsToSnapshot pagination [#54](https://github.com/NEventStore/NEventStore.Persistence.SQL/issues/54)
13+
1014
### Breaking Changes
1115

1216
The following indexes were added to the default schema (upon database initialization), add it manually to an existing database:

‎src/AdditionalTests/AdditionalPersistenceTests.Async.cs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,37 @@
2020
namespace NEventStore.Persistence.AcceptanceTests.Async
2121
{
2222

23+
/// <summary>
24+
/// <para>
25+
/// NEventStore.Persistence.Sql issue #54
26+
/// GetStreamsToSnapshot fails when number of streams exceeds PageSize.
27+
/// </para>
28+
/// <para>
29+
/// Query implementation was wrong: checking how the query and the pagination were implemented
30+
/// it was lacking to check for C.StreamId > @StreamId.
31+
/// which is needed to page through the results (similar to @Coalesce in GetCommitsFromStartingRevision)
32+
/// </para>
33+
/// </summary>
2334
#if MSTEST
2435
[TestClass]
2536
#endif
2637
public class when_getting_streams_to_snapshot_amount_exceeds_PageSize : PersistenceEngineConcernAsync
2738
{
2839
private int _moreThanPageSize;
40+
private Guid _lastStreamId;
2941

3042
protected override async Task BecauseAsync()
3143
{
3244
_moreThanPageSize = ConfiguredPageSizeForTesting + 1;
3345
var eventStore = new OptimisticEventStore(Persistence, null, null);
3446
for (int i = 0; i < _moreThanPageSize; i++)
3547
{
36-
using IEventStream stream = await eventStore.OpenStreamAsync(Guid.NewGuid()).ConfigureAwait(false);
48+
_lastStreamId = Guid.NewGuid();
49+
using IEventStream stream = await eventStore.OpenStreamAsync(_lastStreamId).ConfigureAwait(false);
50+
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
51+
await stream.CommitChangesAsync(Guid.NewGuid(), CancellationToken.None).ConfigureAwait(false);
52+
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
53+
await stream.CommitChangesAsync(Guid.NewGuid(), CancellationToken.None).ConfigureAwait(false);
3754
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
3855
await stream.CommitChangesAsync(Guid.NewGuid(), CancellationToken.None).ConfigureAwait(false);
3956
}
@@ -49,6 +66,17 @@ public void GetStreamsToSnapshot_does_not_crash_and_returns_all_the_streams()
4966
observer.StreamHeads.Count.Should().Be(_moreThanPageSize);
5067
});
5168
}
69+
70+
[Fact]
71+
public void GetFrom_does_not_crash_and_returns_all_the_streams()
72+
{
73+
Assert.DoesNotThrowAsync(async () =>
74+
{
75+
var observer = new CommitStreamObserver();
76+
await Persistence.GetFromAsync(Bucket.Default, _lastStreamId.ToString(), 0, int.MaxValue, observer, CancellationToken.None).ConfigureAwait(false);
77+
observer.Commits.Count.Should().Be(3);
78+
});
79+
}
5280
}
5381

5482
}

‎src/AdditionalTests/AdditionalPersistenceTests.cs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,37 @@
2020
namespace NEventStore.Persistence.AcceptanceTests
2121
{
2222

23+
/// <summary>
24+
/// <para>
25+
/// NEventStore.Persistence.Sql issue #54
26+
/// GetStreamsToSnapshot fails when number of streams exceeds PageSize.
27+
/// </para>
28+
/// <para>
29+
/// Query implementation was wrong: checking how the query and the pagination were implemented
30+
/// it was lacking to check for C.StreamId > @StreamId.
31+
/// which is needed to page through the results (similar to @Coalesce in GetCommitsFromStartingRevision)
32+
/// </para>
33+
/// </summary>
2334
#if MSTEST
2435
[TestClass]
2536
#endif
2637
public class when_getting_streams_to_snapshot_amount_exceeds_PageSize : PersistenceEngineConcern
2738
{
2839
private int _moreThanPageSize;
40+
private Guid _lastStreamId;
2941

3042
protected override void Because()
3143
{
3244
_moreThanPageSize = ConfiguredPageSizeForTesting + 1;
3345
var eventStore = new OptimisticEventStore(Persistence, null, null);
3446
for (int i = 0; i < _moreThanPageSize; i++)
3547
{
36-
using IEventStream stream = eventStore.OpenStream(Guid.NewGuid());
48+
_lastStreamId = Guid.NewGuid();
49+
using IEventStream stream = eventStore.OpenStream(_lastStreamId);
50+
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
51+
stream.CommitChanges(Guid.NewGuid());
52+
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
53+
stream.CommitChanges(Guid.NewGuid());
3754
stream.Add(new EventMessage { Body = new TestEvent() { S = "Hi " + i } });
3855
stream.CommitChanges(Guid.NewGuid());
3956
}
@@ -48,6 +65,16 @@ public void GetStreamsToSnapshot_does_not_crash_and_returns_all_the_streams()
4865
streamHeads.Length.Should().Be(_moreThanPageSize);
4966
});
5067
}
68+
69+
[Fact]
70+
public void GetFrom_does_not_crash_and_returns_all_the_streams()
71+
{
72+
Assert.DoesNotThrow(() =>
73+
{
74+
var streamHeads = Persistence.GetFrom(Bucket.Default, _lastStreamId.ToString(), 0, int.MaxValue).ToArray();
75+
streamHeads.Length.Should().Be(3);
76+
});
77+
}
5178
}
5279

5380
}

‎src/NEventStore.Persistence.Sql/SqlDialects/CommonSqlStatements.Designer.cs‎

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/NEventStore.Persistence.Sql/SqlDialects/CommonSqlStatements.resx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ AND NOT EXISTS
188188
AND C.StreamRevision &gt;= S.StreamRevision
189189
GROUP BY C.StreamId, C.BucketId, C.StreamIdOriginal
190190
HAVING MAX(C.StreamRevision) &gt;= MAX(COALESCE(S.StreamRevision, 0)) + @Threshold
191+
AND C.StreamId &gt; @StreamId
191192
ORDER BY C.StreamId
192193
LIMIT @Limit;</value>
193194
</data>

‎src/NEventStore.Persistence.Sql/SqlDialects/OracleNativeStatements.resx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ FROM (
211211
ON C.BucketId = :BucketId AND C.StreamId = S.StreamId AND C.StreamRevision &gt;= S.StreamRevision
212212
GROUP BY C.StreamId, C.BucketId, C.StreamIdOriginal
213213
HAVING MAX(C.StreamRevision) &gt;= MAX(COALESCE(S.StreamRevision, 0)) + :Threshold
214+
AND C.StreamId &gt; :StreamId
214215
ORDER BY C.StreamId
215216
)
216217
WHERE ROWNUM &lt;= :Limit;</value>

‎src/NEventStore.Persistence.Sql/SqlPersistenceEngine.Async.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ public Task GetStreamsToSnapshotAsync(string bucketId, int maxThreshold, IAsyncO
259259
string statement = _dialect.GetStreamsRequiringSnapshots;
260260
query.AddParameter(_dialect.BucketId, bucketId, DbType.AnsiString);
261261
query.AddParameter(_dialect.Threshold, maxThreshold);
262+
query.AddParameter(_dialect.StreamId, String.Empty, DbType.AnsiString); // (NEventStore.Persistence.Sql #54) to define the parameter for paging
262263
return query.ExecutePagedQueryAsync(
263264
statement, (q, s) => q.SetParameter(_dialect.StreamId, _dialect.CoalesceParameterValue(s.StreamId()), DbType.AnsiString), // todo: I'm not sure this is used, the query does not have a "StreamId" parameter
264265
asyncObserver, cancellationToken);

‎src/NEventStore.Persistence.Sql/SqlPersistenceEngine.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,10 @@ public virtual IEnumerable<IStreamHead> GetStreamsToSnapshot(string bucketId, in
210210
string statement = _dialect.GetStreamsRequiringSnapshots;
211211
query.AddParameter(_dialect.BucketId, bucketId, DbType.AnsiString);
212212
query.AddParameter(_dialect.Threshold, maxThreshold);
213+
query.AddParameter(_dialect.StreamId, String.Empty, DbType.AnsiString); // (NEventStore.Persistence.Sql #54) to define the parameter for paging
213214
return
214215
query.ExecutePagedQuery(statement,
215-
(q, s) => { }) // No next page delegate needed - @Skip parameter is handled automatically by PagedEnumerationCollection
216+
(q, s) => q.SetParameter(_dialect.StreamId, _dialect.CoalesceParameterValue(s.StreamId()), DbType.AnsiString))
216217
.Select(x => x.GetStreamToSnapshot());
217218
});
218219
}

0 commit comments

Comments
 (0)