edgedb-python icon indicating copy to clipboard operation
edgedb-python copied to clipboard

handle recursion in model_dump

Open 1st1 opened this issue 5 months ago • 2 comments

Recursion should be possible:

p = default.LinearPath(label="singleton")
p.next = p

client.save(p)

I'm enabling it in the next PR, but the bigger question is model_dump. It would crash unable to serialize data (the default behavior). We can do better -- we can detect recursion ourselves and stop it by returning a dict with just id.

Granted it would only work on models that were save()-ed, but that's the intended use case for model_dump anyway.

1st1 avatar Jul 14 '25 02:07 1st1

Note that this limitation applies only to models adjusted in Python; data that comes from the DB would just have two different objects for the same ID (we don't have an identity cache)

1st1 avatar Jul 14 '25 02:07 1st1

Looks like we currently overflow the stack and trigger a RecursionError, which is not great but not as bad as the segfault that the xfail comment suggested? (Though maybe sometimes it is a segfault, if it's mixed with rust calls? I'm not sure.)

Using regular models, pydantic fails with the somewhat more well-behaved:

ValueError: Circular reference detected (id repeated)

We could make recursion detection work (and dump an id-only dict) but it would impose some cost (maintaining a map). We might be able to manage to do it using the contexts that dump can be given (though we'd need to be careful to make sure we never lose them--we have a lot of ad-hoc stuff going on). Otherwise we'd need contextvars I suppose.

msullivan avatar Oct 02 '25 23:10 msullivan