mfvanek / pg-index-health

pg-index-health is a Java library for analyzing and maintaining indexes and tables health in Postgresql databases.
Apache License 2.0
128 stars 15 forks source link

Add check for not valid constraints #362

Closed mfvanek closed 6 months ago

mfvanek commented 7 months ago

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);
BLoHny commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

Which sql query more satisfies your request?

I like the first variant more

BLoHny commented 6 months ago

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 commented 6 months ago

@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 https://github.com/mfvanek/pg-index-health/blob/2470e90ec25ba35468b6852c211f46cf83741d59/pg-index-health/src/main/java/io/github/mfvanek/pg/common/maintenance/Diagnostic.java#L25

mfvanek commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

https://github.com/mfvanek/pg-index-health/issues/363 I was thinking about contributing to this.

mfvanek commented 6 months ago

363 I was thinking about contributing to this.

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

mfvanek commented 6 months ago

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