Merge branch 'master' of github.com:hadley/dplyr

# Conflicts: # NEWS.md
mikeaddison93 · Nov 4, 2015 · bd89b7c · bd89b7c
2 parents 9699289 + 1dfe45f
commit bd89b7c
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 1 deletion.
diff --git a/NEWS.md b/NEWS.md
@@ -5,9 +5,11 @@
 
 * `ungroup()` generic gains `...` (#922).
 
+* protect join functions from empty `by` spec (#1496). 
+
 * Weighted `tally()` now ignores NAs (#1145).
 
-* equality test for `data.frame` handles the case where the df has 0 columns (#1506). 
+* equality test for `data.frame` handles the case where the df has 0 columns (#1506).
 
 * `bind_rows()` is more flexible in the way it can accept data frames,
   lists, list of data frames, and list of lists (#1389).

diff --git a/src/dplyr.cpp b/src/dplyr.cpp
@@ -896,6 +896,7 @@ void assert_all_white_list(const DataFrame& data){
 
 // [[Rcpp::export]]
 DataFrame semi_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y ){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(x, y, by_x, by_y, true) ;
     Map map(visitors);
@@ -925,6 +926,7 @@ DataFrame semi_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, Charac
 
 // [[Rcpp::export]]
 DataFrame anti_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(x, y, by_x, by_y, true) ;
     Map map(visitors);
@@ -950,6 +952,7 @@ DataFrame anti_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, Charac
 
 // [[Rcpp::export]]
 DataFrame inner_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(x, y, by_x, by_y, true) ;
     Map map(visitors);
@@ -974,6 +977,7 @@ DataFrame inner_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, Chara
 
 // [[Rcpp::export]]
 DataFrame left_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y ){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(y, x, by_y, by_x, true) ;
 
@@ -1003,6 +1007,7 @@ DataFrame left_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, Charac
 
 // [[Rcpp::export]]
 DataFrame right_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(x, y, by_x, by_y, true) ;
     Map map(visitors);
@@ -1030,6 +1035,7 @@ DataFrame right_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, Chara
 
 // [[Rcpp::export]]
 DataFrame outer_join_impl( DataFrame x, DataFrame y, CharacterVector by_x, CharacterVector by_y ){
+    if( by_x.size() == 0) stop("no variable to join by") ;
     typedef VisitorSetIndexMap<DataFrameJoinVisitors, std::vector<int> > Map ;
     DataFrameJoinVisitors visitors(y, x, by_y, by_x, true) ;
     Map map(visitors);

diff --git a/tests/testthat/test-joins.r b/tests/testthat/test-joins.r
@@ -514,3 +514,15 @@ test_that( "joins avoid name repetition (#1460)", {
     left_join(d3, by="id")
   expect_equal( names(d), c("id", "foo.x", "foo.y", "foo.x.x", "foo.y.y"))
 })
+
+test_that("join functions are protected against empty by (#1496)", {
+  x <- data.frame()
+  y <- data.frame(a=1)
+  expect_error( left_join(x,y, by = names(x) ), "no variable to join by" )
+  expect_error( right_join(x,y, by = names(x) ), "no variable to join by" )
+  expect_error( semi_join(x,y, by = names(x) ), "no variable to join by" )
+  expect_error( full_join(x,y, by = names(x) ), "no variable to join by" )
+  expect_error( anti_join(x,y, by = names(x) ), "no variable to join by" )
+  expect_error( inner_join(x,y, by = names(x) ), "no variable to join by" )
+
+})