TessaIO commented on code in PR #17212:
URL: https://github.com/apache/druid/pull/17212#discussion_r1796550517


##########
extensions-core/lookups-cached-single/src/main/java/org/apache/druid/server/lookup/LoadingLookup.java:
##########


Review Comment:
   > 1. The PR description suggests that it fixes [Unkown lookup type 
loadingLookup when using druid-lookups-cached-singleĀ 
#15936](https://github.com/apache/druid/issues/15936) - but that doesn't seem 
right. Can you confirm this, and update the PR description if needed?
   
   It fixes part of the issue. The frontend issue is not fixed in this PR. 
However, we discovered that even if we bypass the frontend layer, there's still 
a problem with the loading lookup since, after creation, we cannot use it, 
having an error saying that lookup is not iterable. I already mentioned that in 
the description. Should I remove the "fixes #xxxx" part or should I create a 
new Issue and link it with the PR?
   
   
   > 2\. Can you give an example use-case for iterating over all the data? Is 
this for queries like `select * from "lookup"."lookupName"`?
   
   Yes, if we want to see the content of the lookup, sometimes we need to loop 
over all the data. And BTW, the loading lookup fails even if we use it inside a 
query. We tested that on one of our Test environments and it was not working. 
You can confirm this on your side also, that would be much appreciated!
   
   3. These changes were tested on our Test environments and it's working as 
expected so far.



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

Reply via email to