splinter icon indicating copy to clipboard operation
splinter copied to clipboard

Lint to detect missing event triggers for PostgREST automatic schema cache reloading

Open steve-chavez opened this issue 1 year ago • 3 comments

Problem

A user was complaining about supabase-js/PostgREST failing to detect new table changes. The root cause was the user somehow wiped the functions + event triggers (maybe Prisma or other ORM that drops the db).

The functions are documented in https://postgrest.org/en/v12/references/schema_cache.html#finer-grained-event-trigger. On Supabase they're expected to be on the extensions schema.

  • extensions.pgrst_ddl_watch
  • extensions.pgrst_drop_watch

Proposal

Could we WARN whenever the expected triggers are missing?

Note

The issue_graphql_placeholder trigger was also missing for this user, so perhaps this can be a lint for all the expected event triggers.

steve-chavez avatar Mar 29 '24 16:03 steve-chavez

What's the expected remediation for this? The user won't have super so they can't create an event trigger.

Would we tell them to reach out to support? If so, would it make more sense to scan internally and resolve it proactively?

olirice avatar Mar 29 '24 17:03 olirice

Yes, I solved it manually for the user.

The thing is that:

drop event trigger pgrst_drop_watch; -- fails
-- ERROR:  42501: must be owner of event trigger pgrst_drop_watch

drop function extensions.pgrst_drop_watch; --fails
-- ERROR:  2BP01: cannot drop function pgrst_drop_watch() because other objects depend on it
-- DETAIL:  event trigger pgrst_drop_watch depends on function pgrst_drop_watch()
-- HINT:  Use DROP ... CASCADE to drop the dependent objects too.

drop function extensions.pgrst_drop_watch cascade; -- succeeds and drops the event trigger 

Which is really surprising behavior. It seems like a pg bug to me. Maybe we can work around it on supautils?

steve-chavez avatar Mar 29 '24 18:03 steve-chavez

If so, would it make more sense to scan internally and resolve it proactively?

Right, this probably doesn't make sense as a lint then.

steve-chavez avatar Mar 30 '24 21:03 steve-chavez

Added to internal metrics here https://github.com/supabase/data-engineering/pull/651

olirice avatar Jul 10 '24 17:07 olirice