Fix for bug 45639 - cleaned up index logic inside ColumnInfoRecordsAggregate

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@694534 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45639 - Fixed AIOOBE due to bad index logic in ColumnInfoRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">Fixed special cases of INDEX function (single column/single row, errors)</action>
            <action dev="POI-DEVELOPERS" type="add">45761 - Support for Very Hidden excel sheets in HSSF</action>
            <action dev="POI-DEVELOPERS" type="add">45738 - Initial HWPF support for Office Art Shapes</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45639 - Fixed AIOOBE due to bad index logic in ColumnInfoRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="fix">Fixed special cases of INDEX function (single column/single row, errors)</action>
            <action dev="POI-DEVELOPERS" type="add">45761 - Support for Very Hidden excel sheets in HSSF</action>
            <action dev="POI-DEVELOPERS" type="add">45738 - Initial HWPF support for Office Art Shapes</action>

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -1055,7 +1055,7 @@ public final class Sheet implements Model {
 
         ColumnInfoRecord ci = _columnInfos.findColumnInfo(columnIndex);
         if (ci != null) {
-            return ci.getColumnWidth();
+            return (short)ci.getColumnWidth();
         }
         //default column width is measured in characters
         //multiply
@@ -1079,7 +1079,7 @@ public final class Sheet implements Model {
     public short getXFIndexForColAt(short columnIndex) {
         ColumnInfoRecord ci = _columnInfos.findColumnInfo(columnIndex);
         if (ci != null) {
-            return ci.getXFIndex();
+            return (short)ci.getXFIndex();
         }
         return 0xF;
     }
@@ -1138,8 +1138,7 @@ public final class Sheet implements Model {
      * @param indent        if true the group will be indented by one level,
      *                      if false indenting will be removed by one level.
      */
-    public void groupColumnRange(short fromColumn, short toColumn, boolean indent)
+    public void groupColumnRange(int fromColumn, int toColumn, boolean indent) {
-    {
 
         // Set the level for each column
         _columnInfos.groupColumnRange( fromColumn, toColumn, indent);
@@ -1709,14 +1708,10 @@ public final class Sheet implements Model {
     }
 
 
-    public void setColumnGroupCollapsed( short columnNumber, boolean collapsed )
+    public void setColumnGroupCollapsed(int columnNumber, boolean collapsed) {
-    {
+		if (collapsed) {
-        if (collapsed)
-        {
 			_columnInfos.collapseColumn(columnNumber);
-        }
+		} else {
-        else
-        {
 			_columnInfos.expandColumn(columnNumber);
 		}
 	}

src/java/org/apache/poi/hssf/record/ColumnInfoRecord.java

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.record;
 
+import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
@@ -30,19 +31,24 @@ import org.apache.poi.util.BitFieldFactory;
  */
 public final class ColumnInfoRecord extends Record {
     public static final short     sid = 0x7d;
-    private short                 field_1_first_col;
+    private int field_1_first_col;
-    private short                 field_2_last_col;
+    private int field_2_last_col;
-    private short                 field_3_col_width;
+    private int field_3_col_width;
-    private short                 field_4_xf_index;
+    private int field_4_xf_index;
-    private short                 field_5_options;
+    private int field_5_options;
     private static final BitField hidden    = BitFieldFactory.getInstance(0x01);
     private static final BitField outlevel  = BitFieldFactory.getInstance(0x0700);
     private static final BitField collapsed = BitFieldFactory.getInstance(0x1000);
     // Excel seems write values 2, 10, and 260, even though spec says "must be zero"
     private short                 field_6_reserved;
 
-    public ColumnInfoRecord()
+    /**
-    {
+     * Creates a column info record with default width and format
+     */
+    public ColumnInfoRecord() {
+        setColumnWidth(2275);
+        field_5_options = 2; 
+        field_4_xf_index = 0x0f;
         field_6_reserved = 2; // seems to be the most common value
     }
 
@@ -90,7 +96,7 @@ public final class ColumnInfoRecord extends Record {
      * @param fc - the first column index (0-based)
      */
 
-    public void setFirstColumn(short fc)
+    public void setFirstColumn(int fc)
     {
         field_1_first_col = fc;
     }
@@ -100,7 +106,7 @@ public final class ColumnInfoRecord extends Record {
      * @param lc - the last column index (0-based)
      */
 
-    public void setLastColumn(short lc)
+    public void setLastColumn(int lc)
     {
         field_2_last_col = lc;
     }
@@ -110,7 +116,7 @@ public final class ColumnInfoRecord extends Record {
      * @param cw - column width
      */
 
-    public void setColumnWidth(short cw)
+    public void setColumnWidth(int cw)
     {
         field_3_col_width = cw;
     }
@@ -121,20 +127,11 @@ public final class ColumnInfoRecord extends Record {
      * @see org.apache.poi.hssf.record.ExtendedFormatRecord
      */
 
-    public void setXFIndex(short xfi)
+    public void setXFIndex(int xfi)
     {
         field_4_xf_index = xfi;
     }
 
-    /**
-     * set the options bitfield - use the bitsetters instead
-     * @param options - the bitfield raw value
-     */
-
-    public void setOptions(short options)
-    {
-        field_5_options = options;
-    }
 
     // start options bitfield
 
@@ -146,7 +143,7 @@ public final class ColumnInfoRecord extends Record {
 
     public void setHidden(boolean ishidden)
     {
-        field_5_options = hidden.setShortBoolean(field_5_options, ishidden);
+        field_5_options = hidden.setBoolean(field_5_options, ishidden);
     }
 
     /**
@@ -155,9 +152,9 @@ public final class ColumnInfoRecord extends Record {
      * @param olevel -outline level for the cells
      */
 
-    public void setOutlineLevel(short olevel)
+    public void setOutlineLevel(int olevel)
     {
-        field_5_options = outlevel.setShortValue(field_5_options, olevel);
+        field_5_options = outlevel.setValue(field_5_options, olevel);
     }
 
     /**
@@ -168,7 +165,7 @@ public final class ColumnInfoRecord extends Record {
 
     public void setCollapsed(boolean iscollapsed)
     {
-        field_5_options = collapsed.setShortBoolean(field_5_options,
+        field_5_options = collapsed.setBoolean(field_5_options,
                                                     iscollapsed);
     }
 
@@ -179,7 +176,7 @@ public final class ColumnInfoRecord extends Record {
      * @return the first column index (0-based)
      */
 
-    public short getFirstColumn()
+    public int getFirstColumn()
     {
         return field_1_first_col;
     }
@@ -189,7 +186,7 @@ public final class ColumnInfoRecord extends Record {
      * @return the last column index (0-based)
      */
 
-    public short getLastColumn()
+    public int getLastColumn()
     {
         return field_2_last_col;
     }
@@ -199,7 +196,7 @@ public final class ColumnInfoRecord extends Record {
      * @return column width
      */
 
-    public short getColumnWidth()
+    public int getColumnWidth()
     {
         return field_3_col_width;
     }
@@ -210,20 +207,17 @@ public final class ColumnInfoRecord extends Record {
      * @see org.apache.poi.hssf.record.ExtendedFormatRecord
      */
 
-    public short getXFIndex()
+    public int getXFIndex()
     {
         return field_4_xf_index;
     }
 
-    /**
+    public int getOptions() {
-     * get the options bitfield - use the bitsetters instead
-     * @return the bitfield raw value
-     */
-
-    public short getOptions()
-    {
         return field_5_options;
     }
+    public void setOptions(int field_5_options) {
+        this.field_5_options = field_5_options;
+    }
     
     // start options bitfield
 
@@ -244,9 +238,9 @@ public final class ColumnInfoRecord extends Record {
      * @return outline level for the cells
      */
 
-    public short getOutlineLevel()
+    public int getOutlineLevel()
     {
-        return outlevel.getShortValue(field_5_options);
+        return outlevel.getValue(field_5_options);
     }
 
     /**
@@ -261,6 +255,31 @@ public final class ColumnInfoRecord extends Record {
     }
 
     // end options bitfield
+    
+    public boolean containsColumn(int columnIndex) {
+        return field_1_first_col <= columnIndex && columnIndex <= field_2_last_col; 
+    }
+    public boolean isAdjacentBefore(ColumnInfoRecord other) {
+        return field_2_last_col == other.field_1_first_col - 1;
+    }
+   
+    /**
+     * @return <code>true</code> if the format, options and column width match
+     */
+    public boolean formatMatches(ColumnInfoRecord other) {
+        if (field_4_xf_index != other.field_4_xf_index) {
+            return false;
+        }
+        if (field_5_options != other.field_5_options) {
+            return false;
+        }
+        if (field_3_col_width != other.field_3_col_width) {
+            return false;
+        }
+        return true;
+    }
+    
+    
     public short getSid()
     {
         return sid;
@@ -269,13 +288,13 @@ public final class ColumnInfoRecord extends Record {
     public int serialize(int offset, byte [] data)
     {
         LittleEndian.putShort(data, 0 + offset, sid);
-        LittleEndian.putShort(data, 2 + offset, ( short ) 12);
+        LittleEndian.putUShort(data, 2 + offset, 12);
-        LittleEndian.putShort(data, 4 + offset, getFirstColumn());
+        LittleEndian.putUShort(data, 4 + offset, getFirstColumn());
-        LittleEndian.putShort(data, 6 + offset, getLastColumn());
+        LittleEndian.putUShort(data, 6 + offset, getLastColumn());
-        LittleEndian.putShort(data, 8 + offset, getColumnWidth());
+        LittleEndian.putUShort(data, 8 + offset, getColumnWidth());
-        LittleEndian.putShort(data, 10 + offset, getXFIndex());
+        LittleEndian.putUShort(data, 10 + offset, getXFIndex());
-        LittleEndian.putShort(data, 12 + offset, getOptions());
+        LittleEndian.putUShort(data, 12 + offset, field_5_options);
-        LittleEndian.putShort(data, 14 + offset, field_6_reserved);   
+        LittleEndian.putUShort(data, 14 + offset, field_6_reserved);   
         return getRecordSize();
     }
 
@@ -286,24 +305,19 @@ public final class ColumnInfoRecord extends Record {
 
     public String toString()
     {
-        StringBuffer buffer = new StringBuffer();
+        StringBuffer sb = new StringBuffer();
 
-        buffer.append("[COLINFO]\n");
+        sb.append("[COLINFO]\n");
-        buffer.append("colfirst       = ").append(getFirstColumn())
+        sb.append("  colfirst = ").append(getFirstColumn()).append("\n");
-            .append("\n");
+        sb.append("  collast  = ").append(getLastColumn()).append("\n");
-        buffer.append("collast        = ").append(getLastColumn())
+        sb.append("  colwidth = ").append(getColumnWidth()).append("\n");
-            .append("\n");
+        sb.append("  xfindex  = ").append(getXFIndex()).append("\n");
-        buffer.append("colwidth       = ").append(getColumnWidth())
+        sb.append("  options  = ").append(HexDump.shortToHex(field_5_options)).append("\n");
-            .append("\n");
+        sb.append("    hidden   = ").append(getHidden()).append("\n");
-        buffer.append("xfindex        = ").append(getXFIndex()).append("\n");
+        sb.append("    olevel   = ").append(getOutlineLevel()).append("\n");
-        buffer.append("options        = ").append(getOptions()).append("\n");
+        sb.append("    collapsed= ").append(getCollapsed()).append("\n");
-        buffer.append("  hidden       = ").append(getHidden()).append("\n");
+        sb.append("[/COLINFO]\n");
-        buffer.append("  olevel       = ").append(getOutlineLevel())
+        return sb.toString();
-            .append("\n");
-        buffer.append("  collapsed    = ").append(getCollapsed())
-            .append("\n");
-        buffer.append("[/COLINFO]\n");
-        return buffer.toString();
     }
 
     public Object clone() {

src/java/org/apache/poi/hssf/record/aggregates/ColumnInfoRecordsAggregate.java

@@ -18,19 +18,36 @@
 package org.apache.poi.hssf.record.aggregates;
 
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.ColumnInfoRecord;
-import org.apache.poi.hssf.record.Record;
 
 /**
  * @author Glen Stampoultzis
- * @version $Id$
  */
 public final class ColumnInfoRecordsAggregate extends RecordAggregate {
+	/**
+	 * List of {@link ColumnInfoRecord}s assumed to be in order 
+	 */
 	private final List records;
 	
+	
+	private static final class CIRComparator implements Comparator {
+		public static final Comparator instance = new CIRComparator();
+		private CIRComparator() {
+			// enforce singleton
+		}
+		public int compare(Object a, Object b) {
+			return compareColInfos((ColumnInfoRecord)a, (ColumnInfoRecord)b);
+		}
+		public static int compareColInfos(ColumnInfoRecord a, ColumnInfoRecord b) {
+			return a.getFirstColumn()-b.getFirstColumn();
+		}
+	}
+
 	/**
 	 * Creates an empty aggregate
 	 */
@@ -40,25 +57,32 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 	public ColumnInfoRecordsAggregate(RecordStream rs) {
 		this();
 
+		boolean isInOrder = true;
+		ColumnInfoRecord cirPrev = null;
 		while(rs.peekNextClass() == ColumnInfoRecord.class) {
-        	records.add(rs.getNext());
+			ColumnInfoRecord cir = (ColumnInfoRecord) rs.getNext();
+			records.add(cir);
+			if (cirPrev != null && CIRComparator.compareColInfos(cirPrev, cir) > 0) {
+				isInOrder = false;
+			}
+			cirPrev = cir;
 		}
 		if (records.size() < 1) {
 			throw new RuntimeException("No column info records found");
 		}
+		if (!isInOrder) {
+			Collections.sort(records, CIRComparator.instance);
+		}
 	}
 
 	/**
 	 * Performs a deep clone of the record
 	 */
-    public Object clone()
+	public Object clone() {
-    {
 		ColumnInfoRecordsAggregate rec = new ColumnInfoRecordsAggregate();
-        for (int k = 0; k < records.size(); k++)
+		for (int k = 0; k < records.size(); k++) {
-        {
 			ColumnInfoRecord ci = ( ColumnInfoRecord ) records.get(k);
-            ci=(ColumnInfoRecord) ci.clone();
+			rec.records.add(ci.clone());
-            rec.insertColumn( ci );
 		}
 		return rec;
 	}
@@ -66,22 +90,20 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 	/**
 	 * Inserts a column into the aggregate (at the end of the list).
 	 */
-    public void insertColumn( ColumnInfoRecord col )
+	public void insertColumn(ColumnInfoRecord col) {
-    {
 		records.add(col);
+		Collections.sort(records, CIRComparator.instance);
 	}
 
 	/**
-     * Inserts a column into the aggregate (at the position specified
+	 * Inserts a column into the aggregate (at the position specified by
-     * by <code>idx</code>.
+	 * <code>idx</code>.
 	 */
-    public void insertColumn( int idx, ColumnInfoRecord col )
+	private void insertColumn(int idx, ColumnInfoRecord col) {
-    {
 		records.add(idx, col);
 	}
 
-    public int getNumColumns( )
+	/* package */ int getNumColumns() {
-    {
 		return records.size();
 	}
 
@@ -90,60 +112,55 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 		if (nItems < 1) {
 			return;
 		}
+		ColumnInfoRecord cirPrev = null;
 		for(int i=0; i<nItems; i++) {
-    		rv.visitRecord((Record)records.get(i));
+			ColumnInfoRecord cir = (ColumnInfoRecord)records.get(i);
+			rv.visitRecord(cir);
+			if (cirPrev != null && CIRComparator.compareColInfos(cirPrev, cir) > 0) {
+				// Excel probably wouldn't mind, but there is much logic in this class
+				// that assumes the column info records are kept in order
+				throw new RuntimeException("Column info records are out of order");
+			}
+			cirPrev = cir;
 		}
 	}
 
-    public int findStartOfColumnOutlineGroup(int idx)
+	private int findStartOfColumnOutlineGroup(int pIdx) {
-    {
 		// Find the start of the group.
-        ColumnInfoRecord columnInfo = (ColumnInfoRecord) records.get( idx );
+		ColumnInfoRecord columnInfo = (ColumnInfoRecord) records.get(pIdx);
 		int level = columnInfo.getOutlineLevel();
-        while (idx != 0)
+		int idx = pIdx;
-        {
+		while (idx != 0) {
 			ColumnInfoRecord prevColumnInfo = (ColumnInfoRecord) records.get(idx - 1);
-            if (columnInfo.getFirstColumn() - 1 == prevColumnInfo.getLastColumn())
+			if (!prevColumnInfo.isAdjacentBefore(columnInfo)) {
-            {
+				break;
-                if (prevColumnInfo.getOutlineLevel() < level)
+			}
-                {
+			if (prevColumnInfo.getOutlineLevel() < level) {
 				break;
 			}
 			idx--;
 			columnInfo = prevColumnInfo;
 		}
-            else
-            {
-                break;
-            }
-        }
 
 		return idx;
 	}
 
-    public int findEndOfColumnOutlineGroup(int idx)
+	private int findEndOfColumnOutlineGroup(int colInfoIndex) {
-    {
 		// Find the end of the group.
-        ColumnInfoRecord columnInfo = (ColumnInfoRecord) records.get( idx );
+		ColumnInfoRecord columnInfo = (ColumnInfoRecord) records.get(colInfoIndex);
 		int level = columnInfo.getOutlineLevel();
-        while (idx < records.size() - 1)
+		int idx = colInfoIndex;
-        {
+		while (idx < records.size() - 1) {
 			ColumnInfoRecord nextColumnInfo = (ColumnInfoRecord) records.get(idx + 1);
-            if (columnInfo.getLastColumn() + 1 == nextColumnInfo.getFirstColumn())
+			if (!columnInfo.isAdjacentBefore(nextColumnInfo)) {
-            {
+				break;
-                if (nextColumnInfo.getOutlineLevel() < level)
+			}
-                {
+			if (nextColumnInfo.getOutlineLevel() < level) {
 				break;
 			}
 			idx++;
 			columnInfo = nextColumnInfo;
 		}
-            else
-            {
-                break;
-            }
-        }
-
 		return idx;
 	}
 
@@ -151,128 +168,110 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 		return (ColumnInfoRecord) records.get( idx );
 	}
 
-    public ColumnInfoRecord writeHidden( ColumnInfoRecord columnInfo, int idx, boolean hidden )
+	/**
-    {
+	 * 'Collapsed' state is stored in a single column col info record immediately after the outline group
-        int level = columnInfo.getOutlineLevel();
+	 * @param idx
-        while (idx < records.size())
+	 * @return
-        {
+	 */
+	private boolean isColumnGroupCollapsed(int idx) {
+		int endOfOutlineGroupIdx = findEndOfColumnOutlineGroup(idx);
+		int nextColInfoIx = endOfOutlineGroupIdx+1;
+		if (nextColInfoIx >= records.size()) {
+			return false;
+		}
+		ColumnInfoRecord nextColInfo = getColInfo(nextColInfoIx);
+		if (!getColInfo(endOfOutlineGroupIdx).isAdjacentBefore(nextColInfo)) {
+			return false;
+		}
+		return nextColInfo.getCollapsed();
+	}
+
+
+	private boolean isColumnGroupHiddenByParent(int idx) {
+		// Look out outline details of end
+		int endLevel = 0;
+		boolean endHidden = false;
+		int endOfOutlineGroupIdx = findEndOfColumnOutlineGroup( idx );
+		if (endOfOutlineGroupIdx < records.size()) {
+			ColumnInfoRecord nextInfo = getColInfo(endOfOutlineGroupIdx + 1);
+			if (getColInfo(endOfOutlineGroupIdx).isAdjacentBefore(nextInfo)) {
+				endLevel = nextInfo.getOutlineLevel();
+				endHidden = nextInfo.getHidden();
+			}
+		}
+		// Look out outline details of start
+		int startLevel = 0;
+		boolean startHidden = false;
+		int startOfOutlineGroupIdx = findStartOfColumnOutlineGroup( idx );
+		if (startOfOutlineGroupIdx > 0) {
+			ColumnInfoRecord prevInfo = getColInfo(startOfOutlineGroupIdx - 1);
+			if (prevInfo.isAdjacentBefore(getColInfo(startOfOutlineGroupIdx))) {
+				startLevel = prevInfo.getOutlineLevel();
+				startHidden = prevInfo.getHidden();
+			}
+		}
+		if (endLevel > startLevel) {
+			return endHidden;
+		}
+		return startHidden;
+	}
+
+	public void collapseColumn(int columnIndex) {
+		int colInfoIx = findColInfoIdx(columnIndex, 0);
+		if (colInfoIx == -1) {
+			return;
+		}
+
+		// Find the start of the group.
+		int groupStartColInfoIx = findStartOfColumnOutlineGroup(colInfoIx);
+		ColumnInfoRecord columnInfo = getColInfo(groupStartColInfoIx);
+
+		// Hide all the columns until the end of the group
+		int lastColIx = setGroupHidden(groupStartColInfoIx, columnInfo.getOutlineLevel(), true);
+
+		// Write collapse field
+		setColumn(lastColIx + 1, null, null, null, null, Boolean.TRUE);
+	}
+	/**
+	 * Sets all adjacent columns of the same outline level to the specified hidden status.
+	 * @param pIdx the col info index of the start of the outline group
+	 * @return the column index of the last column in the outline group
+	 */
+	private int setGroupHidden(int pIdx, int level, boolean hidden) {
+		int idx = pIdx;
+		ColumnInfoRecord columnInfo = getColInfo(idx);
+		while (idx < records.size()) {
 			columnInfo.setHidden(hidden);
-            if (idx + 1 < records.size())
+			if (idx + 1 < records.size()) {
-            {
 				ColumnInfoRecord nextColumnInfo = getColInfo(idx + 1);
-                if (columnInfo.getLastColumn() + 1 == nextColumnInfo.getFirstColumn())
+				if (!columnInfo.isAdjacentBefore(nextColumnInfo)) {
-                {
-                    if (nextColumnInfo.getOutlineLevel() < level)
 					break;
+				}
+				if (nextColumnInfo.getOutlineLevel() < level) {
+					break;
+				}
 				columnInfo = nextColumnInfo;
 			}
-                else
-                {
-                    break;
-                }
-            }
 			idx++;
 		}
-        return columnInfo;
+		return columnInfo.getLastColumn();
-    }
-
-    public boolean isColumnGroupCollapsed( int idx )
-    {
-        int endOfOutlineGroupIdx = findEndOfColumnOutlineGroup( idx );
-        if (endOfOutlineGroupIdx >= records.size())
-            return false;
-        if (getColInfo(endOfOutlineGroupIdx).getLastColumn() + 1 != getColInfo(endOfOutlineGroupIdx + 1).getFirstColumn())
-            return false;
-        else
-            return getColInfo(endOfOutlineGroupIdx+1).getCollapsed();
 	}
 
 
-    public boolean isColumnGroupHiddenByParent( int idx )
+	public void expandColumn(int columnIndex) {
-    {
+		int idx = findColInfoIdx(columnIndex, 0);
-        // Look out outline details of end
+		if (idx == -1) {
-        int endLevel;
-        boolean endHidden;
-        int endOfOutlineGroupIdx = findEndOfColumnOutlineGroup( idx );
-        if (endOfOutlineGroupIdx >= records.size())
-        {
-            endLevel = 0;
-            endHidden = false;
-        }
-        else if (getColInfo(endOfOutlineGroupIdx).getLastColumn() + 1 != getColInfo(endOfOutlineGroupIdx + 1).getFirstColumn())
-        {
-            endLevel = 0;
-            endHidden = false;
-        }
-        else
-        {
-            endLevel = getColInfo( endOfOutlineGroupIdx + 1).getOutlineLevel();
-            endHidden = getColInfo( endOfOutlineGroupIdx + 1).getHidden();
-        }
-
-        // Look out outline details of start
-        int startLevel;
-        boolean startHidden;
-        int startOfOutlineGroupIdx = findStartOfColumnOutlineGroup( idx );
-        if (startOfOutlineGroupIdx <= 0)
-        {
-            startLevel = 0;
-            startHidden = false;
-        }
-        else if (getColInfo(startOfOutlineGroupIdx).getFirstColumn() - 1 != getColInfo(startOfOutlineGroupIdx - 1).getLastColumn())
-        {
-            startLevel = 0;
-            startHidden = false;
-        }
-        else
-        {
-            startLevel = getColInfo( startOfOutlineGroupIdx - 1).getOutlineLevel();
-            startHidden = getColInfo( startOfOutlineGroupIdx - 1 ).getHidden();
-        }
-
-        if (endLevel > startLevel)
-        {
-            return endHidden;
-        }
-        else
-        {
-            return startHidden;
-        }
-    }
-
-    public void collapseColumn( short columnNumber )
-    {
-        int idx = findColumnIdx( columnNumber, 0 );
-        if (idx == -1)
 			return;
-
-        // Find the start of the group.
-        ColumnInfoRecord columnInfo = getColInfo( findStartOfColumnOutlineGroup( idx ) );
-
-        // Hide all the columns until the end of the group
-        columnInfo = writeHidden( columnInfo, idx, true );
-
-        // Write collapse field
-        setColumn( (short) ( columnInfo.getLastColumn() + 1 ), null, null, null, null, Boolean.TRUE);
 		}
 
-    public void expandColumn( short columnNumber )
+		// If it is already expanded do nothing.
-    {
+		if (!isColumnGroupCollapsed(idx)) {
-        int idx = findColumnIdx( columnNumber, 0 );
-        if (idx == -1)
 			return;
+		}
 
-        // If it is already exapanded do nothing.
+		// Find the start/end of the group.
-        if (!isColumnGroupCollapsed(idx))
-            return;
-
-        // Find the start of the group.
 		int startIdx = findStartOfColumnOutlineGroup(idx);
-        ColumnInfoRecord columnInfo = getColInfo( startIdx );
-
-        // Find the end of the group.
 		int endIdx = findEndOfColumnOutlineGroup(idx);
-        ColumnInfoRecord endColumnInfo = getColInfo( endIdx );
 
 		// expand:
 		// colapsed bit must be unset
@@ -281,217 +280,221 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 		//   to look at the start and the end of the current group to determine which
 		//   is the enclosing group
 		// hidden bit only is altered for this outline level.  ie.  don't uncollapse contained groups
-        if (!isColumnGroupHiddenByParent( idx ))
+		ColumnInfoRecord columnInfo = getColInfo(endIdx);
-        {
+		if (!isColumnGroupHiddenByParent(idx)) {
-            for (int i = startIdx; i <= endIdx; i++)
+			int outlineLevel = columnInfo.getOutlineLevel();
-            {
+			for (int i = startIdx; i <= endIdx; i++) {
-                if (columnInfo.getOutlineLevel() == getColInfo(i).getOutlineLevel())
+				ColumnInfoRecord ci = getColInfo(i);
-                    getColInfo(i).setHidden( false );
+				if (outlineLevel == ci.getOutlineLevel())
+					ci.setHidden(false);
 			}
 		}
 
-        // Write collapse field
+		// Write collapse flag (stored in a single col info record after this outline group)
-        setColumn( (short) ( columnInfo.getLastColumn() + 1 ), null, null, null, null, Boolean.FALSE);
+		setColumn(columnInfo.getLastColumn() + 1, null, null, null, null, Boolean.FALSE);
 	}
 
-    /**
+	private static ColumnInfoRecord copyColInfo(ColumnInfoRecord ci) {
-     * creates the ColumnInfo Record and sets it to a default column/width
+		return (ColumnInfoRecord) ci.clone();
-     * @see org.apache.poi.hssf.record.ColumnInfoRecord
-     * @return record containing a ColumnInfoRecord
-     */
-    public static ColumnInfoRecord createColInfo()
-    {
-        ColumnInfoRecord retval = new ColumnInfoRecord();
-
-        retval.setColumnWidth(( short ) 2275);
-        // was:       retval.setOptions(( short ) 6);
-        retval.setOptions(( short ) 2);
-        retval.setXFIndex(( short ) 0x0f);
-        return retval;
 	}
 
 
-    public void setColumn(short column, Short xfIndex, Short width, Integer level, Boolean hidden, Boolean collapsed)
+	public void setColumn(int targetColumnIx, Short xfIndex, Short width, 
-    {
+					Integer level, Boolean hidden, Boolean collapsed) {
 		ColumnInfoRecord ci = null;
 		int k  = 0;
 
-        for (k = 0; k < records.size(); k++)
+		for (k = 0; k < records.size(); k++) {
-        {
+			ColumnInfoRecord tci = (ColumnInfoRecord) records.get(k);
-            ci = ( ColumnInfoRecord ) records.get(k);
+			if (tci.containsColumn(targetColumnIx)) {
-            if ((ci.getFirstColumn() <= column)
+				ci = tci;
-                    && (column <= ci.getLastColumn()))
-            {
 				break;
 			}
-            ci = null;
+			if (tci.getFirstColumn() > targetColumnIx) {
+				// call column infos after k are for later columns
+				break; // exit now so k will be the correct insert pos
+			}
+		}
+
+		if (ci == null) {
+			// okay so there ISN'T a column info record that covers this column so lets create one!
+			ColumnInfoRecord nci = new ColumnInfoRecord();
+
+			nci.setFirstColumn(targetColumnIx);
+			nci.setLastColumn(targetColumnIx);
+			setColumnInfoFields( nci, xfIndex, width, level, hidden, collapsed );
+			insertColumn(k, nci);
+			attemptMergeColInfoRecords(k);
+			return;
 		}
 
-        if (ci != null)
-        {
 		boolean styleChanged = xfIndex != null && ci.getXFIndex() != xfIndex.shortValue();
 		boolean widthChanged = width != null && ci.getColumnWidth() != width.shortValue();
 		boolean levelChanged = level != null && ci.getOutlineLevel() != level.intValue();
 		boolean hiddenChanged = hidden != null && ci.getHidden() != hidden.booleanValue();
 		boolean collapsedChanged = collapsed != null && ci.getCollapsed() != collapsed.booleanValue();
+
 		boolean columnChanged = styleChanged || widthChanged || levelChanged || hiddenChanged || collapsedChanged;
-            if (!columnChanged)
+		if (!columnChanged) {
-            {
 			// do nothing...nothing changed.
+			return;
 		}
-            else if ((ci.getFirstColumn() == column)
+
-                     && (ci.getLastColumn() == column))
+		if (ci.getFirstColumn() == targetColumnIx && ci.getLastColumn() == targetColumnIx) {
-            {                               // if its only for this cell then
+			// ColumnInfo ci for a single column, the target column
 			setColumnInfoFields(ci, xfIndex, width, level, hidden, collapsed);
+			attemptMergeColInfoRecords(k);
+			return;
 		}
-            else if ((ci.getFirstColumn() == column)
+
-                     || (ci.getLastColumn() == column))
+		if (ci.getFirstColumn() == targetColumnIx || ci.getLastColumn() == targetColumnIx) {
-            {
+			// The target column is at either end of the multi-column ColumnInfo ci
-                // okay so the width is different but the first or last column == the column we'return setting
 			// we'll just divide the info and create a new one
-                if (ci.getFirstColumn() == column)
+			if (ci.getFirstColumn() == targetColumnIx) {
-                {
+				ci.setFirstColumn(targetColumnIx + 1);
-                    ci.setFirstColumn(( short ) (column + 1));
+			} else {
+				ci.setLastColumn(targetColumnIx - 1);
+				k++; // adjust insert pos to insert after
 			}
-                else
+			ColumnInfoRecord nci = copyColInfo(ci);
-                {
-                    ci.setLastColumn(( short ) (column - 1));
-                }
-                ColumnInfoRecord nci = ( ColumnInfoRecord ) createColInfo();
 
-                nci.setFirstColumn(column);
+			nci.setFirstColumn(targetColumnIx);
-                nci.setLastColumn(column);
+			nci.setLastColumn(targetColumnIx);
-                nci.setOptions(ci.getOptions());
-                nci.setXFIndex(ci.getXFIndex());
 			setColumnInfoFields( nci, xfIndex, width, level, hidden, collapsed );
 
 			insertColumn(k, nci);
-            }
+			attemptMergeColInfoRecords(k);
-            else
+		} else {
-            {
 			//split to 3 records
-                short lastcolumn = ci.getLastColumn();
+			ColumnInfoRecord ciStart = ci;
-                ci.setLastColumn(( short ) (column - 1));
+			ColumnInfoRecord ciMid = copyColInfo(ci);
+			ColumnInfoRecord ciEnd = copyColInfo(ci);
+			int lastcolumn = ci.getLastColumn();
 			
-                ColumnInfoRecord nci = ( ColumnInfoRecord ) createColInfo();
+			ciStart.setLastColumn(targetColumnIx - 1);
-                nci.setFirstColumn(column);
-                nci.setLastColumn(column);
-                nci.setOptions(ci.getOptions());
-                nci.setXFIndex(ci.getXFIndex());
-                setColumnInfoFields( nci, xfIndex, width, level, hidden, collapsed );
-                insertColumn(++k, nci);
 
-                nci = ( ColumnInfoRecord ) createColInfo();
+			ciMid.setFirstColumn(targetColumnIx);
-                nci.setFirstColumn((short)(column+1));
+			ciMid.setLastColumn(targetColumnIx);
-                nci.setLastColumn(lastcolumn);
+			setColumnInfoFields(ciMid, xfIndex, width, level, hidden, collapsed);
-                nci.setOptions(ci.getOptions());
+			insertColumn(++k, ciMid);
-                nci.setXFIndex(ci.getXFIndex());
-                nci.setColumnWidth(ci.getColumnWidth());
-                insertColumn(++k, nci);
-            }
-        }
-        else
-        {
 			
-            // okay so there ISN'T a column info record that cover's this column so lets create one!
+			ciEnd.setFirstColumn(targetColumnIx+1);
-            ColumnInfoRecord nci = ( ColumnInfoRecord ) createColInfo();
+			ciEnd.setLastColumn(lastcolumn);
-
+			insertColumn(++k, ciEnd);
-            nci.setFirstColumn(column);
+			// no need to attemptMergeColInfoRecords because we 
-            nci.setLastColumn(column);
+			// know both on each side are different
-            setColumnInfoFields( nci, xfIndex, width, level, hidden, collapsed );
-            insertColumn(k, nci);
 		}
 	}
 
 	/**
 	 * Sets all non null fields into the <code>ci</code> parameter.
 	 */
-    private void setColumnInfoFields( ColumnInfoRecord ci, Short xfStyle, Short width, Integer level, Boolean hidden, Boolean collapsed )
+	private static void setColumnInfoFields( ColumnInfoRecord ci, Short xfStyle, Short width, 
-    {
+				Integer level, Boolean hidden, Boolean collapsed ) {
-	if (xfStyle != null)
+		if (xfStyle != null) {
 			ci.setXFIndex(xfStyle.shortValue());
-        if (width != null)
+		}
+		if (width != null) {
 			ci.setColumnWidth(width.shortValue());
-        if (level != null)
+		}
+		if (level != null) {
 			ci.setOutlineLevel( level.shortValue() );
-        if (hidden != null)
+		}
+		if (hidden != null) {
 			ci.setHidden( hidden.booleanValue() );
-        if (collapsed != null)
+		}
+		if (collapsed != null) {
 			ci.setCollapsed( collapsed.booleanValue() );
 		}
+	}
 
-    private int findColumnIdx(int column, int fromIdx)
+	private int findColInfoIdx(int columnIx, int fromColInfoIdx) {
-    {
+		if (columnIx < 0) {
-        if (column < 0)
+			throw new IllegalArgumentException( "column parameter out of range: " + columnIx );
-            throw new IllegalArgumentException( "column parameter out of range: " + column );
+		}
-        if (fromIdx < 0)
+		if (fromColInfoIdx < 0) {
-            throw new IllegalArgumentException( "fromIdx parameter out of range: " + fromIdx );
+			throw new IllegalArgumentException( "fromIdx parameter out of range: " + fromColInfoIdx );
+		}
 
-        ColumnInfoRecord ci;
+		for (int k = fromColInfoIdx; k < records.size(); k++) {
-        for (int k = fromIdx; k < records.size(); k++)
+			ColumnInfoRecord ci = getColInfo(k);
-        {
+			if (ci.containsColumn(columnIx)) {
-            ci = getColInfo(k);
-            if ((ci.getFirstColumn() <= column)
-                    && (column <= ci.getLastColumn()))
-            {
 				return k;
 			}
-            ci = null;
+			if (ci.getFirstColumn() > columnIx) {
+				break;
+			}
 		}
 		return -1;
 	}
 
-    public void collapseColInfoRecords( int columnIdx )
-    {
-        if (columnIdx == 0)
-            return;
-        ColumnInfoRecord previousCol = getColInfo( columnIdx - 1);
-        ColumnInfoRecord currentCol = getColInfo( columnIdx );
-        boolean adjacentColumns = previousCol.getLastColumn() == currentCol.getFirstColumn() - 1;
-        if (!adjacentColumns)
-            return;
-
-        boolean columnsMatch =
-                previousCol.getXFIndex() == currentCol.getXFIndex() &&
-                previousCol.getOptions() == currentCol.getOptions() &&
-                previousCol.getColumnWidth() == currentCol.getColumnWidth();
-
-        if (columnsMatch)
-        {
-            previousCol.setLastColumn( currentCol.getLastColumn() );
-            records.remove( columnIdx );
-        }
-    }
-
 	/**
-     * Creates an outline group for the specified columns.
+	 * Attempts to merge the col info record at the specified index 
-     * @param fromColumn    group from this column (inclusive)
+	 * with either or both of its neighbours
-     * @param toColumn      group to this column (inclusive)
-     * @param indent        if true the group will be indented by one level,
-     *                      if false indenting will be removed by one level.
 	 */
-    public void groupColumnRange(short fromColumn, short toColumn, boolean indent)
+	private void attemptMergeColInfoRecords(int colInfoIx) {
-    {
+		int nRecords = records.size();
+		if (colInfoIx < 0 || colInfoIx >= nRecords) {
+			throw new IllegalArgumentException("colInfoIx " + colInfoIx 
+					+ " is out of range (0.." + (nRecords-1) + ")");
+		}
+		ColumnInfoRecord currentCol = getColInfo(colInfoIx);
+		int nextIx = colInfoIx+1;
+		if (nextIx < nRecords) {
+			if (mergeColInfoRecords(currentCol, getColInfo(nextIx))) {
+    			records.remove(nextIx);
+			}
+		}
+		if (colInfoIx > 0) {
+			if (mergeColInfoRecords(getColInfo(colInfoIx - 1), currentCol)) {
+    			records.remove(colInfoIx);
+    		}
+		}
+	}
+	/**
+	 * merges two column info records (if they are adjacent and have the same formatting, etc)
+	 * @return <code>false</code> if the two column records could not be merged
+	 */
+	private static boolean mergeColInfoRecords(ColumnInfoRecord ciA, ColumnInfoRecord ciB) {
+		if (ciA.isAdjacentBefore(ciB) && ciA.formatMatches(ciB)) {
+			ciA.setLastColumn(ciB.getLastColumn());
+			return true;
+		}
+		return false;
+	}
+	/**
+	 * Creates an outline group for the specified columns, by setting the level
+	 * field for each col info record in the range. {@link ColumnInfoRecord}s
+	 * may be created, split or merged as a result of this operation.
+	 * 
+	 * @param fromColumnIx
+	 *            group from this column (inclusive)
+	 * @param toColumnIx
+	 *            group to this column (inclusive)
+	 * @param indent
+	 *            if <code>true</code> the group will be indented by one
+	 *            level, if <code>false</code> indenting will be decreased by
+	 *            one level.
+	 */
+	public void groupColumnRange(int fromColumnIx, int toColumnIx, boolean indent) {
 
-        // Set the level for each column
+		int colInfoSearchStartIdx = 0; // optimization to speed up the search for col infos
-        int fromIdx = 0;
+		for (int i = fromColumnIx; i <= toColumnIx; i++) {
-        for (int i = fromColumn; i <= toColumn; i++)
-        {
 			int level = 1;
-            int columnIdx = findColumnIdx( i, Math.max(0,fromIdx) );
+			int colInfoIdx = findColInfoIdx(i, colInfoSearchStartIdx);
-            if (columnIdx != -1)
+			if (colInfoIdx != -1) {
-            {
+				level = getColInfo(colInfoIdx).getOutlineLevel();
-                level = getColInfo(columnIdx).getOutlineLevel();
+				if (indent) {
-                if (indent) level++; else level--;
+					level++;
+				} else {
+					level--;
+				}
 				level = Math.max(0, level);
 				level = Math.min(7, level);
-                fromIdx = columnIdx - 1; // subtract 1 just in case this column is collapsed later.
+				colInfoSearchStartIdx = Math.max(0, colInfoIdx - 1); // -1 just in case this column is collapsed later.
 			}
-            setColumn((short)i, null, null, new Integer(level), null, null);
+			setColumn(i, null, null, new Integer(level), null, null);
-            columnIdx = findColumnIdx( i, Math.max(0, fromIdx ) );
-            collapseColInfoRecords( columnIdx );
 		}
-
 	}
 	/**
 	 * Finds the <tt>ColumnInfoRecord</tt> which contains the specified columnIndex
@@ -502,7 +505,7 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 		int nInfos = records.size();
 		for(int i=0; i< nInfos; i++) {
 			ColumnInfoRecord ci = getColInfo(i);
-			if (ci.getFirstColumn() <= columnIndex && columnIndex <= ci.getLastColumn()) {
+			if (ci.containsColumn(columnIndex)) {
 				return ci;
 			}
 		}
@@ -517,6 +520,4 @@ public final class ColumnInfoRecordsAggregate extends RecordAggregate {
 		}
 		return result;
 	}
-
-
 }

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1595,14 +1595,32 @@ public final class HSSFSheet {
         return patriarch;
     }
 
+    /**
+     * @deprecated (Sep 2008) use {@link #setColumnGroupCollapsed(int, boolean)}
+     */
+    public void setColumnGroupCollapsed(short columnNumber, boolean collapsed) {
+        setColumnGroupCollapsed(columnNumber & 0xFFFF, collapsed);
+    }
+    /**
+     * @deprecated (Sep 2008) use {@link #groupColumn(int, int)}
+     */
+    public void groupColumn(short fromColumn, short toColumn) {
+        groupColumn(fromColumn & 0xFFFF, toColumn & 0xFFFF);
+    }
+    /**
+     * @deprecated (Sep 2008) use {@link #ungroupColumn(int, int)}
+     */
+    public void ungroupColumn(short fromColumn, short toColumn) {
+        ungroupColumn(fromColumn & 0xFFFF, toColumn & 0xFFFF);
+    }
+
     /**
      * Expands or collapses a column group.
      *
      * @param columnNumber      One of the columns in the group.
      * @param collapsed         true = collapse group, false = expand group.
      */
-    public void setColumnGroupCollapsed( short columnNumber, boolean collapsed )
+    public void setColumnGroupCollapsed(int columnNumber, boolean collapsed) {
-    {
         sheet.setColumnGroupCollapsed(columnNumber, collapsed);
     }
 
@@ -1612,13 +1630,11 @@ public final class HSSFSheet {
      * @param fromColumn        beginning of the column range.
      * @param toColumn          end of the column range.
      */
-    public void groupColumn(short fromColumn, short toColumn)
+    public void groupColumn(int fromColumn, int toColumn) {
-    {
         sheet.groupColumnRange(fromColumn, toColumn, true);
     }
 
-    public void ungroupColumn( short fromColumn, short toColumn )
+    public void ungroupColumn(int fromColumn, int toColumn) {
-    {
         sheet.groupColumnRange(fromColumn, toColumn, false);
     }
 

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -357,7 +357,7 @@ public final class TestSheet extends TestCase {
         xfindex = sheet.getXFIndexForColAt((short) 1);
         assertEquals(DEFAULT_IDX, xfindex);
 
-        ColumnInfoRecord nci = ColumnInfoRecordsAggregate.createColInfo();
+        ColumnInfoRecord nci = new ColumnInfoRecord();
         sheet._columnInfos.insertColumn(nci);
 
         // single column ColumnInfoRecord
@@ -567,7 +567,7 @@ public final class TestSheet extends TestCase {
 
         sheet.setMargin(HSSFSheet.LeftMargin, 0.3);
         try {
-            row.createCell((short) 0);
+            row.createCell(0);
         } catch (IllegalStateException e) {
             if (e.getMessage().equals("Cannot create value records before row records exist")) {
                 throw new AssertionFailedError("Identified bug 45717");
@@ -576,4 +576,3 @@ public final class TestSheet extends TestCase {
         }
     }
 }
-

src/testcases/org/apache/poi/hssf/model/TestSheetAdditional.java

@@ -20,7 +20,6 @@ package org.apache.poi.hssf.model;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.record.ColumnInfoRecord;
-import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 
 /**
  * @author Tony Poppleton
@@ -29,7 +28,7 @@ public final class TestSheetAdditional extends TestCase {
 	
 	public void testGetCellWidth() {
 		Sheet sheet = Sheet.createSheet();
-		ColumnInfoRecord nci = ColumnInfoRecordsAggregate.createColInfo();
+		ColumnInfoRecord nci = new ColumnInfoRecord();
 
 		// Prepare test model
 		nci.setFirstColumn((short)5);
@@ -55,5 +54,4 @@ public final class TestSheetAdditional extends TestCase {
 		assertEquals((short)100,sheet.getColumnWidth((short)9));
 		assertEquals((short)100,sheet.getColumnWidth((short)10));
 	}
-
 }

src/testcases/org/apache/poi/hssf/record/aggregates/TestColumnInfoRecordsAggregate.java

@@ -17,9 +17,16 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
+
 import org.apache.poi.hssf.record.ColumnInfoRecord;
+import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 
 /**
  * @author Glen Stampoultzis
@@ -28,11 +35,11 @@ public final class TestColumnInfoRecordsAggregate extends TestCase {
 
 	public void testGetRecordSize() {
 		ColumnInfoRecordsAggregate agg = new ColumnInfoRecordsAggregate();
-		agg.insertColumn(createColumn(1, 3));
+		agg.insertColumn(createColInfo(1, 3));
-		agg.insertColumn(createColumn(4, 7));
+		agg.insertColumn(createColInfo(4, 7));
-		agg.insertColumn(createColumn(8, 8));
+		agg.insertColumn(createColInfo(8, 8));
 		agg.groupColumnRange((short) 2, (short) 5, true);
-		assertEquals(6, agg.getNumColumns());
+		assertEquals(4, agg.getNumColumns());
 
 		confirmSerializedSize(agg);
 
@@ -48,10 +55,91 @@ public final class TestColumnInfoRecordsAggregate extends TestCase {
 		assertEquals(estimatedSize, serializedSize);
 	}
 
-	private static ColumnInfoRecord createColumn(int firstCol, int lastCol) {
+	private static ColumnInfoRecord createColInfo(int firstCol, int lastCol) {
 		ColumnInfoRecord columnInfoRecord = new ColumnInfoRecord();
 		columnInfoRecord.setFirstColumn((short) firstCol);
 		columnInfoRecord.setLastColumn((short) lastCol);
 		return columnInfoRecord;
 	}
+
+	private static final class CIRCollector implements RecordVisitor {
+
+		private List _list;
+		public CIRCollector() {
+			_list = new ArrayList();
+		}
+		public void visitRecord(Record r) {
+			_list.add(r);
+		}
+		public static ColumnInfoRecord[] getRecords(ColumnInfoRecordsAggregate agg) {
+			CIRCollector circ = new CIRCollector();
+			agg.visitContainedRecords(circ);
+			List list = circ._list;
+			ColumnInfoRecord[] result = new ColumnInfoRecord[list.size()];
+			list.toArray(result);
+			return result;
+		}
+	}
+
+	public void testGroupColumns_bug45639() {
+		ColumnInfoRecordsAggregate agg = new ColumnInfoRecordsAggregate();
+		agg.groupColumnRange( 7, 9, true);
+		agg.groupColumnRange( 4, 12, true);
+		try {
+			agg.groupColumnRange( 1, 15, true);
+		} catch (ArrayIndexOutOfBoundsException e) {
+			throw new AssertionFailedError("Identified bug 45639");
+		}
+		ColumnInfoRecord[] cirs = CIRCollector.getRecords(agg);
+		assertEquals(5, cirs.length);
+		confirmCIR(cirs, 0,  1,  3, 1, false, false);
+		confirmCIR(cirs, 1,  4,  6, 2, false, false);
+		confirmCIR(cirs, 2,  7,  9, 3, false, false);
+		confirmCIR(cirs, 3, 10, 12, 2, false, false);
+		confirmCIR(cirs, 4, 13, 15, 1, false, false);
+	}
+
+	/**
+	 * Check that an inner group remains hidden
+	 */
+	public void testHiddenAfterExpanding() {
+		ColumnInfoRecordsAggregate agg = new ColumnInfoRecordsAggregate();
+		agg.groupColumnRange(1, 15, true);
+		agg.groupColumnRange(4, 12, true);
+
+		ColumnInfoRecord[] cirs;
+
+		// collapse both inner and outer groups
+		agg.collapseColumn(6);
+		agg.collapseColumn(3);
+
+		cirs = CIRCollector.getRecords(agg);
+		assertEquals(5, cirs.length);
+		confirmCIR(cirs, 0,  1,  3, 1, true, false);
+		confirmCIR(cirs, 1,  4, 12, 2, true, false);
+		confirmCIR(cirs, 2, 13, 13, 1, true, true);
+		confirmCIR(cirs, 3, 14, 15, 1, true, false);
+		confirmCIR(cirs, 4, 16, 16, 0, false, true);
+
+		// just expand the inner group
+		agg.expandColumn(6);
+
+		cirs = CIRCollector.getRecords(agg);
+		assertEquals(4, cirs.length);
+		if (!cirs[1].getHidden()) {
+			throw new AssertionFailedError("Inner group should still be hidden");
+		}
+		confirmCIR(cirs, 0,  1,  3, 1, true, false);
+		confirmCIR(cirs, 1,  4, 12, 2, true, false);
+		confirmCIR(cirs, 2, 13, 15, 1, true, false);
+		confirmCIR(cirs, 3, 16, 16, 0, false, true);
+	}
+	private static void confirmCIR(ColumnInfoRecord[] cirs, int ix, int startColIx, int endColIx, int level, boolean isHidden, boolean isCollapsed) {
+		ColumnInfoRecord cir = cirs[ix];
+		assertEquals("startColIx", startColIx, cir.getFirstColumn());
+		assertEquals("endColIx", endColIx, cir.getLastColumn());
+		assertEquals("level", level, cir.getOutlineLevel());
+		assertEquals("hidden", isHidden, cir.getHidden());
+		assertEquals("collapsed", isCollapsed, cir.getCollapsed());
+	}
 }