Closed ankane closed 1 year ago
Thanks, confirmed. Would it help if we emitted NULL
instead of NULL::text
only for string values?
Yeah, I think that should solve it.
Imho in this case the correct thing to do would be to have the json
variable to actually be of class json and provide a dbQuoteLiteral()
method for json.
Alternatively (or maybe have that anyway) add an argument implicit
to dbQuoteLiteral()
?
hey, are there any new updates regarding this issue? thank you :)
Would it make sense to extend on https://github.com/r-dbi/RPostgres/pull/370/ and reverse the change applied here https://github.com/r-dbi/RPostgres/pull/370/commits/b6d97c1f26f293f715c49ec895b23d7c8abc5c64#diff-49690042645df85dc9ae7e18aec05c8dd787da2c81d51128d9a2683dacc2dde5R30?
Or would you prefer to extent on the type handling by adding one for json fields?
fwiw, this affects all types (jsonb
was just an example).
This also affects me. The following code won't call the correct stored procedure because of the type cast on the NULL
:
if(is.null(valid_on)) {
valid_on <- NA
}
res <- dbSendQuery(con, sprintf("select * from %sts_read_raw(%s, %s)",
dbQuoteIdentifier(con, Id(schema = schema)),
dbQuoteLiteral(con, valid_on),
dbQuoteLiteral(con, respect_release_date)))
I get the error:
Error: Failed to prepare query: ERROR: function timeseries.ts_read_raw(boolean, boolean) does not exist
LINE 1: select * from "timeseries".ts_read_raw(NULL::bool, FALSE)
Downgrading to 1.4.1 fixes the issue.
Might I suggest to not cast NULL
to bool
? If assigning a literal NA
in code it will always default to a logical, even though the variable might be intended to hold a different type of data. Programmers would have to very explicitly set the type, which is not common in R, like so:
if(is.null(valid_on)) {
valid_on <- as.Date(NA)
}
Expecting this to be the case for all libraries and existing R code is unrealistic and what happens instead is that there are unexpected errors in code that used to work perfectly before.
So my proposal is that if it is not possible to define a more restricted type (with all the checks already in place), the fallback is to translate NA
into NULL
without cast
@krlmlr I can confirm this is interesting, what do you think about @Kjir suggestion?
IIUC Postgres (sometimes) automatically casts untyped NULLs (i.e. those of unknown
type) to text
, e.g. in a VALUE
statement. This is leads to problems with non-text
-ish types, see this answer on SO.
OTOH explicitly casting NULLs of text
-ish types (e.g. json or enum) to text
is also problematic, as we can see here.
(I originally ran into this dbQuoteLiteral()
issue while trying to dbx::dbxUpsert()
an enum with NULL values.)
Another example where Postgres seems to require an explicitly cast NULL on a non-text-ish type, namely a function that expects a date (or timestamp or interval):
library(RPostgres)
con <- postgresDefault()
dbGetQuery(con, "SELECT date_part('year', NULL);")
#> Error: Failed to prepare query: ERROR: function date_part(unknown, unknown) is not unique
#> LINE 1: SELECT date_part('year', NULL);
#> ^
#> HINT: Could not choose a best candidate function. You might need to add explicit type casts.
dbGetQuery(con, "SELECT date_part('year', NULL::date );")
#> date_part
#> 1 NA
While for a function that requires a json this seems to be the other way around.
dbGetQuery(con, "SELECT jsonb_pretty(NULL);")
#> jsonb_pretty
#> 1 <NA>
dbGetQuery(con, "SELECT jsonb_pretty(NULL::text);")
#> Error: Failed to prepare query: ERROR: function jsonb_pretty(text) does not exist
#> LINE 1: SELECT jsonb_pretty(NULL::text);
#> ^
#> HINT: No function matches the given name and argument types. You might need to add explicit type casts.
As an aside: It gets even more π΅ with enum support functions.
Notice that except for the two-argument form of
enum_range
, these functions disregard the specific value passed to them; they care only about its declared data type. Either null or a specific value of the type can be passed, with the same result. It is more common to apply these functions to a table column or function argument than to a hardwired type name as used in the examples. PostgreSQL: Documentation: 15: 9.10.Β Enum Support Functions
Long story short: @krlmlrβs proposal, i.e.Β not type-casting NULLs only for string values, seems the way to go. (Though again with the important AFAIU caveat, i.e.Β I may be missing an important nuance.)
With #425, the motivating example works.
Thanks @krlmlr
Hey @krlmlr, it looks like #370 causes issues for queries that rely on implicit conversion (https://github.com/ankane/dbx/issues/30).
The script succeeds with 1.4.1. Is it possible to revert the change?