From: James Ahlborn Date: Thu, 12 Jan 2023 22:22:29 +0000 (+0000) Subject: Add option to DatabaseBuilder for ignoring broken system catalog indexes. Fixes #46 X-Git-Tag: jackcess-4.0.5~3 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b02d1df66cbd159b1b318405f614827af683bfbb;p=jackcess.git Add option to DatabaseBuilder for ignoring broken system catalog indexes. Fixes #46 git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@1395 f203690c-595d-4dc9-a70b-905162fa7fd2 --- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d0640e8..e821ac4 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -4,6 +4,14 @@ Tim McCune + + + Add option to DatabaseBuilder for ignoring broken system catalog + indexes. This is a workaround to allow jackcess to read tables from + the database even if the index is non-functional.. + + Update parent pom to fix release process. diff --git a/src/main/java/com/healthmarketscience/jackcess/DatabaseBuilder.java b/src/main/java/com/healthmarketscience/jackcess/DatabaseBuilder.java index 18ba994..9b622d9 100644 --- a/src/main/java/com/healthmarketscience/jackcess/DatabaseBuilder.java +++ b/src/main/java/com/healthmarketscience/jackcess/DatabaseBuilder.java @@ -77,7 +77,8 @@ public class DatabaseBuilder private Map _summaryProps; /** database user-defined (if any) */ private Map _userProps; - + /** flag indicating that the system catalog index is borked */ + private boolean _ignoreBrokenSystemCatalogIndex; public DatabaseBuilder() { this((Path)null); @@ -260,12 +261,23 @@ public class DatabaseBuilder return props; } + /** + * Sets flag which, if {@code true}, will make the database ignore the index + * on the system catalog when looking up tables. This will make table + * retrieval slower, but can be used to workaround broken indexes. + */ + public DatabaseBuilder setIgnoreBrokenSystemCatalogIndex(boolean ignore) { + _ignoreBrokenSystemCatalogIndex = ignore; + return this; + } + /** * Opens an existingnew Database using the configured information. */ public Database open() throws IOException { return DatabaseImpl.open(_mdbFile, _readOnly, _channel, _autoSync, _charset, - _timeZone, _codecProvider); + _timeZone, _codecProvider, + _ignoreBrokenSystemCatalogIndex); } /** diff --git a/src/main/java/com/healthmarketscience/jackcess/impl/DatabaseImpl.java b/src/main/java/com/healthmarketscience/jackcess/impl/DatabaseImpl.java index 7370626..7c499dc 100644 --- a/src/main/java/com/healthmarketscience/jackcess/impl/DatabaseImpl.java +++ b/src/main/java/com/healthmarketscience/jackcess/impl/DatabaseImpl.java @@ -393,7 +393,7 @@ public class DatabaseImpl implements Database, DateTimeContext public static DatabaseImpl open( Path mdbFile, boolean readOnly, FileChannel channel, boolean autoSync, Charset charset, TimeZone timeZone, - CodecProvider provider) + CodecProvider provider, boolean ignoreSystemCatalogIndex) throws IOException { boolean closeChannel = false; @@ -439,7 +439,7 @@ public class DatabaseImpl implements Database, DateTimeContext DatabaseImpl db = new DatabaseImpl(mdbFile, channel, closeChannel, autoSync, null, charset, timeZone, provider, - readOnly); + readOnly, ignoreSystemCatalogIndex); success = true; return db; @@ -499,7 +499,7 @@ public class DatabaseImpl implements Database, DateTimeContext channel.force(true); DatabaseImpl db = new DatabaseImpl(mdbFile, channel, closeChannel, autoSync, fileFormat, charset, timeZone, null, - false); + false, false); success = true; return db; } finally { @@ -554,7 +554,7 @@ public class DatabaseImpl implements Database, DateTimeContext protected DatabaseImpl(Path file, FileChannel channel, boolean closeChannel, boolean autoSync, FileFormat fileFormat, Charset charset, TimeZone timeZone, CodecProvider provider, - boolean readOnly) + boolean readOnly, boolean ignoreSystemCatalogIndex) throws IOException { _file = file; @@ -578,7 +578,7 @@ public class DatabaseImpl implements Database, DateTimeContext // needed _pageChannel.initialize(this, provider); _buffer = _pageChannel.createPageBuffer(); - readSystemCatalog(); + readSystemCatalog(ignoreSystemCatalogIndex); } @Override @@ -992,27 +992,40 @@ public class DatabaseImpl implements Database, DateTimeContext /** * Read the system catalog */ - private void readSystemCatalog() throws IOException { + private void readSystemCatalog(boolean ignoreSystemCatalogIndex) + throws IOException { _systemCatalog = loadTable(TABLE_SYSTEM_CATALOG, PAGE_SYSTEM_CATALOG, SYSTEM_OBJECT_FLAGS, TYPE_TABLE); - try { - _tableFinder = new DefaultTableFinder( - _systemCatalog.newCursor() + if(!ignoreSystemCatalogIndex) { + try { + _tableFinder = new DefaultTableFinder( + _systemCatalog.newCursor() .setIndexByColumnNames(CAT_COL_PARENT_ID, CAT_COL_NAME) .setColumnMatcher(CaseInsensitiveColumnMatcher.INSTANCE) .toIndexCursor()); - } catch(IllegalArgumentException e) { + } catch(IllegalArgumentException e) { + if(LOG.isDebugEnabled()) { + LOG.debug(withErrorContext( + "Could not find expected index on table " + + _systemCatalog.getName())); + } + // use table scan instead + _tableFinder = new FallbackTableFinder( + _systemCatalog.newCursor() + .setColumnMatcher(CaseInsensitiveColumnMatcher.INSTANCE) + .toCursor()); + } + } else { if(LOG.isDebugEnabled()) { LOG.debug(withErrorContext( - "Could not find expected index on table " + - _systemCatalog.getName())); + "Ignoring index on table " + _systemCatalog.getName())); } // use table scan instead _tableFinder = new FallbackTableFinder( _systemCatalog.newCursor() - .setColumnMatcher(CaseInsensitiveColumnMatcher.INSTANCE) - .toCursor()); + .setColumnMatcher(CaseInsensitiveColumnMatcher.INSTANCE) + .toCursor()); } _tableParentId = _tableFinder.findObjectId(DB_PARENT_ID, diff --git a/src/main/java/com/healthmarketscience/jackcess/impl/IndexData.java b/src/main/java/com/healthmarketscience/jackcess/impl/IndexData.java index 5afe894..1c430fc 100644 --- a/src/main/java/com/healthmarketscience/jackcess/impl/IndexData.java +++ b/src/main/java/com/healthmarketscience/jackcess/impl/IndexData.java @@ -388,10 +388,12 @@ public class IndexData { /** * Used by unit tests to validate the internal status of the index. + * @param forceLoad if {@code false} only validate currently loaded index + * data pages, otherwise, load and validate all index pages * @usage _advanced_method_ */ - public void validate() throws IOException { - _pageCache.validate(); + public void validate(boolean forceLoad) throws IOException { + _pageCache.validate(forceLoad); } /** diff --git a/src/main/java/com/healthmarketscience/jackcess/impl/IndexPageCache.java b/src/main/java/com/healthmarketscience/jackcess/impl/IndexPageCache.java index d594c1c..b64b93d 100644 --- a/src/main/java/com/healthmarketscience/jackcess/impl/IndexPageCache.java +++ b/src/main/java/com/healthmarketscience/jackcess/impl/IndexPageCache.java @@ -22,13 +22,19 @@ import java.lang.ref.SoftReference; import java.util.AbstractList; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.RandomAccess; +import java.util.Set; import static com.healthmarketscience.jackcess.impl.IndexData.*; +import com.healthmarketscience.jackcess.impl.IndexData.DataPage; import org.apache.commons.lang3.builder.ToStringBuilder; /** @@ -44,7 +50,7 @@ public class IndexPageCache /** max number of pages to cache (unless a write operation is in progress) */ private static final int MAX_CACHE_SIZE = 25; - + /** the index whose pages this cache is managing */ private final IndexData _indexData; /** the root page for the index */ @@ -67,7 +73,7 @@ public class IndexPageCache /** the currently modified index pages */ private final List _modifiedPages = new ArrayList(); - + public IndexPageCache(IndexData indexData) { _indexData = indexData; } @@ -75,11 +81,11 @@ public class IndexPageCache public IndexData getIndexData() { return _indexData; } - + public PageChannel getPageChannel() { return getIndexData().getPageChannel(); } - + /** * Sets the root page for this index, must be called before normal usage. * @@ -90,7 +96,7 @@ public class IndexPageCache // root page has no parent _rootPage.initParentPage(INVALID_INDEX_PAGE_NUMBER, false); } - + /** * Writes any outstanding changes for this index to the file. */ @@ -129,7 +135,7 @@ public class IndexPageCache } } } - + /** * Prepares any non-empty modified pages for writing as the second pass * during a {@link #write} call. Updates entry prefixes, promotes/demotes @@ -158,14 +164,14 @@ public class IndexPageCache if(dpMain.hasChildTail()) { if(size == 1) { demoteTail(cacheDataPage); - } + } } else { if(size > 1) { promoteTail(cacheDataPage); } } } - + // look for pages with more entries than can fit on a page if(cacheDataPage.getTotalEntrySize() > maxPageEntrySize) { @@ -181,7 +187,7 @@ public class IndexPageCache } } } - + } while(splitPages); } @@ -211,7 +217,7 @@ public class IndexPageCache DataPageMain main = getDataPage(pageNumber); return((main != null) ? new CacheDataPage(main) : null); } - + /** * Returns a DataPageMain for the given page number, may be {@code null} if * the given page number is invalid. Loads the given page if necessary. @@ -236,9 +242,9 @@ public class IndexPageCache getIndexData().writeDataPage(cacheDataPage); // lastly, mark the page as no longer modified - cacheDataPage._extra._modified = false; + cacheDataPage._extra._modified = false; } - + /** * Deletes the given index page from the file (clears the page). */ @@ -250,11 +256,11 @@ public class IndexPageCache // discard from our cache _dataPages.remove(cacheDataPage._main._pageNumber); - + // lastly, mark the page as no longer modified - cacheDataPage._extra._modified = false; + cacheDataPage._extra._modified = false; } - + /** * Reads the given index page from the file. */ @@ -268,9 +274,9 @@ public class IndexPageCache // associate the extra info with the main data page dataPage.setExtra(extra); - + return cacheDataPage; - } + } /** * Removes the entry with the given index from the given page. @@ -298,7 +304,7 @@ public class IndexPageCache { updateEntry(cacheDataPage, entryIdx, newEntry, UpdateType.ADD); } - + /** * Updates the entries on the given page according to the given updateType. * @@ -325,7 +331,7 @@ public class IndexPageCache CacheDataPage parentDataPage = (!dpMain.isRoot() ? new CacheDataPage(dpMain.getParentPage()) : null); - + Entry oldLastEntry = dpExtra._entryView.getLast(); Entry oldEntry = null; int entrySizeDiff = 0; @@ -352,7 +358,7 @@ public class IndexPageCache } boolean updateLast = (oldLastEntry != dpExtra._entryView.getLast()); - + // child tail entry updates do not modify the page if(!updateLast || !dpMain.hasChildTail()) { dpExtra._totalEntrySize += entrySizeDiff; @@ -368,7 +374,7 @@ public class IndexPageCache return oldEntry; } - // determine if we need to update our parent page + // determine if we need to update our parent page if(!updateLast || dpMain.isRoot()) { // no parent return oldEntry; @@ -404,7 +410,7 @@ public class IndexPageCache "Empty page but size is not 0? " + dpExtra._totalEntrySize + ", " + cacheDataPage)); } - + if(dpMain.isRoot()) { // clear out this page (we don't actually remove it) dpExtra._entryPrefix = EMPTY_PREFIX; @@ -433,7 +439,7 @@ public class IndexPageCache Integer prevPageNumber = dpMain._prevPageNumber; Integer nextPageNumber = dpMain._nextPageNumber; - + DataPageMain prevMain = dpMain.getPrevPage(); if(prevMain != null) { setModified(new CacheDataPage(prevMain)); @@ -461,7 +467,7 @@ public class IndexPageCache updateParentEntry(parentDataPage, childDataPage, null, childExtra._entryView.getLast(), UpdateType.ADD); } - + /** * Replaces the entry for the given child page in the given parent page. * @@ -478,7 +484,7 @@ public class IndexPageCache updateParentEntry(parentDataPage, childDataPage, oldEntry, childExtra._entryView.getLast(), UpdateType.REPLACE); } - + /** * Updates the entry for the given child page in the given parent page * according to the given updateType. @@ -513,7 +519,7 @@ public class IndexPageCache boolean expectFound = true; int idx = 0; - + switch(upType) { case ADD: expectFound = false; @@ -524,12 +530,12 @@ public class IndexPageCache case REMOVE: idx = parentExtra._entryView.find(oldEntry); break; - + default: throw new RuntimeException(withErrorContext( "unknown update type " + upType)); } - + if(idx < 0) { if(expectFound) { throw new IllegalStateException(withErrorContext( @@ -564,7 +570,6 @@ public class IndexPageCache private void updateParentTail(CacheDataPage parentDataPage, CacheDataPage childDataPage, UpdateType upType) - throws IOException { DataPageMain parentMain = parentDataPage._main; @@ -577,7 +582,7 @@ public class IndexPageCache parentMain._childTailPageNumber = newChildTailPageNumber; } } - + /** * Verifies that the given entry type (node/leaf) is valid for the given * page (node/leaf). @@ -607,13 +612,13 @@ public class IndexPageCache DataPageExtra origExtra = origDataPage._extra; setModified(origDataPage); - + int numEntries = origExtra._entries.size(); if(numEntries < 2) { throw new IllegalStateException(withErrorContext( "Cannot split page with less than 2 entries " + origDataPage)); } - + if(origMain.isRoot()) { // we can't split the root page directly, so we need to put another page // between the root page and its sub-pages, and then split that page. @@ -629,7 +634,7 @@ public class IndexPageCache // start mucking with our entries because our parent may use our entries. DataPageMain parentMain = origMain.getParentPage(); CacheDataPage parentDataPage = new CacheDataPage(parentMain); - + // note, there are many, many ways this could be improved/tweaked. for // now, we just want it to be functional... // so, we will naively move half the entries from one page to a new page. @@ -638,7 +643,7 @@ public class IndexPageCache parentMain._pageNumber, origMain._leaf); DataPageMain newMain = newDataPage._main; DataPageExtra newExtra = newDataPage._extra; - + List headEntries = origExtra._entries.subList(0, ((numEntries + 1) / 2)); @@ -657,7 +662,7 @@ public class IndexPageCache // insert this new page between the old page and any previous page addToPeersBefore(newDataPage, origDataPage); - + if(!newMain._leaf) { // reparent the children pages of the new page reparentChildren(newDataPage); @@ -683,7 +688,7 @@ public class IndexPageCache * split. * * @param rootDataPage the root data page - * + * * @return the newly created page nested under the root page */ private CacheDataPage nestRootDataPage(CacheDataPage rootDataPage) @@ -696,7 +701,7 @@ public class IndexPageCache throw new IllegalArgumentException(withErrorContext( "should be called with root, duh")); } - + CacheDataPage newDataPage = allocateNewCacheDataPage(rootMain._pageNumber, rootMain._leaf); DataPageMain newMain = newDataPage._main; @@ -713,7 +718,7 @@ public class IndexPageCache // we need to re-parent all the child pages reparentChildren(newDataPage); } - + // clear the root page rootMain._leaf = false; rootMain._childTailPageNumber = INVALID_INDEX_PAGE_NUMBER; @@ -727,7 +732,7 @@ public class IndexPageCache return newDataPage; } - + /** * Allocates a new index page with the given parent page and type. * @@ -783,7 +788,7 @@ public class IndexPageCache newMain._nextPageNumber = origMain._pageNumber; newMain._prevPageNumber = origMain._prevPageNumber; origMain._prevPageNumber = newMain._pageNumber; - + if(prevMain != null) { setModified(new CacheDataPage(prevMain)); prevMain._nextPageNumber = newMain._pageNumber; @@ -808,7 +813,7 @@ public class IndexPageCache nextMain._prevPageNumber = INVALID_INDEX_PAGE_NUMBER; dpMain._nextPageNumber = INVALID_INDEX_PAGE_NUMBER; } - + /** * Sets the parent info for the children of the given page to the given * page. @@ -816,7 +821,6 @@ public class IndexPageCache * @param cacheDataPage the page whose children need to be updated */ private void reparentChildren(CacheDataPage cacheDataPage) - throws IOException { DataPageMain dpMain = cacheDataPage._main; DataPageExtra dpExtra = cacheDataPage._extra; @@ -849,7 +853,7 @@ public class IndexPageCache DataPageExtra dpExtra = cacheDataPage._extra; setModified(cacheDataPage); - + DataPageMain tailMain = dpMain.getChildTailPage(); CacheDataPage tailDataPage = new CacheDataPage(tailMain); @@ -858,10 +862,10 @@ public class IndexPageCache Entry tailEntry = dpExtra._entryView.demoteTail(); dpExtra._totalEntrySize += tailEntry.size(); dpExtra._entryPrefix = EMPTY_PREFIX; - + tailMain.setParentPage(dpMain._pageNumber, false); } - + /** * Makes the last normal entry of the given page the tail entry on that * page, done when there are multiple entries on a page and no tail entry. @@ -876,7 +880,7 @@ public class IndexPageCache DataPageExtra dpExtra = cacheDataPage._extra; setModified(cacheDataPage); - + DataPageMain lastMain = dpMain.getChildPage(dpExtra._entryView.getLast()); CacheDataPage lastDataPage = new CacheDataPage(lastMain); @@ -888,7 +892,7 @@ public class IndexPageCache lastMain.setParentPage(dpMain._pageNumber, true); } - + /** * Finds the index page on which the given entry does or should reside. * @@ -904,7 +908,7 @@ public class IndexPageCache // nowhere to go from here return new CacheDataPage(curPage); } - + DataPageExtra extra = curPage.getExtra(); // need to descend @@ -949,24 +953,24 @@ public class IndexPageCache { byte[] b1 = e1.getEntryBytes(); byte[] b2 = e2.getEntryBytes(); - + int maxLen = b1.length; byte[] prefix = b1; if(b1.length > b2.length) { maxLen = b2.length; prefix = b2; } - + int len = 0; while((len < maxLen) && (b1[len] == b2[len])) { ++len; } - + if(len < prefix.length) { if(len == 0) { return EMPTY_PREFIX; } - + // need new prefix prefix = ByteUtil.copyOf(prefix, len); } @@ -977,133 +981,10 @@ public class IndexPageCache /** * Used by unit tests to validate the internal status of the index. */ - void validate() throws IOException { - // copy the values as the validation methods might trigger map updates - for(DataPageMain dpMain : new ArrayList(_dataPages.values())) { - DataPageExtra dpExtra = dpMain.getExtra(); - validateEntries(dpExtra); - validateChildren(dpMain, dpExtra); - validatePeers(dpMain); - } - } - - /** - * Validates the entries for an index page - * - * @param dpExtra the entries to validate - */ - private void validateEntries(DataPageExtra dpExtra) throws IOException { - int entrySize = 0; - Entry prevEntry = FIRST_ENTRY; - for(Entry e : dpExtra._entries) { - entrySize += e.size(); - if(prevEntry.compareTo(e) >= 0) { - throw new IOException(withErrorContext( - "Unexpected order in index entries, " + prevEntry + " >= " + e)); - } - prevEntry = e; - } - if(entrySize != dpExtra._totalEntrySize) { - throw new IllegalStateException(withErrorContext( - "Expected size " + entrySize + - " but was " + dpExtra._totalEntrySize)); - } - } - - /** - * Validates the children for an index page - * - * @param dpMain the index page - * @param dpExtra the child entries to validate - */ - private void validateChildren(DataPageMain dpMain, - DataPageExtra dpExtra) throws IOException { - int childTailPageNumber = dpMain._childTailPageNumber; - if(dpMain._leaf) { - if(childTailPageNumber != INVALID_INDEX_PAGE_NUMBER) { - throw new IllegalStateException(withErrorContext( - "Leaf page has tail " + dpMain)); - } - return; - } - if((dpExtra._entryView.size() == 1) && dpMain.hasChildTail()) { - throw new IllegalStateException(withErrorContext( - "Single child is tail " + dpMain)); - } - for(Entry e : dpExtra._entryView) { - validateEntryForPage(dpMain, e); - Integer subPageNumber = e.getSubPageNumber(); - DataPageMain childMain = _dataPages.get(subPageNumber); - if(childMain != null) { - if(childMain._parentPageNumber != null) { - if(childMain._parentPageNumber != dpMain._pageNumber) { - throw new IllegalStateException(withErrorContext( - "Child's parent is incorrect " + childMain)); - } - boolean expectTail = (subPageNumber == childTailPageNumber); - if(expectTail != childMain._tail) { - throw new IllegalStateException(withErrorContext( - "Child tail status incorrect " + childMain)); - } - } - Entry lastEntry = childMain.getExtra()._entryView.getLast(); - if(e.compareTo(lastEntry) != 0) { - throw new IllegalStateException(withErrorContext( - "Invalid entry " + e + " but child is " + lastEntry)); - } - } - } - } - - /** - * Validates the peer pages for an index page. - * - * @param dpMain the index page - */ - private void validatePeers(DataPageMain dpMain) throws IOException { - DataPageMain prevMain = _dataPages.get(dpMain._prevPageNumber); - if(prevMain != null) { - if(prevMain._nextPageNumber != dpMain._pageNumber) { - throw new IllegalStateException(withErrorContext( - "Prev page " + prevMain + " does not ref " + dpMain)); - } - validatePeerStatus(dpMain, prevMain); - } - - DataPageMain nextMain = _dataPages.get(dpMain._nextPageNumber); - if(nextMain != null) { - if(nextMain._prevPageNumber != dpMain._pageNumber) { - throw new IllegalStateException(withErrorContext( - "Next page " + nextMain + " does not ref " + dpMain)); - } - validatePeerStatus(dpMain, nextMain); - } + void validate(boolean forceLoad) throws IOException { + new Validator(forceLoad).validate(); } - /** - * Validates the given peer page against the given index page - * - * @param dpMain the index page - * @param peerMain the peer index page - */ - private void validatePeerStatus(DataPageMain dpMain, DataPageMain peerMain) - throws IOException - { - if(dpMain._leaf != peerMain._leaf) { - throw new IllegalStateException(withErrorContext( - "Mismatched peer status " + dpMain._leaf + " " + peerMain._leaf)); - } - if(!dpMain._leaf) { - if((dpMain._parentPageNumber != null) && - (peerMain._parentPageNumber != null) && - ((int)dpMain._parentPageNumber != (int)peerMain._parentPageNumber)) { - throw new IllegalStateException(withErrorContext( - "Mismatched node parents " + dpMain._parentPageNumber + " " + - peerMain._parentPageNumber)); - } - } - } - /** * Collects all the cache pages in the cache. * @@ -1139,17 +1020,17 @@ public class IndexPageCache iter.remove(); if(_dataPages.size() <= MAX_CACHE_SIZE) { break; - } + } } } } - + @Override public String toString() { ToStringBuilder sb = CustomToStringStyle.builder(this); if(_rootPage == null) { sb.append("pages", "(uninitialized)"); - } else { + } else { sb.append("pages", collectPages(new ArrayList(), _rootPage)); } return sb.toString(); @@ -1158,7 +1039,7 @@ public class IndexPageCache private String withErrorContext(String msg) { return _indexData.withErrorContext(msg); } - + /** * Keeps track of the main info for an index page. @@ -1181,11 +1062,11 @@ public class IndexPageCache public IndexPageCache getCache() { return IndexPageCache.this; } - + public boolean isRoot() { return(this == _rootPage); } - + public boolean isTail() throws IOException { resolveParent(); @@ -1199,7 +1080,7 @@ public class IndexPageCache public boolean isChildTailPageNumber(int pageNumber) { return(_childTailPageNumber == pageNumber); } - + public DataPageMain getParentPage() throws IOException { resolveParent(); @@ -1212,7 +1093,7 @@ public class IndexPageCache setParentPage(parentPageNumber, isTail); } } - + public void setParentPage(Integer parentPageNumber, boolean isTail) { _parentPageNumber = parentPageNumber; _tail = isTail; @@ -1222,19 +1103,19 @@ public class IndexPageCache { return IndexPageCache.this.getDataPage(_prevPageNumber); } - + public DataPageMain getNextPage() throws IOException { return IndexPageCache.this.getDataPage(_nextPageNumber); } - + public DataPageMain getChildPage(Entry e) throws IOException { Integer childPageNumber = e.getSubPageNumber(); return getChildPage(childPageNumber, isChildTailPageNumber(childPageNumber)); } - + public DataPageMain getChildTailPage() throws IOException { return getChildPage(_childTailPageNumber, true); @@ -1254,7 +1135,7 @@ public class IndexPageCache } return child; } - + public DataPageExtra getExtra() throws IOException { DataPageExtra extra = _extra.get(); @@ -1262,10 +1143,10 @@ public class IndexPageCache extra = readDataPage(_pageNumber)._extra; setExtra(extra); } - + return extra; } - + public void setExtra(DataPageExtra extra) throws IOException { extra.setEntryView(this); @@ -1313,7 +1194,7 @@ public class IndexPageCache public void setEntryView(DataPageMain main) throws IOException { _entryView = new EntryListView(main, this); } - + public void updateEntryPrefix() { if(_entryPrefix.length == 0) { // prefix is only related to *real* entries, tail not included @@ -1321,7 +1202,7 @@ public class IndexPageCache _entries.get(_entries.size() - 1)); } } - + @Override public String toString() { return CustomToStringStyle.builder("DPExtra") @@ -1341,7 +1222,7 @@ public class IndexPageCache private CacheDataPage(DataPageMain dataPage) throws IOException { this(dataPage, dataPage.getExtra()); } - + private CacheDataPage(DataPageMain dataPage, DataPageExtra extra) { _main = dataPage; _extra = extra; @@ -1351,7 +1232,7 @@ public class IndexPageCache public int getPageNumber() { return _main._pageNumber; } - + @Override public boolean isLeaf() { return _main._leaf; @@ -1393,7 +1274,7 @@ public class IndexPageCache _main._childTailPageNumber = pageNumber; } - + @Override public int getTotalEntrySize() { return _extra._totalEntrySize; @@ -1429,12 +1310,12 @@ public class IndexPageCache public void addEntry(int idx, Entry entry) throws IOException { _main.getCache().addEntry(this, idx, entry); } - + @Override public Entry removeEntry(int idx) throws IOException { return _main.getCache().removeEntry(this, idx); } - + } /** @@ -1460,7 +1341,7 @@ public class IndexPageCache private List getEntries() { return _extra._entries; } - + @Override public int size() { int size = getEntries().size(); @@ -1483,31 +1364,31 @@ public class IndexPageCache setChildTailEntry(newEntry) : getEntries().set(idx, newEntry)); } - + @Override public void add(int idx, Entry newEntry) { // note, we will never add to the "tail" entry, that will always be // handled through promoteTail getEntries().add(idx, newEntry); } - + @Override public Entry remove(int idx) { return (isCurrentChildTailIndex(idx) ? setChildTailEntry(null) : getEntries().remove(idx)); } - + public Entry setChildTailEntry(Entry newEntry) { Entry old = _childTailEntry; _childTailEntry = newEntry; return old; } - + private boolean hasChildTail() { return(_childTailEntry != null); } - + private boolean isCurrentChildTailIndex(int idx) { return(idx == getEntries().size()); } @@ -1524,17 +1405,199 @@ public class IndexPageCache getEntries().add(tail); return tail; } - + public Entry promoteTail() { Entry last = getEntries().remove(getEntries().size() - 1); _childTailEntry = last; return last; } - + public int find(Entry e) { return Collections.binarySearch(this, e); } } + /** + * Utility class for running index validation. + */ + private final class Validator { + private final boolean _forceLoad; + private final Map _knownPages = new HashMap<>(); + private final Queue _pendingPages = new LinkedList<>(); + + private Validator(boolean forceLoad) { + _forceLoad = forceLoad; + _knownPages.putAll(_dataPages); + _pendingPages.addAll(_knownPages.values()); + } + + void validate() throws IOException { + DataPageMain dpMain = null; + while((dpMain = _pendingPages.poll()) != null) { + DataPageExtra dpExtra = dpMain.getExtra(); + validateEntries(dpExtra); + validateChildren(dpMain, dpExtra); + validatePeers(dpMain); + } + } + + /** + * Validates the entries for an index page + * + * @param dpExtra the entries to validate + */ + private void validateEntries(DataPageExtra dpExtra) throws IOException { + int entrySize = 0; + Entry prevEntry = FIRST_ENTRY; + for(Entry e : dpExtra._entries) { + entrySize += e.size(); + if(prevEntry.compareTo(e) >= 0) { + throw new IOException(withErrorContext( + "Unexpected order in index entries, " + prevEntry + + " >= " + e)); + } + prevEntry = e; + } + if(entrySize != dpExtra._totalEntrySize) { + throw new IllegalStateException(withErrorContext( + "Expected size " + entrySize + + " but was " + dpExtra._totalEntrySize)); + } + } + + /** + * Validates the children for an index page + * + * @param dpMain the index page + * @param dpExtra the child entries to validate + */ + private void validateChildren(DataPageMain dpMain, + DataPageExtra dpExtra) throws IOException { + int childTailPageNumber = dpMain._childTailPageNumber; + if(dpMain._leaf) { + if(childTailPageNumber != INVALID_INDEX_PAGE_NUMBER) { + throw new IllegalStateException( + withErrorContext("Leaf page has tail " + dpMain)); + } + return; + } + if((dpExtra._entryView.size() == 1) && dpMain.hasChildTail()) { + throw new IllegalStateException( + withErrorContext("Single child is tail " + dpMain)); + } + Integer prevPageNumber = null; + Integer nextPageNumber = null; + for(Entry e : dpExtra._entryView) { + validateEntryForPage(dpMain, e); + Integer subPageNumber = e.getSubPageNumber(); + DataPageMain childMain = getPageForValidate(subPageNumber); + if(childMain != null) { + if((prevPageNumber != null) && + ((int)childMain._prevPageNumber != prevPageNumber)) { + throw new IllegalStateException(withErrorContext( + "Child's prevPageNumber is not the previous child for " + + childMain + " " + dpExtra._entryView + " " + + prevPageNumber)); + } + if((nextPageNumber != null) && + (childMain._pageNumber != nextPageNumber)) { + throw new IllegalStateException(withErrorContext( + "Child's pageNumber is not the expected next child for " + + childMain)); + } + if(childMain._parentPageNumber != null) { + if(childMain._parentPageNumber != dpMain._pageNumber) { + throw new IllegalStateException( + withErrorContext("Child's parent is incorrect " + childMain)); + } + boolean expectTail = (subPageNumber == childTailPageNumber); + if(expectTail != childMain._tail) { + throw new IllegalStateException(withErrorContext( + "Child tail status incorrect " + childMain)); + } + } + Entry lastEntry = childMain.getExtra()._entryView.getLast(); + if(e.compareTo(lastEntry) != 0) { + throw new IllegalStateException(withErrorContext( + "Invalid entry " + e + " but child is " + lastEntry)); + } + nextPageNumber = childMain._nextPageNumber; + prevPageNumber = childMain._pageNumber; + } else { + // if we aren't force loading, we may have gaps in the children so we + // can't validate these for the current child + nextPageNumber = null; + prevPageNumber = null; + } + } + } + + /** + * Validates the peer pages for an index page. + * + * @param dpMain the index page + */ + private void validatePeers(DataPageMain dpMain) + throws IOException { + + DataPageMain prevMain = getPageForValidate(dpMain._prevPageNumber); + if(prevMain != null) { + if(prevMain._nextPageNumber != dpMain._pageNumber) { + throw new IllegalStateException(withErrorContext( + "Prev page " + prevMain + " does not ref " + dpMain)); + } + validatePeerStatus(dpMain, prevMain); + } + + DataPageMain nextMain = + getPageForValidate(dpMain._nextPageNumber); + if(nextMain != null) { + if(nextMain._prevPageNumber != dpMain._pageNumber) { + throw new IllegalStateException(withErrorContext( + "Next page " + nextMain + " does not ref " + dpMain)); + } + validatePeerStatus(dpMain, nextMain); + } + } + + /** + * Validates the given peer page against the given index page + * + * @param dpMain the index page + * @param peerMain the peer index page + */ + private void validatePeerStatus(DataPageMain dpMain, DataPageMain peerMain) + { + if(dpMain._leaf != peerMain._leaf) { + throw new IllegalStateException(withErrorContext( + "Mismatched peer status " + dpMain._leaf + " " + + peerMain._leaf)); + } + if(!dpMain._leaf) { + if((dpMain._parentPageNumber != null) && + (peerMain._parentPageNumber != null) && + ((int)dpMain._parentPageNumber != (int)peerMain._parentPageNumber)) { + throw new IllegalStateException(withErrorContext( + "Mismatched node parents " + dpMain._parentPageNumber + " " + + peerMain._parentPageNumber)); + } + } + } + + private DataPageMain getPageForValidate( + Integer pageNumber) throws IOException { + DataPageMain dpMain = _knownPages.get(pageNumber); + if((dpMain == null) && _forceLoad && + (pageNumber != INVALID_INDEX_PAGE_NUMBER)) { + dpMain = getDataPage(pageNumber); + if(dpMain != null) { + _knownPages.put(pageNumber, dpMain); + _pendingPages.add(dpMain); + } + } + return dpMain; + } + } + } diff --git a/src/main/java/com/healthmarketscience/jackcess/util/CustomLinkResolver.java b/src/main/java/com/healthmarketscience/jackcess/util/CustomLinkResolver.java index e36c569..253351d 100644 --- a/src/main/java/com/healthmarketscience/jackcess/util/CustomLinkResolver.java +++ b/src/main/java/com/healthmarketscience/jackcess/util/CustomLinkResolver.java @@ -268,7 +268,7 @@ public abstract class CustomLinkResolver implements LinkResolver throws IOException { super(file, channel, true, false, fileFormat, null, null, null, - readOnly); + readOnly, false); _resolver = resolver; _customFile = customFile; } diff --git a/src/test/java/com/healthmarketscience/jackcess/BigIndexTest.java b/src/test/java/com/healthmarketscience/jackcess/BigIndexTest.java index 004b3b2..9f2e6cd 100644 --- a/src/test/java/com/healthmarketscience/jackcess/BigIndexTest.java +++ b/src/test/java/com/healthmarketscience/jackcess/BigIndexTest.java @@ -35,7 +35,7 @@ public class BigIndexTest extends TestCase { public BigIndexTest(String name) { super(name); } - + public void testComplexIndex() throws Exception { for (final TestDB testDB : TestDB.getSupportedForBasename(Basename.COMP_INDEX, true)) { @@ -99,12 +99,12 @@ public class BigIndexTest extends TestCase { } } - index.getIndexData().validate(); + index.getIndexData().validate(false); db.flush(); t = null; System.gc(); - + t = (TableImpl)db.getTable("Table1"); index = t.getIndex("col1"); @@ -130,7 +130,7 @@ public class BigIndexTest extends TestCase { assertEquals(2000, rowCount); - index.getIndexData().validate(); + index.getIndexData().validate(false); // delete an entry in the middle Cursor cursor = CursorBuilder.createCursor(index); @@ -147,7 +147,7 @@ public class BigIndexTest extends TestCase { cursor.deleteCurrentRow(); } - index.getIndexData().validate(); + index.getIndexData().validate(false); List found = new ArrayList(); for(Row row : CursorBuilder.createCursor(index)) { @@ -166,7 +166,7 @@ public class BigIndexTest extends TestCase { assertFalse(cursor.moveToNextRow()); assertFalse(cursor.moveToPreviousRow()); - index.getIndexData().validate(); + index.getIndexData().validate(false); // add 50 (pseudo) random entries to the table rand = new Random(42L); @@ -179,14 +179,14 @@ public class BigIndexTest extends TestCase { t.addRow(nextVal, "this is some row data " + nextInt); } - index.getIndexData().validate(); + index.getIndexData().validate(false); cursor = CursorBuilder.createCursor(index); while(cursor.moveToNextRow()) { cursor.deleteCurrentRow(); } - index.getIndexData().validate(); + index.getIndexData().validate(false); db.close(); diff --git a/src/test/java/com/healthmarketscience/jackcess/DatabaseTest.java b/src/test/java/com/healthmarketscience/jackcess/DatabaseTest.java index 3b90545..06a05b8 100644 --- a/src/test/java/com/healthmarketscience/jackcess/DatabaseTest.java +++ b/src/test/java/com/healthmarketscience/jackcess/DatabaseTest.java @@ -989,6 +989,31 @@ public class DatabaseTest extends TestCase assertEquals(expectedUpdateDate, table.getUpdatedDate().toString()); } } + + public void testBrokenIndex() throws Exception { + TestDB testDb = TestDB.getSupportedForBasename(Basename.TEST).get(0); + try (Database db = new DatabaseBuilder(testDb.getFile()) + .setReadOnly(true).setIgnoreBrokenSystemCatalogIndex(true).open()) { + Table test = db.getTable("Table1"); + assertNotNull(test); + verifyFinderType(db, "FallbackTableFinder"); + } + try (Database db = openMem(testDb)) { + Table test = db.getTable("Table1"); + assertNotNull(test); + verifyFinderType(db, "DefaultTableFinder"); + } + } + + private static void verifyFinderType(Database db, String clazzName) + throws Exception{ + java.lang.reflect.Field f = db.getClass().getDeclaredField("_tableFinder"); + f.setAccessible(true); + Object finder = f.get(db); + assertNotNull(finder); + assertEquals(clazzName, finder.getClass().getSimpleName()); + } + private static void checkRawValue(String expected, Object val) { if(expected != null) { diff --git a/src/test/java/com/healthmarketscience/jackcess/IndexTest.java b/src/test/java/com/healthmarketscience/jackcess/IndexTest.java index a312ae8..0ddf171 100644 --- a/src/test/java/com/healthmarketscience/jackcess/IndexTest.java +++ b/src/test/java/com/healthmarketscience/jackcess/IndexTest.java @@ -342,7 +342,7 @@ public class IndexTest extends TestCase { if(expectedSuccess) { assertNull(failure); } else { - assertTrue(failure != null); + assertNotNull(failure); assertTrue(failure.getMessage().contains("uniqueness")); } } diff --git a/src/test/java/com/healthmarketscience/jackcess/TestUtil.java b/src/test/java/com/healthmarketscience/jackcess/TestUtil.java index 7d1d6b3..97139b8 100644 --- a/src/test/java/com/healthmarketscience/jackcess/TestUtil.java +++ b/src/test/java/com/healthmarketscience/jackcess/TestUtil.java @@ -200,10 +200,14 @@ public class TestUtil final Database db = new DatabaseBuilder(file).setReadOnly(readOnly) .setAutoSync(getTestAutoSync()).setChannel(channel) .setCharset(charset).open(); - Assert.assertEquals("Wrong JetFormat.", - DatabaseImpl.getFileFormatDetails(fileFormat).getFormat(), - ((DatabaseImpl)db).getFormat()); - Assert.assertEquals("Wrong FileFormat.", fileFormat, db.getFileFormat()); + if(fileFormat != null) { + Assert.assertEquals( + "Wrong JetFormat.", + DatabaseImpl.getFileFormatDetails(fileFormat).getFormat(), + ((DatabaseImpl)db).getFormat()); + Assert.assertEquals( + "Wrong FileFormat.", fileFormat, db.getFileFormat()); + } return db; }