feat: replace predicate_object with materialized view

[simonas-notcat]

No description provided.

[claude]

Pull Request Review: Replace predicate_object with Materialized View

Overview

This PR replaces the table-based predicate_object and subject_predicate aggregation approach with materialized views. This is a significant architectural change that trades incremental updates for simpler, query-based maintenance.

Code Quality & Best Practices

Strengths

✅ Cleaner architecture: Eliminates 145+ lines of complex trigger logic for maintaining aggregate tables
✅ Simpler maintenance: Materialized views derive data from source tables, reducing data redundancy
✅ Concurrent refresh support: Unique indexes on both views enable REFRESH MATERIALIZED VIEW CONCURRENTLY
✅ Proper indexing: Maintains existing indexes on predicate_id and object_id for query performance

Areas of Concern

1. CRITICAL: Performance Impact of Statement-Level Trigger

Location: infrastructure/hasura/migrations/intuition/1729947331635_consolidated_triggers/up.sql:674

CREATE TRIGGER triple_term_refresh_views_trigger
AFTER INSERT OR UPDATE OR DELETE ON triple_term
FOR EACH STATEMENT
EXECUTE FUNCTION refresh_predicate_object_subject_predicate_views();

Issue: The trigger performs a full refresh of both materialized views on every triple_term change, regardless of how small the change is. Based on the codebase, triple_term is updated frequently via:

• update_triple_term_from_vault() (line 182, 256)
• Vault updates trigger cascading updates to triple_term

Impact:

• Every deposit/redemption that updates a vault → updates triple_term → triggers full view refresh
• With large datasets, this could cause significant latency spikes
• Concurrent refresh helps but still locks briefly and scans entire tables

Recommendation: Consider one of these approaches:

1. Batch refresh: Use a scheduled job (e.g., via pg_cron) to refresh views periodically instead of on every change
2. Incremental maintenance: Keep simplified triggers that update only affected rows (may reduce the benefit of the refactor)
3. Debouncing: Implement a queue-based refresh mechanism that debounces rapid updates

2. Missing Down Migration

Location: infrastructure/hasura/migrations/intuition/1729947331633_consolidated_basic_structure/down.sql:45

Issue: The down migration still attempts to:

DROP TABLE IF EXISTS predicate_object;

But these are now materialized views, not tables. The down migration should use:

DROP MATERIALIZED VIEW IF EXISTS predicate_object;
DROP MATERIALIZED VIEW IF EXISTS subject_predicate;

Impact: Rollback migrations will fail, making it difficult to revert this change in production if issues arise.

3. Missing Index Cleanup in Trigger Down Migration

Location: infrastructure/hasura/migrations/intuition/1729947331635_consolidated_triggers/down.sql

Issue: The down migration drops old triggers/functions but doesn't address the new trigger:

• Missing: DROP TRIGGER IF EXISTS triple_term_refresh_views_trigger ON triple_term;
• Missing: DROP FUNCTION IF EXISTS refresh_predicate_object_subject_predicate_views();

Old references still exist:

DROP TRIGGER IF EXISTS triple_predicate_object_trigger ON triple;  -- Line 26
DROP FUNCTION IF EXISTS update_predicate_object_on_triple_insert(); -- Line 45

Impact: Incomplete rollback capability; old function/trigger names no longer exist in up migration.

4. Data Migration Strategy

Question: How is existing data in the predicate_object and subject_predicate tables being handled during migration?

The migration:

1. Drops the tables (in structure change)
2. Creates materialized views with the same name
3. The views will be empty until first refresh

Recommendation: The migration should include:

-- After creating the materialized views
REFRESH MATERIALIZED VIEW predicate_object;
REFRESH MATERIALIZED VIEW subject_predicate;

This ensures the views are populated immediately and queries don't return empty results.

Security Concerns

⚠️ Concurrent Refresh Locks: While REFRESH MATERIALIZED VIEW CONCURRENTLY avoids full locks, it still requires a brief exclusive lock to swap the new data. High-frequency refreshes could cause lock contention.

No direct security vulnerabilities identified. The change maintains the same data access patterns.

Test Coverage

❓ Testing Strategy: The PR has no description of test coverage. Consider:

1. Performance testing: Measure refresh time with production-scale data
2. Load testing: Verify system behavior under concurrent vault updates
3. Migration testing: Test both up and down migrations on staging data
4. Query performance: Ensure materialized view queries maintain or improve performance vs. tables

Performance Considerations Summary

Aspect	Previous (Tables)	New (Materialized Views)
Write path	Per-row trigger complexity	Simple writes, expensive refresh
Read path	Direct table reads	Direct view reads (same)
Data freshness	Always current	Current only after refresh
Maintenance	Complex trigger logic	Simple view definition
Scalability	Good for frequent writes	Poor for frequent refreshes

Recommendations

Must Fix

1. ✅ Update down migration to use DROP MATERIALIZED VIEW instead of DROP TABLE
2. ✅ Add initial REFRESH MATERIALIZED VIEW statements to populate views after creation
3. ✅ Update trigger down migration to remove new trigger/function

Should Consider

4. 🔄 Re-evaluate the statement-level trigger approach - consider batch/scheduled refreshes
5. 📊 Add performance benchmarks comparing before/after with realistic data volumes
6. 📝 Document expected refresh frequency and impact on data freshness

Nice to Have

7. 📈 Add monitoring for materialized view refresh duration
8. 🔧 Consider adding a manual refresh endpoint for operational flexibility

Overall Assessment

This is a promising simplification that could reduce maintenance burden, but the performance implications need careful evaluation. The statement-level trigger refreshing entire views on every triple_term change is likely too aggressive for high-throughput scenarios.

Recommendation: Address the critical issues (down migration, initial refresh) before merging, and strongly consider moving to scheduled/batch refreshes rather than trigger-based refreshes.

---

Happy to discuss any of these points further! 🚀