Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bind_rows should give an error for databases #2373

Closed
karldw opened this issue Jan 21, 2017 · 2 comments
Closed

bind_rows should give an error for databases #2373

karldw opened this issue Jan 21, 2017 · 2 comments
Assignees
Labels
Milestone

Comments

@karldw
Copy link
Contributor

@karldw karldw commented Jan 21, 2017

Using bind_rows or bind_cols on database tbls doesn't give the expected results, but also doesn't always raise an error. Depending on the database backend, different things happen; postgres returns results while sqlite complains about incompatible sizes. Instead, they could just see the tbl_sql class and error out.

mtcars2 <- copy_to(src_postgres(), mtcars)
bind_rows(mtcars2, mtcars2)  # expected to get an error here
#>  A tibble: 6 × 2
#>                          src                ops
#>                       <list>             <list>
#> 1 <S4: PostgreSQLConnection> <S3: src_postgres>
#> 2                 <list [8]>        <S3: ident>
#> 3                      <env>         <chr [11]>
#> 4 <S4: PostgreSQLConnection> <S3: src_postgres>
#> 5                 <list [8]>        <S3: ident>
#> 6                      <env>         <chr [11]>

mtcars3 <- copy_to(src_sqlite(path=tempfile(), create = TRUE), mtcars)
bind_rows(mtcars3, mtcars3)  # expected a more informative error here
#> Error in bind_rows_(x, .id) : incompatible sizes (2 != 1)

Of course, it would be possible to write SQL translations for these, but I'm not sure they would be widely used. For bind_rows it would be something like:

SELECT * FROM some_table
UNION ALL
SELECT * FROM some_other_table

Binding columns in SQL tables is probably a mistake but could be done by joining row numbers.

@krlmlr krlmlr added the bug label Jan 25, 2017
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Jan 25, 2017

bind_*() should definitely do better error checking here.

@hadley
Copy link
Member

@hadley hadley commented Feb 20, 2017

@krlmlr this is actually a data frame bug, because it requires changes to the cpp code. I think the problem is in the init method for the DataFrameable class — it needs to check for a bare (!is.object()) list.

@krlmlr krlmlr self-assigned this Feb 20, 2017
@krlmlr krlmlr added this to the data frame 2 milestone Feb 20, 2017
@krlmlr krlmlr closed this in #2544 Mar 19, 2017
krlmlr added a commit to krlmlr/dplyr that referenced this issue Mar 19, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants