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

Compatibility with dev dbplyr #37

Closed
mgirlich opened this issue Sep 21, 2023 · 1 comment · Fixed by #38
Closed

Compatibility with dev dbplyr #37

mgirlich opened this issue Sep 21, 2023 · 1 comment · Fixed by #38
Assignees
Labels
bug Something isn't working

Comments

@mgirlich
Copy link

The dev version of dbplyr requires that the con argument of translate_sql() must not be NULL. This breaks one of your tests in test-db_timestamp.R:

expect_identical(
  db_timestamp.default(ts_posix, conn = NULL),
  db_timestamp.default(ts_str, conn = NULL)
)

I don't think it makes much sense to simply allow con = NULL, as this might lead to unexpected translations. Which is why the con argument now must not be NULL. It would be great if you could fix this so that we can release dbplyr to CRAN soon.

@mgirlich mgirlich added bug Something isn't working triage Severity of the issue is TBD labels Sep 21, 2023
@marcusmunch marcusmunch self-assigned this Sep 21, 2023
@marcusmunch
Copy link
Collaborator

Good point; if we're calling translate_sql(), it makes sense that we would require con to actually be a connection object.

The fix should be as simple as adding db_timestamp.NULL method, so that calls to translate_sql will never be NULL. I'm working on it right now.

@marcusmunch marcusmunch removed the triage Severity of the issue is TBD label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants