[CALCITE-4503] Order of fields in records should follow that of the S…

…QL types (Alessandro Solimando)

1. RECORD and RECORD_PROJECTION are now handled in the same way (both require fields)
so there is no point keeping both.
2. Adapt LocalService#toResponse after dropping RECORD_PROJECTION to prevent broken clients,
and add notes about the change of CursorFactory.
3. Update history.md with details on the breaking change.
4. Add javadoc for deduce method.

Close #138

core/src/main/java/org/apache/calcite/avatica/Meta.java

@@ -41,6 +41,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 /**
  * Command handler for getting various metadata. Should be implemented by each
@@ -607,9 +608,8 @@ final class CursorFactory {
 
     private CursorFactory(Style style, Class clazz, List<Field> fields,
         List<String> fieldNames) {
-      assert (fieldNames != null)
-          == (style == Style.RECORD_PROJECTION || style == Style.MAP);
-      assert (fields != null) == (style == Style.RECORD_PROJECTION);
+      assert (fieldNames != null) == (style == Style.RECORD || style == Style.MAP);
+      assert (fields != null) == (style == Style.RECORD);
       this.style = Objects.requireNonNull(style);
       this.clazz = clazz;
       this.fields = fields;
@@ -628,8 +628,6 @@ public static CursorFactory create(@JsonProperty("style") Style style,
       case LIST:
         return LIST;
       case RECORD:
-        return record(clazz);
-      case RECORD_PROJECTION:
         return record(clazz, null, fieldNames);
       case MAP:
         return map(fieldNames);
@@ -647,8 +645,14 @@ public static CursorFactory create(@JsonProperty("style") Style style,
     public static final CursorFactory LIST =
         new CursorFactory(Style.LIST, null, null, null);
 
+    /**
+     *
+     * @deprecated Use {@link #record(Class, List, List)}
+     */
+    @Deprecated // to be removed before 1.19.0
     public static CursorFactory record(Class resultClazz) {
-      return new CursorFactory(Style.RECORD, resultClazz, null, null);
+      List<Field> fields = Arrays.asList(resultClazz.getFields());
+      return new CursorFactory(Style.RECORD, resultClazz, fields, null);
     }
 
     public static CursorFactory record(Class resultClass, List<Field> fields,
@@ -663,14 +667,23 @@ public static CursorFactory record(Class resultClass, List<Field> fields,
           }
         }
       }
-      return new CursorFactory(Style.RECORD_PROJECTION, resultClass, fields,
-          fieldNames);
+      return new CursorFactory(Style.RECORD, resultClass, fields, fieldNames);
     }
 
     public static CursorFactory map(List<String> fieldNames) {
       return new CursorFactory(Style.MAP, null, null, fieldNames);
     }
 
