From 7908aa9749e9d9518436366dc41b524568c5ace8 Mon Sep 17 00:00:00 2001 From: Ivan Vakhrushev <37612014+mfvanek@users.noreply.github.com> Date: Sat, 7 Dec 2019 00:21:09 +0300 Subject: [PATCH] Raw sql queries and tests (#6) * Added raw sql queries * Added all sql queries in raw format * Added links for queries into README.md * Added unit tests for IndexesHealthImpl --- README.md | 16 +- .../HighAvailabilityPgConnectionImpl.java | 2 +- .../maintenance/IndexMaintenanceImpl.java | 14 +- .../sql/foreign_keys_without_index.sql | 19 ++ .../sql/indexes_with_null_values.sql | 14 ++ .../sql/tables_with_missing_indexes.sql | 17 ++ .../sql/tables_without_primary_key.sql | 8 + src/main/resources/sql/unused_indexes.sql | 20 ++ .../HighAvailabilityPgConnectionImplTest.java | 17 +- .../index/health/IndexesHealthImplTest.java | 171 +++++++++++++++++- 10 files changed, 279 insertions(+), 19 deletions(-) create mode 100644 src/main/resources/sql/foreign_keys_without_index.sql create mode 100644 src/main/resources/sql/indexes_with_null_values.sql create mode 100644 src/main/resources/sql/tables_with_missing_indexes.sql create mode 100644 src/main/resources/sql/tables_without_primary_key.sql create mode 100644 src/main/resources/sql/unused_indexes.sql diff --git a/README.md b/README.md index 693bbbb..f3da3a1 100644 --- a/README.md +++ b/README.md @@ -6,14 +6,14 @@ ## Available checks **pg-index-health** allows you to detect the following problems: -1. Invalid (broken) indexes. -1. Duplicated (completely identical) indexes. -1. Intersecting (partially identical) indexes. -1. Unused indexes. -1. Foreign keys without associated indexes. -1. Indexes with null values. -1. Tables with missing indexes. -1. Tables without primary key. +1. Invalid (broken) indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/invalid_indexes.sql)). +1. Duplicated (completely identical) indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/duplicated_indexes.sql)). +1. Intersecting (partially identical) indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/intersecting_indexes.sql)). +1. Unused indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/unused_indexes.sql)). +1. Foreign keys without associated indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/foreign_keys_without_index.sql)). +1. Indexes with null values ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/indexes_with_null_values.sql)). +1. Tables with missing indexes ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/tables_with_missing_indexes.sql)). +1. Tables without primary key ([sql](https://github.com/mfvanek/pg-index-health/blob/master/src/main/resources/sql/tables_without_primary_key.sql)). ## Demo application ```java diff --git a/src/main/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImpl.java b/src/main/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImpl.java index 5415d50..7e3bf98 100644 --- a/src/main/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImpl.java +++ b/src/main/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImpl.java @@ -34,7 +34,7 @@ public Set getConnectionsToReplicas() { @Nonnull public static HighAvailabilityPgConnection of(@Nonnull final PgConnection connectionToMaster) { - return new HighAvailabilityPgConnectionImpl(connectionToMaster, Set.of()); + return new HighAvailabilityPgConnectionImpl(connectionToMaster, Set.of(connectionToMaster)); } @Nonnull diff --git a/src/main/java/com/mfvanek/pg/index/maintenance/IndexMaintenanceImpl.java b/src/main/java/com/mfvanek/pg/index/maintenance/IndexMaintenanceImpl.java index 17f1cf2..464ce6c 100644 --- a/src/main/java/com/mfvanek/pg/index/maintenance/IndexMaintenanceImpl.java +++ b/src/main/java/com/mfvanek/pg/index/maintenance/IndexMaintenanceImpl.java @@ -117,18 +117,17 @@ public class IndexMaintenanceImpl implements IndexMaintenance { " from pg_stat_all_tables\n" + " where\n" + " schemaname = 'public'::text and\n" + - " pg_relation_size(relname::regclass) > 5::integer * 8192 and /*skip small tables*/\n" + - " relname not in ('databasechangelog')\n" + + " pg_relation_size(relname::regclass) > 5::integer * 8192 /*skip small tables*/\n" + ")\n" + "select table_name,\n" + " seq_scan,\n" + " idx_scan\n" + "from tables_without_indexes\n" + "where (seq_scan + idx_scan) > 100::integer and /*table in use*/\n" + - " too_much_seq > 0 -- too much sequential scans\n" + + " too_much_seq > 0 /*too much sequential scans*/\n" + "order by table_name, too_much_seq desc;"; - private static final String TABLES_WITHOUT_PRIMARY_KEYS = + private static final String TABLES_WITHOUT_PRIMARY_KEY = "select tablename as table_name\n" + "from pg_tables\n" + "where\n" + @@ -136,8 +135,7 @@ public class IndexMaintenanceImpl implements IndexMaintenance { " tablename not in (\n" + " select c.conrelid::regclass::text as table_name\n" + " from pg_constraint c\n" + - " where contype = 'p') and\n" + - " tablename not in ('databasechangelog')\n" + + " where contype = 'p')\n" + "order by tablename;"; private static final String INDEXES_WITH_NULL_VALUES = @@ -151,7 +149,7 @@ public class IndexMaintenanceImpl implements IndexMaintenance { "where not x.indisunique and\n" + " not a.attnotnull and\n" + " psai.schemaname = 'public'::text and\n" + - " array_position(x.indkey, a.attnum) = 0 and -- only for first segment\n" + + " array_position(x.indkey, a.attnum) = 0 and /*only for first segment*/\n" + " (x.indpred is null or (position(lower(a.attname) in lower(pg_get_expr(x.indpred, x.indrelid))) = 0))\n" + "group by x.indrelid, x.indexrelid, x.indpred\n" + "order by table_name, index_name;"; @@ -222,7 +220,7 @@ public List getTablesWithMissingIndexes() { @Nonnull @Override public List getTablesWithoutPrimaryKey() { - return executeQuery(TABLES_WITHOUT_PRIMARY_KEYS, rs -> { + return executeQuery(TABLES_WITHOUT_PRIMARY_KEY, rs -> { final String tableName = rs.getString("table_name"); return TableWithoutPrimaryKey.of(tableName); }); diff --git a/src/main/resources/sql/foreign_keys_without_index.sql b/src/main/resources/sql/foreign_keys_without_index.sql new file mode 100644 index 0000000..fc1c068 --- /dev/null +++ b/src/main/resources/sql/foreign_keys_without_index.sql @@ -0,0 +1,19 @@ +select c.conrelid::regclass as table_name, + string_agg(col.attname, ', ' order by u.attposition) as columns, + c.conname as constraint_name +from pg_constraint c + join lateral unnest(c.conkey) with ordinality as u(attnum, attposition) on true + join pg_class t on (c.conrelid = t.oid) + join pg_namespace nsp on nsp.oid = t.relnamespace + join pg_attribute col on (col.attrelid = t.oid and col.attnum = u.attnum) +where contype = 'f' + and nsp.nspname = 'public'::text + and not exists( + select 1 + from pg_index + where indrelid = c.conrelid + and (c.conkey::int[] <@ indkey::int[]) /*all columns of foreign key have to present in index*/ + and array_position(indkey::int[], (c.conkey::int[])[1]) = 0 /*ordering of columns in foreign key and in index is the same*/ + ) +group by c.conrelid, c.conname, c.oid +order by (c.conrelid::regclass)::text, columns; diff --git a/src/main/resources/sql/indexes_with_null_values.sql b/src/main/resources/sql/indexes_with_null_values.sql new file mode 100644 index 0000000..fca5d99 --- /dev/null +++ b/src/main/resources/sql/indexes_with_null_values.sql @@ -0,0 +1,14 @@ +select x.indrelid::regclass as table_name, + x.indexrelid::regclass as index_name, + string_agg(a.attname, ', ') as nullable_fields, + pg_relation_size(x.indexrelid) as index_size +from pg_index x + join pg_stat_all_indexes psai on x.indexrelid = psai.indexrelid + join pg_attribute a ON a.attrelid = x.indrelid AND a.attnum = any (x.indkey) +where not x.indisunique + and not a.attnotnull + and psai.schemaname = 'public'::text + and array_position(x.indkey, a.attnum) = 0 /*only for first segment*/ + and (x.indpred is null or (position(lower(a.attname) in lower(pg_get_expr(x.indpred, x.indrelid))) = 0)) +group by x.indrelid, x.indexrelid, x.indpred +order by table_name, index_name; diff --git a/src/main/resources/sql/tables_with_missing_indexes.sql b/src/main/resources/sql/tables_with_missing_indexes.sql new file mode 100644 index 0000000..603420b --- /dev/null +++ b/src/main/resources/sql/tables_with_missing_indexes.sql @@ -0,0 +1,17 @@ +with tables_without_indexes as ( + select relname::text as table_name, + coalesce(seq_scan, 0) - coalesce(idx_scan, 0) as too_much_seq, + pg_relation_size(relname::regclass) as table_size, + coalesce(seq_scan, 0) as seq_scan, + coalesce(idx_scan, 0) as idx_scan + from pg_stat_all_tables + where schemaname = 'public'::text + and pg_relation_size(relname::regclass) > 5::integer * 8192 /*skip small tables*/ +) +select table_name, + seq_scan, + idx_scan +from tables_without_indexes +where (seq_scan + idx_scan) > 100::integer /*table in use*/ + and too_much_seq > 0 /*too much sequential scans*/ +order by table_name, too_much_seq desc; diff --git a/src/main/resources/sql/tables_without_primary_key.sql b/src/main/resources/sql/tables_without_primary_key.sql new file mode 100644 index 0000000..673b883 --- /dev/null +++ b/src/main/resources/sql/tables_without_primary_key.sql @@ -0,0 +1,8 @@ +select tablename as table_name +from pg_tables +where schemaname = 'public'::text + and tablename not in ( + select c.conrelid::regclass::text as table_name + from pg_constraint c + where contype = 'p') +order by tablename; diff --git a/src/main/resources/sql/unused_indexes.sql b/src/main/resources/sql/unused_indexes.sql new file mode 100644 index 0000000..3424f92 --- /dev/null +++ b/src/main/resources/sql/unused_indexes.sql @@ -0,0 +1,20 @@ +with foreign_key_indexes as ( + select i.indexrelid + from pg_constraint c + join lateral unnest(c.conkey) with ordinality as u(attnum, attposition) on true + join pg_index i on i.indrelid = c.conrelid and (c.conkey::int[] <@ indkey::int[]) + where c.contype = 'f' +) +select psui.relname as table_name, + psui.indexrelname as index_name, + pg_relation_size(i.indexrelid) as index_size, + psui.idx_scan as index_scans +from pg_stat_user_indexes psui + join pg_index i on psui.indexrelid = i.indexrelid +where psui.schemaname = 'public'::text + and not i.indisunique + and i.indexrelid not in (select * from foreign_key_indexes) /*retain indexes on foreign keys*/ + and psui.idx_scan < 50::integer + and pg_relation_size(psui.relid) >= 5::integer * 8192 /*skip small tables*/ + and pg_relation_size(psui.indexrelid) >= 5::integer * 8192 /*skip small indexes*/ +order by psui.relname, pg_relation_size(i.indexrelid) desc; diff --git a/src/test/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImplTest.java b/src/test/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImplTest.java index 01a5f08..00bd541 100644 --- a/src/test/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImplTest.java +++ b/src/test/java/com/mfvanek/pg/connection/HighAvailabilityPgConnectionImplTest.java @@ -10,8 +10,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import java.util.Set; + import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; class HighAvailabilityPgConnectionImplTest { @@ -26,6 +30,17 @@ void ofMaster() { final var pgConnection = PgConnectionImpl.ofMaster(embeddedPostgres.getTestDatabase()); final var haPgConnection = HighAvailabilityPgConnectionImpl.of(pgConnection); assertNotNull(haPgConnection); - assertThat(haPgConnection.getConnectionsToReplicas(), hasSize(0)); + assertThat(haPgConnection.getConnectionsToReplicas(), hasSize(1)); + assertEquals(haPgConnection.getConnectionToMaster(), haPgConnection.getConnectionsToReplicas().iterator().next()); + } + + @Test + void withReplicas() { + final var master = PgConnectionImpl.ofMaster(embeddedPostgres.getTestDatabase()); + final var replica = PgConnectionImpl.of(embeddedPostgres.getTestDatabase(), PgHostImpl.ofName("replica")); + final var haPgConnection = HighAvailabilityPgConnectionImpl.of(master, Set.of(master, replica)); + assertNotNull(haPgConnection); + assertThat(haPgConnection.getConnectionsToReplicas(), hasSize(2)); + assertThat(haPgConnection.getConnectionsToReplicas(), containsInAnyOrder(master, replica)); } } diff --git a/src/test/java/com/mfvanek/pg/index/health/IndexesHealthImplTest.java b/src/test/java/com/mfvanek/pg/index/health/IndexesHealthImplTest.java index e751eeb..519744d 100644 --- a/src/test/java/com/mfvanek/pg/index/health/IndexesHealthImplTest.java +++ b/src/test/java/com/mfvanek/pg/index/health/IndexesHealthImplTest.java @@ -9,6 +9,8 @@ import com.mfvanek.pg.connection.HighAvailabilityPgConnectionImpl; import com.mfvanek.pg.connection.PgConnectionImpl; import com.mfvanek.pg.index.maintenance.IndexMaintenanceFactoryImpl; +import com.mfvanek.pg.model.IndexWithSize; +import com.mfvanek.pg.model.UnusedIndex; import com.mfvanek.pg.utils.DatabasePopulator; import com.mfvanek.pg.utils.TestExecutor; import com.opentable.db.postgres.junit5.EmbeddedPostgresExtension; @@ -19,11 +21,16 @@ import javax.annotation.Nonnull; import java.sql.SQLException; import java.util.function.Consumer; +import java.util.stream.Collectors; +import static java.util.stream.Collectors.toSet; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -// TODO Add tests for non zero cases class IndexesHealthImplTest { @RegisterExtension @@ -53,6 +60,22 @@ void getInvalidIndexesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getInvalidIndexesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createInvalidIndex(); + }, + () -> { + final var invalidIndexes = indexesHealth.getInvalidIndexes(); + assertNotNull(invalidIndexes); + assertEquals(1, invalidIndexes.size()); + final var index = invalidIndexes.get(0); + assertEquals("clients", index.getTableName()); + assertEquals("i_clients_last_name_first_name", index.getIndexName()); + }); + } + @Test void getDuplicatedIndexesOnEmptyDatabase() { final var duplicatedIndexes = indexesHealth.getDuplicatedIndexes(); @@ -70,6 +93,28 @@ void getDuplicatedIndexesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getDuplicatedIndexesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createDuplicatedIndex(); + }, + () -> { + final var duplicatedIndexes = indexesHealth.getDuplicatedIndexes(); + assertNotNull(duplicatedIndexes); + assertEquals(1, duplicatedIndexes.size()); + final var entry = duplicatedIndexes.get(0); + assertEquals("accounts", entry.getTableName()); + assertThat(entry.getTotalSize(), greaterThanOrEqualTo(1L)); + final var indexes = entry.getDuplicatedIndexes(); + assertEquals(2, indexes.size()); + assertThat(indexes.stream() + .map(IndexWithSize::getIndexName) + .collect(Collectors.toList()), + containsInAnyOrder("accounts_account_number_key", "i_accounts_account_number")); + }); + } + @Test void getIntersectedIndexesOnEmptyDatabase() { final var intersectedIndexes = indexesHealth.getIntersectedIndexes(); @@ -87,6 +132,28 @@ void getIntersectedIndexesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getIntersectedIndexesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createDuplicatedIndex(); + }, + () -> { + final var intersectedIndexes = indexesHealth.getIntersectedIndexes(); + assertNotNull(intersectedIndexes); + assertEquals(1, intersectedIndexes.size()); + final var entry = intersectedIndexes.get(0); + assertEquals("clients", entry.getTableName()); + assertThat(entry.getTotalSize(), greaterThanOrEqualTo(1L)); + final var indexes = entry.getDuplicatedIndexes(); + assertEquals(2, indexes.size()); + assertThat(indexes.stream() + .map(IndexWithSize::getIndexName) + .collect(Collectors.toList()), + containsInAnyOrder("i_clients_last_first", "i_clients_last_name")); + }); + } + @Test void getUnusedIndexesOnEmptyDatabase() { final var unusedIndexes = indexesHealth.getUnusedIndexes(); @@ -104,6 +171,21 @@ void getUnusedIndexesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getUnusedIndexesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createDuplicatedIndex(); + }, + () -> { + final var unusedIndexes = indexesHealth.getUnusedIndexes(); + assertNotNull(unusedIndexes); + assertThat(unusedIndexes.size(), equalTo(3)); + final var names = unusedIndexes.stream().map(UnusedIndex::getIndexName).collect(toSet()); + assertThat(names, containsInAnyOrder("i_clients_last_first", "i_clients_last_name", "i_accounts_account_number")); + }); + } + @Test void getForeignKeysNotCoveredWithIndexOnEmptyDatabase() { final var foreignKeys = indexesHealth.getForeignKeysNotCoveredWithIndex(); @@ -121,6 +203,48 @@ void getForeignKeysNotCoveredWithIndexOnDatabaseWithoutThem() throws SQLExceptio }); } + @Test + void getForeignKeysNotCoveredWithIndexOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(DatabasePopulator::populateOnlyTablesAndReferences, + () -> { + var foreignKeys = indexesHealth.getForeignKeysNotCoveredWithIndex(); + assertNotNull(foreignKeys); + assertEquals(1, foreignKeys.size()); + final var foreignKey = foreignKeys.get(0); + assertEquals("accounts", foreignKey.getTableName()); + assertThat(foreignKey.getColumnsInConstraint(), containsInAnyOrder("client_id")); + }); + } + + @Test + void getForeignKeysNotCoveredWithIndexOnDatabaseWithNotSuitableIndex() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateOnlyTablesAndReferences(); + databasePopulator.createNotSuitableIndexForForeignKey(); + }, + () -> { + var foreignKeys = indexesHealth.getForeignKeysNotCoveredWithIndex(); + assertNotNull(foreignKeys); + assertEquals(1, foreignKeys.size()); + final var foreignKey = foreignKeys.get(0); + assertEquals("accounts", foreignKey.getTableName()); + assertThat(foreignKey.getColumnsInConstraint(), containsInAnyOrder("client_id")); + }); + } + + @Test + void getForeignKeysNotCoveredWithIndexOnDatabaseWithSuitableIndex() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateOnlyTablesAndReferences(); + databasePopulator.createSuitableIndexForForeignKey(); + }, + () -> { + var foreignKeys = indexesHealth.getForeignKeysNotCoveredWithIndex(); + assertNotNull(foreignKeys); + assertEquals(0, foreignKeys.size()); + }); + } + @Test void getTablesWithMissingIndexesOnEmptyDatabase() { final var tables = indexesHealth.getTablesWithMissingIndexes(); @@ -138,6 +262,23 @@ void getTablesWithMissingIndexesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getTablesWithMissingIndexesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.tryToFindAccountByClientId(101); + }, + () -> { + var tables = indexesHealth.getTablesWithMissingIndexes(); + assertNotNull(tables); + assertEquals(1, tables.size()); + var table = tables.get(0); + assertEquals("accounts", table.getTableName()); + assertThat(table.getSeqScans(), greaterThanOrEqualTo(101L)); + assertEquals(0, table.getIndexScans()); + }); + } + @Test void getTablesWithoutPrimaryKeyOnEmptyDatabase() { final var tables = indexesHealth.getTablesWithoutPrimaryKey(); @@ -155,6 +296,21 @@ void getTablesWithoutPrimaryKeyOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getTablesWithoutPrimaryKeyOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createTableWithoutPrimaryKey(); + }, + () -> { + var tables = indexesHealth.getTablesWithoutPrimaryKey(); + assertNotNull(tables); + assertEquals(1, tables.size()); + var table = tables.get(0); + assertEquals("bad_clients", table.getTableName()); + }); + } + @Test void getIndexesWithNullValuesOnEmptyDatabase() { final var indexes = indexesHealth.getIndexesWithNullValues(); @@ -172,6 +328,19 @@ void getIndexesWithNullValuesOnDatabaseWithoutThem() throws SQLException { }); } + @Test + void getIndexesWithNullValuesOnDatabaseWithThem() throws SQLException { + executeTestOnDatabase(databasePopulator -> { + databasePopulator.populateWithDataAndReferences(); + databasePopulator.createIndexWithNulls(); + }, + () -> { + final var indexes = indexesHealth.getIndexesWithNullValues(); + assertNotNull(indexes); + assertEquals(1, indexes.size()); + }); + } + private void executeTestOnDatabase(@Nonnull final Consumer databasePopulatorConsumer, @Nonnull final TestExecutor testExecutor) throws SQLException {