-
Notifications
You must be signed in to change notification settings - Fork 686
feat(materialize): add new backend for Materialize streaming database #11703
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.
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
base: main
Are you sure you want to change the base?
feat(materialize): add new backend for Materialize streaming database #11703
Conversation
35b0a11 to
349400c
Compare
|
@jasonhernandez Thanks for the PR! I will try to give it a review this weekend or later this evening. It looks like the materialize test suite is timing out at 60 minutes. Any idea what's going on there? |
cpcloud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good! This is one of the most thorough initial backend PRs I've ever seen, great work.
A few comments, mostly around exception handling and too-broad-try clauses.
39004fc to
46a9f19
Compare
I was hoping you might have known of something obvious that might cause that timeout. I'm going to investigate it further. |
7e62c90 to
bf29fa1
Compare
e130a91 to
e410ec0
Compare
8cdab23 to
1f2df71
Compare
| interval: 2s | ||
| retries: 30 | ||
| timeout: 5s | ||
| test: ["CMD", "curl", "-f", "localhost:6878/api/readyz"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is our recommended healthcheck endpoint
|
@cpcloud I added two commits - the first one specifically addresses the feedback on this review. The second one makes some fixes to testing (resolving the timeouts of the backend test), bumps to the latest version of Materialize, and generally incorporates recommendations from our head of QA. I can squash these all together / submit a new PR or change this however would simplify review / merging on your end. |
This commit adds comprehensive support for Materialize, including: - Core backend implementation with cluster and connection management - Support for materialized views, indexes, sources, sinks, and secrets - Idiomatic Materialize SQL patterns (mz_now(), SUBSCRIBE, etc.) - Test coverage for Materialize-specific features - Integration with existing Ibis test suite Co-Authored-By: Seth Wiesman <[email protected]>
This commit addresses several issues to improve Materialize test reliability: - Update to Materialize v26 syntax: Replace deprecated COUNTER load generator with AUCTION, update to FOR ALL TABLES syntax for multi-output sources - Fix test parallelization: Serialize array, source, and subscribe tests to avoid resource contention and subsource name collisions - Optimize data loading: Cache list_tables() result during CSV loading to reduce expensive database calls - Fix doctest failures: Reduce to 54 doctest +SKIP markers matching postgres backend pattern (68% reduction) - Fix dead fixture warnings: Mark TPC benchmark tests as skipped instead of deselecting them to prevent pytest-deadfixtures from flagging their fixtures - Fix test_map: Make assertion Materialize-specific to avoid breaking other backends' pytest diff output - Remove deprecated enable_load_generator_counter and enable_create_table_from_source flags from compose.yaml Co-Authored-By: Dennis Felsing <[email protected]>
1f2df71 to
4dca804
Compare
|
@cpcloud I just wanted to check when you might have a chance to look at this PR again. I know it's rather massive. |
Materialize does not support MERGE INTO syntax, which the upsert() method uses. This commit marks the upsert tests with pytest.mark.notyet for materialize, similar to how other backends that don't support MERGE INTO are handled (risingwave, mysql, sqlite, etc.). The following tests are now expected failures for Materialize: - test_upsert_from_dataframe - test_upsert_from_memtable (all variants) - test_upsert_from_expr (both variants)
Description of changes
Adds a Materialize backend for Ibis
This PR introduces a new backend for Materialize, a live data layer with native support for generalized incremental view maintenance. Materialize strives to maintain PostgreSQL wire compatibility, but it offers a number of unique capabilities that require expanding beyond the existing Postgres backend for Materialize users. In particular, Materialize provides:
The Materialize team is excited to collaborate with the Ibis community and is committed to maintaining and improving this backend going forward.