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

Should dbDataType dispatch to the default AsIs method in custom backends? #514

Open
MichaelChirico opened this issue Mar 17, 2025 · 2 comments

Comments

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Mar 17, 2025

I have a custom backend where we need a custom method for dbDataType() for logical() ("BOOL" instead of "SMALLINT").

https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types

I'd rather back up to the default dispatch where possible, so instead of writing out a full custom R type->backend type mapping, I've just added one method for logical():

setMethod(
  "dbDataType", c("MyDBIConnection", "logical"),
  \(dbObj, obj, ...) "BOOL"
)

However, that fails the {DBItest} suite:

`expect_identical(...)` threw an unexpected error. ('test-DBITest.R:54:1')
--------------------------------------------------------------------------------
`expect_identical(...)` threw an unexpected error.
Message: dbDataType(dbObj, I(FALSE)) (`actual`) not identical to dbDataType(dbObj, FALSE) (`expected`).

`actual`:   "SMALLINT"
`expected`: "BOOL"    
Class:   expectation_failure/expectation/error/condition

My S4 knowledge is pretty weak, but IIUC the issue is that AsIs does not dispatch to the correct method:

dbiDataType_AsIs <- function(x) {
oldClass(x) <- oldClass(x)[-1]
dbiDataType(x)
}
setMethod("dbiDataType", signature("AsIs"), dbiDataType_AsIs)

So I had to wind up just copy-pasting that as a new c("MyDBIConnection", "AsIs") method.

But shouldn't that be the default behavior so I don't need to copy over boilerplate? That would just mean adding a specific c("DBIConnection", "AsIs") method, right?

(happy to file a PR, but checking my understanding first)

@krlmlr
Copy link
Member

krlmlr commented Mar 18, 2025

Thanks. There is a nasty message that can't be turned off when more than one S4 method matches. Copying the boilerplate is the easiest way to shut off that message.

I'm not sure if this applies here, so worth a check.

@MichaelChirico
Copy link
Contributor Author

I'll investigate when I get to a suitable machine for doing so :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants