You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/05/08 18:38:56 UTC

[GitHub] [incubator-pinot] npawar opened a new pull request #5353: Pass valid tableConfig to SegmentGeneratorConfig in all tests

npawar opened a new pull request #5353:
URL: https://github.com/apache/incubator-pinot/pull/5353


   We have 2 entry points to SegmentGeneratorConfig https://github.com/apache/incubator-pinot/issues/5322
   1) SegmentGeneratorConfig(@Nullable TableConfig, Schema)
   2) @Deprecated SegmentGeneratorConfig()
   
   We want to standardize on just 1. We also want to remove "@Nullable" from 1. 
   
   **This PR mainly touches tests. I**t attempts to:
   - ensure no one uses 2
   - All usages of 1 pass valid table config
   
   Motivation:
   As part of https://github.com/apache/incubator-pinot/issues/2756 we will begin to have multiple DateTime columns. We must rely ONLY on table config to get the primary time column name. In SegmentGeneratorConfig constructor, we will **stop falling back to Schema to fetch time column** if tableConfig==null (which happens as of today). We cannot do this with multiple DateTime columns because
   1) We cannot know which of the multiple date time columns to use
   2) It is possible that table doesn't have a time column (REFRESH) but still has datetimefieldSpecs. We cannot know whether one of the dateTimeFieldSpecs is intended to be timeColumn or not. Even if there's only 1 dateTimeFieldSpec, we cannot determine if it should be used as time column name.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [incubator-pinot] npawar commented on pull request #5353: Pass valid tableConfig to SegmentGeneratorConfig in all tests

Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #5353:
URL: https://github.com/apache/incubator-pinot/pull/5353#issuecomment-626075253


   > If all `null` tableConfig usages are removed, can you remove the nullable and null check for tableConfig? If all the tests passed, then that means there is no usage of `null` tableConfig.
   > LGTM otherwise.
   
   Done


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org