[Feature] SQL: Add all combinations of element refs and ids for 'create' methods

[EotT123]

Enhance the create methods for SQL objects by offering more flexibility for handling foreign keys. Currently, users have to choose between object references or IDs, but not a mix of them.

Example:

• Currently:

  create(Foo fooRef, Bar barRef, ...) // all by reference
  create(Integer fooId, Integer barId, ...) // all by id

• Wanted:

  create(Foo fooRef, Bar barRef, ...)
  create(Integer fooId, Integer barId, ...) // new: mix
  create(Foo fooRef, Integer barId, ...)
  create(Integer fooId, Bar barRef, ...) // new: mix

[EotT123]

PR which implements this: https://github.com/manifold-systems/manifold/pull/576

[rsmckinney]

Hi @EotT123. Thanks for the PR. My only reservation is the exponential growth of generated functions with FKs. Four or more FKs feels like too many. This is why I chose to have either all refs or all IDs. And use the all ID version when you don't have all the required FK refs. Calling getId() isn't so bad?

Perhaps I'll make this a configurable option... Support a boolean generatefullCreateApi dbconfig option, default to false to be on the conservative side.

What I really want is a union type:

create(Foo | Integer foo, Bar | Integer bar, ...)

But that's probably a bridge too far, for now.

[EotT123]

Thanks for your answer. I was also thinking about the exponential growth, and the fact that calling getId() isn't so bad. However, calling getId() is not always possible when the object is not yet persisted. e.g.:

Foo foo = Foo.create(...);
Bar bar = Bar.create(foo, ...);
MyDB.commit();

In this example, calling Bar.create(foo.getId(), ...); would not work. Currently, the only way to make this work is to persist the foo object first, before persisting the bar object (but no rollback is done for foo when an error occurs during persisting bar).

The union type is a nice idea, but that's a complete new feature? And how would such a type be handled, because you don't know what the type is?

[rsmckinney]

calling Bar.create(foo.getId(), ...); would not work

That can be made to work... In fact the information is there, it's just not made available until after the commit. Basically, after a create or update any generated data in the record is automatically retrieved and placed "on hold" in a private data section until the commit succeeds or fails. If it succeeds, the on-hold data becomes active, otherwise it is discarded.

I am considering delegating to on-hold/generated data during the commit. It's a straightforward change that probably should be done regardless of the outcome of this issue.

The union type is a nice idea, but that's a complete new feature? And how would such a type be handled, because you don't know what the type is?

Yep, it would be a new feature. The type can be identified with simple instanceof checks or the union type can provide a discriminant, which is a field to identify the type such as an enum. While I like union types, I don't really want to go down this path, at least not any time soon.

In any case I do like your PR, but I think it should be optional via dbconfig settings. I'll merge it and add the option and get a feel for it. I'll also probably experiment with making the on-hold data indirectly accessible during the commit.

[EotT123]

That might be a better solution. Then there even might not be the need to pass the objects by reference, only by id?

I'm a bit intrigued how you managed to do this. Autogenerated id's are generated by the database, when persisting the object. How do you already have access to the id's, before the object is persisted?

[rsmckinney]

Well, there's no free lunch, but the price is low. Here's an example illustrating what I mean by making generated data available during the commit.

Foo foo = Foo.create(...);
MyDatabase.addSqlChange(ctx -> Bar.create(foo.getId(), ... ));
. . .
MyDatabase.commit();

During a commit addSqlChange calls force the tx to execute CRUD calls such as create and build that precede it. This behavior guarantees the sequence of CRUD changes are reflected in the tx as direct SQL ops execute. Thus, we can use addSqlChange as the basis for accessing generated data, which is already stored in entity bindings, hence foo.getId() can work by delegating to the "on hold" generated data internal to all manifold entity bindings.

The addSqlChange call is the price of the "free lunch". It works, but it's not intuitive. So another tweak to the changes I'm considering is to make CRUD operations execute eagerly instead of in batch when in a addSqlChange or commit call.

MyDatabase.commit(ctx -> {
    Foo foo = Foo.create(...);
    Bar.create(foo.getId(), ... );
    . . .
} );

With eager execution instead of batching INSERT ops we execute them with create calls, thus the INSERT corresponding with Foo.create() executes before Bar.create() enabling foo.getId().

Still not a free lunch as it must execute in a commit() call. Personally, I prefer having more fluid, tx-independent CRUD with the ability to commit changes that span code contexts, requests, etc. But this change is nice when a commit can be gathered in one place.

Probably more than you wanted to know, but that's the gist of it.

[rsmckinney]

Maybe another way to understand this, your example involving refs works by virtue of the "on hold" bindings.

Foo foo = Foo.create(...);
Bar bar = Bar.create(foo, ...);
MyDB.commit();

Here the foo reference works because, although INSERTS are batched, the "on hold"/generated bindings are late bound and accessible during the INSERT for bar. This is primarily how the either/or API on create & build get by. Most it suffices, but there are plenty enough use-cases where only IDs are available. While fetch calls fill the gaps, they are less efficient than using straight IDs when you have'm.

Ok. Time for a weekend!

[EotT123]

Thanks for the explanation. I was thinking about something like it, but I was wondering if I maybe was missing something. All seems logical now. Thanks.

Probably the following would work too (although this is also not an optimal solution, and it doesn't solve anything, as we need separate methods for this to, same as what the PR adds.):

Foo foo = Foo.create(...);
Bar.create(foo::getId, ... );

In this example, getting the id is function, which is lazily executed, when the id is available.

Have a nice weekend!