bug #1681954. git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@174 f203690c-595d-4dc9-a70b-905162fa7fd2tags/rel_1_1_10
@@ -6,6 +6,10 @@ | |||
</properties> | |||
<body> | |||
<release version="1.1.10" date="TBD"> | |||
<action dev="jahlborn" type="fix" issue="1681954"> | |||
Update table row count correctly on row deletion or bulk row addition, | |||
bug #1681954. | |||
</action> | |||
<action dev="jahlborn" type="update" issue="1565216"> | |||
Add experimental support for auto-number columns, feature request | |||
#1565216. |
@@ -32,6 +32,7 @@ import java.io.IOException; | |||
import java.nio.ByteBuffer; | |||
import java.nio.ByteOrder; | |||
import java.util.ArrayList; | |||
import java.util.Arrays; | |||
import java.util.Collection; | |||
import java.util.Collections; | |||
import java.util.Comparator; | |||
@@ -42,7 +43,10 @@ import java.util.List; | |||
import java.util.Map; | |||
import java.util.SortedSet; | |||
import java.util.TreeSet; | |||
import org.apache.commons.lang.builder.CompareToBuilder; | |||
import org.apache.commons.logging.Log; | |||
import org.apache.commons.logging.LogFactory; | |||
@@ -52,6 +56,8 @@ import org.apache.commons.lang.builder.CompareToBuilder; | |||
*/ | |||
public class Index implements Comparable<Index> { | |||
private static final Log LOG = LogFactory.getLog(Index.class); | |||
/** Max number of columns in an index */ | |||
private static final int MAX_COLUMNS = 10; | |||
@@ -182,7 +188,9 @@ public class Index implements Comparable<Index> { | |||
/** Page number of the index data */ | |||
private int _pageNumber; | |||
private int _parentPageNumber; | |||
/** Number of rows in the index */ | |||
/** Number of rows in the index | |||
NOTE: this does not actually seem to be the row count, unclear what the | |||
value means*/ | |||
private int _rowCount; | |||
private JetFormat _format; | |||
private SortedSet<Entry> _entries; | |||
@@ -317,9 +325,7 @@ public class Index implements Comparable<Index> { | |||
buffer.put((byte) 0); //Unknown | |||
byte[] entryMask = new byte[_format.SIZE_INDEX_ENTRY_MASK]; | |||
int totalSize = 0; | |||
Iterator iter = _entries.iterator(); | |||
while (iter.hasNext()) { | |||
Entry entry = (Entry) iter.next(); | |||
for(Entry entry : _entries) { | |||
int size = entry.size(); | |||
totalSize += size; | |||
int idx = totalSize / 8; | |||
@@ -330,9 +336,7 @@ public class Index implements Comparable<Index> { | |||
entryMask[idx] |= (1 << (totalSize % 8)); | |||
} | |||
buffer.put(entryMask); | |||
iter = _entries.iterator(); | |||
while (iter.hasNext()) { | |||
Entry entry = (Entry) iter.next(); | |||
for(Entry entry : _entries) { | |||
entry.write(buffer); | |||
} | |||
buffer.putShort(2, (short) (_format.PAGE_SIZE - buffer.position())); | |||
@@ -498,7 +502,7 @@ public class Index implements Comparable<Index> { | |||
/** | |||
* Add a row to this index | |||
* Adds a row to this index | |||
* <p> | |||
* Forces index initialization. | |||
* | |||
@@ -511,10 +515,48 @@ public class Index implements Comparable<Index> { | |||
{ | |||
// make sure we've parsed the entries | |||
initialize(); | |||
++_rowCount; | |||
_entries.add(new Entry(row, pageNumber, rowNumber)); | |||
} | |||
/** | |||
* Removes a row from this index | |||
* <p> | |||
* Forces index initialization. | |||
* | |||
* @param row Row to remove | |||
* @param pageNumber Page number on which the row is removed | |||
* @param rowNumber Row number at which the row is removed | |||
*/ | |||
public void deleteRow(Object[] row, int pageNumber, byte rowNumber) | |||
throws IOException | |||
{ | |||
// make sure we've parsed the entries | |||
initialize(); | |||
--_rowCount; | |||
Entry oldEntry = new Entry(row, pageNumber, rowNumber); | |||
if(!_entries.remove(oldEntry)) { | |||
// the caller may have only read some of the row data, if this is the | |||
// case, just search for the page/row numbers | |||
boolean removed = false; | |||
for(Iterator<Entry> iter = _entries.iterator(); iter.hasNext(); ) { | |||
Entry entry = iter.next(); | |||
if((entry.getPage() == pageNumber) && | |||
(entry.getRow() == rowNumber)) { | |||
iter.remove(); | |||
removed = true; | |||
break; | |||
} | |||
} | |||
if(!removed) { | |||
LOG.warn("Failed removing index entry " + oldEntry + " for row: " + | |||
Arrays.asList(row)); | |||
} | |||
} | |||
} | |||
@Override | |||
public String toString() { | |||
StringBuilder rtn = new StringBuilder(); | |||
@@ -719,7 +761,7 @@ public class Index implements Comparable<Index> { | |||
return new FixedEntryColumn(col); | |||
} | |||
public List getEntryColumns() { | |||
public List<EntryColumn> getEntryColumns() { | |||
return _entryColumns; | |||
} | |||
@@ -733,9 +775,8 @@ public class Index implements Comparable<Index> { | |||
public int size() { | |||
int rtn = 4; | |||
Iterator iter = _entryColumns.iterator(); | |||
while (iter.hasNext()) { | |||
rtn += ((EntryColumn) iter.next()).size(); | |||
for(EntryColumn entryCol : _entryColumns) { | |||
rtn += entryCol.size(); | |||
} | |||
return rtn; | |||
} | |||
@@ -744,9 +785,8 @@ public class Index implements Comparable<Index> { | |||
* Write this entry into a buffer | |||
*/ | |||
public void write(ByteBuffer buffer) throws IOException { | |||
Iterator iter = _entryColumns.iterator(); | |||
while (iter.hasNext()) { | |||
((EntryColumn) iter.next()).write(buffer); | |||
for(EntryColumn entryCol : _entryColumns) { | |||
entryCol.write(buffer); | |||
} | |||
buffer.put((byte) (_page >>> 16)); | |||
buffer.put((byte) (_page >>> 8)); | |||
@@ -763,15 +803,15 @@ public class Index implements Comparable<Index> { | |||
if (this == other) { | |||
return 0; | |||
} | |||
Iterator myIter = _entryColumns.iterator(); | |||
Iterator otherIter = other.getEntryColumns().iterator(); | |||
Iterator<EntryColumn> myIter = _entryColumns.iterator(); | |||
Iterator<EntryColumn> otherIter = other.getEntryColumns().iterator(); | |||
while (myIter.hasNext()) { | |||
if (!otherIter.hasNext()) { | |||
throw new IllegalArgumentException( | |||
"Trying to compare index entries with a different number of entry columns"); | |||
} | |||
EntryColumn myCol = (EntryColumn) myIter.next(); | |||
EntryColumn otherCol = (EntryColumn) otherIter.next(); | |||
EntryColumn myCol = myIter.next(); | |||
EntryColumn otherCol = otherIter.next(); | |||
int i = myCol.compareTo(otherCol); | |||
if (i != 0) { | |||
return i; |
@@ -30,6 +30,7 @@ package com.healthmarketscience.jackcess; | |||
import java.io.IOException; | |||
import java.nio.ByteBuffer; | |||
import java.util.ArrayList; | |||
import java.util.Arrays; | |||
import java.util.Collection; | |||
import java.util.Collections; | |||
import java.util.Comparator; | |||
@@ -161,7 +162,7 @@ public class Table | |||
readTableDefinition(tableBuffer); | |||
tableBuffer = null; | |||
_rowState = new RowState(true); | |||
_rowState = new RowState(true, _maxColumnCount); | |||
} | |||
/** | |||
@@ -236,13 +237,29 @@ public class Table | |||
// FIXME, want to make this static, but writeDataPage is not static, also, this may screw up other rowstates... | |||
// see if row was already deleted | |||
if(_rowState.isDeleted()) { | |||
throw new IllegalStateException("Deleting already deleted row"); | |||
} | |||
// delete flag always gets set in the "root" page (even if overflow row) | |||
ByteBuffer rowBuffer = _rowState.getPage(_pageChannel); | |||
int index = getRowStartOffset(_currentRowInPage, _format); | |||
rowBuffer.putShort(index, (short)(rowBuffer.getShort(index) | |||
int pageNumber = _rowState.getPageNumber(); | |||
int rowIndex = getRowStartOffset(_currentRowInPage, _format); | |||
rowBuffer.putShort(rowIndex, (short)(rowBuffer.getShort(rowIndex) | |||
| DELETED_ROW_MASK | OVERFLOW_ROW_MASK)); | |||
writeDataPage(rowBuffer, _rowState.getPageNumber()); | |||
writeDataPage(rowBuffer, pageNumber); | |||
_rowState.setDeleted(true); | |||
// update the indexes | |||
for(Index index : _indexes) { | |||
index.deleteRow(_rowState.getRowValues(), pageNumber, | |||
(byte)_currentRowInPage); | |||
} | |||
// make sure table def gets updated | |||
--_rowCount; | |||
updateTableDefinition(); | |||
} | |||
/** | |||
@@ -265,7 +282,7 @@ public class Table | |||
return null; | |||
} | |||
return getRow(rowBuffer, getRowNullMask(rowBuffer), _columns, | |||
return getRow(_rowState, rowBuffer, getRowNullMask(rowBuffer), _columns, | |||
columnNames); | |||
} | |||
@@ -312,7 +329,7 @@ public class Table | |||
return null; | |||
} | |||
return getRow(rowBuffer, getRowNullMask(rowBuffer), columns, | |||
return getRow(rowState, rowBuffer, getRowNullMask(rowBuffer), columns, | |||
columnNames); | |||
} | |||
@@ -320,6 +337,7 @@ public class Table | |||
* Reads the row data from the given row buffer. Leaves limit unchanged. | |||
*/ | |||
private static Map<String, Object> getRow( | |||
RowState rowState, | |||
ByteBuffer rowBuffer, | |||
NullMask nullMask, | |||
Collection<Column> columns, | |||
@@ -329,10 +347,18 @@ public class Table | |||
Map<String, Object> rtn = new LinkedHashMap<String, Object>( | |||
columns.size()); | |||
for(Column column : columns) { | |||
Object value = null; | |||
if((columnNames == null) || (columnNames.contains(column.getName()))) { | |||
// Add the value to the row data | |||
rtn.put(column.getName(), getRowColumn(rowBuffer, nullMask, column)); | |||
value = getRowColumn(rowBuffer, nullMask, column); | |||
rtn.put(column.getName(), value); | |||
} | |||
// cache the row values in order to be able to update the index on row | |||
// deletion. note, most of the returned values are immutable, except | |||
// for binary data (returned as byte[]), but binary data shouldn't be | |||
// indexed anyway. | |||
rowState._rowValues[column.getColumnNumber()] = value; | |||
} | |||
return rtn; | |||
@@ -1033,26 +1059,40 @@ public class Table | |||
dataPage.put(rowData[i]); | |||
// update the indexes | |||
Iterator<Index> indIter = _indexes.iterator(); | |||
while (indIter.hasNext()) { | |||
Index index = indIter.next(); | |||
index.addRow(rows.get(i), pageNumber, (byte) rowNum); | |||
for(Index index : _indexes) { | |||
index.addRow(rows.get(i), pageNumber, (byte)rowNum); | |||
} | |||
} | |||
writeDataPage(dataPage, pageNumber); | |||
//Update tdef page | |||
_rowCount += rows.size(); | |||
updateTableDefinition(); | |||
} | |||
/** | |||
* Updates the table definition after rows are modified. | |||
*/ | |||
private void updateTableDefinition() throws IOException | |||
{ | |||
// load table definition | |||
ByteBuffer tdefPage = _pageChannel.createPageBuffer(); | |||
_pageChannel.readPage(tdefPage, _tableDefPageNumber); | |||
tdefPage.putInt(_format.OFFSET_NUM_ROWS, ++_rowCount); | |||
// make sure rowcount and autonumber are up-to-date | |||
tdefPage.putInt(_format.OFFSET_NUM_ROWS, _rowCount); | |||
tdefPage.putInt(_format.OFFSET_NEXT_AUTO_NUMBER, _lastAutoNumber); | |||
// write any index changes | |||
Iterator<Index> indIter = _indexes.iterator(); | |||
for (int i = 0; i < _indexes.size(); i++) { | |||
tdefPage.putInt(_format.OFFSET_INDEX_DEF_BLOCK + | |||
i * _format.SIZE_INDEX_DEFINITION + 4, _rowCount); | |||
(i * _format.SIZE_INDEX_DEFINITION) + 4, _rowCount); | |||
Index index = indIter.next(); | |||
index.update(); | |||
} | |||
// write modified table definition | |||
_pageChannel.writePage(tdefPage, _tableDefPageNumber); | |||
} | |||
@@ -1393,15 +1433,19 @@ public class Table | |||
/** the row buffer which contains the final data (after following any | |||
overflow pointers) */ | |||
public ByteBuffer _finalRowBuffer; | |||
public RowState(boolean hardRowBuffer) { | |||
/** values read from the last row */ | |||
public Object[] _rowValues; | |||
public RowState(boolean hardRowBuffer, int colCount) { | |||
_rowBufferH = TempPageHolder.newHolder(hardRowBuffer); | |||
_rowValues = new Object[colCount]; | |||
} | |||
public void reset() { | |||
_finalRowBuffer = null; | |||
_deleted = false; | |||
_overflow = false; | |||
Arrays.fill(_rowValues, null); | |||
} | |||
public int getPageNumber() { | |||
@@ -1430,6 +1474,10 @@ public class Table | |||
return _overflow; | |||
} | |||
public Object[] getRowValues() { | |||
return _rowValues; | |||
} | |||
public void possiblyInvalidate(int modifiedPageNumber, | |||
ByteBuffer modifiedBuffer) { | |||
_rowBufferH.possiblyInvalidate(modifiedPageNumber, |
@@ -238,7 +238,8 @@ public class DatabaseTest extends TestCase { | |||
Object[] row3 = createTestRow("Tim3"); | |||
Table table = db.getTable("Test"); | |||
table.addRows(Arrays.asList(row1, row2, row3)); | |||
assertRowCount(3, table); | |||
table.reset(); | |||
table.getNextRow(); | |||
table.getNextRow(); | |||
@@ -250,6 +251,7 @@ public class DatabaseTest extends TestCase { | |||
assertEquals("Tim1", outRow.get("A")); | |||
outRow = table.getNextRow(); | |||
assertEquals("Tim3", outRow.get("A")); | |||
assertRowCount(2, table); | |||
// test multi row delete/add | |||
db = create(); | |||
@@ -261,29 +263,29 @@ public class DatabaseTest extends TestCase { | |||
table.addRow(row); | |||
} | |||
row[3] = 1974; | |||
assertEquals(10, countRows(table)); | |||
assertRowCount(10, table); | |||
table.reset(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
assertEquals(9, countRows(table)); | |||
assertRowCount(9, table); | |||
table.reset(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
assertEquals(8, countRows(table)); | |||
assertRowCount(8, table); | |||
table.reset(); | |||
for (int i = 0; i < 8; i++) { | |||
table.getNextRow(); | |||
} | |||
table.deleteCurrentRow(); | |||
assertEquals(7, countRows(table)); | |||
assertRowCount(7, table); | |||
table.addRow(row); | |||
assertEquals(8, countRows(table)); | |||
assertRowCount(8, table); | |||
table.reset(); | |||
for (int i = 0; i < 3; i++) { | |||
table.getNextRow(); | |||
} | |||
table.deleteCurrentRow(); | |||
assertEquals(7, countRows(table)); | |||
assertRowCount(7, table); | |||
table.reset(); | |||
assertEquals(2, table.getNextRow().get("D")); | |||
} | |||
@@ -607,7 +609,7 @@ public class DatabaseTest extends TestCase { | |||
new ArrayList<Object>(row.values())); | |||
table.reset(); | |||
assertEquals(7, countRows(table)); | |||
assertRowCount(7, table); | |||
} | |||
@@ -809,7 +811,14 @@ public class DatabaseTest extends TestCase { | |||
String str = builder.toString(); | |||
return str; | |||
} | |||
static void assertRowCount(int expectedRowCount, Table table) | |||
throws Exception | |||
{ | |||
assertEquals(expectedRowCount, countRows(table)); | |||
assertEquals(expectedRowCount, table.getRowCount()); | |||
} | |||
static int countRows(Table table) throws Exception { | |||
int rtn = 0; | |||
for(Map<String, Object> row : table) { |
@@ -122,5 +122,45 @@ 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"); | |||
for(int i = 0; i < 10; ++i) { | |||
table.addRow("foo" + i, "bar" + i, (byte)42 + i, (short)53 + i, 13 * i, | |||
(6.7d / i), null, null, true); | |||
} | |||
table.reset(); | |||
assertRowCount(12, table); | |||
for(Index index : table.getIndexes()) { | |||
assertEquals(12, index.getEntryCount()); | |||
} | |||
table.reset(); | |||
table.getNextRow(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
table.getNextRow(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
table.getNextRow(); | |||
table.getNextRow(); | |||
table.getNextRow(); | |||
table.deleteCurrentRow(); | |||
table.reset(); | |||
assertRowCount(8, table); | |||
for(Index index : table.getIndexes()) { | |||
assertEquals(8, index.getEntryCount()); | |||
} | |||
} | |||
} |