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/15 21:55:28 UTC

[GitHub] [incubator-druid] clintropolis opened a new pull request #8876: fix sql compatible null handling config work with runtime.properties

clintropolis opened a new pull request #8876: fix sql compatible null handling config work with runtime.properties
URL: https://github.com/apache/incubator-druid/pull/8876
 
 
   
   ### Description
   This terrible PR modifies `NullHandling` to no longer have a default value since it appears in practice to overwrite the instance injected by `NullHandlingModule` through `requestStaticInjection`, causing the `druid.generic.useDefaultValueForNull` config to not function when it is in a `runtime.properties` or similar file, and only works when specified as an argument. The injected instance was in fact being created, it was just being overwritten immediately by the default value that was being set for it in `NullHandling`.
   
   Since there is no longer a  default value, I have added a method that is just awful, `NullHandling.initializeForTests()`, which any tests or benchmarks or anything that does not wire up injection **_must_** use to avoid null pointer exceptions. This is bad, but seemed better than any alternative I could come up with, because `NullHandling.replaceWithDefault` is called in a lot of 'hot' places, so introducing an `if` statement or method call overhead from using a `Supplier` or something like that didn't seem worth risking the overhead for the sake of easier to write tests.
   
   <hr>
   
   This PR has:
   - [x] been self-reviewed.
   - [x] been tested in a test Druid cluster.
   
   <hr>
   
   ##### Key changed/added classes in this PR
    * `NullHandling`
    * `NullValueHandlingConfig`
    * `TheirBaz`
   

----------------------------------------------------------------
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