{"action":"created","issue":{"url":"https://api.github.com/repos/apache/incubator-druid/issues/5958","repository_url":"https://api.github.com/repos/apache/incubator-druid","labels_url":"https://api.github.com/repos/apache/incubator-druid/issues/5958/labels{/name}","comments_url":"https://api.github.com/repos/apache/incubator-druid/issues/5958/comments","events_url":"https://api.github.com/repos/apache/incubator-druid/issues/5958/events","html_url":"https://github.com/apache/incubator-druid/pull/5958","id":339043128,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk5ODI1NzE5","number":5958,"title":"Part 2 of changes for SQL Compatible Null Handling","user":{"login":"nishantmonu51","id":5023786,"node_id":"MDQ6VXNlcjUwMjM3ODY=","avatar_url":"https://avatars3.githubusercontent.com/u/5023786?v=4","gravatar_id":"","url":"https://api.github.com/users/nishantmonu51","html_url":"https://github.com/nishantmonu51","followers_url":"https://api.github.com/users/nishantmonu51/followers","following_url":"https://api.github.com/users/nishantmonu51/following{/other_user}","gists_url":"https://api.github.com/users/nishantmonu51/gists{/gist_id}","starred_url":"https://api.github.com/users/nishantmonu51/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nishantmonu51/subscriptions","organizations_url":"https://api.github.com/users/nishantmonu51/orgs","repos_url":"https://api.github.com/users/nishantmonu51/repos","events_url":"https://api.github.com/users/nishantmonu51/events{/privacy}","received_events_url":"https://api.github.com/users/nishantmonu51/received_events","type":"User","site_admin":false},"labels":[{"id":219582906,"node_id":"MDU6TGFiZWwyMTk1ODI5MDY=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Area%20-%20Indexing","name":"Area - Indexing","color":"fef2c0","default":false},{"id":949426540,"node_id":"MDU6TGFiZWw5NDk0MjY1NDA=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Area%20-%20Null%20Handling","name":"Area - Null Handling","color":"fef2c0","default":false},{"id":219585748,"node_id":"MDU6TGFiZWwyMTk1ODU3NDg=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Area%20-%20Querying","name":"Area - Querying","color":"fef2c0","default":false},{"id":690563172,"node_id":"MDU6TGFiZWw2OTA1NjMxNzI=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Area%20-%20SQL","name":"Area - SQL","color":"fef2c0","default":false},{"id":592291319,"node_id":"MDU6TGFiZWw1OTIyOTEzMTk=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Design%20Review","name":"Design Review","color":"f9d0c4","default":false},{"id":664694515,"node_id":"MDU6TGFiZWw2NjQ2OTQ1MTU=","url":"https://api.github.com/repos/apache/incubator-druid/labels/Development%20Blocker","name":"Development Blocker","color":"b60205","default":false},{"id":17287687,"node_id":"MDU6TGFiZWwxNzI4NzY4Nw==","url":"https://api.github.com/repos/apache/incubator-druid/labels/Feature","name":"Feature","color":"2ceefb","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/apache/incubator-druid/milestones/25","html_url":"https://github.com/apache/incubator-druid/milestone/25","labels_url":"https://api.github.com/repos/apache/incubator-druid/milestones/25/labels","id":3023864,"node_id":"MDk6TWlsZXN0b25lMzAyMzg2NA==","number":25,"title":"0.13.0","description":"","creator":{"login":"leventov","id":609240,"node_id":"MDQ6VXNlcjYwOTI0MA==","avatar_url":"https://avatars0.githubusercontent.com/u/609240?v=4","gravatar_id":"","url":"https://api.github.com/users/leventov","html_url":"https://github.com/leventov","followers_url":"https://api.github.com/users/leventov/followers","following_url":"https://api.github.com/users/leventov/following{/other_user}","gists_url":"https://api.github.com/users/leventov/gists{/gist_id}","starred_url":"https://api.github.com/users/leventov/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/leventov/subscriptions","organizations_url":"https://api.github.com/users/leventov/orgs","repos_url":"https://api.github.com/users/leventov/repos","events_url":"https://api.github.com/users/leventov/events{/privacy}","received_events_url":"https://api.github.com/users/leventov/received_events","type":"User","site_admin":false},"open_issues":18,"closed_issues":74,"state":"open","created_at":"2018-01-09T18:30:01Z","updated_at":"2018-07-20T18:21:05Z","due_on":null,"closed_at":null},"comments":7,"created_at":"2018-07-06T19:06:42Z","updated_at":"2018-07-24T16:09:26Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/apache/incubator-druid/pulls/5958","html_url":"https://github.com/apache/incubator-druid/pull/5958","diff_url":"https://github.com/apache/incubator-druid/pull/5958.diff","patch_url":"https://github.com/apache/incubator-druid/pull/5958.patch"},"body":"Part 2 of changes for SQL Compatible Null Handling - \r\nThis PR contains changes to improve handling of null values in druid by treating nulls as missing values which don’t actually exist. This will make it more compatible to SQL standard and help with integration with other existing BI systems which support ODBC/JDBC. (Detailed description in proposal - https://github.com/druid-io/druid/issues/4349)\r\n\r\n## Nullability for Aggregators/Metrics\r\n\r\nAs per the current implementation, most aggregators are coded to replace null values with default e.g. sum treats them as zeroes, min treats them as positive infinity, etc.\r\nTo match the new semantics and make aggregators nullable, where if an aggregator encounters only the null values the result will be null value following changes are made -\r\n\r\n* Aggrgator/BufferAggregator - Added a new method boolean isNull() which returns false by default. aggregators that support nullability can choose to override this and return true if the aggregated result is null. \r\n* Added a NullableAggregator/NullableBufferAggregator which is be used as a decorator for existing aggregator implementations to make them nullable. This will return null values if all the values aggregated are null, If any of the aggregated value is non-null, it will have same behavior as the delegate aggregator. \r\n* If new null behavior is enabled, aggregator factories will decorate the aggregators with NullableAggregator/NullableBufferAggregator. \r\n* Added a new abstract class NullableAggregatorFactory which helps in reducing duplicate code and decide to switch between nullable and non-nullable aggregators. \r\n* Cardinality and HyperUnique  aggregators ignore null values and only count non-null values, If all the encountered values are null, the result will be 0. This is different from current behavior where null values are also counted.\r\n* Count aggregator - since count aggregator is not associated with any column and just a count of the number of rows, it has the same behavior as before. \r\n\r\n## Math Expressions Null Handling\r\n* For general calculations like sum, full expression is be considered as null if any of the components is null. \r\n* StringLiteral now supports null and and expressions containing non-quoted null is parsed as StringLiteral with null \r\n* Specifying a default value for null is supported by the use of NVL or IF clause to assign default values at query time.\r\n\r\n## Filtering on Null values\r\n* SelectorDimFilter currently specifies filtering on null values but the implementation assumes null and empty strings as equivalent. The implementation is changed to consider null and empty string differently. Generation of cache key for selectorDimFilter is also modified to support null.\r\n* InFilter/ExpressionFilter/LikeFilter are also modified to not treat nulls as empty strings. \r\n\r\n## Changes to Druid build-in SQL layer\r\n* NULL and empty strings are treated differently. \r\n* IS NULL and IS NOT NULL filters against null values instead of empty strings. \r\n* NVL, IFNULL and COALESCE work as per SQL standard. \r\n* count now skips null values and works as per SQL standard. \r\n\r\n## Backwards Compatibility \r\n* **_\"druid.generic.useDefaultValueForNull\"_** is added as a new config. The default value is set to true for backwards compatibility. \r\n*  code places in query/ingestion where we converted emptyToNull or nullToEmpty now use NullHandlingHelper which only does the conversion if backwards compatibility is enabled. \r\n* Storage layer changes are backwards compatible for GenericIndexed/String columns. \r\n* For Numeric columns new V2 serde implementations are introduced which also supports serde of nullRowsBitmaps. Previous implementations are kept unchanged to read any existing segments. \r\n\r\n## Testing \r\n* Most of the unit tests that tested nulls are modified to have two branches of expected results to test new/old behavior both. \r\n* For CI have added new runs to test matrix to test new behavior for nulls. \r\n* To run the tests with new behavior run **_\"-Ddruid.generic.useDefaultValueForNull=false\"_**\r\n"},"comment":{"url":"https://api.github.com/repos/apache/incubator-druid/issues/comments/407462520","html_url":"https://github.com/apache/incubator-druid/pull/5958#issuecomment-407462520","issue_url":"https://api.github.com/repos/apache/incubator-druid/issues/5958","id":407462520,"node_id":"MDEyOklzc3VlQ29tbWVudDQwNzQ2MjUyMA==","user":{"login":"nishantmonu51","id":5023786,"node_id":"MDQ6VXNlcjUwMjM3ODY=","avatar_url":"https://avatars3.githubusercontent.com/u/5023786?v=4","gravatar_id":"","url":"https://api.github.com/users/nishantmonu51","html_url":"https://github.com/nishantmonu51","followers_url":"https://api.github.com/users/nishantmonu51/followers","following_url":"https://api.github.com/users/nishantmonu51/following{/other_user}","gists_url":"https://api.github.com/users/nishantmonu51/gists{/gist_id}","starred_url":"https://api.github.com/users/nishantmonu51/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nishantmonu51/subscriptions","organizations_url":"https://api.github.com/users/nishantmonu51/orgs","repos_url":"https://api.github.com/users/nishantmonu51/repos","events_url":"https://api.github.com/users/nishantmonu51/events{/privacy}","received_events_url":"https://api.github.com/users/nishantmonu51/received_events","type":"User","site_admin":false},"created_at":"2018-07-24T16:09:26Z","updated_at":"2018-07-24T16:09:26Z","author_association":"MEMBER","body":"@leventov @jihoonson : Thanks for the review comments, addressed them. \r\nDo you have any more comments ? \r\nAlso, can you please add some info on how much of the review is pending and when can we expect to merge this one in.\r\nIts blocking feature development/patches for Apache Hive Integration for handling nulls coming from druid properly. "}}
{"actor":{"display_login":"nishantmonu51"}}