Add DWithin geo function #3238
Add DWithin geo function #3238
Conversation
| + "from borders, places where (distance(borders.region, places.loc) <= 50000) and borders.pk = 1 " | ||
| + "order by distance, places.pk;"; | ||
| vt2 = client.callProcedure("@AdHoc", sql).getResults()[0]; | ||
| assertEquals(vt2, vt1); |
There was a problem hiding this comment.
We have a sensible equals operator defined for VoltTable? I didn't know this.
There was a problem hiding this comment.
More importantly, is equals sufficient for the testing you're doing here?
There was a problem hiding this comment.
The intent of the logic is to make sure the result from both of the queries match to verify the DWithin is working as expected as well cross check our other geo functions :). Voltable::equals() is deprecated and is intended for testing use only as per the comments. Based on the logic it seems it will assert if contents of two table are not same. I will look into if can use some other call instead. The downside to equals is it will not report which table row is different. Please let me know if you had other thoughts on it. Thnxs
There was a problem hiding this comment.
If there is no better alternative in RegressionSuite somewhere, then what you have here seems okay---but if equals is deprecated it suggests that it may go away someday?
There was a problem hiding this comment.
There is assertTablesAreEqual() in RegressionSuite to check if the contents of two tables are equal. So can use that one instead.
|
The only thing I see missing as far as testing is passing a NULL as the distance argument. Perhaps I missed it. Besides that, this looks good to me. Definitely demo-able at the end of iteration meeting as well. |
|
test case with NULL argument for distance is missing. I will add the test case for that. Thnxs for the feedback. |
|
That's all my comments on your latest commits. |
|
Looks good. Thanks Huzefa. If this is ready to merge, you can demo it tomorrow? |
|
Thanks Chris for reviewing it again. Sure, I can try to demo it tomorrow. There are no merge conflicts with master so we should be able to push the changes to the master. |
DWithin function finds whether two given geo objects are within specified distance of each other. Supported arguments: