piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

asyncpg.exceptions.InvalidTextRepresentationError

Open powellnorma opened this issue 2 years ago • 2 comments

When storing a string into a JSONB column I get:

asyncpg.exceptions.InvalidTextRepresentationError: invalid input syntax for type json

I guess it is because somewhere piccolo assumes that if the type of the input argument is str, it is already json encoded. Maybe we can add a quick check, like assuming that the first char is in "[{? Or we just always assume that the input is not already encoded, and thus always encode it ourselves even if the type is str?

powellnorma avatar Feb 10 '23 10:02 powellnorma

Yeah, Piccolo doesn't validate that the string is valid JSON.

JSON is quite flexible in terms of what you can store - for example, this works:

>>> import json
>>> foo = "1"
>>> json.loads(foo)
1

The main problem is if it's a plain string:

>>> import json
>>> foo = "hello world"
>>> json.loads(foo)
Error

We could validate every JSON string, but then it adds overhead. Not a big problem if only inserting a few rows, but could slow things down if there are lots of rows. So I'm on the fence... That asyncpg error isn't super helpful. What do you think?

dantownsend avatar Feb 10 '23 14:02 dantownsend

I think that it would be good if JSONB would transparently encode anything that is put into it, no matter the type - So also a str should get json encoded. In case that the user wants to add already encoded json data, perhaps a flag raw_json could be set?

But since this would be a breaking change, I am not sure if this would be a good decision.

The current situation however feels a bit unintuitive, when one has e.g. a "data" colum, where one first only storse dicts, but then later also stores simple strs - This then throws an error (because the str currently needs to be json encoded manually while the dict does not)

The checking I proposed was not only to display a nicer error message, but to conditionally encode the str that was passed in. However there could be cases where this mechanism thinks a input is already encoded, but actually it is just valid json by coincidence. So I don't think that would be a good solution

powellnorma avatar Feb 10 '23 22:02 powellnorma