IbisJsonCompatStore

[geoHeil]

No description provided.

[geoHeil]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• postgres store #234
• IbisJsonCompatStore #382 👈 (View in Graphite)
• [core] FlatVersioningEngine #381
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Coverage Report (Python 3.13) •

File	Stmts	Miss	Cover	Missing
__init__.py	20	19	5%	1, 3–5, 10–11, 23, 30, 36, 41, 47–48, 63, 66, 88, 108, 112–113, 116
_testing
duckdb_json_compat_store.py	32	20	37%	19, 27, 29–30, 32–33, 35, 37, 39, 44–45, 47–48, 61, 63–64, 66, 69, 75–76
metaxy_project.py	250	28	88%	93, 129, 136, 199, 306, 308–314, 353, 419, 440, 475, 489, 521, 534, 733, 735–741, 747
metadata_store
ibis_json_compat.py	126	87	30%	69–70, 74, 77, 128, 139, 146–147, 152–153, 158–159, 161, 182, 187, 190, 192, 199–206, 208, 216, 219, 221–222, 224–226, 228, 230, 233–235, 237, 240–243, 245–246, 248–249, 252–253, 263, 304–306, 308, 310–311, 313, 316–317, 319, 321–325, 328, 332, 337–338, 340, 343–344, 450, 454–455, 457–459, 462, 465, 467, 490–492, 497, 500, 503
json_struct_serializer.py	158	142	10%	29–32, 35–36, 39, 44, 53, 57, 59, 61–62, 64, 66–70, 73–78, 82–83, 87–88, 93–94, 97–99, 101–103, 106–107, 109, 115, 117–119, 121–123, 130–131, 136–138, 146, 149, 152, 155–156, 162, 165, 168–169, 173–174, 177, 180–183, 186–187, 190–191, 194–196, 199–201, 204–205, 208–209, 214–215, 227–228, 232–233, 245–246, 251, 259, 261–262, 265, 268, 271–272, 274–275, 292–293, 311–312, 314, 317–319, 322–324, 328–329, 333, 337, 341, 345, 349, 352, 355–356, 365, 374–375, 378, 383–384, 387, 389, 393–394, 398, 403, 409, 413, 417–418, 422, 427, 434, 440, 443
utils.py	48	17	64%	1–4, 6, 9, 14, 21–22, 35–37, 39, 44, 49, 123, 125
TOTAL	8982	3663	59%

Tests	Skipped	Failures	Errors	Time
1976	22 💤	0 ❌	0 🔥	6m 33s ⏱️

[github-actions]

Coverage Report (Python 3.12) •

File	Stmts	Miss	Cover	Missing
__init__.py	20	19	5%	1, 3–5, 10–11, 23, 30, 36, 41, 47–48, 63, 66, 88, 108, 112–113, 116
_testing
duckdb_json_compat_store.py	32	20	37%	19, 27, 29–30, 32–33, 35, 37, 39, 44–45, 47–48, 61, 63–64, 66, 69, 75–76
metaxy_project.py	250	28	88%	93, 129, 136, 199, 306, 308–314, 353, 419, 440, 475, 489, 521, 534, 733, 735–741, 747
metadata_store
ibis_json_compat.py	126	87	30%	69–70, 74, 77, 128, 139, 146–147, 152–153, 158–159, 161, 182, 187, 190, 192, 199–206, 208, 216, 219, 221–222, 224–226, 228, 230, 233–235, 237, 240–243, 245–246, 248–249, 252–253, 263, 304–306, 308, 310–311, 313, 316–317, 319, 321–325, 328, 332, 337–338, 340, 343–344, 450, 454–455, 457–459, 462, 465, 467, 490–492, 497, 500, 503
json_struct_serializer.py	158	142	10%	29–32, 35–36, 39, 44, 53, 57, 59, 61–62, 64, 66–70, 73–78, 82–83, 87–88, 93–94, 97–99, 101–103, 106–107, 109, 115, 117–119, 121–123, 130–131, 136–138, 146, 149, 152, 155–156, 162, 165, 168–169, 173–174, 177, 180–183, 186–187, 190–191, 194–196, 199–201, 204–205, 208–209, 214–215, 227–228, 232–233, 245–246, 251, 259, 261–262, 265, 268, 271–272, 274–275, 292–293, 311–312, 314, 317–319, 322–324, 328–329, 333, 337, 341, 345, 349, 352, 355–356, 365, 374–375, 378, 383–384, 387, 389, 393–394, 398, 403, 409, 413, 417–418, 422, 427, 434, 440, 443
utils.py	48	17	64%	1–4, 6, 9, 14, 21–22, 35–37, 39, 44, 49, 123, 125
TOTAL	8982	3663	59%

