Bug 33054

Attempting to recreate (TreeMap definitely in use). The example to output the a
sql statement seems to work fine:

  <%-- Get the column names for the header of the table --%>
  <c:forEach var="columnName" items="${deejays.columnNames}">
    <th><c:out value="${columnName}"/></th>
  </c:forEach>

  <%-- Get the value of each column while iterating over rows --%>
  <c:forEach var="row" items="${db.rowsByIndex}">
    <tr>
      <c:forEach var="column" items="${row}">
        <td><c:out value="${column}"/></td>
      </c:forEach>
  </c:forEach>

I suspect because the columnNames variable is the correct order. So that's a
definite workaround for this issue.

Scratch that last comment. It's not the columnNames.

So despite the TreeMap; I can't reproduce this easily. I have a pretty small
table (3 rows) and my select statement returns 28 columns without any obvious
change in the order. So needs more digging to understand how this might come about.

So here's a better test:

  <c:forEach var="row" items="${db.rows}">
    <tr>
      <c:forEach var="column" items="${row}">
        <td><c:out value="${column}"/></td>
      </c:forEach>
    </tr>
  </c:forEach>

You have to use rows and not rowIndex. The former uses the TreeMap and the
latter doesn't.

Definitely proves the bug. Things are ordered case insensitively by column name
(as the code is pretty honest about). From the javadoc for getRows:

     * Returns an array of SortedMap objects. The SortedMap
     * object key is the ColumnName and the value is the ColumnValue.
     * SortedMap was created using the CASE_INSENSITIVE_ORDER
     * Comparator so the key is the case insensitive representation
     * of the ColumnName.

There's nothing in the spec, so I think the next step here is to develop a
patch, and then to consider whether a test can easily be created.

Pulling in
http://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk/src/java/org/apache/commons/collections/SequencedHashMap.java
seems quite possible. Its deprecated because a version that is more embedded in
Collections now exists (ListOrderedMap), but pulling that version in involves a
lot more in the way of classes.

It's not thread-safe, but neither is TreeMap so that shouldn't be a problem. The
only backwards compatibility issue I can see is that TreeMap implements
SortedMap which adds some methods which SequencedHashMap won't be offering. It
seems unlikely that many people are using those methods.

So next up - a test case.

Created attachment 19347 [details]
Unit test for this issue

Adds Derby as a dependency for the cactus tests

So while I can happily test for this, and we can easily put a SequencedHashMap
in, the spec API for javax.servlet.jsp.jstl.sql.Result defines the method
getRows() as returning a SortedMap and not a Map.

SequencedHashMap is not a SortedMap - the insertion-ordering that this issue
calls for doesn't match the SortedMap API which indicates ordering should be
managed via  comparator.

It would be possible to hack the fork of SequencedHashMap around a bit to make
it a SortedMap implementation (given that it won't be public); but I think
that's abusing the spec far too much.

---

Looking at the sample page, I believe the following is a useable workaround for
this issue:

  <%-- Get the column names for the header of the table --%>
  <c:forEach var="columnName" items="${db.columnNames}">
    <th><c:out value="${columnName}"/></th>
  </c:forEach>

  <%-- Get the value of each column while iterating over rows --%>
  <c:forEach var="row" items="${db.rowsByIndex}">
    <tr>
      <c:forEach var="column" items="${row}">
        <td><c:out value="${column}"/></td>
      </c:forEach>
  </c:forEach>

We should put that in a FAQ.

svn ci -m "Adding unit test for Bug 33054 - however the assertion is turned off
because this is not something we can fix. It's being added as an example of how
to do a sql tag test so it doesn't get lost. " 

Sending        build-tests.xml
Sending        build_sample_standard.properties
Adding         test/org/apache/taglibs/standard/tag/el/sql
Adding         test/org/apache/taglibs/standard/tag/el/sql/Test33054.java
Adding         test/web/org/apache/taglibs/standard/tag/el/sql
Adding         test/web/org/apache/taglibs/standard/tag/el/sql/Test33054.jsp
Transmitting file data ....
Committed revision 501245.

Added to the FAQ:

http://wiki.apache.org/jakarta-taglibs/Standard1%2e1%2e3FAQ

Otherwise, I can't see us fixing this so I'm resoling it WONTFIX.