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

Handle CONSTRAINT TRIGGER on DB Manager/PostgreSQL. #35983

Merged
merged 2 commits into from Apr 27, 2020

Conversation

espinafre
Copy link
Contributor

Fixes #35967

Despite appearing as constraints on the catalog pg_constraint, they are not really constraints, but rather a special kind of trigger, as seen here: https://www.postgresql.org/docs/12/catalog-pg-constraint.html and here: https://www.postgresql.org/docs/12/sql-createtrigger.html .

The following table/trigger shows the issue:

CREATE FUNCTION hi_trigger() RETURNS TRIGGER AS $$ BEGIN RAISE NOTICE 'Hello world'; RETURN NEW; END; $$ LANGUAGE plpgsql;
CREATE TABLE test_trigger(id SERIAL NOT NULL PRIMARY KEY, value varchar(32));
CREATE CONSTRAINT TRIGGER tg_test AFTER INSERT ON test_trigger DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE hi_trigger();

All the other constraint types that might appear on the catalog pg_constraint according to the PostgreSQL documentation are handled by DB Manager:

types = {"c": TypeCheck, "f": TypeForeignKey, "p": TypePrimaryKey, "u": TypeUnique, "x": TypeExclusion}

Those are SQL standard, the CONSTRAINT TRIGGER is a PostgreSQL extension (according to the PostgreSQL docs), and I think it is not really a constraint in the traditional sense.

I believe this should be backported to 3.12 and 3.10.

@github-actions github-actions bot added this to the 3.14.0 milestone Apr 24, 2020
@espinafre
Copy link
Contributor Author

@gioman , all green. Does it look good to you?

@gioman
Copy link
Contributor

gioman commented Apr 25, 2020

@gioman , all green. Does it look good to you?

I'm not a reviewer. A dev will do it.

@espinafre
Copy link
Contributor Author

@gioman , all green. Does it look good to you?

I'm not a reviewer. A dev will do it.

Oh, I thought you were, sorry. Maybe @m-kuhn could take a look at this, since I've recently pestered him about another PR?

@gioman
Copy link
Contributor

gioman commented Apr 27, 2020

@espinafre is this PR safe also when using old PostgreSQL releases like 9.6?

@m-kuhn
Copy link
Member

m-kuhn commented Apr 27, 2020

@gioman it should be safe, but a test of current master before merging the backport would be appreciated. Please note that the 3.10 backport has been frozen for a dev cycle to have time to deal with potential issues before merging.

@gioman
Copy link
Contributor

gioman commented Apr 27, 2020

it should be safe, but a test of current master before merging the backport would be appreciated

@m-kuhn sure. Master in the nightly repo for Ubuntu bionic is stuck at code revision from 16 days ago, any idea why? At home with a tiny laptop compiling is not really an option.

@m-kuhn
Copy link
Member

m-kuhn commented Apr 27, 2020

Maybe @jef-n knows more?

@jef-n
Copy link
Member

jef-n commented Apr 27, 2020

Maybe @jef-n knows more?

the addition of qgis_process broke the packaging process - fixed in 8ee4d05 - the current build should complete (http://qgis2.qgis.org:1080/cgi-bin/build.sh)

@gioman
Copy link
Contributor

gioman commented Apr 27, 2020

the addition of qgis_process broke the packaging process - fixed in 8ee4d05 - the current build should complete (http://qgis2.qgis.org:1080/cgi-bin/build.sh)

@jef-n thanks!

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.

DB Manager/PostgreSQL doesn't show tables with CONSTRAINT TRIGGER
4 participants