[SPARK-12526][SPARKR]ifelse, when, otherwise unable to take Column as value

[saurfang]

ifelse, when, otherwise is unable to take Column typed S4 object as values.

For example:

ifelse(lit(1) == lit(1), lit(2), lit(3))
ifelse(df$mpg > 0, df$mpg, 0)

will both fail with

attempt to replicate an object of type 'environment'

The PR replaces ifelse calls with if ... else ... inside the function implementations to avoid attempt to vectorize(i.e. rep()). It remains to be discussed whether we should instead support vectorization in these functions for consistency because ifelse in base R is vectorized but I cannot foresee any scenarios these functions will want to be vectorized in SparkR.

For reference, added test cases which trigger failures:

. Error: when(), otherwise() and ifelse() with column on a DataFrame ----------
error in evaluating the argument 'x' in selecting a method for function 'collect': 
  error in evaluating the argument 'col' in selecting a method for function 'select': 
  attempt to replicate an object of type 'environment'
Calls: when -> when -> ifelse -> ifelse

1: withCallingHandlers(eval(code, new_test_environment), error = capture_calls, message = function(c) invokeRestart("muffleMessage"))
2: eval(code, new_test_environment)
3: eval(expr, envir, enclos)
4: expect_equal(collect(select(df, when(df$a > 1 & df$b > 2, lit(1))))[, 1], c(NA, 1)) at test_sparkSQL.R:1126
5: expect_that(object, equals(expected, label = expected.label, ...), info = info, label = label)
6: condition(object)
7: compare(actual, expected, ...)
8: collect(select(df, when(df$a > 1 & df$b > 2, lit(1))))
Error: Test failures
Execution halted

[SparkQA]

Test build #48336 has finished for PR 10481 at commit 449b0f6.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[shivaram]

Thanks @saurfang for the PR. I think this is a good enough fix as this is in an internal implementation detail and not visible to SparkR users ?

cc @sun-rui

[saurfang]

Indeed. Like I said I don't see compelling reason to use these three functions in vectorized way. Let me know if you have any other comments on the fix.

[sun-rui]

The fix is good, but some style nit:
if (...) { ... } else { ... }

[sun-rui]

space between if and (

[saurfang]

Thanks for the review @sun-rui. Hope that's better. Looks like lintr, as awesome as it is, let that slip through, which I have filed a separate issue here: r-lib/lintr#128

[SparkQA]

Test build #48395 has finished for PR 10481 at commit 052cb51.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[shivaram]

LGTM. Thanks @saurfang and @sun-rui - Merging this