yupeng9 commented on pull request #6096: URL: https://github.com/apache/incubator-pinot/pull/6096#issuecomment-703194819
> After some consideration, I still think it is better to wrap the primary key columns inside the `UpsertConfig` for the following reasons: > > * The schema for the offline segment is re-constructed from the segment metadata, where primary key columns might not be maintained This is fine for now. As upsert applies to real-time segment only. In the future, I think we shall consider adding the primary ey concept to offline segment too > * For regular use cases, there is no primary key concept in Pinot. Putting it in `UpsertConfig` can keep all related configs at the same place > * For the upcoming JOIN feature, we should have another config for the in-memory lookup table, where we can put the primary key there. This 2 features cannot be applied to the same table, and I don't see the benefit of keeping this info inside the schema For fact-dim join, the primary key actually needs to be defined on the regular table. > > Since this is the first feature using the primary key concept, I suggest putting all the related configs at the same place for easier management. If we find this as a common concept in the future, we can move it then. ---------------------------------------------------------------- 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
