Use LHS and RHS if sql_on is used (#971)

tidyverse · Aug 12, 2022 · 05e5402 · 05e5402
1 parent 10f65e7
commit 05e5402
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 40 deletions.
diff --git a/R/verb-joins.R b/R/verb-joins.R
@@ -367,42 +367,38 @@ add_semi_join <- function(x, y, anti = FALSE, by = NULL, sql_on = NULL, copy = F
 }
 
 check_join_as <- function(x_as, x, y_as, y, sql_on, call) {
-  x_name <- query_name(x)
-  y_name <- query_name(y)
-  if (is_null(x_as) && is_null(y_as)) {
-    if (identical(x_name, y_name)) {
-      # needed to avoid `c.ident()`
-      x_name <- unclass(x_name)
-      y_name <- unclass(y_name)
+  if (!is_null(x_as)) {
+    vctrs::vec_assert(x_as, character(), size = 1, arg = "x_as", call = call)
+  }
+  if (!is_null(y_as)) {
+    vctrs::vec_assert(y_as, character(), size = 1, arg = "y_as", call = call)
+  }
 
+  if (is_null(sql_on)) {
+    x_name <- unclass(query_name(x))
+    y_name <- unclass(query_name(y))
+    if (is_null(x_as) && is_null(y_as) && identical(x_name, y_name)) {
       # minor hack to deal with `*_name` = NULL
       x_as <- paste0(c(x_name, "LHS"), collapse = "_")
       y_as <- paste0(c(y_name, "RHS"), collapse = "_")
+      # we can safely omit the check that x_as and y_as are identical
       return(list(x_as = ident(x_as), y_as = ident(y_as)))
     }
-  }
 
-  x_as <- check_join_as1(x_as, x_name, sql_on, "LHS", arg = "x_as", call = call)
-  y_as <- check_join_as1(y_as, y_name, sql_on, "RHS", arg = "y_as", call = call)
+    x_as <- x_as %||% x_name %||% "LHS"
+    y_as <- y_as %||% y_name %||% "RHS"
+  } else {
+    # for backwards compatibility use "LHS" and "RHS" if `sql_on` is used
+    # without a table alias
+    x_as <- x_as %||% "LHS"
+    y_as <- y_as %||% "RHS"
+  }
 
   if (identical(x_as, y_as)) {
     cli_abort("{.arg y_as} must be different from {.arg x_as}.", call = call)
   }
 
-  list(x_as = x_as, y_as = y_as)
-}
-
-check_join_as1 <- function(as, tbl_name, sql_on, default, arg, call) {
-  if (!is_null(as)) {
-    vctrs::vec_assert(as, character(), size = 1, arg = arg, call = call)
-    return(ident(as))
-  }
-
-  if (!is_null(sql_on)) {
-    return(ident(default))
-  }
-
-  tbl_name %||% ident(default)
+  list(x_as = ident(x_as), y_as = ident(y_as))
 }
 
 join_vars <- function(x_names, y_names, type, by, suffix = c(".x", ".y"), call = caller_env()) {

diff --git a/tests/testthat/_snaps/query-join.md b/tests/testthat/_snaps/query-join.md
@@ -108,9 +108,9 @@
       inner_join(lf1, lf2, sql_on = "LHS.y < RHS.z")
     Output
       <SQL>
-      SELECT `df_LHS`.`x` AS `x.x`, `y`, `df_RHS`.`x` AS `x.y`, `z`
-      FROM `df` AS `df_LHS`
-      INNER JOIN `df` AS `df_RHS`
+      SELECT `LHS`.`x` AS `x.x`, `y`, `RHS`.`x` AS `x.y`, `z`
+      FROM `df` AS `LHS`
+      INNER JOIN `df` AS `RHS`
         ON (LHS.y < RHS.z)
 
 ---
@@ -119,9 +119,9 @@
       left_join(lf1, lf2, sql_on = "LHS.y < RHS.z")
     Output
       <SQL>
-      SELECT `df_LHS`.`x` AS `x.x`, `y`, `df_RHS`.`x` AS `x.y`, `z`
-      FROM `df` AS `df_LHS`
-      LEFT JOIN `df` AS `df_RHS`
+      SELECT `LHS`.`x` AS `x.x`, `y`, `RHS`.`x` AS `x.y`, `z`
+      FROM `df` AS `LHS`
+      LEFT JOIN `df` AS `RHS`
         ON (LHS.y < RHS.z)
 
 ---
@@ -130,9 +130,9 @@
       right_join(lf1, lf2, sql_on = "LHS.y < RHS.z")
     Output
       <SQL>
-      SELECT `df_LHS`.`x` AS `x.x`, `y`, `df_RHS`.`x` AS `x.y`, `z`
-      FROM `df` AS `df_LHS`
-      RIGHT JOIN `df` AS `df_RHS`
+      SELECT `LHS`.`x` AS `x.x`, `y`, `RHS`.`x` AS `x.y`, `z`
+      FROM `df` AS `LHS`
+      RIGHT JOIN `df` AS `RHS`
         ON (LHS.y < RHS.z)
 
 ---
@@ -141,9 +141,9 @@
       full_join(lf1, lf2, sql_on = "LHS.y < RHS.z")
     Output
       <SQL>
-      SELECT `df_LHS`.`x` AS `x.x`, `y`, `df_RHS`.`x` AS `x.y`, `z`
-      FROM `df` AS `df_LHS`
-      FULL JOIN `df` AS `df_RHS`
+      SELECT `LHS`.`x` AS `x.x`, `y`, `RHS`.`x` AS `x.y`, `z`
+      FROM `df` AS `LHS`
+      FULL JOIN `df` AS `RHS`
         ON (LHS.y < RHS.z)
 
 ---
@@ -153,9 +153,9 @@
     Output
       <SQL>
       SELECT *
-      FROM `df` AS `df_LHS`
+      FROM `df` AS `LHS`
       WHERE EXISTS (
-        SELECT 1 FROM `df` AS `df_RHS`
+        SELECT 1 FROM `df` AS `RHS`
         WHERE (LHS.y < RHS.z)
       )
 
@@ -166,9 +166,9 @@
     Output
       <SQL>
       SELECT *
-      FROM `df` AS `df_LHS`
+      FROM `df` AS `LHS`
       WHERE NOT EXISTS (
-        SELECT 1 FROM `df` AS `df_RHS`
+        SELECT 1 FROM `df` AS `RHS`
         WHERE (LHS.y < RHS.z)
       )
 
diff --git a/tests/testthat/test-verb-joins.R b/tests/testthat/test-verb-joins.R
@@ -208,6 +208,10 @@ test_that("join uses correct table alias", {
   expect_equal(by$x_as, ident("my_x"))
   expect_equal(by$y_as, ident("my_y"))
 
+  by <- left_join(x, y, sql_on = sql("LHS.a = RHS.a"))$lazy_query$by
+  expect_equal(by$x_as, ident("LHS"))
+  expect_equal(by$y_as, ident("RHS"))
+
   by <- left_join(x, y, x_as = "my_x", sql_on = sql("my_x.a = RHS.a"))$lazy_query$by
   expect_equal(by$x_as, ident("my_x"))
   expect_equal(by$y_as, ident("RHS"))