Cast the values read in the EXTERN function to the type in the row signature #15183
Conversation
github-actions
bot
added
Area - Batch Ingestion
Area - MSQ
There was a problem hiding this comment.
👍 i suppose there is going to be a perf cost to this, but since InputFormat are not currently responsible for coercing values to common druid types I'm not sure what else we can do here.
I think since this is going to be sitting on top of a row based column selector factory getLong/getDouble/getFloat should probably be handled ok and don't need the same help.
I suppose this coercion could also be done in RowBasedColumnSelectorFactory, though the impact might be potentially larger because it is used in other places too, so we would want to be cautious probably. It is already doing some similar coercions, but nothing for getObject.
| @@ -97,7 +103,11 @@ public void inspectRuntimeShape(RuntimeShapeInspector inspector) | |||
| public Object getObject() | |||
| { | |||
| try { | |||
| return delegateDimensionSelector.getObject(); | |||
| ExpressionType expressionType = ExpressionType.fromColumnType(dimensionSpec.getOutputType()); | |||
There was a problem hiding this comment.
should probably save this outside of getObject so don't have to recreate every time (it uses interned objects so it won't be making new ones really, but could still save some overhead)
| @@ -195,7 +205,13 @@ public boolean isNull() | |||
| public Object getObject() | |||
| { | |||
| try { | |||
| return delegateColumnValueSelector.getObject(); | |||
| ExpressionType expressionType = ExpressionType.fromColumnType(rowSignature | |||
There was a problem hiding this comment.
same comment on saving outside of getObject
| @@ -857,6 +858,53 @@ public void testScanWithOrderByOnDoubleArray() | |||
| .verifyResults(); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testScanExternBooleanArray() | |||
There was a problem hiding this comment.
might be nice to add some other mismatched types, maybe even some that are not convertible to show what happens (like maybe json input to a double?)
Description
Currently, when reading values from an external data source (
EXTERN), we don't cast the values to the signature passed by the user. This can cause an issue when reading values that are not casted by the underlying selectors.For example, when reading a boolean array [true, false], we may run into a bug where the array is not casted to
ARRAY<LONG>(internal representation for boolean) by the underlying selector, we pass it to methods assuming thegetObject()returns a valid array type (Object[] with long values). Therefore it's important to cast the externally read values to the row signature passed by the user. This PR addresses the issue and preserves the type that is entered by the user in the query, regardless of what gets read.This PR has: