Skip to content

Commit

Permalink
Fix overlapping index checking (#117)
Browse files Browse the repository at this point in the history
* Fix overlapping index checking

- Ensure we check only overlap on the current table
- Exclude local indexes
- Make sure PK are not taken into account

* Simplify tablename passing
  • Loading branch information
Raveline authored Oct 28, 2024
1 parent d83fb05 commit a993e40
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
8 changes: 4 additions & 4 deletions src/Database/PostgreSQL/PQTypes/Checks.hs
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ checkDBStructure options tables = fmap mconcat . forM tables $ \(table, version)
runQuery_ $ sqlGetForeignKeys table
fkeys <- fetchMany fetchForeignKey
triggers <- getDBTriggers tblName
checkedOverlaps <- checkOverlappingIndexes
checkedOverlaps <- checkOverlappingIndexes tblName
return $
mconcat
[ checkColumns 1 tblColumns desc
Expand Down Expand Up @@ -714,8 +714,8 @@ checkDBStructure options tables = fmap mconcat . forM tables $ \(table, version)
\expected output into source code.)"
]

checkOverlappingIndexes :: MonadDB m => m ValidationResult
checkOverlappingIndexes =
checkOverlappingIndexes :: MonadDB m => RawSQL () -> m ValidationResult
checkOverlappingIndexes tableName =
if eoCheckOverlappingIndexes options
then go
else pure mempty
Expand All @@ -728,7 +728,7 @@ checkDBStructure options tables = fmap mconcat . forM tables $ \(table, version)
, " contains index "
, contained
]
runSQL_ checkOverlappingIndexesQuery
runSQL_ $ checkOverlappingIndexesQuery tableName
overlaps <- fetchMany handleOverlap
pure $
if null overlaps
Expand Down
19 changes: 13 additions & 6 deletions src/Database/PostgreSQL/PQTypes/Checks/Util.hs
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ objectHasMore otype ptype extra =
arrListTable :: RawSQL () -> Text
arrListTable tableName = " ->" <+> unRawSQL tableName <> ": "

checkOverlappingIndexesQuery :: SQL
checkOverlappingIndexesQuery =
checkOverlappingIndexesQuery :: RawSQL () -> SQL
checkOverlappingIndexesQuery tableName =
smconcat
[ "WITH"
, -- get predicates (WHERE clause) definition in text format (ugly but the parsed version
Expand All @@ -201,7 +201,8 @@ checkOverlappingIndexesQuery =
, " , ((regexp_match(pg_get_indexdef(indexrelid)"
, " , 'WHERE (.*)$')))[1] AS preddef"
, " FROM pg_index"
, " WHERE indexprs IS NULL)"
, " WHERE indexprs IS NULL"
, " AND indrelid = '" <> raw tableName <> "'::regclass)"
, -- add the rest of metadata and do the join
" , indexdata2 AS (SELECT t1.*"
, " , pg_get_indexdef(t1.indexrelid) AS contained"
Expand All @@ -218,13 +219,19 @@ checkOverlappingIndexesQuery =
, " SELECT contained"
, " , contains"
, " FROM indexdata2"
, " JOIN pg_class c ON (c.oid = indexdata2.indexrelid)"
, -- The indexes are the same or the "other" is larger than us
" WHERE (colotherindex = colindex"
, " OR colotherindex LIKE colindex || '+%')"
, -- and this is not a local index
" AND NOT c.relname ILIKE 'local_%'"
, -- and we have the same predicate
" AND other_preddef IS NOT DISTINCT FROM preddef"
, -- and either the other is unique (so better than us) or none of us is unique
" AND (other_indisunique"
, " OR (NOT other_indisunique"
, " AND NOT indisunique));"
" AND (NOT indisunique)"
, " OR ("
, " indisunique"
, " AND other_indisunique"
, " AND colindex = colotherindex"
, ");"
]
15 changes: 13 additions & 2 deletions test/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2166,7 +2166,18 @@ overlapingIndexesTests connSource = do
testCaseSteps' "Overlapping indexes tests" connSource $ \step -> do
freshTestDB step

step "Check that overlapping indexes get flagged"
step "Migration is correct if not checking for overlapping indexes"
do
let options = defaultExtrasOptions {eoCheckOverlappingIndexes = False}
migrateDatabase
options
["pgcrypto"]
[]
[]
[table1]
[createTableMigration table1]

step "Migration invalid when flagging overlapping indexes"
do
let options = defaultExtrasOptions {eoCheckOverlappingIndexes = True}
assertException "Some indexes are overlapping" $
Expand All @@ -2181,7 +2192,7 @@ overlapingIndexesTests connSource = do
table1 :: Table
table1 =
tblTable
{ tblName = "idxTest"
{ tblName = "idx_test"
, tblVersion = 1
, tblColumns =
[ tblColumn {colName = "id", colType = UuidT, colNullable = False}
Expand Down

0 comments on commit a993e40

Please sign in to comment.