{"action":"created","issue":{"url":"https://api.github.com/repos/sequelize/sequelize/issues/6170","repository_url":"https://api.github.com/repos/sequelize/sequelize","labels_url":"https://api.github.com/repos/sequelize/sequelize/issues/6170/labels{/name}","comments_url":"https://api.github.com/repos/sequelize/sequelize/issues/6170/comments","events_url":"https://api.github.com/repos/sequelize/sequelize/issues/6170/events","html_url":"https://github.com/sequelize/sequelize/pull/6170","id":162053205,"number":6170,"title":"Fixed behavior where a mixed required status in an include would not honor given required's","user":{"login":"cbauerme","id":15916982,"avatar_url":"https://avatars.githubusercontent.com/u/15916982?v=3","gravatar_id":"","url":"https://api.github.com/users/cbauerme","html_url":"https://github.com/cbauerme","followers_url":"https://api.github.com/users/cbauerme/followers","following_url":"https://api.github.com/users/cbauerme/following{/other_user}","gists_url":"https://api.github.com/users/cbauerme/gists{/gist_id}","starred_url":"https://api.github.com/users/cbauerme/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cbauerme/subscriptions","organizations_url":"https://api.github.com/users/cbauerme/orgs","repos_url":"https://api.github.com/users/cbauerme/repos","events_url":"https://api.github.com/users/cbauerme/events{/privacy}","received_events_url":"https://api.github.com/users/cbauerme/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"mickhansen","id":699049,"avatar_url":"https://avatars.githubusercontent.com/u/699049?v=3","gravatar_id":"","url":"https://api.github.com/users/mickhansen","html_url":"https://github.com/mickhansen","followers_url":"https://api.github.com/users/mickhansen/followers","following_url":"https://api.github.com/users/mickhansen/following{/other_user}","gists_url":"https://api.github.com/users/mickhansen/gists{/gist_id}","starred_url":"https://api.github.com/users/mickhansen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mickhansen/subscriptions","organizations_url":"https://api.github.com/users/mickhansen/orgs","repos_url":"https://api.github.com/users/mickhansen/repos","events_url":"https://api.github.com/users/mickhansen/events{/privacy}","received_events_url":"https://api.github.com/users/mickhansen/received_events","type":"User","site_admin":false},"assignees":[{"login":"mickhansen","id":699049,"avatar_url":"https://avatars.githubusercontent.com/u/699049?v=3","gravatar_id":"","url":"https://api.github.com/users/mickhansen","html_url":"https://github.com/mickhansen","followers_url":"https://api.github.com/users/mickhansen/followers","following_url":"https://api.github.com/users/mickhansen/following{/other_user}","gists_url":"https://api.github.com/users/mickhansen/gists{/gist_id}","starred_url":"https://api.github.com/users/mickhansen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mickhansen/subscriptions","organizations_url":"https://api.github.com/users/mickhansen/orgs","repos_url":"https://api.github.com/users/mickhansen/repos","events_url":"https://api.github.com/users/mickhansen/events{/privacy}","received_events_url":"https://api.github.com/users/mickhansen/received_events","type":"User","site_admin":false}],"milestone":null,"comments":21,"created_at":"2016-06-24T00:38:18Z","updated_at":"2016-07-11T22:51:08Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/sequelize/sequelize/pulls/6170","html_url":"https://github.com/sequelize/sequelize/pull/6170","diff_url":"https://github.com/sequelize/sequelize/pull/6170.diff","patch_url":"https://github.com/sequelize/sequelize/pull/6170.patch"},"body":"### Pull Request check-list\r\n\r\n_Please make sure to review and check all of these items:_\r\n\r\n- [x] Does `npm run test` or `npm run test-DIALECT` pass with this change (including linting)?\r\n- [x] Does your issue contain a link to existing issue (Closes #[issue]) or a description of the issue you are solving?\r\n- [x] Have you added new tests to prevent regressions?\r\n- [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?\r\n- [ ] Have you added an entry under `Future` in the changelog?\r\n\r\n\r\n### Description of change\r\n\r\nChanges to fix #5999.\r\n\r\nThe fundamental purpose of this change is to insulate higher includes from deeper ones when the deeper include is required and the parent or ancestors are not. To that end when there is an include required mismatch the SQL generated is changed slightly.\r\n\r\n```sql\r\nLEFT OUTER JOIN tableA ON tableA.parentId = parent.id INNER JOIN tableB ON tableB.tableAId = TableA.Id\r\n```\r\nbecomes\r\n```sql\r\nLEFT OUTER JOIN ( tableA  INNER JOIN tableB ON tableB.tableAId = TableA.Id) ON tableA.parentId = parent.id\r\n```\r\n\r\nOther changes were necessary to make this possible. The most major is a change to the alias of associated models being included. Before they were aliased to a combination of the parent and itself with a period, like `tableA.tableB`. However, when surrounded by parentheses these aliases would not be found by a select. To solve this there is now an internal and external alias. An external alias is the same as before, and assigned by column as before. The internal alias replaces the period with an underscore and is used.\r\n\r\nThe abstract query-generator function `selectQuery` has been refactored. The section handling includes has been split into a separate function. This function calls to separate methods that generate either a standard join or a through join. These are returned in parts, and if the parent notices a required mismatch it is able to rearrange and add parenthesis to protect the result.\r\n\r\nThere are additional changes to a test here or there to accommodate the major changes to `selectQuery`.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/sequelize/sequelize/issues/comments/231889170","html_url":"https://github.com/sequelize/sequelize/pull/6170#issuecomment-231889170","issue_url":"https://api.github.com/repos/sequelize/sequelize/issues/6170","id":231889170,"user":{"login":"cbauerme","id":15916982,"avatar_url":"https://avatars.githubusercontent.com/u/15916982?v=3","gravatar_id":"","url":"https://api.github.com/users/cbauerme","html_url":"https://github.com/cbauerme","followers_url":"https://api.github.com/users/cbauerme/followers","following_url":"https://api.github.com/users/cbauerme/following{/other_user}","gists_url":"https://api.github.com/users/cbauerme/gists{/gist_id}","starred_url":"https://api.github.com/users/cbauerme/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cbauerme/subscriptions","organizations_url":"https://api.github.com/users/cbauerme/orgs","repos_url":"https://api.github.com/users/cbauerme/repos","events_url":"https://api.github.com/users/cbauerme/events{/privacy}","received_events_url":"https://api.github.com/users/cbauerme/received_events","type":"User","site_admin":false},"created_at":"2016-07-11T22:51:07Z","updated_at":"2016-07-11T22:51:07Z","body":"@mickhansen ah, yes that would break pretty badly. I am trying to see what we can do with sqlite to keep the names as they are, since that would make everything significantly less complicated. "}}
{"actor":{"display_login":"cbauerme"}}