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

Add check for not valid constraints #362

Closed
mfvanek opened this issue Apr 7, 2024 · 13 comments
Closed

Add check for not valid constraints #362

mfvanek opened this issue Apr 7, 2024 · 13 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mfvanek
Copy link
Owner

mfvanek commented Apr 7, 2024

See article https://habr.com/ru/articles/800121/

SELECT
	t.relname, -- наименование отношения
    c.conname, -- наименование ограничения
    c.contype  -- тип ограничения 
FROM pg_catalog.pg_constraint AS c
INNER JOIN pg_catalog.pg_class AS t
    ON t.oid = c.conrelid AND c.contype IN ('c', 'f') 
        AND (NOT c.convalidated);
@mfvanek mfvanek added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Apr 7, 2024
@BLoHny
Copy link
Collaborator

BLoHny commented Apr 8, 2024

I have a question regarding this issue; may I ask it?

Should I add content to the final class named Validators in the validation package of the pg-index-health-model module and write test code for it?

@mfvanek
Copy link
Owner Author

mfvanek commented Apr 8, 2024

Hi @BLoHny

Should I add content to the final class named Validators in the validation package of the pg-index-health-model module and write test code for it?

No, you shouldn't

Please take a look at

There I briefly described things you need to do to implement a new check

@BLoHny
Copy link
Collaborator

BLoHny commented Apr 9, 2024

would this satisfy your request?

SELECT
    t.relname AS table_name, -- Name of the table
    c.conname AS constraint_name, -- Name of the constraint
    c.contype AS constraint_type  -- Type of the constraint
FROM pg_catalog.pg_constraint c
JOIN pg_catalog.pg_class t ON t.oid = c.conrelid
WHERE NOT c.convalidated; -- Constraints that have not yet been validated

@mfvanek
Copy link
Owner Author

mfvanek commented Apr 9, 2024

would this satisfy your request?

It's almost ready)

I think we need to keep filtering on c.contype IN ('c', 'f') in where-clause.
Other constraint types usually aren't interested for us.
See https://www.postgresql.org/docs/current/catalog-pg-constraint.html

Also all sql queries have to be schema-aware
https://github.com/mfvanek/pg-index-health-sql/blob/6a5b823d2f86f3fed946f073de93a20245b8d312/sql/foreign_keys_without_index.sql#L22

I use SQLFluff for analysing sql queries syntax and codestyle
https://github.com/mfvanek/pg-index-health-sql/blob/master/.github/linters/.sqlfluff
Please reformat your query with my custom rules.

P.S. Then in Java code we need to implement a new base class for database constraints.
Existing class ForeignKey should be derived from that new class
https://github.com/mfvanek/pg-index-health/blob/master/pg-index-health-model/src/main/java/io/github/mfvanek/pg/model/constraint/ForeignKey.java

@BLoHny
Copy link
Collaborator

BLoHny commented Apr 9, 2024

After understanding it, I applied it. Which sql query more satisfies your request?

SELECT
    t.relname AS table_name, -- Name of the table
    c.conname AS constraint_name, -- Name of the constraint
    c.contype AS constraint_type  -- Type of the constraint
FROM
    pg_catalog.pg_constraint c
    JOIN pg_catalog.pg_class t ON t.oid = c.conrelid
    JOIN pg_catalog.pg_namespace n ON n.oid = t.relnamespace
WHERE
    NOT c.convalidated -- Constraints that have not yet been validated
    AND c.contype IN ('c', 'f') -- Focus on check and foreign key constraints
    AND n.nspname = :schema_name_param::text; -- Make the query schema-aware

or

SELECT
    t.relname AS table_name, -- Name of the table
    c.conname AS constraint_name, -- Name of the constraint
    c.contype AS constraint_type, -- Type of the constraint
    CASE c.contype 
        WHEN 'c' THEN 'Check constraint'
        WHEN 'f' THEN 'Foreign key constraint'
        WHEN 'p' THEN 'Primary key constraint'
        WHEN 'u' THEN 'Unique constraint'
        WHEN 't' THEN 'Constraint trigger'
        WHEN 'x' THEN 'Exclusion constraint'
        ELSE 'Unknown' 
    END AS constraint_type_description, -- Description of the constraint type
    n.nspname AS schema_name -- Name of the schema
