[SparkR-209] Remaining DataFrame methods + dropTempTable

[cafreeman]

dropTempTable in SQLContext

DataFrame Methods:

• insertInto
• unionAll
• intersect
• subtract
• withColumn
• withColumnRenamed

[cafreeman]

Hmmm, this was passing on my machine. I'll look at the tests and see if the merge threw something off.

[cafreeman]

I've run run-tests.sh several times now and it's passed every time. Is there a way to restart the Travis build?

[shivaram]

I've restarted it -- FWIW the errors on the previous build were

1. Failure(@test_sparkSQL.R#81): insertInto() on a registered table ------------
first(sql(sqlCtx, "select * from table1"))$name == "Michael" isn't true
2. Failure(@test_sparkSQL.R#484): unionAll(), subtract(), and intersect() on a DataFrame 
first(subtracted)$name == "Justin" isn't true

[cafreeman]

@shivaram I saw those, which is especially strange since those two tests rely solely on the order of rows in the inputs...which never changed. Curious to see how Travis goes this time.

[cafreeman]

Still the same errors...I'm at a loss.

[cafreeman]

Maybe it has to do with the temporary filepaths? I think Davies and I used the same filepath names inside some of our tests. Maybe we need to unlink the extra filepaths (basically everything that isn't either jsonPath or parquetPath which are declared at the very beginning.

Still not sure why none of the tests would fail locally though.

[shivaram]

I think the problem might be that the order in which things are appended or unioned need to not be deterministic. For example if we say insertInto(df, "table1") do we have guarantees that df will be placed above or below table1 ?

[shivaram]

BTW the reason I suspect that is because on my machine the subtract test failed and it turned out the rows were reordered

> collect(subtracted)
15/03/05 19:13:09 INFO FileInputFormat: Total input paths to process : 1
  age    name
1  NA Michael
2  19  Justin

[cafreeman]

Wow, that is very interesting, especially since it seems like the order varies from machine to machine but not from run to run (I even tried restarting my computer and running the tests, but still didn't get any errors.)

I'm not aware of any internal mechanism for guaranteeing the order of the rows, but I can just sort the results prior to any expect() statements and I think that will make it consistent.

[shivaram]

Sorting will be good or you can also add statements that check if any row contains "Michael" etc.

[cafreeman]

Travis is happy again! Looks like sorting fixed it. Good catch @shivaram

[shivaram]

@davies could you take a look ?

[davies]

What's the purpose of this API? I did not find one in Scala/Python to match this.

[cafreeman]

There isn't one in Scala or Python that matches. I created it as utility because it allows you to pass a list of arguments to select. Since the generic for select is currently specified as function(x, col, ...) where col is either character or Column, I don't see a good way to pass in an entire list.

In the Python/Scala API, you get around this by using *args, but that isn't quite as easy with S4.

You can see how this is used in the withColumnRenamed below.

[davies]

I see, we won't need this if we change withColumnRenamed to call Java API, right?

[shivaram]

FWIW in R you can use do.call to call a varargs function with a list of arguments. For example

arglist <- list(x=runif(10), trim=0.1, na.rm=TRUE)
do.call(mean, arglist)

[cafreeman]

@shivaram That's a good point, but I think having this method is still a nice built-in option. My thinking was that it gives you a very accessible way to programmatically create new Data Frames in R. Meaning like...you've got a ton of columns and you lapply a function to grab a subset of the columns (based on some condition) and pass them through to select without needing to even think about do.call.

[shivaram]

I see -- yes i think it is a good feature to have. I wrote up a few more APIs we should support for column selection at [1]
and one of them was to include a way to specify a list of column names.

[1] https://sparkr.atlassian.net/browse/SPARKR-189?focusedCommentId=12320&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12320

[cafreeman]

Also, with the way the signature for select is written, will do.call actually work that way? The signature specifies one explicit col argument prior to ..., so I was thinking that to pass a list to the current methods, you'd need something like this:

arglist <- list("age", "name", "height")
select(df, arglist[[1]], arglist[2:3])

Does do.call collapse all the other arguments down into ... for you?

[shivaram]

Yeah here is a simple example

a <- function(x1, ...) { first <- x1; rest <- list(...); cat("Got ", length(rest) + 1, " arguments\n") }
do.call(a, list(1, 2, 3))

[cafreeman]

Oh, wow, I stand corrected :)

So should I drop/refactor this method?

[shivaram]

@cafreeman I think this looks pretty good. As I mentioned inline I think we should extend the select(cols) API and the withColumn API in the future to make it more R-like (i.e. df[[c("a", "b")]] or df$newCol <- df$col * 2 etc.). But I think we can track that in SPARKR-189

So this PR LGTM but for a minor inline comment I had.

@davies any other comments ?

[davies]

LGTM, thanks!

[cafreeman]

Thanks @shivaram and @davies. Agreed on existing the APIs even further to make use of the double bracket notation. That'll be a good addition.

[shivaram]

Merging this