hard deletions

[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

• deletion documentation #511
• deletion dagster integration #510
• hard deletions cli integration #509
• cleanup ibis/clickhouse and use metadata for deletions where available #508
• hard delete lance #507
• hard delete delta #506
• hard delete for ibis #505
• hard deletions #504 👈 (View in Graphite)
• [core] add metaxy_deleted_at column #502
• main

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

[github-actions]

Coverage Report (Python 3.12) •

File	Stmts	Miss	Cover	Missing
metadata_store
base.py	505	155	69%	3, 5–11, 13–17, 19–21, 27–29, 34, 38, 51–53, 58–60, 67, 70, 89, 91, 96, 101, 107, 112, 124, 152, 159, 162, 164, 234–235, 248–249, 262, 383, 444, 467, 553, 655, 712, 741, 743–744, 804, 881, 964–965, 1008–1009, 1012–1013, 1030, 1060–1061, 1066–1067, 1088, 1093, 1105, 1110, 1142, 1159, 1177, 1188, 1202, 1226, 1240, 1244, 1257, 1267–1268, 1292, 1319, 1354, 1388, 1445, 1519, 1535, 1542, 1550, 1556, 1560, 1596, 1618, 1644–1645, 1658, 1680, 1682–1683, 1686, 1729, 1758, 1783, 1817, 1843, 1856, 1858, 1915, 1994, 1996, 1999–2000, 2003–2004, 2008, 2018, 2031, 2033, 2035–2037, 2042–2043, 2046–2047, 2049–2051, 2053, 2056–2057, 2060–2063, 2066, 2075–2076, 2078–2079, 2082, 2085, 2087–2089, 2093–2094, 2097–2099, 2102, 2104, 2108
delta.py	123	13	89%	114, 128, 131, 164–165, 227, 271, 301, 304–305, 308, 316, 366
duckdb.py	160	30	81%	84–90, 92, 203–204, 208, 276, 286–288, 290, 298–299, 320, 325, 330, 344–345, 356, 361, 419, 426, 432, 437, 450
ibis.py	176	39	77%	205, 224, 375–376, 378, 420, 438, 469, 472–473, 476, 479, 482–483, 486, 489, 493, 496–497, 500, 503, 509, 511, 517–518, 520, 524, 526, 529, 531–532, 534, 538, 540–541, 562, 606, 674, 701
lancedb.py	105	9	91%	158–160, 163, 309–311, 361, 374
memory.py	104	46	55%	3–5, 7–10, 12–17, 20, 33, 36, 64–65, 67, 78, 82, 86–87, 96, 99, 101–102, 105, 107, 112, 139, 141, 149, 151, 169, 181, 189, 196, 201, 240, 257, 267–268, 306, 311–312
models
types.py	152	81	46%	3, 5–6, 16, 30, 33–34, 37, 46–48, 51, 54, 65, 70, 88–90, 100–104, 114–115, 117, 136–138, 158, 185, 192–193, 201–202, 206, 210, 214, 216, 222, 226, 230, 232, 236, 238, 242, 244, 248, 250, 254–255, 263, 267, 271, 275, 283, 320, 324, 328, 334, 339, 376, 380, 384, 391, 393, 396, 401, 437, 468, 471, 475, 483, 492, 495, 502, 507, 513, 516, 520, 522
TOTAL	8674	3441	60%

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

[github-actions]

Coverage Report (Python 3.11) •

File	Stmts	Miss	Cover	Missing
metadata_store
base.py	505	155	69%	3, 5–11, 13–17, 19–21, 27–29, 34, 38, 51–53, 58–60, 67, 70, 89, 91, 96, 101, 107, 112, 124, 152, 159, 162, 164, 234–235, 248–249, 262, 383, 444, 467, 553, 655, 712, 741, 743–744, 804, 881, 964–965, 1008–1009, 1012–1013, 1030, 1060–1061, 1066–1067, 1088, 1093, 1105, 1110, 1142, 1159, 1177, 1188, 1202, 1226, 1240, 1244, 1257, 1267–1268, 1292, 1319, 1354, 1388, 1445, 1519, 1535, 1542, 1550, 1556, 1560, 1596, 1618, 1644–1645, 1658, 1680, 1682–1683, 1686, 1729, 1758, 1783, 1817, 1843, 1856, 1858, 1915, 1994, 1996, 1999–2000, 2003–2004, 2008, 2018, 2031, 2033, 2035–2037, 2042–2043, 2046–2047, 2049–2051, 2053, 2056–2057, 2060–2063, 2066, 2075–2076, 2078–2079, 2082, 2085, 2087–2089, 2093–2094, 2097–2099, 2102, 2104, 2108
delta.py	123	13	89%	114, 128, 131, 164–165, 227, 271, 301, 304–305, 308, 316, 366
duckdb.py	160	30	81%	84–90, 92, 203–204, 208, 276, 286–288, 290, 298–299, 320, 325, 330, 344–345, 356, 361, 419, 426, 432, 437, 450
ibis.py	176	39	77%	205, 224, 375–376, 378, 420, 438, 469, 472–473, 476, 479, 482–483, 486, 489, 493, 496–497, 500, 503, 509, 511, 517–518, 520, 524, 526, 529, 531–532, 534, 538, 540–541, 562, 606, 674, 701
lancedb.py	105	9	91%	158–160, 163, 309–311, 361, 374
memory.py	104	46	55%	3–5, 7–10, 12–17, 20, 33, 36, 64–65, 67, 78, 82, 86–87, 96, 99, 101–102, 105, 107, 112, 139, 141, 149, 151, 169, 181, 189, 196, 201, 240, 257, 267–268, 306, 311–312
models
types.py	152	81	46%	3, 5–6, 16, 30, 33–34, 37, 46–48, 51, 54, 65, 70, 88–90, 100–104, 114–115, 117, 136–138, 158, 185, 192–193, 201–202, 206, 210, 214, 216, 222, 226, 230, 232, 236, 238, 242, 244, 248, 250, 254–255, 263, 267, 271, 275, 283, 320, 324, 328, 334, 339, 376, 380, 384, 391, 393, 396, 401, 437, 468, 471, 475, 483, 492, 495, 502, 507, 513, 516, 520, 522
TOTAL	8674	3441	60%

Tests	Skipped	Failures	Errors	Time
1999	46 💤	0 ❌	0 🔥	6m 33s ⏱️

[github-actions]

Test Results (Python 3.10)

1 999 tests  +30   1 953 ✅ + 6   6m 54s ⏱️ +21s
    1 suites ± 0      46 💤 +24 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 3a3a985. ± Comparison against base commit ab1b584.

♻️ This comment has been updated with latest results.

[github-actions]

Coverage Report (Python 3.10) •

File	Stmts	Miss	Cover	Missing
metadata_store
base.py	505	155	69%	3, 5–11, 13–17, 19–21, 27–29, 34, 38, 51–53, 58–60, 67, 70, 89, 91, 96, 101, 107, 112, 124, 152, 159, 162, 164, 234–235, 248–249, 262, 383, 444, 467, 553, 655, 712, 741, 743–744, 804, 881, 964–965, 1008–1009, 1012–1013, 1030, 1060–1061, 1066–1067, 1088, 1093, 1105, 1110, 1142, 1159, 1177, 1188, 1202, 1226, 1240, 1244, 1257, 1267–1268, 1292, 1319, 1354, 1388, 1445, 1519, 1535, 1542, 1550, 1556, 1560, 1596, 1618, 1644–1645, 1658, 1680, 1682–1683, 1686, 1729, 1758, 1783, 1817, 1843, 1856, 1858, 1915, 1994, 1996, 1999–2000, 2003–2004, 2008, 2018, 2031, 2033, 2035–2037, 2042–2043, 2046–2047, 2049–2051, 2053, 2056–2057, 2060–2063, 2066, 2075–2076, 2078–2079, 2082, 2085, 2087–2089, 2093–2094, 2097–2099, 2102, 2104, 2108
delta.py	123	13	89%	114, 128, 131, 164–165, 227, 271, 301, 304–305, 308, 316, 366
duckdb.py	160	30	81%	84–90, 92, 203–204, 208, 276, 286–288, 290, 298–299, 320, 325, 330, 344–345, 356, 361, 419, 426, 432, 437, 450
ibis.py	176	39	77%	205, 224, 375–376, 378, 420, 438, 469, 472–473, 476, 479, 482–483, 486, 489, 493, 496–497, 500, 503, 509, 511, 517–518, 520, 524, 526, 529, 531–532, 534, 538, 540–541, 562, 606, 674, 701
lancedb.py	105	9	91%	158–160, 163, 309–311, 361, 374
memory.py	104	46	55%	3–5, 7–10, 12–17, 20, 33, 36, 64–65, 67, 78, 82, 86–87, 96, 99, 101–102, 105, 107, 112, 139, 141, 149, 151, 169, 181, 189, 196, 201, 240, 257, 267–268, 306, 311–312
models
types.py	152	81	46%	3, 5–6, 16, 30, 33–34, 37, 46–48, 51, 54, 65, 70, 88–90, 100–104, 114–115, 117, 136–138, 158, 185, 192–193, 201–202, 206, 210, 214, 216, 222, 226, 230, 232, 236, 238, 242, 244, 248, 250, 254–255, 263, 267, 271, 275, 283, 320, 324, 328, 334, 339, 376, 380, 384, 391, 393, 396, 401, 437, 468, 471, 475, 483, 492, 495, 502, 507, 513, 516, 520, 522
TOTAL	8674	3441	60%

Tests	Skipped	Failures	Errors	Time
1999	46 💤	0 ❌	0 🔥	6m 54s ⏱️

[github-actions]

Coverage Report (Python 3.13) •

File	Stmts	Miss	Cover	Missing
metadata_store
base.py	505	155	69%	3, 5–11, 13–17, 19–21, 27–29, 34, 38, 51–53, 58–60, 67, 70, 89, 91, 96, 101, 107, 112, 124, 152, 159, 162, 164, 234–235, 248–249, 262, 383, 444, 467, 553, 655, 712, 741, 743–744, 804, 881, 964–965, 1008–1009, 1012–1013, 1030, 1060–1061, 1066–1067, 1088, 1093, 1105, 1110, 1142, 1159, 1177, 1188, 1202, 1226, 1240, 1244, 1257, 1267–1268, 1292, 1319, 1354, 1388, 1445, 1519, 1535, 1542, 1550, 1556, 1560, 1596, 1618, 1644–1645, 1658, 1680, 1682–1683, 1686, 1729, 1758, 1783, 1817, 1843, 1856, 1858, 1915, 1994, 1996, 1999–2000, 2003–2004, 2008, 2018, 2031, 2033, 2035–2037, 2042–2043, 2046–2047, 2049–2051, 2053, 2056–2057, 2060–2063, 2066, 2075–2076, 2078–2079, 2082, 2085, 2087–2089, 2093–2094, 2097–2099, 2102, 2104, 2108
delta.py	123	13	89%	114, 128, 131, 164–165, 227, 271, 301, 304–305, 308, 316, 366
duckdb.py	160	30	81%	84–90, 92, 203–204, 208, 276, 286–288, 290, 298–299, 320, 325, 330, 344–345, 356, 361, 419, 426, 432, 437, 450
ibis.py	176	39	77%	205, 224, 375–376, 378, 420, 438, 469, 472–473, 476, 479, 482–483, 486, 489, 493, 496–497, 500, 503, 509, 511, 517–518, 520, 524, 526, 529, 531–532, 534, 538, 540–541, 562, 606, 674, 701
lancedb.py	105	9	91%	158–160, 163, 309–311, 361, 374
memory.py	104	46	55%	3–5, 7–10, 12–17, 20, 33, 36, 64–65, 67, 78, 82, 86–87, 96, 99, 101–102, 105, 107, 112, 139, 141, 149, 151, 169, 181, 189, 196, 201, 240, 257, 267–268, 306, 311–312
models
types.py	152	81	46%	3, 5–6, 16, 30, 33–34, 37, 46–48, 51, 54, 65, 70, 88–90, 100–104, 114–115, 117, 136–138, 158, 185, 192–193, 201–202, 206, 210, 214, 216, 222, 226, 230, 232, 236, 238, 242, 244, 248, 250, 254–255, 263, 267, 271, 275, 283, 320, 324, 328, 334, 339, 376, 380, 384, 391, 393, 396, 401, 437, 468, 471, 475, 483, 492, 495, 502, 507, 513, 516, 520, 522
TOTAL	8674	3441	60%

Tests	Skipped	Failures	Errors	Time
1999	46 💤	0 ❌	0 🔥	6m 29s ⏱️

[danielgafni]

Maybe instead just @pytest.mark.xfail(raises=NotImplementedError) the entire test?

[geoHeil]

we later want to implement integration by integration so being able to remoe/choose which ones work

makes sense for me

is there a better way?

[danielgafni]

IMO these deletion tests (the one from the previous PR as well) should be moved into other testing modules, this is called test_provenance_golden because it's testing... provenance engines

[geoHeil]

but then we would have to copy the test initialization logic as well or externalize it?

[geoHeil]

isnt the deletion also a part of the provenance i.e. especially in the case of soft deletions where we add rows and want to ensure that the right data is read?

[danielgafni]

Hmm ok that's a good point, actually to verify this we need to attempt to call .resolve_update for the feature and make sure increment.deleted is calculated correctly

[danielgafni]

Alright, looking better, let's drop the DeletionResult thing for now?
Check my comment for reasoning

[danielgafni]

hmm why exactly do we need this collect? is it just to return the row_count? probably we should avoid calculating the row count in this case.

We could get back to this later once we implement the metaxy_metadata column with arbitrary key-value pairs, we could include a soft_deletion_id there for backtracking the row count.

[geoHeil]

We should generalize that - also for hard deletions and ideally from the DB driver retrieve the affected/deleted records.

[geoHeil]

But we have to collect it somewhere: df: IntoFrame, for write_metadata is not lazy. So by dropping the Deletionresult we already got rid of a couple of collects. Is it fine to keep this one here for starters?

[danielgafni]

Another thing: this PR currently implements both hard and soft deletes.

But only hard deletes are tested.

Could you either split the PR or add soft deletion tests?

For example, ensure that soft-deleting historical feature versions does not affect the current feature versions.

Ensure that writing new metadata on top of soft-deleted rows properly undo the soft-deletion.

Same with hard deletes.

[geoHeil]

Another thing: this PR currently implements both hard and soft deletes.

But only hard deletes are tested.

Could you either split the PR or add soft deletion tests?

For example, ensure that soft-deleting historical feature versions does not affect the current feature versions.

Ensure that writing new metadata on top of soft-deleted rows properly undo the soft-deletion.

Same with hard deletes.

can you explain what you mean. I think the soft deletes are downstack and have their test - and hard deletes here have their own tests see

metaxy/tests/metadata_stores/test_provenance_golden_reference.py

Lines 314 to 407 in 89cc2da

	def test_soft_delete(any_store: MetadataStore):
	"""Soft delete for stores that support it."""
	unsupported = {
	"ClickHouseMetadataStore",
	"LanceDBMetadataStore",
	}
	if any_store.__class__.__name__ in unsupported:
	pytest.xfail(f"Hard delete pending for {any_store.__class__.__name__}")
	class UserProfile(
	SampleFeature,
	spec=SampleFeatureSpec(
	key="hard_delete_profile",
	fields=["email", "status"],
	),
	):
	email: str | None = None
	status: str | None = None
	df = pl.DataFrame(
	{
	"sample_uid": ["u1", "u2", "u3"],
	"email": ["a@example.com", "b@example.com", "c@example.com"],
	"status": ["active", "active", "inactive"],
	METAXY_PROVENANCE_BY_FIELD: [
	{"email": "p1", "status": "p1"},
	{"email": "p2", "status": "p2"},
	{"email": "p3", "status": "p3"},
	],
	}
	)
	with any_store.open("write"):
	any_store.write_metadata(UserProfile, df)
	result = any_store.delete_metadata(
	UserProfile, filters=nw.col("status") == "inactive"
	)
	assert result.row_count == 1
	with any_store:
	remaining = any_store.read_metadata(UserProfile).collect().to_polars()
	assert set(remaining["sample_uid"]) == {"u1", "u2"}
	with_deleted = (
	any_store.read_metadata(UserProfile, include_soft_deleted=True)
	.collect()
	.to_polars()
	)
	assert set(with_deleted["sample_uid"]) == {"u1", "u2", "u3"}
	def test_hard_delete(any_store: MetadataStore):
	"""Hard delete removes rows from storage."""
	if any_store.__class__.__name__ != "InMemoryMetadataStore":
	pytest.xfail("Hard delete pending for non-memory backends")
	class UserProfile(
	SampleFeature,
	spec=SampleFeatureSpec(
	key="hard_delete_profile",
	fields=["email", "status"],
	),
	):
	email: str | None = None
	status: str | None = None
	df = pl.DataFrame(
	{
	"sample_uid": ["u1", "u2", "u3"],
	"email": ["a@example.com", "b@example.com", "c@example.com"],
	"status": ["active", "active", "inactive"],
	METAXY_PROVENANCE_BY_FIELD: [
	{"email": "p1", "status": "p1"},
	{"email": "p2", "status": "p2"},
	{"email": "p3", "status": "p3"},
	],
	}
	)
	with any_store.open("write"):
	any_store.write_metadata(UserProfile, df)
	result = any_store.delete_metadata(
	UserProfile, filters=nw.col("status") == "inactive", soft=False
	)
	assert result.row_count == 1
	with any_store:
	remaining = any_store.read_metadata(UserProfile).collect().to_polars()
	assert remaining.height == 2
	assert set(remaining["sample_uid"]) == {"u1", "u2"}