Update table row count correctly on row deletion or bulk row addition,

bug #1681954. git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@174 f203690c-595d-4dc9-a70b-905162fa7fd2
author: James Ahlborn <jtahlborn@yahoo.com> 2007-11-17 04:14:00 +0000
committer: James Ahlborn <jtahlborn@yahoo.com> 2007-11-17 04:14:00 +0000
commit: 1fcd851a9f5d780b3d78b92f89bd35a2d634b467 (patch)
tree: 1b6f37f25a95d3aaeb3d907f8d925f5889b39f4a /src
parent: 9ba0e78522e68b40c75b0cb86fe74815146ce20d (diff)
download: jackcess-1fcd851a9f5d780b3d78b92f89bd35a2d634b467.tar.gz
jackcess-1fcd851a9f5d780b3d78b92f89bd35a2d634b467.zip
3 files changed, 127 insertions, 35 deletions
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 11d8e23..b92065b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -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.
diff --git a/src/java/com/healthmarketscience/jackcess/Index.java b/src/java/com/healthmarketscience/jackcess/Index.java
index 0cd6689..073ce57 100644
--- a/src/java/com/healthmarketscience/jackcess/Index.java
+++ b/src/java/com/healthmarketscience/jackcess/Index.java
@@ -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;
diff --git a/src/java/com/healthmarketscience/jackcess/Table.java b/src/java/com/healthmarketscience/jackcess/Table.java
index 3bed3cb..6acab8d 100644
--- a/src/java/com/healthmarketscience/jackcess/Table.java
+++ b/src/java/com/healthmarketscience/jackcess/Table.java
@@ -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,
author	James Ahlborn <jtahlborn@yahoo.com>	2007-11-17 04:14:00 +0000
committer	James Ahlborn <jtahlborn@yahoo.com>	2007-11-17 04:14:00 +0000
commit	1fcd851a9f5d780b3d78b92f89bd35a2d634b467 (patch)
tree	1b6f37f25a95d3aaeb3d907f8d925f5889b39f4a /src
parent	9ba0e78522e68b40c75b0cb86fe74815146ce20d (diff)
download	jackcess-1fcd851a9f5d780b3d78b92f89bd35a2d634b467.tar.gz jackcess-1fcd851a9f5d780b3d78b92f89bd35a2d634b467.zip