sqlalchemy todict intbitset conversion fails on infinite sets

[dset0x]

Given an infinite intbitset,

intbitset(trailing_bits=True)

and this SQLAlchemy type,

class IntBitSetType(types.TypeDecorator):

    impl = types.BLOB

    def process_bind_param(self, value, dialect):
        if value is None:
            return None
        return intbitset(value).fastdump()

    def process_result_value(self, value, dialect):
        if value is None:
            return None
        return intbitset(value)

calling dict() on an instance that contains it, raises this exception:

...
File "/home/gs-sis/.virtualenvs/invenio/src/invenio-checker/invenio_checker/views/admin.py", line 201, in get_tasks_data
 rule_d = dict(rule)
   File "/home/gs-sis/.virtualenvs/invenio/src/invenio/invenio/ext/sqlalchemy/utils.py", line 116, in todict
     value = value.tolist()
   File "intbitset.pyx", line 770, in intbitset.intbitset.tolist (intbitset/intbitset.c:12711)
   File "intbitset.pyx", line 775, in intbitset.intbitset.tolist (intbitset/intbitset.c:12641)
OverflowError: It's impossible to retrieve a list of an infinite set

---

intbitset behaves reasonably here, but this conversion was not asked for. Why are these conversions forced in todict?

[jirikuncar]

intbitset(trailing_bits=True) is an infinite structure that can't be converted to list. IMHO this is r_wontfix issue.

[dset0x]

The question here is: why we are converting in todict? All I requested in my code was a dictionary of my entry.

[jirikuncar]

@dset0x you have answered yourself - because you are requesting a dict.

[jirikuncar]

see https://github.com/inveniosoftware/invenio-ext/blob/master/invenio_ext/sqlalchemy/utils.py#L133-L135

[dset0x]

@dset0x you have answered yourself - because you are requesting a dict.

A dictionary can hold this object just like any other:

In [75942]: {'infinity': intbitset(trailing_bits=True)}
Out[75942]: {'infinity': intbitset([0], trailing_bits=True)}

see https://github.com/inveniosoftware/invenio-ext/blob/master/invenio_ext/sqlalchemy/utils.py#L133-L135

Iterables should too.

[jirikuncar]

intbitset has it's own handling because we were not able to create a JSON out of it. Maybe it's time to use better JSON encoder/decorer that can handle it. See also https://github.com/inveniosoftware/invenio-ext/blob/master/invenio_ext/sqlalchemy/utils.py#L115-L116

[jirikuncar]

There is still a question: Why do you need to store an intbitset with trailings_bit=True in database?

This structure does not have any Python equivalent except of an infinite generator.

[dset0x]

There is still a question: Why do you need to store an intbitset with trailings_bit=True in database?

I wish to express infinity, therefore I store a dump of an expression of that (in the same format that I store non-infinity), instead of another magic value. This keeps my TypeDecorator above clean. I only afterwards interpret what infinity means for my logic. I do not necessarily iterate over it.

intbitset has it's own handling because we were not able to create a JSON out of it.

I don't understand what JSON has to do with a method that is called when __builtin__.dict is called. I do not wish to serialize my data. Perhaps this was added assuming that the only reason to call __builtin__.dict was to serialize?

[jirikuncar]

(a) use the model directly
(b) serialize only fields you really need
(c) fix the serialization and check all dependent code

[dset0x]

(a) use the model directly

I wanted a dictionary of the entire object, like SQLAlchemy normally hands over.

(b) serialize only fields you really need

Again, I am not serializing anything (But yes, I want this field in my dictionary).

(c) fix the serialization and check all dependent code

Since there is no reason to do these conversions, then this is the solution that will prevent future pain. I suspect however, that removing this code will break edge cases in all kinds of hard-to-find code, especially since __iter__ was overriden as well.

The use-cases are not obvious to me even in inveniosoftware/invenio@1ee2c0c, where this code seems to originate.

Perhaps someone can come up with a better plan to revert this.

[kaplun]

@jirikuncar indeed re-reading all this issue and the referred original code, seems like that the .todict() method was indeed trying to do too much. Looks like was more a .tojsondict() i.e. returning something that is safely serializable to JSON. However as @dset0x points out, the .todict() method is called indirectly also in contexts where JSON serialization is not needed, and where a real instance of an intbitset is perfectly OK.

So in your opinion can .todict() be amended to not be too smart and simply return a dictionary with values not converted/serialized?

[jirikuncar]

We can amend it as anyway the intbitset will not be necessary for core packages anymore.