imply-cheddar commented on code in PR #13085:
URL: https://github.com/apache/druid/pull/13085#discussion_r996620329
##########
sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalDataSource.java:
##########
@@ -141,7 +141,7 @@ public DataSource withUpdatedDataSource(DataSource
newSource)
@Override
public byte[] getCacheKey()
{
- return null;
+ return new byte[0];
Review Comment:
This should either not be cacheable or it needs to build up an identifier
for the external data source... `null` seems correct for now.
##########
processing/src/main/java/org/apache/druid/query/QueryDataSource.java:
##########
@@ -109,7 +109,7 @@ public DataSource withUpdatedDataSource(DataSource
newSource)
@Override
public byte[] getCacheKey()
{
- return null;
+ return new byte[0];
Review Comment:
So, the query data source itself is not likely cacheable. The sub-query
that is run from inside the data source is perhaps cacheable, but not the
datasource. Well, unless it's a result level cache, that could work. But, I'm
like 80% that `null` is actually correct here.
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/InputNumberDataSource.java:
##########
@@ -112,7 +119,7 @@ public DataSource withUpdatedDataSource(DataSource
newSource)
@Override
public byte[] getCacheKey()
{
- return null;
+ return new byte[0];
Review Comment:
This seems uncacheable to me.
##########
processing/src/main/java/org/apache/druid/query/InlineDataSource.java:
##########
@@ -252,7 +252,7 @@ public DataSource withUpdatedDataSource(DataSource
newSource)
@Override
public byte[] getCacheKey()
{
- return null;
+ return new byte[0];
Review Comment:
Are you sure that this was cached previously? It seems like an
`InlineDataSource` would either have to include all of the data in the cache
key or cause caching to never happen...
--
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]
