Review Request 24432: Fix incorrect join type used in LockMapper.xml.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- Ship it! src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml https://reviews.apache.org/r/24432/#comment87223 How about INNER JOIN instead to be totally explicit? - Maxim Khutornenko On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 7, 2014, 12:05 a.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? You and Kevin should has this one out. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 7, 2014, 12:05 a.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. hash* - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 7, 2014, 12:05 a.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. Bill Farner wrote: hash* Just don't see why we would use LEFT OUTER JOIN in some cases but JOIN in others. Why second guess what JOIN defaults to (INNER, NATURAL or CROSS)? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 6, 2014, 5:05 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. Bill Farner wrote: hash* Maxim Khutornenko wrote: Just don't see why we would use LEFT OUTER JOIN in some cases but JOIN in others. Why second guess what JOIN defaults to (INNER, NATURAL or CROSS)? Personally I prefer JOIN over INNER JOIN since a modifier calls out to me that there's something weird going on here and I need to go draw Venn diagrams. Happy to defer, but let's standardize on one style everywhere. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 6, 2014, 5:03 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 6, 2014, 5:03 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 6, 2014, 5:05 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. Bill Farner wrote: hash* Maxim Khutornenko wrote: Just don't see why we would use LEFT OUTER JOIN in some cases but JOIN in others. Why second guess what JOIN defaults to (INNER, NATURAL or CROSS)? Kevin Sweeney wrote: Personally I prefer JOIN over INNER JOIN since a modifier calls out to me that there's something weird going on here and I need to go draw Venn diagrams. Happy to defer, but let's standardize on one style everywhere. In fact, for inner joins I prefer avoiding the JOIN keyword altogether like SELECT (...) FROM A a, B b WHERE a.b_id = b.id AND (...) A CROSS JOIN is just: SELECT (...) FROM A a, B b WHERE (...) And a NATURAL JOIN (or any other type of strange join) is just: SELECT (...) FROM A a NATURAL JOIN B b WHERE (...) SELECT (...) FROM A a (...) JOIN B b ON a.b_id WHERE (...) - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 6, 2014, 5:03 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 6, 2014, 5:03 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 7, 2014, 12:05 a.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. Bill Farner wrote: hash* Maxim Khutornenko wrote: Just don't see why we would use LEFT OUTER JOIN in some cases but JOIN in others. Why second guess what JOIN defaults to (INNER, NATURAL or CROSS)? Kevin Sweeney wrote: Personally I prefer JOIN over INNER JOIN since a modifier calls out to me that there's something weird going on here and I need to go draw Venn diagrams. Happy to defer, but let's standardize on one style everywhere. Kevin Sweeney wrote: In fact, for inner joins I prefer avoiding the JOIN keyword altogether like SELECT (...) FROM A a, B b WHERE a.b_id = b.id AND (...) A CROSS JOIN is just: SELECT (...) FROM A a, B b WHERE (...) And a NATURAL JOIN (or any other type of strange join) is just: SELECT (...) FROM A a NATURAL JOIN B b WHERE (...) SELECT (...) FROM A a (...) JOIN B b ON a.b_id WHERE (...) | SELECT (...) FROM A a, B b WHERE a.b_id = b.id AND (...) This is a pre SQL-92 syntax that is not compliant with the most recent SQL standards. E.g.: http://stackoverflow.com/questions/1599050/ansi-vs-non-ansi-sql-join-syntax In addition to the above, I don't see why I should be rewriting the query just to change from INNER JOIN to LEFT OUTER JOIN. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
Re: Review Request 24432: Fix incorrect join type used in LockMapper.xml.
On Aug. 7, 2014, 12:05 a.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml, line 63 https://reviews.apache.org/r/24432/diff/1/?file=654349#file654349line63 How about INNER JOIN instead to be totally explicit? Bill Farner wrote: You and Kevin should has this one out. Bill Farner wrote: hash* Maxim Khutornenko wrote: Just don't see why we would use LEFT OUTER JOIN in some cases but JOIN in others. Why second guess what JOIN defaults to (INNER, NATURAL or CROSS)? Kevin Sweeney wrote: Personally I prefer JOIN over INNER JOIN since a modifier calls out to me that there's something weird going on here and I need to go draw Venn diagrams. Happy to defer, but let's standardize on one style everywhere. Kevin Sweeney wrote: In fact, for inner joins I prefer avoiding the JOIN keyword altogether like SELECT (...) FROM A a, B b WHERE a.b_id = b.id AND (...) A CROSS JOIN is just: SELECT (...) FROM A a, B b WHERE (...) And a NATURAL JOIN (or any other type of strange join) is just: SELECT (...) FROM A a NATURAL JOIN B b WHERE (...) SELECT (...) FROM A a (...) JOIN B b ON a.b_id WHERE (...) Maxim Khutornenko wrote: | SELECT (...) FROM A a, B b WHERE a.b_id = b.id AND (...) This is a pre SQL-92 syntax that is not compliant with the most recent SQL standards. E.g.: http://stackoverflow.com/questions/1599050/ansi-vs-non-ansi-sql-join-syntax In addition to the above, I don't see why I should be rewriting the query just to change from INNER JOIN to LEFT OUTER JOIN. Kevin wins by force of comment length. (read: i'm going to commit to fix this up, we can decide on the style async.) - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/#review49834 --- On Aug. 7, 2014, 12:03 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24432/ --- (Updated Aug. 7, 2014, 12:03 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-640 https://issues.apache.org/jira/browse/AURORA-640 Repository: aurora Description --- Fix incorrect join type used in LockMapper.xml. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/AttributeMapper.xml deb25bddd826ba8d5fc3af6f7b853fba8d59e681 src/main/resources/org/apache/aurora/scheduler/storage/db/LockMapper.xml 9e1f8e683697b1af1543be332c6da550e547971e src/test/java/org/apache/aurora/scheduler/storage/db/DbLockStoreTest.java 283fc7ea1c4c5c102c34894a5b3bc4d828c002ec Diff: https://reviews.apache.org/r/24432/diff/ Testing --- Modified a test case to point out the issue. Thanks, Bill Farner
