From b9dafb970e0c42345d8953969bf67d6b9e5b643c Mon Sep 17 00:00:00 2001 From: rbasralian Date: Wed, 18 Sep 2024 14:04:48 -0400 Subject: [PATCH 1/5] Include column name in exception when ColumnSource.cast fails --- .../io/deephaven/engine/table/impl/TableDefaults.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java index 9600e9021d0..95ae43d0498 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java @@ -108,8 +108,13 @@ default ColumnSource getColumnSource(String sourceName, Class componentType) { @SuppressWarnings("rawtypes") ColumnSource rawColumnSource = getColumnSource(sourceName); - // noinspection unchecked - return rawColumnSource.cast(clazz, componentType); + try { + // noinspection unchecked + return rawColumnSource.cast(clazz, componentType); + } catch (ClassCastException ex) { + throw new RuntimeException( + "Error retrieving ColumnSource with type " + clazz.getName() + " for column " + sourceName, ex); + } } // ----------------------------------------------------------------------------------------------------------------- From dc184e2b7382091a3a5c71656c17d26e2d962da6 Mon Sep 17 00:00:00 2001 From: rbasralian Date: Wed, 18 Sep 2024 16:24:12 -0400 Subject: [PATCH 2/5] include column name in prefix for `checkCastTo` instead --- .../deephaven/engine/table/ColumnSource.java | 60 ++++++++++++++++++- .../engine/table/impl/TableDefaults.java | 11 +--- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java index c70c2fe65a0..9ba13992d3d 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java @@ -177,8 +177,34 @@ default void exportAllTo(final Object @NotNull [] dest, @NotNull final T tuple) */ @FinalDefault default ColumnSource cast(Class clazz) { + return cast(clazz, (String) null); + } + + /** + * Returns this {@code ColumnSource}, parameterized by {@code }, if the data type of this column (as given by + * {@link #getType()}) can be cast to {@code clazz}. This is analogous to casting the objects provided by this + * column source to {@code clazz}. + *

+ * For example, the following code will throw an exception if the "MyString" column does not actually contain + * {@code String} data: + * + * 

+     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class)
+     *
+ *

+ * Due to the nature of type erasure, the JVM will still insert an additional cast to {@code TYPE} when elements are + * retrieved from the column source, such as with {@code String myStr = colSource.get(0)}. + * + * @param clazz The target type. + * @param The target type, as a type parameter. Intended to be inferred from {@code clazz}. + * @param colName An optional column name, which will be included in exception messages. + * @return A {@code ColumnSource} parameterized by {@code TYPE}. + */ + @FinalDefault + default ColumnSource cast(Class clazz, @Nullable String colName) { Require.neqNull(clazz, "clazz"); - TypeHelper.checkCastTo("ColumnSource", getType(), clazz); + final String castCheckPrefix = colName == null ? "ColumnSource" : "ColumnSource[" + colName + ']'; + TypeHelper.checkCastTo(castCheckPrefix, getType(), clazz); // noinspection unchecked return (ColumnSource) this; } @@ -208,8 +234,38 @@ default ColumnSource cast(Class clazz) { */ @FinalDefault default ColumnSource cast(Class clazz, @Nullable Class componentType) { + return cast(clazz, componentType, null); + } + + /** + * Returns this {@code ColumnSource}, parameterized by {@code }, if the data type of this column (as given by + * {@link #getType()}) can be cast to {@code clazz}. This is analogous to casting the objects provided by this + * column source to {@code clazz}. Additionally, this checks that the component type of this column (as given by + * {@link #getComponentType()}) can be cast to {@code componentType} (both must be present and castable, or both + * must be {@code null}). + * + *

+ * For example, the following code will throw an exception if the "MyString" column does not actually contain + * {@code String} data: + * + * 

+     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class, null)
+     *
+ *

