Add sql + ingestion compatibility for first/last on numeric values#15607
Conversation
|
Tested out the changes locally, and went through the test cases and the code changes. Thank you for your continued efforts in the area 🚀 |
| if (type instanceof RowSignatures.ComplexSqlType) { | ||
| ColumnType complexColumnType = ((RowSignatures.ComplexSqlType) type).getColumnType(); | ||
| String complexTypeName = complexColumnType.getComplexTypeName(); | ||
| if (complexTypeName != null && (complexTypeName.equals(SerializablePairLongLongComplexMetricSerde.TYPE_NAME) || complexTypeName.equals(SerializablePairLongFloatComplexMetricSerde.TYPE_NAME) || complexTypeName.equals(SerializablePairLongDoubleComplexMetricSerde.TYPE_NAME))) { |
There was a problem hiding this comment.
Is this correct? Shouldn't we be returning the scalar return type then? Why are we not doing this for string as well?
There was a problem hiding this comment.
We can return the same for String but since its return type is already string which is handled in the below if condition, I dint bother changing this
There was a problem hiding this comment.
SELECT LATEST_BY("last_float_added", "__time") + 4.0 FROM "wikiticker_long_string_last_first"
This doesn't work because the return type is incorrect.
There was a problem hiding this comment.
If the input type is complex, it should return the scalar value for that complex type (float for longFloatPair etc), else if the input is numeric, it should return that numeric type, else it should return varchar (in unknown complex, string and any other sql type cases)
There was a problem hiding this comment.
Thanks for catching this @LakshSingla
updated the PR
| 'longFirst', | ||
| 'longLast', | ||
| 'doubleFirst', | ||
| 'doubleLast', | ||
| 'floatFirst', | ||
| 'floatLast', | ||
| 'stringFirst', | ||
| 'stringLast', |
There was a problem hiding this comment.
Perhaps this change isn't required. KNOWN_TYPES already contains the first/last aggregations.
Follow-up on 14462
Description
Enables sql querying for numeric last and first value. Instead of returning pair object as a string, now
latest, latest_byandearliest, earilest_bywill return the actual numeric value for column type lastNumeric and firstNumericRelease note
SQL compatibility for numeric last and first column types.
Ingestion UI now provides option for first and last aggregation as well.
Key changed/added classes in this PR
EarliestLatestAnySqlAggregatorMetricSpecThis PR has: