abhishekrb19 commented on code in PR #16557:
URL: https://github.com/apache/druid/pull/16557#discussion_r1628399933
##########
processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java:
##########
@@ -40,4 +40,9 @@ public interface LookupExtractorFactoryContainerProvider
* Returns a lookup container for the provided lookupName, if it exists.
*/
Optional<LookupExtractorFactoryContainer> get(String lookupName);
+
+ /**
+ * Returns the canonical lookup name from a lookup name.
+ */
+ String getCanonicalLookupName(String lookupName);
Review Comment:
@LakshSingla, good points. I think we need to clarify the Javadoc a bit more
to clarify the intent of the new method and/or rename accordingly. We can also
call it `getLookupName(String name)` too, but that's a bit ambiguous imo.
Naming, thoughts? 😅
As far as 1 is concerned, the interface isn't annotated `@PublicAPI` or
`@ExtensionsPoint`. Also, this PR https://github.com/apache/druid/pull/9281/
changed the method signature and added a new interface method without default
implementations. Following that pattern, I believe it's safe to add new methods
without providing a default implementation. In general, my understanding is
that for custom extensions that directly use/extend interfaces that are not
public APIs (i.e., not annotated with `PublicAPI` or `ExtensionsPoint`), the
onus is on the developers maintaining custom implementations to sync with
changes coming from upstream.
For 2, `LookupSchema` is powering the SQL view for the lookups configured in
the system. For consistency, I think it'd make sense to also call the new
function there as you pointed out. I'm okay with doing that as a follow up too.
--
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]
