fix: SelectColumn Should Unbox Result Type #6052
Conversation
nbauernfeind
added
core
| @@ -20290,6 +20290,10 @@ public static boolean greaterEquals(float a, BigDecimal b) { | |||
| return compareTo(a, b) >= 0; | |||
| } | |||
|
|
|||
| public static BigDecimal negate(BigDecimal a) { | |||
There was a problem hiding this comment.
Maybe we should assess if there are further gaps? Maybe the way to get there is by updating the generator and adding it to our process.
| final ColumnDefinition<?> cd = ColumnDefinition.fromGenericType(name, cs.getType(), cs.getComponentType()); | ||
| columnDefinitions.put(name, cd); | ||
| } | ||
| parentTable.getDefinition().getColumns().forEach(cd -> columnDefinitions.put(cd.getName(), cd)); |
There was a problem hiding this comment.
I would definitely run nightlies. I would also like you to test out what happens if you run select(), view(), update(), and updateView() preserving a partitioning column: does the output reflect that the column is partitioning, and is that a change?
| @@ -189,7 +189,7 @@ public List<String> initDef( | |||
| .endl(); | |||
|
|
|||
| applyUsedVariables(columnDefinitionMap, result.getVariablesUsed(), result.getPossibleParams()); | |||
| returnedType = result.getType(); | |||
| returnedType = TypeUtils.getUnboxedTypeIfBoxed(result.getType()); | |||
There was a problem hiding this comment.
This seems like a healthy change. That said, should we be changing the consumer of returned type to ensure that things are better for all select columns? Or should we check the other implementations of getReturnedType? I'm mainly concerned about the python formula column.
| // Verify that the operator overload method returns the original expected type (or its unboxed form): | ||
| Assert.equals(TypeUtils.getUnboxedTypeIfBoxed(ret), "TypeUtils.getUnboxedTypeIfBoxed(ret)", | ||
| result, "result"); |
There was a problem hiding this comment.
Should we allow boxed or unboxed?
Assert.equals(TypeUtils.getUnboxedTypeIfBoxed(ret), "TypeUtils.getUnboxedTypeIfBoxed(ret)",
TypeUtils.getUnboxedTypeIfBoxed(result), "TypeUtils.getUnboxedTypeIfBoxed(result)");
| @@ -1717,10 +1717,11 @@ public Class<?> visit(UnaryExpr n, VisitArgs printer) { | |||
| // since the original expression was visited by getTypeWithCaching at the beginning of this method. | |||
| final Class<?> result = unaryOpOverloadMethod.accept(this, printer); | |||
|
|
|||
| // Verify that the operator overload method returns the original expected type: | |||
| Assert.equals(ret, "ret", result, "result"); | |||
| // Verify that the operator overload method returns the original expected type (or its unboxed form): | |||
There was a problem hiding this comment.
We talked about "smart unboxing" when given a null input that matches a single primitive method. I'm not sure if it makes sense, yet.
Fixes #5998.