LakshSingla commented on code in PR #16557:
URL: https://github.com/apache/druid/pull/16557#discussion_r1628189672
##########
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:
naming nit:
1. This can be getLookupName(String name). Canonical is confusing, and isn't
defined in the javadoc here. See point (2) below as well.
other comments:
1. This method isn't marked as `PublicApi` however custom extensions can
extend it for own lookup implementations. Please add a default implementation
for the same.
2. When should this method be called. As of now, this is a super specialized
method that is called in `QueryLookupOperatorConversion`. Perhaps it should be
called out and named as such. As a developer, it is unclear to me when must I
call it and when not. For example, why am I not calling it on
https://github.com/apache/druid/blob/master/sql/src/main/java/org/apache/druid/sql/calcite/schema/LookupSchema.java#L61?
--
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]
