SOLR-10085: SQL result set fields should be ordered by the field list

solr/core/src/java/org/apache/solr/handler/SQLHandler.java

@@ -34,7 +34,9 @@ import org.apache.solr.client.solrj.io.stream.ExceptionStream;
 import org.apache.solr.client.solrj.io.stream.JDBCStream;
 import org.apache.solr.client.solrj.io.stream.TupleStream;
 import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.handler.sql.CalciteSolrDriver;
@@ -74,6 +76,9 @@ public class SQLHandler extends RequestHandlerBase implements SolrCoreAware, Per
 
   public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
     ModifiableSolrParams params = new ModifiableSolrParams(req.getParams());
+    params = adjustParams(params);
+    req.setParams(params);
+
     String sql = params.get("stmt");
     // Set defaults for parameters
     params.set("numWorkers", params.getInt("numWorkers", 1));
@@ -139,6 +144,8 @@ public class SQLHandler extends RequestHandlerBase implements SolrCoreAware, Per
   private class SqlHandlerStream extends JDBCStream {
     private final boolean includeMetadata;
     private boolean firstTuple = true;
+    List<String> metadataFields = new ArrayList<>();
+    Map<String, String> metadataAliases = new HashMap<>();
 
     SqlHandlerStream(String connectionUrl, String sqlQuery, StreamComparator definedSort,
                      Properties connectionProperties, String driverClassName, boolean includeMetadata)
@@ -151,7 +158,7 @@ public class SQLHandler extends RequestHandlerBase implements SolrCoreAware, Per
     @Override
     public Tuple read() throws IOException {
       // Return a metadata tuple as the first tuple and then pass through to the JDBCStream.
-      if(includeMetadata && firstTuple) {
+      if(firstTuple) {
         try {
           Map<String, Object> fields = new HashMap<>();
 
@@ -159,8 +166,6 @@ public class SQLHandler extends RequestHandlerBase implements SolrCoreAware, Per
 
           ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
 
-          List<String> metadataFields = new ArrayList<>();
-          Map<String, String> metadataAliases = new HashMap<>();
           for(int i = 1; i <= resultSetMetaData.getColumnCount(); i++) {
             String columnName = resultSetMetaData.getColumnName(i);
             String columnLabel = resultSetMetaData.getColumnLabel(i);
@@ -168,16 +173,30 @@ public class SQLHandler extends RequestHandlerBase implements SolrCoreAware, Per
             metadataAliases.put(columnName, columnLabel);
           }
 
-          fields.put("isMetadata", true);
+          if(includeMetadata) {
-          fields.put("fields", metadataFields);
+            fields.put("isMetadata", true);
-          fields.put("aliases", metadataAliases);
+            fields.put("fields", metadataFields);
-          return new Tuple(fields);
+            fields.put("aliases", metadataAliases);
+            return new Tuple(fields);
+          }
         } catch (SQLException e) {
           throw new IOException(e);
         }
-      } else {
-        return super.read();
       }
+
+      Tuple tuple = super.read();
+      if(!tuple.EOF) {
+        tuple.fieldNames = metadataFields;
+        tuple.fieldLabels = metadataAliases;
+      }
+      return tuple;
     }
   }
+
+  private ModifiableSolrParams adjustParams(SolrParams params) {
+    ModifiableSolrParams adjustedParams = new ModifiableSolrParams();
+    adjustedParams.add(params);
+    adjustedParams.add(CommonParams.OMIT_HEADER, "true");
+    return adjustedParams;
+  }
 }

solr/core/src/test/org/apache/solr/handler/TestSQLHandler.java

@@ -16,21 +16,30 @@
  */
 package org.apache.solr.handler;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
 
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
 import org.apache.solr.client.solrj.io.Tuple;
 import org.apache.solr.client.solrj.io.stream.ExceptionStream;
 import org.apache.solr.client.solrj.io.stream.SolrStream;
 import org.apache.solr.client.solrj.io.stream.TupleStream;
+import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 
+import org.apache.solr.common.util.NamedList;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -161,6 +170,9 @@ public class TestSQLHandler extends AbstractFullDistribZkTestBase {
       assert(tuple.getLong("field_i") == 7);
       assert(tuple.get("str_s").equals("a"));
 
+      //Assert field order
+      assertResponseContains(clients.get(0), sParams, "{\"docs\":[{\"id\":8,\"field_i\":60,\"str_s\":\"c\"}");
+
       //Test unlimited unsorted result. Should sort on _version_ desc
       sParams = mapParams(CommonParams.QT, "/sql", "stmt", "select id, field_i, str_s from collection1 where text='XXXX'");
 
@@ -2362,4 +2374,23 @@ public class TestSQLHandler extends AbstractFullDistribZkTestBase {
     return params;
   }
 
+  public void assertResponseContains(SolrClient server, SolrParams requestParams, String json) throws IOException, SolrServerException {
+    String p = requestParams.get("qt");
+    if(p != null) {
+      ModifiableSolrParams modifiableSolrParams = (ModifiableSolrParams) requestParams;
+      modifiableSolrParams.remove("qt");
+    }
+
+    QueryRequest query = new QueryRequest( requestParams );
+    query.setPath(p);
+    query.setResponseParser(new InputStreamResponseParser("json"));
+    query.setMethod(SolrRequest.METHOD.POST);
+    NamedList<Object> genericResponse = server.request(query);
+    InputStream stream = (InputStream)genericResponse.get("stream");
+    InputStreamReader reader = new InputStreamReader(stream, "UTF-8");
+    BufferedReader bufferedReader = new BufferedReader(reader);
+    String response = bufferedReader.readLine();
+    assertTrue(response.contains(json));
+  }
+
 }

solr/solrj/src/java/org/apache/solr/client/solrj/io/Tuple.java

@@ -45,6 +45,8 @@ public class Tuple implements Cloneable, MapWriter {
   public boolean EXCEPTION;
 
   public Map fields = new HashMap();
+  public List<String> fieldNames;
+  public Map<String, String> fieldLabels;
 
   public Tuple(Map fields) {
     if(fields.containsKey("EOF")) {
@@ -198,12 +200,19 @@ public class Tuple implements Cloneable, MapWriter {
 
   @Override
   public void writeMap(EntryWriter ew) throws IOException {
-    fields.forEach((k, v) -> {
+    if(fieldNames == null) {
-      try {
+      fields.forEach((k, v) -> {
-        ew.put((String)k,v);
+        try {
-      } catch (IOException e) {
+          ew.put((String) k, v);
-        throw new RuntimeException(e);
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+      });
+    } else {
+      for(String fieldName : fieldNames) {
+        String label = fieldLabels.get(fieldName);
+        ew.put(label, fields.get(label));
       }
-    });
+    }
   }
 }