Support non-lowercase table names in Druid connector

[imjalpreet]

This PR introduces druid.case-insensitive-name-matching to allow querying tables with mixed case/uppercase names.

Fixes #15587

== RELEASE NOTES ==

General Changes
* Add support for querying non-lowercase table names in Druid connector (:pr:`15920`)

[beinan]

what's the benefit of using an Optional here? can we just put the remoteTableObject as the value directly?

[imjalpreet]

I am using Optional.empty() to mark a table which doesn't exist in Druid.

[beinan]

Ok, it make sense. Thank you for the clarification!

[beinan]

hmmm, can we use cache's get(K key,
Callable<? extends V> valueLoader) to avoid returning a null? it's something like if cached, return; otherwise create, cache and return

[imjalpreet]

@beinan In the current design, to create a new cache entry it involves looping through all the tables present in Druid. If I make use of cacheLoader to create, cache and return, it will result in looping through all the tables every time a new table is accessed as cache will be updated for only the table which is accessed. Whereas in my current implementation, if a cache reload takes place, I load all the tables into the cache which are present in Druid at the instant which means cache will only be reloaded again when a table is accessed which did not exist when the cache was refreshed the last time.

Please let me know your views on the same and correct me in case I misunderstood something.

[beinan]

Got it, I'm convinced.

[beinan]

I assume the cache you're using is thread-safe, correct me if I'm wrong.

[imjalpreet]

@beinan Yes, that's right the cache is thread-safe.

[beinan]

Cool, thanks for the clarification.

[beinan]

I see, you're using Optional.empty to indicate the non-existing table name. Now I understand why you're using an Optional as the V.

But we don't have to update mapping here, am I right? if the mapping does not contains the key, we can just do remoteTables.put(schemaTableName,, Optional.empty() and then return Optional.empty(). the logic should be identical I think

[imjalpreet]

@beinan I agree, I have made the changes as suggested by you.

[beinan]

Thank you so much for this great contribution! @imjalpreet
sorry for my delay, quite a few things on-going these days. Just left a couple comments for some minor issues, I will try to make a further review in the next 2~3 days.

[beinan]

It looks good to me except a couple of minor issue as I commented above. Thank you for the contribution!

[imjalpreet]

@beinan thank you for the review. Sorry I couldn't look into it yet, but I will work on the comments and revert back soon.

[imjalpreet]

@beinan Sorry, I couldn't revert back earlier. I have made some changes as you requested, can you have a look at my comments and let me know what are your views? Thanks again for the review.

[beinan]

Cool, thanks for the clarification.

[beinan]

Got it, I'm convinced.

[beinan]

Ok, it make sense. Thank you for the clarification!