You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2023/04/19 07:59:34 UTC

[GitHub] [pinot] gortiz commented on issue #10615: About FieldConfig.IndexType values

gortiz commented on issue #10615:
URL: https://github.com/apache/pinot/issues/10615#issuecomment-1514302380

   I'm just describing the problems I found to open a discussion looking for solutions.
   Recently, I was involved in at least two discussion about the _timestamp index_ or the _sorted index_ where different people had different assumptions about the _indexTypes_ values and their meaning and I wanted to make clear how are they used from code's perspective in order to use that as a base on which we can build the _business_ or _product_ terms we want to define to talk with users and even between devs.
   
   About what to do with them. The options I see are:
   1. Remove them
   2. Ignore them (like what we do right now, but in a more consistent way)
   3. Honor them.
   
   I think the second is the better right now. Easier to implement and the one that doesn't actually impact existent table configs. It is not great from the developer perspective, but it is not that harmful.
   
   ## Remove them
   
   As you say, we cannot just remove literals without breaking backward compatibility.
   At most, we may remove literals that were never documented.
   If we do that, Pinot won't be able to read TableConfigs that were using undocumented things, but we can consider that if they are not documented, they are not part of the stable config and therefore there is no backward compatibility break.
   
   ## Ignore them
   
   But even if we do not remove them, we can use them in a deterministic way. 
   For example, we annotate them as deprecated and ensure that we don't use them anywhere (including tests).
   We can also include a warning when they are using, to indicate users that these literals are actually doing nothing.
   
   Specially in the case of _timestamp_, we should actually ignore the field and actually do the check if _timestampConfig_ is not null, which is what actually enable the _index_.
   
   ## Honor them
   
   Another alternative would be just the opposite.
   Modify the code to actually change its behavior when these literals are present.
   That would also be a backward incompatible change, but it may be seen as a bugfix, given that if the user included `range` in their `indexTypes`, we can assume they though that a range index was going to be created.
   


-- 
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: commits-unsubscribe@pinot.apache.org

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