Codecov Report

❌ Patch coverage is 88.84758% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.11%. Comparing base (ab928a3) to head (5a1707f).

@@             Coverage Diff              @@
##                1.x    #2408      +/-   ##
============================================
- Coverage     85.11%   85.11%   -0.01%     
- Complexity        0    21198   +21198     
============================================
  Files          1599     1597       -2     
  Lines         65334    65401      +67     
============================================
+ Hits          55612    55668      +56     
- Misses         9722     9733      +11     

@MrHDOLEK as we discussed offline, please find some feedback and guidance below.

Extractor / Loader

In general I like the idea of having one extractor that accepts Seal EngineInterface which is aligned with how Doctrine Adapter works right now. Same goes for the loaders.

It would also work very well with frameworks as seal provides a bundle for symfony that literally configures engine so to use it with flow it would be a simple matter of dependency injection.

What to test

I had to think about it a bit more but I dont think we should retest every backend that seal is testing. Doctrine adapter is testing MySQL, PostgreSql nad Sqlite because flow-php/docrtine-dbal-bulk which extends doctrine default behaviors that needs to be tested.

At this time seal provides following adapters:

• Elasticsearch - cmsig/seal-elasticsearch-adapter
• Opensearch - cmsig/seal-opensearch-adapter
• Meilisearch - cmsig/seal-meilisearch-adapter
• Algolia - cmsig/seal-algolia-adapter
• Solr - cmsig/seal-solr-adapter
• RediSearch - cmsig/seal-redisearch-adapter
• Typesense - cmsig/seal-typesense-adapter
• Loupe - cmsig/seal-loupe-adapter

I would chose one of them (that requires the least configuration and is the easiest to setup in docker) and cover it with proper integration tests that would confirm that moving data from flow to seal works as expected.
Heck you can even use MemoryAdapter in tests unless other adapters have some features that requires some custom configuration.
The only thing you might want to research first is are there any differences between seal adapters. Flow needs to be flexible enough to use all of them, so if one requires something custom, flow extractor/loader needs to be able to pass it through (but I dont think thats the case).

How to test

So in general we want to cover in tests that seal adapter can handle all types of flow Entries. You can achieve that by using one of the two extractors:

• FakeStaticOrdersExtractor
• FakeRandomOrdersExtractor

They both expose static method schema() : Schema but here is where the tricky part starts. I believe you need to be able to first create an index in tests.
And that brings us to a missing piece of this PR - SchemaConverter.

SchemaConverter

Schema converters are recursive algorithms that can convert schema in both directiosn Flow to Seal and Seal to Flow. You can find some inspirations here:

• Parquet
• Doctrine
• PostgreSql

It might be the easiest to use LLM to help you create one based on those 3 examples for Seal (it's a recursive brain damaging exercise that might not be worth spending time on).

Of course schema converters would need a DSL method.

Search Engine in Tests

So in this PR you are using traits to configure backends in tests, which is fine but there is a different pattern which I found cleaner and easier to maintain.

Contexts.

Here is a good example of DatabaseContext that is used in DatabaseTableListCommandTest

If there will be more than one integration tests you can extract an abstract SealTestCase extends FlowTestCase and setup SealContext in the setUp method making it available through sealContext() : SealContext method.

In case of any questions, you know where to find me 😁

@norberttech I found an issue while testing with the 20k dataset.

SEAL cannot read data beyond 10K from elastic because it only supports offset-based pagination, and Elasticsearch blocks offsets outside the 10K result window. To work around this problem, I added cursor-based pagination.

SEAL cannot read data beyond 10K from elastic because it only supports offset-based pagination, and Elasticsearch blocks offsets outside the 10K result window. To work around this problem, I added cursor-based pagination.

That's excellent finding!

Does other engines also comes with such limitations? Is this keyset pagination going to work only with Elastic or all seal adapters?
When someone tries to read above 10k values without keyset, is the exception error going to point him to the keyset? Are docs updated?
What is the cost of moving to such pagination? Is it really keyset or maybe a cursor pagination? Are there other types of pagination supported?

@alexander-schranz if by any chance you could take a look at this integration, your feedback would be greatly appreciated 🙏

SEAL cannot read data beyond 10K from elastic because it only supports offset-based pagination, and Elasticsearch blocks offsets outside the 10K result window. To work around this problem, I added cursor-based pagination.

That's excellent finding!

Does other engines also comes with such limitations? Is this keyset pagination going to work only with Elastic or all seal adapters? When someone tries to read above 10k values without keyset, is the exception error going to point him to the keyset? Are docs updated? What is the cost of moving to such pagination? Is it really keyset or maybe a cursor pagination? Are there other types of pagination supported?

@alexander-schranz if by any chance you could take a look at this integration, your feedback would be greatly appreciated 🙏

Algolia — 1 000 (paginationLimitedTo, max 20k)
Meilisearch — 1 000 (maxTotalHits)
Typesense — max_per_page 250 + limit_hits ~500
2. A raw Elasticsearch error is returned. There is currently no error handling (catch) for this exception on the SEAL side.
3. I would not worry about performance here. The real problem is its pagination limitations.
4. This could be solved by adding some primitive scroller in SEAL.

4. This could be solved by adding some primitive scroller in SEAL.

In general pagination is a complex problem, especially over large datasets.
There are 3 patterns I'm using in PostgreSql and Doctrine adapters.

Limit Offset Pagination - usually works up to 5-10k rows
Cursor Pagination - you need to understand cursors first as they might expire while you are paginating and they might highly impact the performance (cursors should be configurable)
KeySet Pagination - my usual default but storage must support it.

I guess the biggest question is if SEAL would let us now build something that would leverage KeySet pagination without any changes in SEAL or maybe some adjustments are first required there?

@norberttech I can just give a quick response here without having a deeper look at the Pull Request yet. Limitations on Elasticsearch can be controlled by Elasticsearch settings, so if somebody want to fetch more they can increase it on Elasticsearch instance config. There are some discussions to allow configure more settings via DSN params, but that will take a longer time.

While there is an issue around adding a Scroller: PHP-CMSIG/search#535 it won't solve the Elasticsearch 10k issue, as @MrHDOLEK mention the offset is blocked by Elastic and the Scroller issue is designed that it not need be implemented per Adapter = uses Limit / Offset like Scrolling. Maybe something we need to rethink.

KeySet Pagination

Are you in control of the Schema and the Data? If I understand the KeySet Pagination correctly you would require additional field or a value which increases. If document id is integer maybe sort by id, else a datetime field or something which can be sorted by and is possible unique or atleast not exist more as 250 (typesense default limit).

@norberttech How are you handling KeySet Pagination in doctrine if the identifier are not auto incremented like uuids?

@norberttech I can just give a quick response here without having a deeper look at the Pull Request yet.

Thank you!

Limitations on Elasticsearch can be controlled by Elasticsearch settings,

That's unfortunately not acceptable solution in data processing world, it does not scale and it's also not very complex to hit that limit even when the scale is not massive + there is a limit of how much you can increase those limits up to. So

@norberttech How are you handling KeySet Pagination in doctrine if the identifier are not auto incremented like uuids?

Yes, KeySet pagination requires something you can sort by, it can be either auto-incremented id, Uuid V7, additional timestamp column.
I don't think I ever had to deal with a dataset that wouldn't have at least one of those, and even if not, adding something as small as createdAt timestamp solves the problem usually.

Are you in control of the Schema and the Data?

Of course, if you don't have a control over it, you can't build an reliable ETL pipeline.