Auto migration not working with JSONB fields

[heliumbrain]

Hello!

@dantownsend I'm afraid something in #122 made the migration manager sad at the sight of a JSONB field.

piccolo migrations new blog --auto

Running Postgres version 12.5
Creating new migration ...
The command failed...
Traceback (most recent call last):
...
    return json.dumps(data, default=str)
  File "/Users/nilskanevad/.asdf/installs/python/3.9.4/lib/python3.9/json/__init__.py", line 234, in dumps
    return cls(
  File "/Users/nilskanevad/.asdf/installs/python/3.9.4/lib/python3.9/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/Users/nilskanevad/.asdf/installs/python/3.9.4/lib/python3.9/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/Users/nilskanevad/dev/kekou.eu/kekou-piccolo/.venv/lib/python3.9/site-packages/piccolo/columns/base.py", line 608, in __str__
    return self.querystring.__str__()
  File "/Users/nilskanevad/dev/kekou.eu/kekou-piccolo/.venv/lib/python3.9/site-packages/piccolo/querystring.py", line 82, in __str__
    return template.format(*converted_args)
IndexError: Replacement index 0 out of range for positional args tuple

And with orjson:

...
  File "/Users/nilskanevad/dev/kekou.eu/kekou-piccolo/.venv/lib/python3.9/site-packages/piccolo/utils/sql_values.py", line 25, in convert_to_sql_value
    return dump_json(value)
  File "/Users/nilskanevad/dev/kekou.eu/kekou-piccolo/.venv/lib/python3.9/site-packages/piccolo/utils/encoding.py", line 16, in dump_json
    return orjson.dumps(data, default=str).decode("utf8")
TypeError: Type is not JSON serializable: JSONB

It works fine with JSON. And both JSON and JSONB works in 0.23.0.

My knowledge of JSONB is very lacking, and I have no clue how to find the cause of this

[dantownsend]

Hmm, strange. I can replicate this locally - will try and figure it out.

[heliumbrain]

Indeed strange... I just tried creating a new project with a new venv and a newly scaffold ASGI app. It seems to work fine. This might just be my environment acting up. Sorry for the noise.

Will dig in to it a bit more though.

[heliumbrain]

Ok, so it only seems to happen after adding a new JSONB field to a table...

In a completely new project, only installing dependencies and running piccolo asgi new with FastAPI and Uvicorn

class Task(Table):
    """
    An example table.
    """

    name = Varchar()
    completed = Boolean(default=False)
    data = JSONB()

~/tmp-dev/piccolo-issue-123-2 via 🐍 v3.9.4 (.venv) 
❯ piccolo migrations new home --auto
Running Postgres version 12.5
Creating new migration ...
Created tables                           1
Dropped tables                           0
Renamed tables                           0
Created table columns                    3
Dropped columns                          0
Columns added to existing tables         0
Renamed columns                          0
Altered columns                          0

Then after updating Table:

class Task(Table):
    """
    An example table.
    """

    name = Varchar()
    completed = Boolean(default=False)
    data = JSONB()
    data2 = JSONB()

~/tmp-dev/piccolo-issue-123-2 via 🐍 v3.9.4 (.venv) took 2s 
❯ piccolo migrations new home --auto
Running Postgres version 12.5
Creating new migration ...
The command failed.
Replacement index 0 out of range for positional args tuple

[heliumbrain]

Hmm, it seems to happen no matter if you try to add a new JSONB (or any other field for that matter) or not. Even running the migrations --auto command after a succesful run without changing the table gives the same error.

[dantownsend]

Yeah, seems to be an issue when Piccolo is trying to check which columns have been added.

It uses sets to do this.

piccolo/piccolo/apps/migrations/auto/diffable_table.py

Lines 67 to 76 in 98de5b6

	add_columns = [
	AddColumn(
	table_class_name=self.class_name,
	column_name=i._meta.name,
	column_class_name=i.__class__.__name__,
	column_class=i.__class__,
	params=i._meta.params,
	)
	for i in (set(self.columns) - set(value.columns))
	]

Under the hood it seems like sets call the __eq__ method to compare two objects. However, Column has a custom __eq__ method, which is what allows something like SomeTable.some_column == 'cats' to work in a where cause.

I need to work out a different way to check equality between columns without triggering this custom __eq__ method.

Or just refactor the code I added, so convert_to_sql_value is called in a different place.

[dantownsend]

@heliumbrain Should be fixed now - it's on PyPI. Give it a go when you have a chance.

I need to add a new Github action, which will try and run a big suite of migration files to catch these edge cases.

[heliumbrain]

@dantownsend that's fast, great job :)

Working as expected now 👍