+ * Due to the nature of type erasure, the JVM will still insert an additional cast to {@code TYPE} when elements are + * retrieved from the column source, such as with {@code String myStr = colSource.get(0)}. + * + * @param clazz The target type. + * @param componentType The target component type, may be {@code null}. + * @param colName An optional column name, which will be included in exception messages. + * @param The target type, as a type parameter. Intended to be inferred from {@code clazz}. + * @return A {@code ColumnSource} parameterized by {@code TYPE}. + */ + @FinalDefault + default ColumnSource cast(Class clazz, @Nullable Class componentType, @Nullable String colName) { Require.neqNull(clazz, "clazz"); - TypeHelper.checkCastTo("ColumnSource", getType(), getComponentType(), clazz, componentType); + final String castCheckPrefix = colName == null ? "ColumnSource" : "ColumnSource[" + colName + ']'; + TypeHelper.checkCastTo(castCheckPrefix, getType(), getComponentType(), clazz, componentType); // noinspection unchecked return (ColumnSource) this; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java index 95ae43d0498..d815bc5bd22 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java @@ -99,7 +99,7 @@ default ColumnSource getColumnSource(String sourceName, Class ColumnSource getColumnSource(String sourceName, Class componentType) { @SuppressWarnings("rawtypes") ColumnSource rawColumnSource = getColumnSource(sourceName); - try { - // noinspection unchecked - return rawColumnSource.cast(clazz, componentType); - } catch (ClassCastException ex) { - throw new RuntimeException( - "Error retrieving ColumnSource with type " + clazz.getName() + " for column " + sourceName, ex); - } + // noinspection unchecked + return rawColumnSource.cast(clazz, componentType, sourceName); } // ----------------------------------------------------------------------------------------------------------------- From 07395445a447b37fa7db3adbe172720ef82712d6 Mon Sep 17 00:00:00 2001 From: rbasralian Date: Wed, 18 Sep 2024 16:27:45 -0400 Subject: [PATCH 3/5] update example code in docs to match the method --- .../src/main/java/io/deephaven/engine/table/ColumnSource.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java index 9ba13992d3d..7401898b3e8 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java @@ -189,7 +189,7 @@ default ColumnSource cast(Class clazz) { * {@code String} data: * * 

-     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class)
+     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class, "MyString")
      *
*

* Due to the nature of type erasure, the JVM will still insert an additional cast to {@code TYPE} when elements are @@ -249,7 +249,7 @@ default ColumnSource cast(Class clazz, @Nullable Cl * {@code String} data: * * 

-     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class, null)
+     *     ColumnSource<String> colSource = table.getColumnSource("MyString").cast(String.class, null, "MyString")
      *
*

