You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2019/11/19 11:30:06 UTC
[GitHub] [incubator-druid] clintropolis opened a new pull request #8908: fix
sql compatible null handling config work with runtime.properties
clintropolis opened a new pull request #8908: fix sql compatible null handling config work with runtime.properties
URL: https://github.com/apache/incubator-druid/pull/8908
### Description
This PR is an alternate to #8876, which was just awful, but I was afraid of introducing an if statement to `NullHandling. replaceWithDefault` in something that is called often on the query path. However, curiosity got the best of me, so I added a new benchmark schema to help determine how much of a performance hit introducing an if statement. It turns it that it isn't really noticeable, so I'm closing #8876 in favor of this PR instead.
before PR:
```
Benchmark (defaultStrategy) (initialBuckets) (numProcessingThreads) (numSegments) (queryGranularity) (rowsPerSegment) (schemaAndQuery) (vectorize) Mode Cnt Score Error Units
GroupByBenchmark.queryMultiQueryableIndexWithSerde v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 8610.792 ± 260.568 us/op
GroupByBenchmark.queryMultiQueryableIndexWithSpilling v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 7769.964 ± 184.177 us/op
GroupByBenchmark.queryMultiQueryableIndexX v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 8424.455 ± 268.964 us/op
GroupByBenchmark.querySingleIncrementalIndex v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 15782.469 ± 3313.596 us/op
GroupByBenchmark.querySingleQueryableIndex v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 3978.851 ± 284.614 us/op
```
after PR:
```
Benchmark (defaultStrategy) (initialBuckets) (numProcessingThreads) (numSegments) (queryGranularity) (rowsPerSegment) (schemaAndQuery) (vectorize) Mode Cnt Score Error Units
GroupByBenchmark.queryMultiQueryableIndexWithSerde v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 8104.835 ± 275.995 us/op
GroupByBenchmark.queryMultiQueryableIndexWithSpilling v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 8215.000 ± 228.912 us/op
GroupByBenchmark.queryMultiQueryableIndexX v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 8294.836 ± 189.671 us/op
GroupByBenchmark.querySingleIncrementalIndex v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 16709.884 ± 3195.983 us/op
GroupByBenchmark.querySingleQueryableIndex v2 -1 2 4 all 100000 simple-nulls.A false avgt 15 3882.968 ± 93.530 us/op
```
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
<hr>
##### Key changed/added classes in this PR
* `NullHandling`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org