[calcite-avatica] branch master updated: [CALCITE-2776] Wrong value when accessing struct types with one attribute

[za...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git


The following commit(s) were added to refs/heads/master by this push:
     new 5a305f1  [CALCITE-2776] Wrong value when accessing struct types with one attribute
5a305f1 is described below

commit 5a305f166cf06419a0c28e496a9bf853ba540047
Author: Stamatis Zampetakis <za...@gmail.com>
AuthorDate: Tue Jan 8 10:37:27 2019 +0100

    [CALCITE-2776] Wrong value when accessing struct types with one attribute
    
    1. Remove special treatment for struct types with one attribute in AbstractCursor#createAccessor.
    
    2. Add parameterized unit test for testing struct accessors using multiple internal representations for struct types.
---
 .../calcite/avatica/util/AbstractCursor.java       |   6 +-
 .../calcite/avatica/util/StructImplTest.java       | 196 +++++++++++++++------
 2 files changed, 147 insertions(+), 55 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java b/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
index 9372bfb..5559f60 100644
--- a/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
+++ b/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
@@ -202,12 +202,8 @@ public abstract class AbstractCursor implements Cursor {
             (ColumnMetaData.StructType) columnMetaData.type;
         List<Accessor> accessors = new ArrayList<>();
         for (ColumnMetaData column : structType.columns) {
-          final Getter fieldGetter =
-              structType.columns.size() == 1
-                  ? getter
-                  : new StructGetter(getter, column);
           accessors.add(
-              createAccessor(column, fieldGetter, localCalendar, factory));
+              createAccessor(column, new StructGetter(getter, column), localCalendar, factory));
         }
         return new StructAccessor(getter, accessors);
       default:
diff --git a/core/src/test/java/org/apache/calcite/avatica/util/StructImplTest.java b/core/src/test/java/org/apache/calcite/avatica/util/StructImplTest.java
index 625c538..1058125 100644
--- a/core/src/test/java/org/apache/calcite/avatica/util/StructImplTest.java
+++ b/core/src/test/java/org/apache/calcite/avatica/util/StructImplTest.java
@@ -17,74 +17,170 @@
 package org.apache.calcite.avatica.util;
 
 import org.apache.calcite.avatica.ColumnMetaData;
-import org.apache.calcite.avatica.ColumnMetaData.StructType;
 import org.apache.calcite.avatica.MetaImpl;
 import org.apache.calcite.avatica.util.Cursor.Accessor;
 
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
+import java.sql.SQLException;
 import java.sql.Struct;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertArrayEquals;
 
 /**
  * Test class for StructImpl.
  */
+@RunWith(Parameterized.class)
 public class StructImplTest {
 
-  @Test public void structAccessor() throws Exception {
-    // Define the struct type we're creating
-    ColumnMetaData intMetaData = MetaImpl.columnMetaData("MY_INT", 1, int.class, false);
-    ColumnMetaData stringMetaData = MetaImpl.columnMetaData("MY_STRING", 2, String.class, true);
-    StructType structType = ColumnMetaData.struct(Arrays.asList(intMetaData, stringMetaData));
-    // Create some structs
-    Struct struct1 = new StructImpl(Arrays.<Object>asList(1, "one"));
-    Struct struct2 = new StructImpl(Arrays.<Object>asList(2, "two"));
-    Struct struct3 = new StructImpl(Arrays.<Object>asList(3));
-    Struct struct4 = new StructImpl(Arrays.<Object>asList(4, "four", "ignored"));
-    ColumnMetaData structMetaData = MetaImpl.columnMetaData("MY_STRUCT", 1, structType, false);
-    List<List<Object>> rows = Arrays.asList(Collections.<Object>singletonList(struct1),
-        Collections.<Object>singletonList(struct2), Collections.<Object>singletonList(struct3),
-        Collections.<Object>singletonList(struct4));
-    // Create four rows, each with one (struct) column
+  /**
+   * A class holding the necessary information for performing tests related with one table column.
+   *
+   * @param <T> the type of the expected values for the column
+   */
+  private static class ColumnInputBundle<T> {
+    private final ColumnMetaData metaData;
+    /**
+     * The input values for the given column.
+     *
+     * These values are used to construct the rows of a ResultSet.
+     */
+    private final List<Object> inputValues;
+    /**
+     * The expected values for the given column.
+     *
+     * These values are used to verify that the result obtained from a ResultSet is correct. Note,
+     * that inputValues and expectedValues are not necessarily equal.
+     */
+    private final List<T> expectedValues;
+
+    ColumnInputBundle(List<Object> inputValues, List<T> expected, ColumnMetaData metaData) {
+      this.inputValues = inputValues;
+      this.expectedValues = expected;
+      this.metaData = metaData;
+    }
+
+    @Override public String toString() {
+      return "column=" + metaData.columnName + ", inputType=" + inputValues.get(0).getClass();
+    }
+  }
+
+  private final ColumnInputBundle<Struct> columnInputBundle;
+
+  public StructImplTest(ColumnInputBundle<Struct> columnInputBundle) {
+    this.columnInputBundle = columnInputBundle;
+  }
+
+  @Parameterized.Parameters(name = "{0}")
+  public static Collection<ColumnInputBundle<Struct>> data() throws SQLException {
+    List<ColumnInputBundle<Struct>> data = new ArrayList<>();
+    final int numRows = 5;
+
+    ColumnMetaData oneAttrStructMeta = MetaImpl.columnMetaData(
+        "ONE_ATTRIBUTE_STRUCT",
+        0,
+        ColumnMetaData.struct(
+            Arrays.asList(
+                MetaImpl.columnMetaData("INT", 0, int.class, false)
+            )
+        ),
+        false);
+
+    ColumnMetaData twoAttrStructMeta = MetaImpl.columnMetaData(
+        "TWO_ATTRIBUTES_STRUCT",
+        0,
+        ColumnMetaData.struct(
+            Arrays.asList(
+                MetaImpl.columnMetaData("INT", 0, int.class, false),
+                MetaImpl.columnMetaData("INT", 1, int.class, false)
+            )
+        ),
+        false);
+
+    List<Struct> oneAttrStructData = new ArrayList<>(numRows);
+    List<Struct> twoAttrStructData = new ArrayList<>(numRows);
+    for (int i = 0; i < numRows; i++) {
+      oneAttrStructData.add(new StructImpl(Arrays.asList(i)));
+      twoAttrStructData.add(new StructImpl(Arrays.asList(i, i)));
+    }
+
+    // A struct has various internal representations. The most common are the following:
+    Class[] structTypes = new Class[]{Object[].class, Struct.class, List.class};
+    // Generate column bundles for every possible representation that is supported currently.
+    for (Class<?> type : structTypes) {
+      data.add(newStructColBundle(oneAttrStructData, type, oneAttrStructMeta));
+      data.add(newStructColBundle(twoAttrStructData, type, twoAttrStructMeta));
+    }
+
+    return data;
+  }
+
+  private static ColumnInputBundle newStructColBundle(
+      List<Struct> data, Class<?> structType, ColumnMetaData meta) throws SQLException {
+    List<Object> input = new ArrayList<>();
+    List<Struct> expected = new ArrayList<>();
+    for (Struct struct : data) {
+      input.add(structOf(structType, struct));
+      // The result obtained from StructAccessor is always of type Struct.class
+      expected.add(structOf(Struct.class, struct));
+    }
+    return new ColumnInputBundle(input, expected, meta);
+  }
+
+  /**
+   * Creates a struct in the representation dictated by the <code>structClass</code> parameter.
+   */
+  private static <T> T structOf(Class<T> structClass, Struct structData) throws SQLException {
+    Object[] fieldValues = structData.getAttributes();
+    if (structClass.equals(Object[].class)) {
+      Object[] arrayStruct = new Object[fieldValues.length];
+      for (int i = 0; i < fieldValues.length; i++) {
+        arrayStruct[i] = fieldValues[i];
+      }
+      return (T) arrayStruct;
+    } else if (structClass.equals(Struct.class)) {
+      List<Object> listStruct = new ArrayList<>();
+      for (int i = 0; i < fieldValues.length; i++) {
+        listStruct.add(fieldValues[i]);
+      }
+      return (T) new StructImpl(listStruct);
+    } else if (structClass.equals(List.class)) {
+      List<Object> listStruct = new ArrayList<>();
+      for (int i = 0; i < fieldValues.length; i++) {
+        listStruct.add(fieldValues[i]);
+      }
+      return (T) listStruct;
+    } else {
+      throw new IllegalStateException("Cannot create struct of type" + structClass);
+    }
+  }
+
+  @Test public void testStructAccessor() throws Exception {
+    // Create rows based on the inputValues data
+    List<List<Object>> rows = new ArrayList<>();
+    for (Object o : columnInputBundle.inputValues) {
+      rows.add(Collections.singletonList(o));
+    }
     try (Cursor cursor = new ListIteratorCursor(rows.iterator())) {
-      List<Accessor> accessors = cursor.createAccessors(Collections.singletonList(structMetaData),
-          Unsafe.localCalendar(), null);
-      assertEquals(1, accessors.size());
+      List<Accessor> accessors =
+          cursor.createAccessors(
+              Collections.singletonList(columnInputBundle.metaData), Unsafe.localCalendar(), null);
       Accessor accessor = accessors.get(0);
-
-      assertTrue(cursor.next());
-      Struct s = accessor.getObject(Struct.class);
-      Object[] structData = s.getAttributes();
-      assertEquals(2, structData.length);
-      assertEquals(1, structData[0]);
-      assertEquals("one", structData[1]);
-
-      assertTrue(cursor.next());
-      s = accessor.getObject(Struct.class);
-      structData = s.getAttributes();
-      assertEquals(2, structData.length);
-      assertEquals(2, structData[0]);
-      assertEquals("two", structData[1]);
-
-      assertTrue(cursor.next());
-      s = accessor.getObject(Struct.class);
-      structData = s.getAttributes();
-      assertEquals(1, structData.length);
-      assertEquals(3, structData[0]);
-
-      assertTrue(cursor.next());
-      s = accessor.getObject(Struct.class);
-      structData = s.getAttributes();
-      assertEquals(3, structData.length);
-      assertEquals(4, structData[0]);
-      assertEquals("four", structData[1]);
-      // We didn't provide metadata, but we still expect to see it.
-      assertEquals("ignored", structData[2]);
+      int i = 0;
+      while (cursor.next()) {
+        Struct s = accessor.getObject(Struct.class);
+        Object[] expectedStructAttributes = columnInputBundle.expectedValues.get(i).getAttributes();
+        Object[] actualStructAttributes = s.getAttributes();
+        assertArrayEquals(expectedStructAttributes, actualStructAttributes);
+        i++;
+      }
     }
   }
 }