cecemei commented on PR #17954: URL: https://github.com/apache/druid/pull/17954#issuecomment-2843616527
The main purpose is to be able to identify whether a segment is backed by a table or not, which seems like might be better if we just return null for `getId()`, non-table segment should not have a valid `SegmentId`. I opened a new PR to implement it: https://github.com/apache/druid/pull/17960. This PR is therefore deprecated. > I am a little skeptical of adding a new field to segment id, since it is used pretty much in the entirety of Druid. Adding a new field which is going to be the same in all of the existing segments in the system feels like it is going to add a lot of unnecessary overhead in memory footprint. > > Since the datasource field will always be empty for the non-table datasource types, how about we use some reserved datasource names for those cases instead? > > The `SegmentId` class may still expose a method `getDatasourceType()` which will just return the appropriate value based on the value of the datasource. `SegmentId` can also have static `create` or `of` methods that create `SegmentId`s for specific datasource types. But we should avoid addition of a new field in the object. > > cc: @gianm , @clintropolis -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
