Vamsi-klu commented on PR #18661: URL: https://github.com/apache/pinot/pull/18661#issuecomment-4966078527
Compared the removed `PRelToPlanNodeConverter.convertToColumnDataType` against `RelToPlanNodeConverter.convertToColumnDataType` (line 1131) and the mapping is identical right down to `resolveDecimal` and the UBIGINT rejection, so pointing `toDataSchema` at it is behavior preserving. The target is already `public static` in the same module and this class was importing `RexExpression`/`RexExpressionUtils` from the `logical` package anyway, and the dropped imports plus the `LOGGER` field were only used by the deleted methods, so nothing is left dangling. Removing `PRelToPlanNodeConverterTest` is fine too, since it was the only external caller of the deleted method and `RelToPlanNodeConverterTest` already exercises the same unsigned matrix (UTINYINT/USMALLINT to INT, UINTEGER to LONG, UBIGINT throws, plus the array variants) directly on the method the v2 path now calls. No objections from me. -- 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]
