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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to