👌 IMPROVE: Use sqlalchemy.func for JSONB QB filters

[ltalirz]

related to #5282

The QueryBuilder declared a custom jsonb_typeof FunctionElement,
for which sqlalchemy disables query caching by default.

[ltalirz]

Removes the warning locally for me

[chrisjsewell]

Cheers, can you try something of the lines of:

QueryBuilder().append(Node, filters={"attributes.x": {'of_type': 'bool'}}).as_sql()
QueryBuilder().append(Node, filters={"attributes.x": {'of_length': 3}}).as_sql()

just to check that this still compiles correctly.
I seem to remember some issues around this previously

[ltalirz]

You mean this? (note: replaced 'bool' => 'boolean')

In [2]: QueryBuilder().append(Node, filters={"attributes.x": {'of_type': 'boolean'}}).as_sql()
Out[2]: "'SELECT db_dbnode_1.id, db_dbnode_1.uuid, db_dbnode_1.node_type, db_dbnode_1.process_type, db_dbnode_1.label, db_dbnode_1.description, db_dbnode_1.ctime, db_dbnode_1.mtime, db_dbnode_1.attributes, db_dbnode_1.extras, db_dbnode_1.repository_metadata, db_dbnode_1.dbcomputer_id, db_dbnode_1.user_id \\nFROM db_dbnode AS db_dbnode_1 \\nWHERE CAST(db_dbnode_1.node_type AS VARCHAR) LIKE %(param_1)s AND jsonb_typeof((db_dbnode_1.attributes #> %(attributes_1)s)) = %(jsonb_typeof_1)s' % {'param_1': '%', 'attributes_1': ('x',), 'jsonb_typeof_1': 'boolean'}\n"

In [3]: QueryBuilder().append(Node, filters={"attributes.x": {'of_length': 3}}).as_sql()
Out[3]: "'SELECT db_dbnode_1.id, db_dbnode_1.uuid, db_dbnode_1.node_type, db_dbnode_1.process_type, db_dbnode_1.label, db_dbnode_1.description, db_dbnode_1.ctime, db_dbnode_1.mtime, db_dbnode_1.attributes, db_dbnode_1.extras, db_dbnode_1.repository_metadata, db_dbnode_1.dbcomputer_id, db_dbnode_1.user_id \\nFROM db_dbnode AS db_dbnode_1 \\nWHERE CAST(db_dbnode_1.node_type AS VARCHAR) LIKE %(param_1)s AND CASE WHEN (jsonb_typeof((db_dbnode_1.attributes #> %(attributes_1)s)) = %(jsonb_typeof_1)s) THEN jsonb_array_length(CAST((db_dbnode_1.attributes #> %(attributes_1)s) AS JSONB)) = %(jsonb_array_length_1)s ELSE %(param_2)s END' % {'param_1': '%', 'attributes_1': ('x',), 'jsonb_typeof_1': 'array', 'jsonb_array_length_1': 3, 'param_2': False}\n"

The one failing test is about a change in the SQL representation

E       -'SELECT db_dbnode_1.id, db_dbnode_1.uuid \nFROM db_dbnode AS db_dbnode_1 \nWHERE CAST(db_dbnode_1.node_type AS VARCHAR) LIKE %(param_1)s AND CASE WHEN (jsonb_typeof((db_dbnode_1.extras #> %(extras_1)s)) = %(param_2)s) THEN (db_dbnode_1.extras #>> %(extras_1)s) = %(param_3)s ELSE %(param_4)s END' % {'param_1': '%', 'extras_1': ('tag4',), 'param_2': 'string', 'param_3': 'appl_pecoal', 'param_4': False}
E       +'SELECT db_dbnode_1.id, db_dbnode_1.uuid \nFROM db_dbnode AS db_dbnode_1 \nWHERE CAST(db_dbnode_1.node_type AS VARCHAR) LIKE %(param_1)s AND CASE WHEN (jsonb_typeof((db_dbnode_1.extras #> %(extras_1)s)) = %(jsonb_typeof_1)s) THEN (db_dbnode_1.extras #>> %(extras_1)s) = %(param_2)s ELSE %(param_3)s END' % {'param_1': '%', 'extras_1': ('tag4',), 'jsonb_typeof_1': 'string', 'param_2': 'appl_pecoal', 'param_3': False}

but I think it's just about naming - the placeholder that used to be param_2 is now called jsonb_typeof_1

[ltalirz]

@chrisjsewell Does this look good to you?

Do I need to update some test data to make this pass?

Also, do you have an idea what causes this mypy error in pre-commit & how to fix it?

aiida/storage/psql_dos/orm/querybuilder/main.py:336: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "str")  [assignment]

Is this due to some change I made or is mypy changing?~~

Ok, my bad, I removed a mypy ignore (because my local mypy complained about it not doing anything).

[chrisjsewell]

Do I need to update some test data to make this pass?

Yeh the one you mentioned already: tests/orm/test_querybuilder.py::TestRepresentations::test_as_sql

The change looks fine, it's a pytest-regressions test, so you can even run tox tests/orm/test_querybuilder.py::TestRepresentations::test_as_sql -- --force-regen to update it

[ltalirz]

Great, thanks for the hints!

[chrisjsewell]

Maybe just change the PR/commit name to something like 👌 IMPROVE: Use `sqlalchemy.func` for JSONB QB filters

[ltalirz]

Done! I'll still need your approving review for this to auto-merge

[chrisjsewell]

I'll still need your approving review for this to auto-merge

change the PR name ☝️

[chrisjsewell]

cheers!