Skip to content
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

Fix the way replace_metadata drops event triggers (fix #5461) #6137

Merged
merged 6 commits into from Nov 10, 2020

Conversation

lexi-lambda
Copy link
Contributor

@lexi-lambda lexi-lambda commented Nov 5, 2020

Description

This PR fixes #5461 by changing the way replace_metadata updates event triggers. Instead of dropping and recreating them, HGE now performs a diff-and-patch against the existing metadata, which goes through the appropriate code paths to update the associated SQL functions and archive events.

Affected components

  • Server

Related Issues

#5461

Server checklist

Metadata

Does this PR add a new Metadata feature?

  • No

GraphQL

  • No new GraphQL schema is generated

Breaking changes

  • No Breaking changes

@netlify
Copy link

netlify bot commented Nov 5, 2020

Deploy preview for hasura-docs ready!

Built with commit c5a4bbf

https://deploy-preview-6137--hasura-docs.netlify.app

@0x777 0x777 requested a review from rakeshkky November 9, 2020 13:10
@tirumaraiselvan tirumaraiselvan marked this pull request as ready for review November 10, 2020 06:19
@tirumaraiselvan tirumaraiselvan requested a review from a team as a code owner November 10, 2020 06:19
Previously, we were relying on "ON UPDATE CASCADE" on the foreign key constraint to do this automatically.
Copy link
Contributor

@codingkarthik codingkarthik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rakeshkky rakeshkky self-assigned this Nov 10, 2020
@rakeshkky
Copy link
Member

rakeshkky commented Nov 10, 2020

@lexi-lambda
With the metadata storage separation (#5797) the migration present in the PR becomes obsolete and event triggers are no longer stored in hdb_catalog.event_triggers. I'll take this up. Assigning myself.

cc @tirumaraiselvan

@tirumaraiselvan
Copy link
Contributor

@rakeshkky This PR should be merged regardless because we need to cherry-pick it for v1.3.3

The new event trigger changes should happen on top of this.

@tirumaraiselvan
Copy link
Contributor

tirumaraiselvan commented Nov 10, 2020

test_server_upgrade is failing because circleci/postgres:11-alpine-postgis has a different FK constraint name:

postgres=# \d hdb_catalog.event_triggers;
          Table "hdb_catalog.event_triggers"
    Column     | Type | Collation | Nullable | Default 
---------------+------+-----------+----------+---------
 name          | text |           | not null | 
 type          | text |           | not null | 
 schema_name   | text |           | not null | 
 table_name    | text |           | not null | 
 configuration | json |           |          | 
 comment       | text |           |          | 
Indexes:
    "event_triggers_pkey" PRIMARY KEY, btree (name)
Foreign-key constraints:
    "event_triggers_schema_name_fkey" FOREIGN KEY (schema_name, table_name) REFERENCES hdb_catalog.hdb_table(table_schema, table_name) ON UPDATE CASCADE

@rakeshkky rakeshkky removed their assignment Nov 10, 2020
Copy link
Member

@rakeshkky rakeshkky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server LGTM. However, these changes will become obsolete with metadata storage separation (see #5797).

@kodiakhq kodiakhq bot merged commit 6787a1b into hasura:master Nov 10, 2020
codingkarthik pushed a commit to codingkarthik/graphql-engine that referenced this pull request Nov 10, 2020
codingkarthik pushed a commit to codingkarthik/graphql-engine that referenced this pull request Nov 10, 2020
codingkarthik pushed a commit to codingkarthik/graphql-engine that referenced this pull request Nov 10, 2020
@lexi-lambda lexi-lambda deleted the sync-event-triggers-gh-5461 branch November 10, 2020 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"table or event-trigger not found in schema cache" error in logs
4 participants