You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "kgyrtkirk (via GitHub)" <gi...@apache.org> on 2024/03/04 11:40:27 UTC
[PR] Pull up literals in InputAccessor (druid)
kgyrtkirk opened a new pull request, #16033:
URL: https://github.com/apache/druid/pull/16033
* pull up literals in `InputAccessor`
* remove the need to pass `constants` of `Window` operator
Fixes #15353
This PR has:
- [ ] been self-reviewed.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Pull up literals in InputAccessor (druid)
Posted by "soumyava (via GitHub)" <gi...@apache.org>.
soumyava merged PR #16033:
URL: https://github.com/apache/druid/pull/16033
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Pull up literals in InputAccessor (druid)
Posted by "soumyava (via GitHub)" <gi...@apache.org>.
soumyava commented on code in PR #16033:
URL: https://github.com/apache/druid/pull/16033#discussion_r1514757469
##########
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java:
##########
@@ -1318,6 +1320,20 @@ public void testFloatAndDoubleAreConsideredTheSame()
);
}
+ @Test
+ public void testDsHllOnTopOfNested()
+ {
+ // this query was not planable: https://github.com/apache/druid/issues/15353
+ testBuilder()
+ .queryContext(ImmutableMap.of(QueryContexts.ENABLE_DEBUG, true))
Review Comment:
Probably just use QUERY_CONTEXT_DEFAULT here and not introduce debug mode for a test case
##########
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java:
##########
@@ -1318,6 +1320,20 @@ public void testFloatAndDoubleAreConsideredTheSame()
);
}
+ @Test
+ public void testDsHllOnTopOfNested()
+ {
+ // this query was not planable: https://github.com/apache/druid/issues/15353
+ testBuilder()
+ .queryContext(ImmutableMap.of(QueryContexts.ENABLE_DEBUG, true))
+ .sql(
+ "SELECT d1,dim2,APPROX_COUNT_DISTINCT_DS_HLL(dim2, 18) as val"
+ + " FROM (select d1,dim1,dim2 from druid.foo group by d1,dim1,dim2 order by dim1 limit 3) t "
+ + " group by 1,2"
+ )
+ .run();
Review Comment:
We should add the expected results of this query as well to make sure the results do not change later down the line
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Pull up literals in InputAccessor (druid)
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #16033:
URL: https://github.com/apache/druid/pull/16033#discussion_r1514844345
##########
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java:
##########
@@ -1318,6 +1320,20 @@ public void testFloatAndDoubleAreConsideredTheSame()
);
}
+ @Test
+ public void testDsHllOnTopOfNested()
+ {
+ // this query was not planable: https://github.com/apache/druid/issues/15353
+ testBuilder()
+ .queryContext(ImmutableMap.of(QueryContexts.ENABLE_DEBUG, true))
+ .sql(
+ "SELECT d1,dim2,APPROX_COUNT_DISTINCT_DS_HLL(dim2, 18) as val"
+ + " FROM (select d1,dim1,dim2 from druid.foo group by d1,dim1,dim2 order by dim1 limit 3) t "
+ + " group by 1,2"
+ )
+ .run();
Review Comment:
yeah - I've added it! :)
thank you for taking a look!
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org