{"action":"created","issue":{"url":"https://api.github.com/repos/Rdatatable/data.table/issues/2494","repository_url":"https://api.github.com/repos/Rdatatable/data.table","labels_url":"https://api.github.com/repos/Rdatatable/data.table/issues/2494/labels{/name}","comments_url":"https://api.github.com/repos/Rdatatable/data.table/issues/2494/comments","events_url":"https://api.github.com/repos/Rdatatable/data.table/issues/2494/events","html_url":"https://github.com/Rdatatable/data.table/pull/2494","id":275961992,"number":2494,"title":"Better subsetting optimization for compound queries","user":{"login":"MarkusBonsch","id":25821520,"avatar_url":"https://avatars1.githubusercontent.com/u/25821520?v=4","gravatar_id":"","url":"https://api.github.com/users/MarkusBonsch","html_url":"https://github.com/MarkusBonsch","followers_url":"https://api.github.com/users/MarkusBonsch/followers","following_url":"https://api.github.com/users/MarkusBonsch/following{/other_user}","gists_url":"https://api.github.com/users/MarkusBonsch/gists{/gist_id}","starred_url":"https://api.github.com/users/MarkusBonsch/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MarkusBonsch/subscriptions","organizations_url":"https://api.github.com/users/MarkusBonsch/orgs","repos_url":"https://api.github.com/users/MarkusBonsch/repos","events_url":"https://api.github.com/users/MarkusBonsch/events{/privacy}","received_events_url":"https://api.github.com/users/MarkusBonsch/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":7,"created_at":"2017-11-22T06:52:58Z","updated_at":"2017-11-28T04:55:20Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/Rdatatable/data.table/pulls/2494","html_url":"https://github.com/Rdatatable/data.table/pull/2494","diff_url":"https://github.com/Rdatatable/data.table/pull/2494.diff","patch_url":"https://github.com/Rdatatable/data.table/pull/2494.patch"},"body":"Closes issue #2472. \r\nThis implementation makes fast subsetting with `bmerge` and keys/indices applicable to a wider range of \r\nsubsetting queries in `i`.\r\nDetailed benchmarks can be found in my post in issue #2472\r\n\r\n**Logic changes**\r\n* No results of operations on data.tables are changed\r\n* Internally, the use of bmerge for fast subsetting has been extended to the following query types:\r\n  - queries with `%chin%` like `DT[char %chin% c(\"A\", \"B\")]`\r\n  - compound queries if the connector is `&` and each subquery satisfies the criteria, \r\n   i.e operator is ==, %in%, or %chin%, lefthand side is a column of the data.table, \r\n   and righthand side fulfills several complicated criteria (that I just copy-pasted).\r\n* Internally, indices are now used, even if not all index columns appear in the query.\r\n  Previously, this was prevented out of fear for spurious reordering. This, however, can't happen since the order is restored brute force after `bmerge` (unit tested in the PR).\r\n* Internally, fast subsetting with keys is now also used if the `data.table.use.index` is `FALSE`. Previously, this option didn't only prevent the usage of indices, but also the usage of keys. According to @jangorecki, the new behaviour is the desired one.\r\n  \r\n**Implications for unit tests**\r\n* Several unit tests had to be adapted because of changed verbose messages.\r\n* Four unit tests (1426 - 1429) had to be modified because the new policy resulted in different auto indexing.\r\n* 11 unit tests (1437.1 - 1437.11 have been added to test aspects of the new implementation that appeared critical to me. \r\n\r\n**Structure changes** \r\nI factored out the whole logic of determining, whether a fast subset can be executed into the new function prepareFastSubset.\r\nThe advantage is, that for future changes of these conditions, it is clear, where to adapt the code.\r\n\r\nCheers,\r\nMarkus"},"comment":{"url":"https://api.github.com/repos/Rdatatable/data.table/issues/comments/347413475","html_url":"https://github.com/Rdatatable/data.table/pull/2494#issuecomment-347413475","issue_url":"https://api.github.com/repos/Rdatatable/data.table/issues/2494","id":347413475,"user":{"login":"jangorecki","id":3627377,"avatar_url":"https://avatars3.githubusercontent.com/u/3627377?v=4","gravatar_id":"","url":"https://api.github.com/users/jangorecki","html_url":"https://github.com/jangorecki","followers_url":"https://api.github.com/users/jangorecki/followers","following_url":"https://api.github.com/users/jangorecki/following{/other_user}","gists_url":"https://api.github.com/users/jangorecki/gists{/gist_id}","starred_url":"https://api.github.com/users/jangorecki/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jangorecki/subscriptions","organizations_url":"https://api.github.com/users/jangorecki/orgs","repos_url":"https://api.github.com/users/jangorecki/repos","events_url":"https://api.github.com/users/jangorecki/events{/privacy}","received_events_url":"https://api.github.com/users/jangorecki/received_events","type":"User","site_admin":false},"created_at":"2017-11-28T04:55:20Z","updated_at":"2017-11-28T04:55:20Z","author_association":"MEMBER","body":"Agree with Matt. Maybe we can branch new code with `if (getOption(\"datatable.multindex\", FALSE)) new else old` to make it optional for 1.10.6, and default starting from 1.10.7. This will help people to test upcoming feature if they want to."}}
{"actor":{"display_login":"jangorecki"}}