+    /**
+     * Deduces the appropriate {@code CursorFactory} for accessing the underlying
+     * result set. For result sets composed by records, {@code resultClazz} must
+     * be not null, and each field name in {@code columns} must match one of its
+     * public fields.
+     * @param columns The columns metadata for the result set
+     * @param resultClazz The class representing the records, if any
+     * @return the appropriate {@code CursorFactory} for the underlying result set
+     * @throws RuntimeException if the field name validation fails
+     */
     public static CursorFactory deduce(List<ColumnMetaData> columns,
         Class resultClazz) {
       if (columns.size() == 1) {
@@ -685,7 +698,8 @@ public static CursorFactory deduce(List<ColumnMetaData> columns,
       if (List.class.isAssignableFrom(resultClazz)) {
         return LIST;
       }
-      return record(resultClazz);
+      return record(resultClazz, null,
+          columns.stream().map(c -> c.columnName).collect(Collectors.toList()));
     }
 
     public Common.CursorFactory toProto() {
@@ -737,7 +751,6 @@ public static CursorFactory fromProto(Common.CursorFactory proto) {
   enum Style {
     OBJECT,
     RECORD,
-    RECORD_PROJECTION,
     ARRAY,
     LIST,
     MAP;

core/src/main/java/org/apache/calcite/avatica/MetaImpl.java

@@ -89,14 +89,8 @@ protected Getter createGetter(int ordinal) {
           (Iterable<Object[]>) (Iterable) iterable;
       return new ArrayIteratorCursor(iterable1.iterator());
     case RECORD:
-      @SuppressWarnings("unchecked") final Class<Object> clazz =
-          cursorFactory.clazz;
-      return new RecordIteratorCursor<Object>(iterable.iterator(), clazz);
-    case RECORD_PROJECTION:
-      @SuppressWarnings("unchecked") final Class<Object> clazz2 =
-          cursorFactory.clazz;
-      return new RecordIteratorCursor<Object>(iterable.iterator(), clazz2,
-          cursorFactory.fields);
+      @SuppressWarnings("unchecked") final Class<Object> clazz = cursorFactory.clazz;
+      return new RecordIteratorCursor<>(iterable.iterator(), clazz, cursorFactory.fields);
     case LIST:
       @SuppressWarnings("unchecked") final Iterable<List<Object>> iterable2 =
           (Iterable<List<Object>>) (Iterable) iterable;
@@ -138,16 +132,8 @@ public static List<List<Object>> collect(CursorFactory cursorFactory,
       }
       return list;
     case RECORD:
-    case RECORD_PROJECTION:
-      final Field[] fields;
-      switch (cursorFactory.style) {
-      case RECORD:
-        fields = cursorFactory.clazz.getFields();
-        break;
-      default:
-        fields = cursorFactory.fields.toArray(
-            new Field[cursorFactory.fields.size()]);
-      }
+      final Field[] fields = cursorFactory.fields.toArray(
+          new Field[cursorFactory.fields.size()]);
       for (Object o : iterable) {
         final Object[] objects = new Object[fields.length];
         for (int i = 0; i < fields.length; i++) {

core/src/main/java/org/apache/calcite/avatica/remote/LocalService.java

@@ -90,27 +90,33 @@ public ResultSetResponse toResponse(Meta.MetaResultSet resultSet) {
     }
 
     Meta.Signature signature = resultSet.signature;
+    // TODO Revise modification of CursorFactory see:
+    // https://issues.apache.org/jira/browse/CALCITE-4567
     Meta.CursorFactory cursorFactory = resultSet.signature.cursorFactory;
     Meta.Frame frame = null;
     int updateCount = -1;
     final List<Object> list;
 
     if (resultSet.firstFrame != null) {
       list = list(resultSet.firstFrame.rows);
-      switch (cursorFactory.style) {
-      case ARRAY:
+      if (list.isEmpty()) {
         cursorFactory = Meta.CursorFactory.LIST;
-        break;
-      case MAP:
-      case LIST:
-        break;
-      case RECORD:
-        cursorFactory = Meta.CursorFactory.LIST;
-        break;
-      default:
-        cursorFactory = Meta.CursorFactory.map(cursorFactory.fieldNames);
+      } else {
+        switch (cursorFactory.style) {
+        case ARRAY:
+          cursorFactory = Meta.CursorFactory.LIST;
+          break;
+        case MAP:
+        case LIST:
+          break;
+        case RECORD:
+          cursorFactory = Meta.CursorFactory.map(cursorFactory.fieldNames);
+          break;
+        default:
+          throw new IllegalStateException("Unknown cursor factory style: "
+              + cursorFactory.style);
+        }
       }
-
       final boolean done = resultSet.firstFrame.done;
 
       frame = new Meta.Frame(0, done, list);

core/src/main/java/org/apache/calcite/avatica/util/RecordIteratorCursor.java

@@ -37,7 +37,9 @@ public class RecordIteratorCursor<E> extends IteratorCursor<E> {
    *
    * @param iterator Iterator
    * @param clazz Element type
+   * @deprecated Use {@link #RecordIteratorCursor(Iterator, Class, List)}
    */
+  @Deprecated // to be removed before 2.0
   public RecordIteratorCursor(Iterator<E> iterator, Class<E> clazz) {
     this(iterator, clazz, Arrays.asList(clazz.getFields()));
   }