FROM
    pg_catalog.pg_constraint c
JOIN
    pg_catalog.pg_class t ON c.conrelid = t.oid
JOIN
    pg_catalog.pg_namespace n ON t.relnamespace = n.oid
WHERE
    n.nspname = :schema_name_param::text -- Recognizing the schema
    AND NOT c.convalidated -- Constraints that have not yet been validated
    AND c.contype IN ('c', 'f'); -- Focusing on specific types of constraints

@mfvanek
Copy link
Owner Author

mfvanek commented Apr 10, 2024

Which sql query more satisfies your request?

I like the first variant more

@BLoHny
Copy link
Collaborator

BLoHny commented Apr 25, 2024

I have a question.
Should we now create a new class for checking whether constraints are not valid in checks.host package of pg-index-health?

@mfvanek
Copy link
Owner Author

mfvanek commented Apr 25, 2024

@BLoHny

Should we now create a new class for checking whether constraints are not valid in checks.host package of pg-index-health?

Yes, we should.
Please take a look at https://github.com/mfvanek/pg-index-health/blob/master/CONTRIBUTING.md#implementing-a-new-check

First of all you need to add a new element to

@mfvanek
Copy link
Owner Author

mfvanek commented Apr 27, 2024

Hi @BLoHny,
please take a look at updated query https://github.com/mfvanek/pg-index-health-sql/blob/master/sql/check_not_valid_constraints.sql
I've fixed it a bit: added order by and changed getting table name.

Query was tested with

create schema demo;

CREATE TABLE demo.c1001_1
(
    id    integer GENERATED ALWAYS AS IDENTITY NOT NULL,
    parent_id integer NOT NULL,
    value integer NOT NULL,
    CONSTRAINT c1001_1_pk PRIMARY KEY (id)
);

ALTER TABLE demo.c1001_1 ADD CONSTRAINT c1001_1_fk FOREIGN KEY (parent_id) REFERENCES public.c1001_1(id) NOT VALID;
ALTER TABLE demo.c1001_1 ADD CONSTRAINT c1001_1_chk CHECK ( value > 0 ) NOT VALID;

create sequence if not exists demo.accounts_seq;
create table if not exists demo.accounts (
id bigint not null primary key default nextval('demo.accounts_seq'),
client_id bigint not null,
account_number varchar(50) not null unique,
account_balance numeric(22,2) not null default 0,
deleted boolean not null default false);

create sequence if not exists demo.clients_seq;
create table if not exists demo.clients (
id bigint not null primary key default nextval('demo.clients_seq'),
last_name varchar(255) not null,
first_name varchar(255) not null,
middle_name varchar(255),
info jsonb);

alter table if exists demo.accounts
add constraint c_accounts_fk_client_id foreign key (client_id) references demo.clients (id);

@mfvanek mfvanek added the work in progress Work on this issue has already begun label Apr 28, 2024
@mfvanek mfvanek added this to the 0.10.4 milestone Apr 28, 2024
@mfvanek
Copy link
Owner Author

mfvanek commented May 3, 2024

Hi @BLoHny,
do you plan to send PRs to starter and demo apps?
If not I'll make it myself.
I'm going to release a new version this weekend.

@BLoHny
Copy link
Collaborator

BLoHny commented May 3, 2024

#363
I was thinking about contributing to this.

@mfvanek
Copy link
Owner Author

mfvanek commented May 3, 2024

#363 I was thinking about contributing to this.

Ok! Drop a comment in that issue and I'll assign it to you

@mfvanek mfvanek assigned mfvanek and unassigned BLoHny May 3, 2024
@mfvanek
Copy link
Owner Author

mfvanek commented May 3, 2024

@BLoHny
I've just released version 0.11.0
Thank you very much for being part of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants