[GitHub] [pinot] Jackie-Jiang commented on pull request #10836: refactor ConcurrentMapPartitionUpsertMetadataManager constructor

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

Jackie-Jiang commented on PR #10836:
URL: https://github.com/apache/pinot/pull/10836#issuecomment-1574401359

   > @Jackie-Jiang Can I assign the `comparisonColumns` value to upsertConfig and pass it to PartitionUpsertMetadataManager? then we don't need to repeat the same steps in partitionlevel upsert metadata managers.
   > 
   > The purpose of the change is to address a comment from @KKcorps. [#10047 (comment)](https://github.com/apache/pinot/pull/10047#discussion_r1189404582)
   
   That can fix the issue, but also means it is harder to manage (next person might make the same mistake). I'd suggest keeping the constructor as is without overriding the config. @KKcorps wdyt?


-- 
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