Tests	Skipped	Failures	Errors	Time
1976	22 💤	0 ❌	0 🔥	7m 20s ⏱️

[github-actions]

Test Results (Python 3.10)

1 976 tests  +7   1 954 ✅ +7   6m 6s ⏱️ -7s
    1 suites ±0      22 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit f61a4e3. ± Comparison against base commit 8554f08.

♻️ This comment has been updated with latest results.

[github-actions]

Coverage Report (Python 3.10) •

File	Stmts	Miss	Cover	Missing
__init__.py	20	19	5%	1, 3–5, 10–11, 23, 30, 36, 41, 47–48, 63, 66, 88, 108, 112–113, 116
_testing
duckdb_json_compat_store.py	32	20	37%	19, 27, 29–30, 32–33, 35, 37, 39, 44–45, 47–48, 61, 63–64, 66, 69, 75–76
metaxy_project.py	250	28	88%	93, 129, 136, 199, 306, 308–314, 353, 419, 440, 475, 489, 521, 534, 733, 735–741, 747
metadata_store
ibis_json_compat.py	126	87	30%	69–70, 74, 77, 128, 139, 146–147, 152–153, 158–159, 161, 182, 187, 190, 192, 199–206, 208, 216, 219, 221–222, 224–226, 228, 230, 233–235, 237, 240–243, 245–246, 248–249, 252–253, 263, 304–306, 308, 310–311, 313, 316–317, 319, 321–325, 328, 332, 337–338, 340, 343–344, 450, 454–455, 457–459, 462, 465, 467, 490–492, 497, 500, 503
json_struct_serializer.py	158	142	10%	29–32, 35–36, 39, 44, 53, 57, 59, 61–62, 64, 66–70, 73–78, 82–83, 87–88, 93–94, 97–99, 101–103, 106–107, 109, 115, 117–119, 121–123, 130–131, 136–138, 146, 149, 152, 155–156, 162, 165, 168–169, 173–174, 177, 180–183, 186–187, 190–191, 194–196, 199–201, 204–205, 208–209, 214–215, 227–228, 232–233, 245–246, 251, 259, 261–262, 265, 268, 271–272, 274–275, 292–293, 311–312, 314, 317–319, 322–324, 328–329, 333, 337, 341, 345, 349, 352, 355–356, 365, 374–375, 378, 383–384, 387, 389, 393–394, 398, 403, 409, 413, 417–418, 422, 427, 434, 440, 443
utils.py	48	17	64%	1–4, 6, 9, 14, 21–22, 35–37, 39, 44, 49, 123, 125
TOTAL	8982	3663	59%

Tests	Skipped	Failures	Errors	Time
1976	22 💤	0 ❌	0 🔥	6m 6s ⏱️

[github-actions]

Coverage Report (Python 3.11) •

