npawar commented on pull request #5378: URL: https://github.com/apache/incubator-pinot/pull/5378#issuecomment-629497281
> Not related to this PR, but inside the Schema can we do the TimeFieldSpec to DateTimeFieldSpec conversion on the setter side instead of the getter side? > The problem of conversion on the getter side is that the schema still stores the deprecated TimeFieldSpec, and `getFieldSpecMap()` and `getFieldSpecFor()` will still return the TimeFieldSpec instead of DateTimeFieldSpec. This can easily cause bugs inside the code. That was exactly the initial plan - to modify the setter to save it as a DateTimeFieldspec. But now we changed the plan. We are not going to do any changes to ser/deser. Reason mainly being external integrations (presto, superset etc) which still need TimeFieldSpec, and also upgrades. I've updated the design doc: https://docs.google.com/document/d/1SU1jCjfsIDSA960fD5YWQbD72p8UdGF0c7CroFNt9Ho/edit#heading=h.6gaudq1t6ys8 (Step C and also put the previous approach in Discarded section) New plan is to make sure Pinot can treat TIME or DATE_TIME either of them as the primary time column. ---------------------------------------------------------------- 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
