From: James Ahlborn Date: Tue, 11 Mar 2008 00:49:42 +0000 (+0000) Subject: add unit tests (and fix some bugs) for ignoreNull and unique index handling X-Git-Tag: rel_1_1_13~28 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5c1b068826d0e897b0ea3c4b77c2b8c08503f112;p=jackcess.git add unit tests (and fix some bugs) for ignoreNull and unique index handling git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@264 f203690c-595d-4dc9-a70b-905162fa7fd2 --- diff --git a/src/java/com/healthmarketscience/jackcess/Column.java b/src/java/com/healthmarketscience/jackcess/Column.java index 9d3583a..6f2ac35 100644 --- a/src/java/com/healthmarketscience/jackcess/Column.java +++ b/src/java/com/healthmarketscience/jackcess/Column.java @@ -1112,7 +1112,7 @@ public class Column implements Comparable { @Override public String toString() { StringBuilder rtn = new StringBuilder(); - rtn.append("\tName: " + _name); + rtn.append("\tName: (" + _table.getName() + ") " + _name); rtn.append("\n\tType: 0x" + Integer.toHexString(_type.getValue()) + " (" + _type + ")"); rtn.append("\n\tNumber: " + _columnNumber); diff --git a/src/java/com/healthmarketscience/jackcess/Index.java b/src/java/com/healthmarketscience/jackcess/Index.java index c87f766..0146c2b 100644 --- a/src/java/com/healthmarketscience/jackcess/Index.java +++ b/src/java/com/healthmarketscience/jackcess/Index.java @@ -498,10 +498,9 @@ public class Index implements Comparable { } if(isLeaf) { - entries.add(new Entry(curEntryBuffer, curEntryLen, _columns)); + entries.add(new Entry(curEntryBuffer, curEntryLen)); } else { - nodeEntries.add(new NodeEntry( - curEntryBuffer, curEntryLen, _columns)); + nodeEntries.add(new NodeEntry(curEntryBuffer, curEntryLen)); } // read any shared "compressed" bytes @@ -558,16 +557,22 @@ public class Index implements Comparable { public void addRow(Object[] row, RowId rowId) throws IOException { - if(shouldIgnoreNulls() && isNullEntry(row)) { + int nullCount = countNullValues(row); + boolean isNullEntry = (nullCount == _columns.size()); + if(shouldIgnoreNulls() && isNullEntry) { // nothing to do return; } + if(isPrimaryKey() && (nullCount > 0)) { + throw new IOException("Null value found in row " + Arrays.asList(row) + + " for primary key index " + this); + } // make sure we've parsed the entries initialize(); - Entry newEntry = new Entry(row, rowId, this); - if(addEntry(newEntry)) { + Entry newEntry = new Entry(createEntryBytes(row), rowId); + if(addEntry(newEntry, isNullEntry, row)) { ++_rowCount; ++_modCount; } else { @@ -587,7 +592,8 @@ public class Index implements Comparable { public void deleteRow(Object[] row, RowId rowId) throws IOException { - if(shouldIgnoreNulls() && isNullEntry(row)) { + int nullCount = countNullValues(row); + if(shouldIgnoreNulls() && (nullCount == _columns.size())) { // nothing to do return; } @@ -595,7 +601,7 @@ public class Index implements Comparable { // make sure we've parsed the entries initialize(); - Entry oldEntry = new Entry(row, rowId, this); + Entry oldEntry = new Entry(createEntryBytes(row), rowId); if(removeEntry(oldEntry)) { --_rowCount; ++_modCount; @@ -638,18 +644,16 @@ public class Index implements Comparable { initialize(); Position startPos = FIRST_POSITION; if(startRow != null) { - Entry startEntry = new Entry(startRow, + Entry startEntry = new Entry(createEntryBytes(startRow), (startInclusive ? - RowId.FIRST_ROW_ID : RowId.LAST_ROW_ID), - this); + RowId.FIRST_ROW_ID : RowId.LAST_ROW_ID)); startPos = new Position(FIRST_ENTRY_IDX, startEntry); } Position endPos = LAST_POSITION; if(endRow != null) { - Entry endEntry = new Entry(endRow, + Entry endEntry = new Entry(createEntryBytes(endRow), (endInclusive ? - RowId.LAST_ROW_ID : RowId.FIRST_ROW_ID), - this); + RowId.LAST_ROW_ID : RowId.FIRST_ROW_ID)); endPos = new Position(LAST_ENTRY_IDX, endEntry); } return new EntryCursor(startPos, endPos); @@ -674,7 +678,7 @@ public class Index implements Comparable { /** * Adds an entry to the _entries list, maintaining the order. */ - private boolean addEntry(Entry newEntry) + private boolean addEntry(Entry newEntry, boolean isNullEntry, Object[] row) throws IOException { int idx = findEntry(newEntry); @@ -683,16 +687,17 @@ public class Index implements Comparable { idx = missingIndexToInsertionPoint(idx); // determine if the addition of this entry would break the uniqueness - // constraint - if(isUnique() && + // constraint (note, access does not seem to consider multiple null + // entries invalid for a "unique" index) + if(isUnique() && !isNullEntry && (((idx > 0) && newEntry.equalsEntryBytes(_entries.get(idx - 1))) || ((idx < _entries.size()) && newEntry.equalsEntryBytes(_entries.get(idx))))) { throw new IOException( - "New entry " + newEntry + - " violates uniqueness constrain for index " + this); + "New row " + Arrays.asList(row) + + " violates uniqueness constraint for index " + this); } _entries.add(idx, newEntry); @@ -787,7 +792,7 @@ public class Index implements Comparable { @Override public String toString() { StringBuilder rtn = new StringBuilder(); - rtn.append("\tName: " + _name); + rtn.append("\tName: (" + _table.getName() + ") " + _name); rtn.append("\n\tNumber: " + _indexNumber); rtn.append("\n\tPage number: " + _pageNumber); rtn.append("\n\tIs Primary Key: " + isPrimaryKey()); @@ -809,27 +814,29 @@ public class Index implements Comparable { } /** - * Determines if all values for this index from the given row are - * {@code null}. + * Determines the number of {@code null} values for this index from the + * given row. */ - private boolean isNullEntry(Object[] values) + private int countNullValues(Object[] values) { if(values == null) { - return true; + return _columns.size(); } // annoyingly, the values array could come from different sources, one // of which will make it a different size than the other. we need to // handle both situations. boolean useColNumber = (values.length >= _table.getMaxColumnCount()); + int nullCount = 0; for(ColumnDescriptor col : _columns) { Object value = values[ useColNumber ? col.getColumnNumber() : col.getColumnIndex()]; - if(!col.isNullValue(value)) { - return false; + if(col.isNullValue(value)) { + ++nullCount; } } - return true; + + return nullCount; } /** @@ -1039,7 +1046,7 @@ public class Index implements Comparable { */ private static Entry createSpecialEntry(RowId rowId) { try { - return new Entry(null, rowId, null); + return new Entry((byte[])null, rowId); } catch(IOException e) { // should never happen throw new IllegalStateException(e); @@ -1369,47 +1376,41 @@ public class Index implements Comparable { /** * Create a new entry - * @param values Indexed row values + * @param entryBytes encoded bytes for this index entry * @param rowId rowId in which the row is stored - * @param columns map of columns for this index */ - private Entry(Object[] values, RowId rowId, Index parent) + private Entry(byte[] entryBytes, RowId rowId) throws IOException { _rowId = rowId; - if(values != null) { - _entryBytes = parent.createEntryBytes(values); + _entryBytes = entryBytes; + if(_entryBytes != null) { _type = ((_rowId.getType() == RowId.Type.NORMAL) ? EntryType.NORMAL : ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ? EntryType.FIRST_VALID : EntryType.LAST_VALID)); + } else if(!_rowId.isValid()) { + // this is a "special" entry (first/last) + _type = ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ? + EntryType.ALWAYS_FIRST : EntryType.ALWAYS_LAST); } else { - if(!_rowId.isValid()) { - // this is a "special" entry (first/last) - _entryBytes = null; - _type = ((_rowId.getType() == RowId.Type.ALWAYS_FIRST) ? - EntryType.ALWAYS_FIRST : EntryType.ALWAYS_LAST); - } else { - throw new IllegalArgumentException("Values was null"); - } + throw new IllegalArgumentException("Values was null for valid entry"); } } /** * Read an existing entry in from a buffer */ - private Entry(ByteBuffer buffer, int entryLen, - List columns) + private Entry(ByteBuffer buffer, int entryLen) throws IOException { - this(buffer, entryLen, columns, 0); + this(buffer, entryLen, 0); } /** * Read an existing entry in from a buffer */ - private Entry(ByteBuffer buffer, int entryLen, - List columns, int extraTrailingLen) + private Entry(ByteBuffer buffer, int entryLen, int extraTrailingLen) throws IOException { // we need 4 trailing bytes for the rowId, plus whatever the caller @@ -1439,7 +1440,7 @@ public class Index implements Comparable { public boolean isValid() { return(_entryBytes != null); } - + protected final byte[] getEntryBytes() { return _entryBytes; } @@ -1534,12 +1535,11 @@ public class Index implements Comparable { /** * Read an existing node entry in from a buffer */ - private NodeEntry(ByteBuffer buffer, int entryLen, - List columns) + private NodeEntry(ByteBuffer buffer, int entryLen) throws IOException { // we need 4 trailing bytes for the sub-page number - super(buffer, entryLen, columns, 4); + super(buffer, entryLen, 4); _subPageNumber = ByteUtil.getInt(buffer, ByteOrder.BIG_ENDIAN); } @@ -1665,7 +1665,8 @@ public class Index implements Comparable { public void beforeEntry(Object[] row) throws IOException { - restorePosition(new Entry(row, RowId.FIRST_ROW_ID, Index.this)); + restorePosition( + new Entry(Index.this.createEntryBytes(row), RowId.FIRST_ROW_ID)); } /** @@ -1675,7 +1676,8 @@ public class Index implements Comparable { public void afterEntry(Object[] row) throws IOException { - restorePosition(new Entry(row, RowId.LAST_ROW_ID, Index.this)); + restorePosition( + new Entry(Index.this.createEntryBytes(row), RowId.LAST_ROW_ID)); } /** diff --git a/src/java/com/healthmarketscience/jackcess/Table.java b/src/java/com/healthmarketscience/jackcess/Table.java index eeea925..311f0ad 100644 --- a/src/java/com/healthmarketscience/jackcess/Table.java +++ b/src/java/com/healthmarketscience/jackcess/Table.java @@ -1094,14 +1094,17 @@ public class Table } getPageChannel().readPage(dataPage, tmpPageNumber); if(dataPage.get() == PageTypes.DATA) { - // found last data page - pageNumber = tmpPageNumber; + // found last data page, only use if actually listed in free space + // pages + if(_freeSpacePages.containsPageNumber(tmpPageNumber)) { + pageNumber = tmpPageNumber; + } break; } } if(pageNumber == PageChannel.INVALID_PAGE_NUMBER) { - //No data pages exist. Create a new one. + // No data pages exist (with free space). Create a new one. pageNumber = newDataPage(dataPage); } @@ -1111,12 +1114,13 @@ public class Table short freeSpaceInPage = dataPage.getShort(getFormat().OFFSET_FREE_SPACE); if (freeSpaceInPage < rowSpaceUsage) { - //Last data page is full. Create a new one. + // Last data page is full. Create a new one. writeDataPage(dataPage, pageNumber); - dataPage.clear(); _freeSpacePages.removePageNumber(pageNumber); + dataPage.clear(); pageNumber = newDataPage(dataPage); + freeSpaceInPage = dataPage.getShort(getFormat().OFFSET_FREE_SPACE); } @@ -1131,7 +1135,7 @@ public class Table } writeDataPage(dataPage, pageNumber); - //Update tdef page + // Update tdef page updateTableDefinition(rows.size()); } diff --git a/src/java/com/healthmarketscience/jackcess/UsageMap.java b/src/java/com/healthmarketscience/jackcess/UsageMap.java index 551c8a5..9484ffb 100644 --- a/src/java/com/healthmarketscience/jackcess/UsageMap.java +++ b/src/java/com/healthmarketscience/jackcess/UsageMap.java @@ -384,7 +384,8 @@ public class UsageMap @Override public String toString() { - StringBuilder builder = new StringBuilder("page numbers: ["); + StringBuilder builder = new StringBuilder( + "page numbers (range " + _startPage + " " + _endPage + "): ["); PageCursor pCursor = cursor(); while(true) { int nextPage = pCursor.getNextPage(); diff --git a/test/data/testIndexProperties.mdb b/test/data/testIndexProperties.mdb new file mode 100644 index 0000000..bb1c158 Binary files /dev/null and b/test/data/testIndexProperties.mdb differ diff --git a/test/src/java/com/healthmarketscience/jackcess/CursorTest.java b/test/src/java/com/healthmarketscience/jackcess/CursorTest.java index 0aa9c70..531805a 100644 --- a/test/src/java/com/healthmarketscience/jackcess/CursorTest.java +++ b/test/src/java/com/healthmarketscience/jackcess/CursorTest.java @@ -105,12 +105,7 @@ public class CursorTest extends TestCase { } static Database createTestIndexTable() throws Exception { - File srcFile = new File("test/data/indexCursorTest.mdb"); - File dbFile = File.createTempFile("databaseTest", ".mdb"); - dbFile.deleteOnExit(); - copyFile(srcFile, dbFile); - - Database db = Database.open(dbFile); + Database db = openCopy(new File("test/data/indexCursorTest.mdb")); Table table = db.getTable("test"); diff --git a/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java b/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java index 8e3ee2b..c057656 100644 --- a/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java +++ b/test/src/java/com/healthmarketscience/jackcess/IndexCodesTest.java @@ -31,7 +31,6 @@ import java.io.File; import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -376,7 +375,7 @@ public class IndexCodesTest extends TestCase { return builder.toString(); } - private static String entryToString(Cursor.Position curPos) + static String entryToString(Cursor.Position curPos) throws Exception { Field eField = curPos.getClass().getDeclaredField("_entry"); diff --git a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java index 917fc81..bb3b7f1 100644 --- a/test/src/java/com/healthmarketscience/jackcess/IndexTest.java +++ b/test/src/java/com/healthmarketscience/jackcess/IndexTest.java @@ -28,6 +28,7 @@ King of Prussia, PA 19406 package com.healthmarketscience.jackcess; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -149,12 +150,7 @@ public class IndexTest extends TestCase { db.close(); // copy to temp file and attemp to edit - File testFile = File.createTempFile("databaseTest", ".mdb"); - testFile.deleteOnExit(); - - copyFile(origFile, testFile); - - db = Database.open(testFile); + db = openCopy(origFile); t = db.getTable("Table1"); try { @@ -167,12 +163,7 @@ public class IndexTest extends TestCase { } public void testEntryDeletion() throws Exception { - File srcFile = new File("test/data/test.mdb"); - File dbFile = File.createTempFile("databaseTest", ".mdb"); - dbFile.deleteOnExit(); - copyFile(srcFile, dbFile); - - Table table = Database.open(dbFile).getTable("Table1"); + Table table = openCopy(new File("test/data/test.mdb")).getTable("Table1"); for(int i = 0; i < 10; ++i) { table.addRow("foo" + i, "bar" + i, (byte)42 + i, (short)53 + i, 13 * i, @@ -207,6 +198,116 @@ public class IndexTest extends TestCase { } } + public void testIgnoreNulls() throws Exception + { + Database db = openCopy(new File("test/data/testIndexProperties.mdb")); + + doTestIgnoreNulls(db, "TableIgnoreNulls1"); + doTestIgnoreNulls(db, "TableIgnoreNulls2"); + + db.close(); + } + + private void doTestIgnoreNulls(Database db, String tableName) + throws Exception + { + Table orig = db.getTable(tableName); + Index origI = orig.getIndex("DataIndex"); + Table temp = db.getTable(tableName + "_temp"); + Index tempI = temp.getIndex("DataIndex"); + + // copy from orig table to temp table + for(Map row : orig) { + temp.addRow(orig.asRow(row)); + } + + assertEquals(origI.getEntryCount(), tempI.getEntryCount()); + + Cursor origC = Cursor.createIndexCursor(orig, origI); + Cursor tempC = Cursor.createIndexCursor(temp, tempI); + + while(true) { + boolean origHasNext = origC.moveToNextRow(); + boolean tempHasNext = tempC.moveToNextRow(); + assertTrue(origHasNext == tempHasNext); + if(!origHasNext) { + break; + } + + Map origRow = origC.getCurrentRow(); + Cursor.Position origCurPos = origC.getSavepoint().getCurrentPosition(); + Map tempRow = tempC.getCurrentRow(); + Cursor.Position tempCurPos = tempC.getSavepoint().getCurrentPosition(); + + assertEquals(origRow, tempRow); + assertEquals(IndexCodesTest.entryToString(origCurPos), + IndexCodesTest.entryToString(tempCurPos)); + } + } + + public void testUnique() throws Exception + { + Database db = openCopy(new File("test/data/testIndexProperties.mdb")); + + Table t = db.getTable("TableUnique1_temp"); + Index index = t.getIndex("DataIndex"); + + doTestUnique(t, index, 1, + null, true, + "unique data", true, + null, true, + "more", false, + "stuff", false, + "unique data", false); + + t = db.getTable("TableUnique2_temp"); + index = t.getIndex("DataIndex"); + + doTestUnique(t, index, 2, + null, null, true, + "unique data", 42, true, + "unique data", null, true, + null, null, true, + "some", 42, true, + "more unique data", 13, true, + null, -4242, true, + "another row", -3462, false, + null, 49, false, + "more", null, false, + "unique data", 42, false, + "unique data", null, false, + null, -4242, false); + + db.close(); + } + + private void doTestUnique(Table t, Index index, int numValues, + Object... testData) + throws Exception + { + for(int i = 0; i < testData.length; i += (numValues + 1)) { + Object[] row = new Object[numValues + 1]; + row[0] = "testRow" + i; + for(int j = 1; j < (numValues + 1); ++j) { + row[j] = testData[i + j - 1]; + } + boolean expectedSuccess = (Boolean)testData[i + numValues]; + + IOException failure = null; + try { + index.addRow(row, new RowId(400 + i, 0)); + } catch(IOException e) { + failure = e; + } + if(expectedSuccess) { + assertNull(failure); + } else { + assertTrue(failure != null); + assertTrue(failure.getMessage().contains("uniqueness")); + } + } + } + private void checkIndexColumns(Table table, String... idxInfo) throws Exception {