Jackie-Jiang commented on pull request #6728: URL: https://github.com/apache/incubator-pinot/pull/6728#issuecomment-811347857
> High level question - can we consider merging these different specifications of data types. The DataType captured in the schema has both SV and MV for primitives so why can't we use that all the way even in functions. As mentioned in the description, I think we should replace `DataType` with `ColumnDataType` where the SV and MV info is embedded within the type. This way, the data type within the schema and the response will be consistent. That will be a large refactor, so will do that separately. > Also, not necessarily related to this PR but I don't think I fully understand the purpose of PinotDataType. It is currently used in record transformer and functions. For functions only in scalar transform functions and ingest transform functions for type conversion. Why do we need to convert the types between parameter type and argument type? > > For example, if we have foo (long 123), then parameterType is long and argument type is int ? Why do we need to convert ? > > The only place where we should convert is for cast functions but there we don't even use PinotDataType > > I feel a lot of type related code in FunctionInvoker and FunctionUtils can be cleaned up if we have a unified type throughout the engine. Currently Pinot handles the type conversion automatically when argument cannot be directly assigned to the parameter. I agree we can force argumentType and parameterType to be the same which can simplify the logic a lot, but that also means user need to explicitly cast the value to the correct type. This is backward-incompatible change, so I decided to keep the current behavior for this PR. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