* Due to the nature of type erasure, the JVM will still insert an additional cast to {@code TYPE} when elements are From 22761590087c2b1e41e7edc5af131dab14fef782 Mon Sep 17 00:00:00 2001 From: rbasralian Date: Wed, 18 Sep 2024 16:32:54 -0400 Subject: [PATCH 4/5] spotless --- .../src/main/java/io/deephaven/engine/table/ColumnSource.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java index 7401898b3e8..2e469486904 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java @@ -262,7 +262,8 @@ default ColumnSource cast(Class clazz, @Nullable Cl * @return A {@code ColumnSource} parameterized by {@code TYPE}. */ @FinalDefault - default ColumnSource cast(Class clazz, @Nullable Class componentType, @Nullable String colName) { + default ColumnSource cast(Class clazz, @Nullable Class componentType, + @Nullable String colName) { Require.neqNull(clazz, "clazz"); final String castCheckPrefix = colName == null ? "ColumnSource" : "ColumnSource[" + colName + ']'; TypeHelper.checkCastTo(castCheckPrefix, getType(), getComponentType(), clazz, componentType); From 8582f88ffad72891c927951553c43eb362afef8a Mon Sep 17 00:00:00 2001 From: rbasralian Date: Wed, 18 Sep 2024 18:12:56 -0400 Subject: [PATCH 5/5] test getColumnSource/ColumnSource.cast --- .../engine/table/impl/QueryTableTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java index 29b7ea4d488..1e81065b84d 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java @@ -3848,6 +3848,79 @@ public void testMultipleUpdateGraphs() { assertEquals(g2, g2.sharedLock().computeLocked(() -> merge(s1, s2, r2).getUpdateGraph())); } + public void testColumnSourceCast() { + // Create a test table with a String column and an array column + final Table testTable = TableTools.newTable( + TableTools.stringCol("MyTestStrCol", "A", "B", "C"), + TableTools.col("MyTestArrCol", new String[] {"A0", "A1"}, new String[] {"B0", "B1"}, + new String[] {"C0", "C1"})); + + /* Test getting column sources with checked types */ + + // Test getColumnSource for MyTestStrCol + ColumnSource stringColSource = testTable.getColumnSource("MyTestStrCol", CharSequence.class); + assertNotNull(stringColSource); + assertEquals(String.class, stringColSource.getType()); // actual type is still String + + // Test getColumnSource for MyTestStrCol with wrong type and verify exception message + ClassCastException colTypeException = Assert.assertThrows(ClassCastException.class, () -> { + ColumnSource intColSource = testTable.getColumnSource("MyTestStrCol", Integer.class); + }); + assertEquals("Cannot convert ColumnSource[MyTestStrCol] of type java.lang.String to type java.lang.Integer", + colTypeException.getMessage()); + + // Test getColumnSource for MyTestArrCol + ColumnSource arrColSource = + testTable.getColumnSource("MyTestArrCol", CharSequence[].class, CharSequence.class); + assertNotNull(arrColSource); + assertEquals(String[].class, arrColSource.getType()); + assertEquals(String.class, arrColSource.getComponentType()); + + // Test getColumnSource for MyTestArrCol with a wrong component type and verify exception message + ClassCastException wrongComponentException = Assert.assertThrows(ClassCastException.class, () -> { + ColumnSource wrongComponentTypeSource = + testTable.getColumnSource("MyTestArrCol", CharSequence[].class, Integer.class); + }); + assertEquals( + "Cannot convert ColumnSource[MyTestArrCol] componentType of type java.lang.String to java.lang.Integer (for [Ljava.lang.String; / [Ljava.lang.CharSequence;)", + wrongComponentException.getMessage()); + + + /* Verify exception messages of underlying ColumnSource.cast method, with and without column name specified */ + + ColumnSource rawStrColSource = testTable.getColumnSource("MyTestStrCol"); + // cast() without component type, with column name specified + ClassCastException castExceptionNoCompWithColName = Assert.assertThrows(ClassCastException.class, () -> { + rawStrColSource.cast(Boolean.class, "MyTestStrCol"); + }); + assertEquals("Cannot convert ColumnSource[MyTestStrCol] of type java.lang.String to type java.lang.Boolean", + castExceptionNoCompWithColName.getMessage()); + + // cast() without component type and no column name specified + ClassCastException castExceptionNoCompNoColName = Assert.assertThrows(ClassCastException.class, () -> { + rawStrColSource.cast(Boolean.class); + }); + assertEquals("Cannot convert ColumnSource of type java.lang.String to type java.lang.Boolean", + castExceptionNoCompNoColName.getMessage()); + + ColumnSource rawArrColSource = testTable.getColumnSource("MyTestArrCol"); + // cast() with component type and column name specified + ClassCastException castExceptionWithCompAndColName = Assert.assertThrows(ClassCastException.class, () -> { + rawArrColSource.cast(Object[].class, Integer.class, "MyTestArrCol"); + }); + assertEquals( + "Cannot convert ColumnSource[MyTestArrCol] componentType of type java.lang.String to java.lang.Integer (for [Ljava.lang.String; / [Ljava.lang.Object;)", + castExceptionWithCompAndColName.getMessage()); + + // cast() with component type and no column name specified + ClassCastException castExceptionWithCompNoColName = Assert.assertThrows(ClassCastException.class, () -> { + rawArrColSource.cast(Object[].class, Integer.class); + }); + assertEquals( + "Cannot convert ColumnSource componentType of type java.lang.String to java.lang.Integer (for [Ljava.lang.String; / [Ljava.lang.Object;)", + castExceptionWithCompNoColName.getMessage()); + } + private static final class DummyUpdateGraph implements UpdateGraph { private final String name;