kgyrtkirk commented on code in PR #16800:
URL: https://github.com/apache/druid/pull/16800#discussion_r1760622194
##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -982,13 +983,40 @@ void validate(int row, int column, ValueType type, Object
expectedCell, Object r
mismatchMessage(row, column),
(Float) expectedCell,
(Float) resultCell,
- ASSERTION_EPSILON);
+ ASSERTION_EPSILON
+ );
} else if (expectedCell instanceof Double) {
assertEquals(
mismatchMessage(row, column),
(Double) expectedCell,
(Double) resultCell,
- ASSERTION_EPSILON);
+ ASSERTION_EPSILON
+ );
+ } else if (expectedCell instanceof Object[] || expectedCell instanceof
List) {
+ final Object[] expectedCellCasted;
+ if (expectedCell instanceof Object[]) {
+ expectedCellCasted = (Object[]) expectedCell;
+ } else {
+ expectedCellCasted = ExprEval.coerceListToArray((List) resultCell,
true).rhs;
+ }
Review Comment:
why repeat this part twice? if it can be `Object[]` or a `List` can't it be
some other type?
if that's okay - why isn't this part of the `coerceListToArray` method?
##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -982,13 +983,40 @@
mismatchMessage(row, column),
(Float) expectedCell,
(Float) resultCell,
- ASSERTION_EPSILON);
+ ASSERTION_EPSILON
+ );
} else if (expectedCell instanceof Double) {
assertEquals(
mismatchMessage(row, column),
(Double) expectedCell,
(Double) resultCell,
- ASSERTION_EPSILON);
+ ASSERTION_EPSILON
+ );
+ } else if (expectedCell instanceof Object[] || expectedCell instanceof
List) {
+ final Object[] expectedCellCasted;
+ if (expectedCell instanceof Object[]) {
+ expectedCellCasted = (Object[]) expectedCell;
+ } else {
+ expectedCellCasted = ExprEval.coerceListToArray((List) resultCell,
true).rhs;
+ }
+ final Object[] resultCellCasted;
+ if (resultCell instanceof List) {
+ resultCellCasted = ExprEval.coerceListToArray((List) resultCell,
true).rhs;
+ } else {
+ resultCellCasted = (Object[]) resultCell;
+ }
+ if (expectedCellCasted.length != resultCellCasted.length) {
+ throw new RE(
+ "Mismatched array lengths: expected[%s] with length[%d],
actual[%s] with length[%d]",
+ expectedCellCasted,
Review Comment:
shouldn't this warning be addressed; wouldn't it will produce garbage
excpetion messages?
##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -1019,6 +1047,31 @@ void validate(int row, int column, ValueType type,
Object expectedCell, Object r
(Double) resultCell,
eps
);
+ } else if (expectedCell instanceof Object[] || expectedCell instanceof
List) {
Review Comment:
what's the difference between this block and the above starting in line 995?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