File	Stmts	Miss	Cover	Missing
__init__.py	20	19	5%	1, 3–5, 10–11, 23, 30, 36, 41, 47–48, 63, 66, 88, 108, 112–113, 116
_testing
duckdb_json_compat_store.py	32	20	37%	19, 27, 29–30, 32–33, 35, 37, 39, 44–45, 47–48, 61, 63–64, 66, 69, 75–76
metaxy_project.py	250	28	88%	93, 129, 136, 199, 306, 308–314, 353, 419, 440, 475, 489, 521, 534, 733, 735–741, 747
metadata_store
ibis_json_compat.py	126	87	30%	69–70, 74, 77, 128, 139, 146–147, 152–153, 158–159, 161, 182, 187, 190, 192, 199–206, 208, 216, 219, 221–222, 224–226, 228, 230, 233–235, 237, 240–243, 245–246, 248–249, 252–253, 263, 304–306, 308, 310–311, 313, 316–317, 319, 321–325, 328, 332, 337–338, 340, 343–344, 450, 454–455, 457–459, 462, 465, 467, 490–492, 497, 500, 503
json_struct_serializer.py	158	142	10%	29–32, 35–36, 39, 44, 53, 57, 59, 61–62, 64, 66–70, 73–78, 82–83, 87–88, 93–94, 97–99, 101–103, 106–107, 109, 115, 117–119, 121–123, 130–131, 136–138, 146, 149, 152, 155–156, 162, 165, 168–169, 173–174, 177, 180–183, 186–187, 190–191, 194–196, 199–201, 204–205, 208–209, 214–215, 227–228, 232–233, 245–246, 251, 259, 261–262, 265, 268, 271–272, 274–275, 292–293, 311–312, 314, 317–319, 322–324, 328–329, 333, 337, 341, 345, 349, 352, 355–356, 365, 374–375, 378, 383–384, 387, 389, 393–394, 398, 403, 409, 413, 417–418, 422, 427, 434, 440, 443
utils.py	48	17	64%	1–4, 6, 9, 14, 21–22, 35–37, 39, 44, 49, 123, 125
TOTAL	8982	3663	59%

Tests	Skipped	Failures	Errors	Time
1976	22 💤	0 ❌	0 🔥	7m 12s ⏱️

[Copilot]

Pull request overview

This PR introduces IbisJsonCompatStore, a new abstract base class for Ibis-based metadata stores that work with databases lacking native struct support (PostgreSQL, SQLite, MySQL). The implementation uses a dict-based versioning engine with JSON serialization at database boundaries, allowing struct-like behavior through flattened column naming conventions during computation and JSON storage in the database.

• Adds abstract base class IbisJsonCompatStore that extends IbisMetadataStore
• Implements JSON pack/unpack logic to serialize/deserialize struct-like columns
• Forces use of IbisDictBasedVersioningEngine instead of standard IbisVersioningEngine

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
src/metaxy/metadata_store/ibis_json_compat.py	New abstract base class implementing JSON-compatible metadata storage with pack/unpack methods for struct serialization
tests/metadata_stores/test_ibis_json_compat_store.py	Comprehensive tests for JSON pack/unpack functionality including roundtrip tests and lazy execution verification
tests/metadata_stores/test_ibis_json_compat_store_simple.py	Basic tests verifying abstract class behavior and dict-based engine configuration

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[danielgafni]

I think in principle it makes sense.

My main problem with this PR is code quality, and the volume of LOC, it's just hard to review it this way.

Please do a pass and refactor bad code like hasattr checks or unnecessary error handling - even Opus 4.5 will help a lot here.

Consider splitting the PR to introduce the versioning engine and the store separately.

Now, the most important thing I would like to see is a sanity check: what if we implement DuckDB store with this new json code path?

It should work in the same way as the existing store.

It would be a very easy and convincing way to verify the correctness of this huge PR.

Let's add this test. This json-compatible DuckDB class can stay in _testing. It would be much better than mocked tests (I never trust them).

[geoHeil]

we should have a duckdb json test actually in the massive PG PR

[danielgafni]

we should have a duckdb json test actually in the massive PG PR

Well, it should be moved here, tests should not be in some other PRs

[danielgafni]

The tests are very much not convincing. These are typical AI tests, very lengthy and test almost nothing, mostly small scope unit tests and mocks.

I was able to count one write_metadata_to_store and zero write_metadata in these tests.

A very easy thing to do is to add DuckDBJsonCompatStore to AllStoreCases. It would run it through our entire test suite.

[geoHeil]

The tests are very much not convincing. These are typical AI tests, very lengthy and test almost nothing, mostly small scope unit tests and mocks.

I was able to count one write_metadata_to_store and zero write_metadata in these tests.

A very easy thing to do is to add DuckDBJsonCompatStore to AllStoreCases. It would run it through our entire test suite.

I added this.