From 7c1e1f24785294d90c8dc76a1b033a07aeeb4d9d Mon Sep 17 00:00:00 2001
From: Josh Micich <josh@apache.org>
Date: Mon, 26 May 2008 18:02:23 +0000
Subject: [PATCH] Follow-on fix for bug 42564 (r653668). Array elements are
 stored internally column by column.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@660256 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/hssf/record/formula/ArrayPtg.java     |  44 ++++++++++--------
 .../poi/hssf/data/ex42564-elementOrder.xls    | Bin 0 -> 16384 bytes
 .../poi/hssf/record/formula/TestArrayPtg.java |  29 ++++++++++--
 3 files changed, 48 insertions(+), 25 deletions(-)
 create mode 100644 src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls

diff --git a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
index 251009b287..49c66e0720 100644
--- a/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
+++ b/src/java/org/apache/poi/hssf/record/formula/ArrayPtg.java
@@ -47,7 +47,7 @@ public class ArrayPtg extends Ptg {
 	protected Object[] token_3_arrayValues;
 
 	protected ArrayPtg() {
-	  //Required for clone methods
+		//Required for clone methods
 	}
 
 	public ArrayPtg(RecordInputStream in)
@@ -89,12 +89,16 @@ public class ArrayPtg extends Ptg {
 		for (int x=0;x<getColumnCount();x++) {
 			for (int y=0;y<getRowCount();y++) {
 				Object o = token_3_arrayValues[getValueIndex(x, y)];
-	   			buffer.append("[").append(x).append("][").append(y).append("] = ").append(o).append("\n"); 
+				buffer.append("[").append(x).append("][").append(y).append("] = ").append(o).append("\n"); 
 			}
 		}
 		return buffer.toString();
 	}
 
+	/**
+	 * Note - (2D) array elements are stored column by column 
+	 * @return the index into the internal 1D array for the specified column and row
+	 */
 	/* package */ int getValueIndex(int colIx, int rowIx) {
 		if(colIx < 0 || colIx >= token_1_columns) {
 			throw new IllegalArgumentException("Specified colIx (" + colIx 
@@ -104,7 +108,7 @@ public class ArrayPtg extends Ptg {
 			throw new IllegalArgumentException("Specified rowIx (" + rowIx 
 					+ ") is outside the allowed range (0.." + (token_2_rows-1) + ")");
 		}
-		return rowIx * token_1_columns + colIx;
+		return rowIx + token_2_rows * colIx;
 	}
 
 	public void writeBytes(byte[] data, int offset) {
@@ -137,22 +141,21 @@ public class ArrayPtg extends Ptg {
 		return size;
 	}
 
-	public String toFormulaString(HSSFWorkbook book)
-	{
+	public String toFormulaString(HSSFWorkbook book) {
 		StringBuffer b = new StringBuffer();
 		b.append("{");
-		for (int x=0;x<getColumnCount();x++) {
-		  	if (x > 0) {
+		for (int x = 0; x < getColumnCount(); x++) {
+			if (x > 0) {
 				b.append(";");
 			}
-		  	for (int y=0;y<getRowCount();y++) {
+			for (int y = 0; y < getRowCount(); y++) {
 				if (y > 0) {
 					b.append(",");
 				}
-		  		Object o = token_3_arrayValues[getValueIndex(x, y)];
-		  		b.append(getConstantText(o));
-		  	}
-		  }
+				Object o = token_3_arrayValues[getValueIndex(x, y)];
+				b.append(getConstantText(o));
+			}
+		}
 		b.append("}");
 		return b.toString();
 	}
@@ -166,6 +169,7 @@ public class ArrayPtg extends Ptg {
 			return "\"" + ((UnicodeString)o).getString() + "\"";
 		}
 		if (o instanceof Double) {
+			// TODO - numeric array elements need default Excel number formatting
 			return ((Double)o).toString();
 		}
 		if (o instanceof Boolean) {
@@ -182,13 +186,13 @@ public class ArrayPtg extends Ptg {
 	}
 	
 	public Object clone() {
-	  ArrayPtg ptg = new ArrayPtg();
-	  ptg.field_1_reserved = (byte[]) field_1_reserved.clone();
-	  
-	  ptg.token_1_columns = token_1_columns;
-	  ptg.token_2_rows = token_2_rows;
-	  ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone();
-	  ptg.setClass(ptgClass);
-	  return ptg;
+		ArrayPtg ptg = new ArrayPtg();
+		ptg.field_1_reserved = (byte[]) field_1_reserved.clone();
+
+		ptg.token_1_columns = token_1_columns;
+		ptg.token_2_rows = token_2_rows;
+		ptg.token_3_arrayValues = (Object[]) token_3_arrayValues.clone();
+		ptg.setClass(ptgClass);
+		return ptg;
 	}
 }
diff --git a/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls b/src/testcases/org/apache/poi/hssf/data/ex42564-elementOrder.xls
new file mode 100644
index 0000000000000000000000000000000000000000..3c49fc257214d0bfb88fe570aefa5e7cc4a0c28e
GIT binary patch
literal 16384
zcmeHOdvI078UN1BO>#mI5(oi#<q!yjki?Jxf;?^jpJRB8;6O(#gydd`#DvC+g&7%Q
ztN&E%U{aw%TF0t0wOXubeN1hIj-&o@?NjS$wSygXZ0)p_+G%x=+uyf)_TF>$Jnp@9
z#!-jO?76$&+5LUre*5ir_LblKy7JV$k4}Dzw6KDT=xn--N*r_pX^tXP5KTdX^JmlP
zv?LNCc`kf`H1G*zorcaULX097BXXaYBASS0h@%k85l17Qhd2gtEFw&RDiOyajz_FQ
zoPb!3I1zCY;`xY^5!psKPIsigKx+EZih33G<1a}TT|#~M9iV;aq5MNNEJi6Of@+q@
zPofjzNtg1^v7Qsr4bQ%4SZDk1-i+U=pbg4mv<ch?Xcui4a_o||3V4pAh{1NNWXPu~
zv+i$^C5veT?V;=O*D2b51+<V5GBG7dJxKS3fYyUQ`#>H)EJH|9*Q*!pTrph?4qZZq
zxafhx<XH^<s_psmXMKHW^Di?oic5;=!J3y!Pr{p*;N@Esp`S!g;z{&p#4WU&{k-*A
zhE+m_PJ?AQ0p`n*GH3+U3iuSJqUZ4(FP@Kxa?gn8a=bZZ+;5C*GV2zj1b?G8N7_ZX
zZ;NNGc#gr-peIG0*9B!hO{X_d-lT)vr4hQW)L0EG=!F$*hjsMO=c&X_!=T0V3@afc
z>GigpokG$^pAzo2M01vS13J>VxG`8`l{qz5m9xaMoF!J|C}B`c(}ympK?`z}Ae0F2
zeFNV_x7Kwo{KX8%h~WDeUc!JC=HgNoh%r4n6@D4t;M!TWGaF~lY}q<@>6Utx*)n%}
zbOyfi@WYCP_t#t%@4nVus20-zOb%3`<`y>&@(eWxqSorAG&H;Dh15W;L|a*u21`k1
zXSdV~!j`#9Z9;8RE#%r--@X-GnrSZnP}Nzl!E#n=uv|4_)JPY(naLKo`3cb+EMu`O
z(?v87BPq*N)z9!%l?Cf;%~N77MD*{ph0&qH35)6MS_ECWU<#-60QV~&L}zY(<-g2y
zgDh^I3=yeMp2-S5JZ{sY_Xz!a=|2vj|0sZdEP#GAfc{7T{ow%m_X6m*1<-@!3BrF&
z;P6b{t)K4)(4{=y@=_iz{r&))M+4}O2he{JK>t<%{pkRDq4NAx;P8satsiM8yz&wB
z?^ISEE<apO(L<s<uY@E$k$AS|*&2#IDd@%UA%gy26c9n#=iC5#V*tIum(D9Mfv@=!
zUbP8&#{Q)}mn!<pv>wSC%&&x;yxJ4xB^}L|bY214bPGu|U!!Y$d4(v-XY?cOpI3`E
z&Nd{mD3mz7suc8$oDbQ0v*{z1mDT0d(DM;N=hdgemwJ}^FUEIS&^0~tYF5xSIja=?
zWlZsCMMe+2n$^qSWLD8lte{L-K|8K|J+<F#qWvy97l>e$TqRZn?Cbd>iO);J7<`c<
z-U?i#pbB!rR{#fIniRx=*9QevV3%4@1@4oADzMQORDnCCpbBi}1y$hjRZs<XC<RsE
zkx@_u9(n~;;IULt1$KS~RhU<3g~mcFG!<H*xzGxjL>AWQrLGD*{+v$0*l(`Xyawi&
z@E2-K>lGrV?M`@L%K5Nmc|kC-&JB`|g@d%^1Jc^&2GO110)S+##M2zF=)4&t+t1z&
z{y6PaSR{(}^NZ*I@vC%j`|DjGBUIuKl71l!<U%({JV284`y&oX?$o+-AwWt+ce2#6
z*o^^_r0&jul-f-^aNt1NvIBQJ12}1A4jNvGFv^Fv-_(U#g0fDbV*Wy%K7HCFR4f+r
z2<4<Hp_pdNge9;P-bwQcg%zDcsE_vtsM}8{r&d0lru5Oc2tsA8(Ib>%mL4=Rn)0@+
zA=5Ipe-6GT{lEhcWI&V+I902dIcfQ;TOXLqA%+~7Y**+jw+(n(!5e7OXt))g>5Lkf
z>2%7|isQ$RdyK|OQ>|bcutiWt<D_|~Oc$VRwiRp#F4gWguhz!nGE|6W+^eS*taomn
z)2XrE6-|vtuivUgt*qZh#@opXB-n59#5fD$k&bm-MDc`EI<F*r=8_X6)e#PY4Iu{z
zGJeg`oxeVMX66GT51N8?!i%Y#&50*?;u#FWXHYpoQXSzS@^DOjnt|xAlW>q(E=dAF
z<iT5Tb9iBrvpMku&#i+&CglT?>IetnL%ggc$oT6d9HcHE5cWP{&EdswR+7N66Ar>>
zlsQ3C9pNBYKRQ5=@z+T>$n1PT<iTuk7xTh4XLI5SULgg8@R@B+kW@!Fh~)x7#$PAl
zAan8okq6bm&EZX|oXv?Rco7&3!uK6=f}}daL1wr>knz`v6J$6t4yRsN+_kpUSxwf;
zuC91ra{iFHhK5*mb}*vT)8}D#y@l$?q9(+ZI9Kc<d8#N^7zWPpOQZ3`AK&>PUCe17
zF_;C^@!kZpsnprDdfC*5u#syJ!KOH#IQ`}u{%pqSY}&kRW{0qms}#YeB%b)|2XFeb
z8LzWh;APVg!iLu|MVR$KQKj+3JE#8U&!$Rev(U??IfM-_7i=~r`sVNd@@F$aXS2x5
zrZt3(oYsogl*JRD{N*)&Hr32#LAa&WfX#wDEG^aXRO)SiHWPI=K9)8!gpJeEQXTI;
z{ggkONje)JOPdqI#%XD(j#uA)#-GjkIvXELn-{{yX=$mB)QQ*q*-X~i_*mKnA#9wM
zmg;!o&G-D-Owrl+SXx^M8>gkEI?lZExIdez%w|!zrCDIJC=W|(ZM*BSS0x*EXb2gf
zHce;aV`;NO*f=e%we7XTclooKuCwv6wE7S>PD^WT`{f^w__L|e+4xvmV+b3krM0#l
zzxM@yHkQuD$I@Ct*f=e%wQcw%+e7PpGec+NV`&RQ*f=e%we8~*2mM+z8pqR^cV)ht
zE4A<Dv~+3Uw~}bx%|G`CotXuNMd_e3G*G7%;pk)i8h_APSy0%I4qB&yI&FsN=+D05
z4_cQ6g|+CQb2U(>h2WxA_YeF*XJ<iS8#-u{2I{m2qAMSM-XC;M78I7CgU;7LoxM-=
zpF@}Wftpm0uPd$U>>e_gxO?81k}jfE*lz63e1VI3Qv^GNJUikvZzI~CqyqO3u!q08
zv$tzcZ)dWnzi-IA*$o(}PM4FsGQ?N9dI6oiLRKnf5)8`QS&9K~F9`-ExL3(9c|XH|
z8k{-Pl`C`Qh9hHfs{*GHt9Qn`c3G?Ay}d)`0Zp!n=~0ktCnkekxNK)39fx$iXm(aA
z-ii{FLgye=Qt|#%Nk%G}Q&P#Cl1k==v1vPRuqZubT5eH_i@;<x?!3u6ci=@_wsEio
zW$U%FCAfi{Eo;#=_!3;pRYaYp!6@p?*6KvlChF|LeXRj8_wBN8G%}aw!BW&{)@qcd
zM|*0xc}<`T__pgMdw2JC_H`!v2liPT<9m}s=4EcE$ij35E>iBrg*oo~KGFYt!EqmS
zz7gW=g+1`*mFj)&Y}>rb&^Bz(T-M(YLtW~|H72K{bQzS^ubC?Eh$}2GQ&?kgYXvKa
z%j2EhJ$*YYtQ9mtiz=Ym%OMckeh&oQ0RiP2L6`srOwAShB^;Y#DI}AQa5ypsiwp-i
ze4h?p=>snPUjTSI_QMl@LPHq5%?S=4sDqF50hfL-09;NDA_9DY6CA!!a|lJ_eZbit
z${s6qs%1DbiWZ_H>59JHdteHC++7x_LVNf&>2BB}cY1c9rLpg?I*ktpKIQPQzyt|?
zsiUWFSG?OE|5s|lOoCtP5FNb>o!;&807V&V%=jYZeRzwz1d1C?7eano8{d;0=!ARG
z9J4VMj(IJBxupZJDRwM!MI-zv4og&fM{uHI8$3*EkT4wE+_D-3mTO_@1L9IHL)qFl
zkjySt%Wy9d5NrFB@gZ}&8zV9b<ApJj5Q_I2dDSCC!A7-mX}rmtb+j_4T$;=&mnL(=
zk#br>*wk9LCn<XT>l)JufL<qb;O%p#%V!Vi>T@P!C6PHLiOeZUWR6|gMT5oA-7L2V
z#l`4|^=R`zX5r5&V^dwqrn<2w*$ZvWch@Q!i|*M7>qw&GSYKXiHyC1>fap)M1!#K@
zMdu3$R=Di_s4yzm{sQY=`4QP#m17gDq~+)%+SEDF$1Y2{*O*(-NI5+9CU6+QxKdur
z&#m%DTxj?Xb2HuH*d#fK)>YzfBS5x`)^32v`$TFD+Ro09uY)gv7JKMMVL{mvJc4%M
zE1HC>TtwT%)!|#ws)>B049nQCGagUQKVnXzBMzR@Y~|_6eYkFYpW_|ZyB%h1;O`hN
zvg0>85%(!oos))1Cy|-<zCmX|S0Rl)GO5d~MpQKUj~ASa&>`?S<ao2{9fOX7cGN**
z9=C&byMxBH`C@~-7GB}H7G55y;@2W@-6ovFl;Py|Jn-Z36@m7~igYEO{O=(qoq6zs
z->uocKDJfR8w7oxcz#1r1|4mxgH;n*#?5$hGRnLrAbwMnzC}D|qjW-qcVm3(RHf@P
zDDTDQI@0&@y;LBwb`5Bmwa_NQ?_=sWNh6p-jA%MQwM<yXUl$Rsh7>4>j}!;lNBlG+
ze1^jm%O;M6H9Rb6d}D&cSKBe`Cpt5lwnbSx_}3UzLQ>YCf5z+twc80(B`Q>+N+l+#
z#8j2ARH9ZTW~sz%m8e&V29;=3iRSiKA@OrUt`*n^e1iAxsa@pd<2G2zZ7wZEa5s1?
zPpvs9;dhu5X@{d2+Q*CWqKl<n$-?KJpBoKuzp*)TH}d~y;UfV<kjE~Z=Y44YJr};5
z8aRtjWwH2>p`#sv?c&s}_niJ{-Oh@K?lfuMoX1{+ucz-K@);JtZ%1U8eK{fz`HhI<
z5Vs-XQ;x-s{C~k)@H-yyAR^n^+o;4xh+Nk~)YI%t*YtD^^xx2*NLrWd?TYtGS9Hhv
z-(H3PA8W^ZG6v>sMA~*gO!>b;So<Tdy!6UQ^OTC=!)SlwN5{DR&mr=d;P$UaWcR)q
zk<XgD5&4oIPg%JQJnQFn--K9=cnczSJn_Mo`nd<cx;{X%UFv9o#d*f$p#M$>6PAPf
zLry(tG)N4|D&^vv`UiI1z_XGY1S*$y2g6=p@5an^J6-2~nMaQQ3(Gk&@~9$gWgwq|
zd6p@Zf1a>;7yBafH{jiiFyrGk=>NMp8yd)t!*z*7Pgh(HN2|4^rOnD1m-u3=>PaLP
eV=~O!Bp9*H!Xld&(}zore=ToSo-6nnYv6yj^X`2B

literal 0
HcmV?d00001

diff --git a/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java b/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java
index 16f80bb791..39464b5e03 100644
--- a/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java
+++ b/src/testcases/org/apache/poi/hssf/record/formula/TestArrayPtg.java
@@ -19,8 +19,10 @@ package org.apache.poi.hssf.record.formula;
 
 import java.util.Arrays;
 
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.TestcaseRecordInputStream;
 import org.apache.poi.hssf.record.UnicodeString;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
@@ -77,7 +79,7 @@ public final class TestArrayPtg extends TestCase {
 	}
 
 	/**
-	 * make sure constant elements are stored row by row 
+	 * Excel stores array elements column by column.  This test makes sure POI does the same.
 	 */
 	public void testElementOrdering() {
 		ArrayPtg ptg = new ArrayPtgV(new TestcaseRecordInputStream(ArrayPtgV.sid, ENCODED_PTG_DATA));
@@ -86,10 +88,27 @@ public final class TestArrayPtg extends TestCase {
 		assertEquals(2, ptg.getRowCount());
 		
 		assertEquals(0, ptg.getValueIndex(0, 0));
-		assertEquals(1, ptg.getValueIndex(1, 0));
-		assertEquals(2, ptg.getValueIndex(2, 0));
-		assertEquals(3, ptg.getValueIndex(0, 1));
-		assertEquals(4, ptg.getValueIndex(1, 1));
+		assertEquals(2, ptg.getValueIndex(1, 0));
+		assertEquals(4, ptg.getValueIndex(2, 0));
+		assertEquals(1, ptg.getValueIndex(0, 1));
+		assertEquals(3, ptg.getValueIndex(1, 1));
 		assertEquals(5, ptg.getValueIndex(2, 1));
 	}
+	
+	/**
+	 * Test for a bug which was temporarily introduced by the fix for bug 42564.
+	 * A spreadsheet was added to make the ordering clearer.
+	 */
+	public void testElementOrderingInSpreadsheet() {
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex42564-elementOrder.xls");
+
+		// The formula has an array with 3 rows and 5 column 
+		String formula = wb.getSheetAt(0).getRow(0).getCell((short)0).getCellFormula();
+		// TODO - These number literals should not have '.0'. Excel has different number rendering rules
+
+		if (formula.equals("SUM({1.0,6.0,11.0;2.0,7.0,12.0;3.0,8.0,13.0;4.0,9.0,14.0;5.0,10.0,15.0})")) {
+			throw new AssertionFailedError("Identified bug 42564 b");
+		}
+		assertEquals("SUM({1.0,2.0,3.0;4.0,5.0,6.0;7.0,8.0,9.0;10.0,11.0,12.0;13.0,14.0,15.0})", formula);
+	}
 }
-- 
2.39.5