Skip to content

Commit

Permalink
fix sql results mixed array and scalar values (#16105)
Browse files Browse the repository at this point in the history
* fix sql results mixed array and scalar values

* simplify
  • Loading branch information
clintropolis committed Mar 13, 2024
1 parent 82fced5 commit 795e342
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -70,7 +71,7 @@ public static Object coerce(
} else if (value instanceof Boolean) {
coercedValue = String.valueOf(value);
} else {
final Object maybeList = maybeCoerceArrayToList(value, false);
final Object maybeList = coerceArrayToList(value, false);

// Check if "maybeList" was originally a Collection of some kind, or was able to be coerced to one.
// Then Iterate through the collection, coercing each value. Useful for handling multi-value dimensions.
Expand Down Expand Up @@ -152,10 +153,7 @@ public static Object coerce(
// the protobuf jdbc handler prefers lists (it actually can't handle java arrays as sql arrays, only java lists)
// the json handler could handle this just fine, but it handles lists as sql arrays as well so just convert
// here if needed
coercedValue = maybeCoerceArrayToList(value, true);
if (coercedValue == null) {
throw cannotCoerce(value, sqlTypeName, fieldName);
}
coercedValue = coerceArrayToList(value, true);
}
} else {
throw cannotCoerce(value, sqlTypeName, fieldName);
Expand All @@ -166,11 +164,11 @@ public static Object coerce(

/**
* Attempt to coerce a value to {@link List}. If it cannot be coerced, either return the original value (if mustCoerce
* is false) or return null (if mustCoerce is true).
* is false) or return the value as a single element list (if mustCoerce is true).
*/
@VisibleForTesting
@Nullable
static Object maybeCoerceArrayToList(Object value, boolean mustCoerce)
static Object coerceArrayToList(Object value, boolean mustCoerce)
{
if (value instanceof List) {
return value;
Expand All @@ -184,7 +182,7 @@ static Object maybeCoerceArrayToList(Object value, boolean mustCoerce)
final Object[] array = (Object[]) value;
final ArrayList<Object> lst = new ArrayList<>(array.length);
for (Object o : array) {
lst.add(maybeCoerceArrayToList(o, false));
lst.add(coerceArrayToList(o, false));
}
return lst;
} else if (value instanceof long[]) {
Expand All @@ -199,7 +197,7 @@ static Object maybeCoerceArrayToList(Object value, boolean mustCoerce)
}
return lst;
} else if (mustCoerce) {
return null;
return Collections.singletonList(value);
}
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ public void testCoerceStringArrays()
assertCoerceArrayToList(stringList, stringList);
assertCoerceArrayToList(stringList, stringArray);
assertCoerceArrayToList(stringList, stringArray2);
assertCoerceArrayToList(null, null);
assertCoerceArrayToList(Collections.singletonList("a"), "a");
}

@Test
Expand All @@ -76,6 +78,8 @@ public void testCoerceLongArrays()
assertCoerceArrayToList(listWithNull, arrayWithNull);
assertCoerceArrayToList(list, list);
assertCoerceArrayToList(list, array);
assertCoerceArrayToList(null, null);
assertCoerceArrayToList(Collections.singletonList(1L), 1L);
}

@Test
Expand All @@ -90,6 +94,8 @@ public void testCoerceDoubleArrays()
assertCoerceArrayToList(listWithNull, arrayWithNull);
assertCoerceArrayToList(list, list);
assertCoerceArrayToList(list, array);
assertCoerceArrayToList(null, null);
assertCoerceArrayToList(Collections.singletonList(1.1), 1.1);
}

@Test
Expand All @@ -104,6 +110,8 @@ public void testCoerceFloatArrays()
assertCoerceArrayToList(listWithNull, arrayWithNull);
assertCoerceArrayToList(list, list);
assertCoerceArrayToList(list, array);
assertCoerceArrayToList(null, null);
assertCoerceArrayToList(Collections.singletonList(1.1f), 1.1f);
}

@Test
Expand Down Expand Up @@ -225,11 +233,6 @@ public void testCoerceOfArrayOfPrimitives()
Assert.assertEquals("Cannot coerce field [fieldName] from type [Byte Array] to type [BIGINT]", e.getMessage());
}
}
@Test
public void testCoerceArrayFails()
{
assertCannotCoerce("xyz", SqlTypeName.ARRAY);
}

@Test
public void testCoerceUnsupportedType()
Expand All @@ -238,15 +241,9 @@ public void testCoerceUnsupportedType()
}

@Test
public void testMustCoerce()
{
Assert.assertNull(SqlResults.maybeCoerceArrayToList("hello", true));
}

@Test
public void testMayNotCoerce()
public void testMayNotCoerceList()
{
Assert.assertEquals("hello", SqlResults.maybeCoerceArrayToList("hello", false));
Assert.assertEquals("hello", SqlResults.coerceArrayToList("hello", false));
}

private void assertCoerce(Object expected, Object toCoerce, SqlTypeName typeName)
Expand All @@ -269,9 +266,9 @@ private void assertCannotCoerce(Object toCoerce, SqlTypeName typeName)
MatcherAssert.assertThat(e, ThrowableMessageMatcher.hasMessage(CoreMatchers.containsString("Cannot coerce")));
}

private static void assertCoerceArrayToList(Object expected, Object toCoerce)
private void assertCoerceArrayToList(Object expected, Object toCoerce)
{
Object coerced = SqlResults.maybeCoerceArrayToList(toCoerce, true);
Object coerced = SqlResults.coerce(jsonMapper, DEFAULT_CONTEXT, toCoerce, SqlTypeName.ARRAY, "");
Assert.assertEquals(expected, coerced);
}
}

0 comments on commit 795e342

Please sign in to comment.