Inheritance Problems

[mariotoffia]

I'm in learning stage so I'd setup a test case where:

• IResource - Interface for a resource
  • Id : Guid
• Resource - Base type implementing the IResource interface
• ZoneResource - Base type for zone resources
  • Zone : int
  • Kiln
• Kiln - Derives from Resource
• CFan - Derives from ZoneResource

I've started out doing a interface for the ZoneResource since both ZoneResource and Resource are IResource (but that did not work at all).

Now with "flatternd" inheritance / implementation chain I explicitly setup subclassing (tried to just add IResource when all resources where implementing that but it just created a table for each resource):

  For<Resource>()
    .AddSubclass<Ce>()
    .AddSubclass<DigitalInputPort>()
    .AddSubclass<Kiln>()
    .AddSubclass<Sawmill>();

  For<ZoneResource>().AddSubclass<CFan>();

I add a set of resources and do a initial query - session.Query().Where(x => x.Name = "CFAN1").ToList(); and it works perfectly.

In another class I do:

  var kiln = Kilns().First();
  var cf = _session.Query<CFan>().Where(c => c.Kiln == kiln.Id);
  if (_config.KilnZone != -1)
    return cf.Where(c => c.Zone == _config.KilnZone).ToList();

  return cf.ToList();

I see in the log that it creates a new table 'mt_doc_cfan' and queries against that instead of table 'mt_doc_zoneresource' as earlier.

What I'm I doing wrong or is this a bug?

Is the intention that it shall be possible to do a For and then it shall use all subtypes even if the inheritance chain is 3 or more on the same table?

Cheers, Mario

[jeremydmiller]

@mariotoffia I'm thinking it's a bug -- unless you were using a different DocumentStore object in the second class. It should have created the table for the parent type and used that throughout. All the tests for that behavior are passing in CI, so if it is broken, my theory is that it's either losing the configuration somehow or fetching the configuration incorrectly somewhere downstream.

[mariotoffia]

Hi, I've have written more proper unittests (I can send you the project if you want) to isolate problems. The only thing I've found is that either the base type is queryable or the most concrete. The derived classes in between do not work.

Is this possible to fix? Do you want my test suite?

Cheers, Mario

[jeremydmiller]

@mariotoffia It'll get fixed a lot faster with your test suite;) Yes, please, and I'm sure we can fix it.

[mariotoffia]

Hi, I've created a temp repo and uploaded the Mario.Tests.zip file of the ms-test project...

It is using a set of nugget packages that you need to restore. I've done a simple nugget package to fire up docker containers and tear them down for each test (or for a whole testclass) that I'm using in the test. If you wish to test differently just copy'n'paste those test to your own solution.

Hovever, I do not use nugget for Martens (I've cloned it and placed in the Marten.sln so it is a direct project reference to Marten (in master branch). I'm using a postgres 9.5 docker image for my tests - "clkao/postgres-plv8:latest".

The tests I refer to is in the Inheritance/\ namespace

• InheritanceQueryDirectConfigTests
• InheritanceQueryMartenRegistryConfigTests
• LightWeightSessionInheritanceQueryMartenRegistryConfigTests All three emit same behaviour.

Please let me know if there's something that you need me to do!

Cheers, Mario :)

[jeremydmiller]

@mariotoffia Thanks Mario. Gimme a couple days to get to it, but I will sometime early next week.

[nieve]

@jeremydmiller unless someone beats you to it ;)

[nieve]

Ok, the first issue I discovered is with two levels down inheritance, ie A -> B -> C any instances of C saved by marten will not be actually created & persisted in the DB. Hopefully will get this sorted tomorrow/this weekend.

[jeremydmiller]

C still has to be explicitly mapped as "save as A", it doesn't infer anything

[nieve]

Ok, First of all, sorry @jeremydmiller - it turns out that wasn't the issue, that was me being sloppy. The good news is that A->B->C actually works OOTB.

@mariotoffia I finally understood the issue you've raised; it's something we should probably discuss before implementing. The issue is if you have A->B->C and you save an instance of C and you Query() you won't get the C you just saved as long as your schema is _.Schema.For().AddSubclass().AddSubclass()

The reason being Marten currently supports only singular inheritance/interface-implementation-relations. If you look at the row created in the db, it looks like this: ID: "0fad5873-ba87-45b6-ab46-46fc76622d90"; DOC: "{"Id": "0fad5873-ba87-45b6-ab46-46fc76622d90", "Name": "KILN1", "Zones": [{"Zone": 1, "ZoneType": "climate"}], "Ordinal": 1, "CommonName": null}"; DOC TYPE: "kiln"

We will need to modify the way the doc type saving is done, maybe use an array type? @jeremydmiller - what say you?

Personally, I'm not 100% sure about adding such a feature. It does make complex inheritance relationships a possibility, which could be a code smell- don't know to what extent marten should be opinionated though.

[mariotoffia]

In our case it's all about porting from (my, custom, anno -2003) document (xml) store to marten. And make use of these chains since to allow for more generic algorithms to be executed on base types and only add specialized for the specifics (if needed). It's a drying kiln process driver application along with PLC:s on the field. This configuration if both used for persistence and augmented into the OPC (PLC communication) resources.

@nieve Do the DOC_TYPE really need to be an array? Doesn't it just be the most concrete type stored e.g. in your example A->B->C DOC_TYPE: "C" and since schema do have A,B,C (and knows the inheritance chain?) if I Query could it not generate query "DOC_TYPE eq "C" || DOC_TYPE eq "B"?

Thus this will not inflict any performance while not using long inheritance chains - for those users and still allow e.g. my customer use-case and accept less performant queries in those cases?

Or I'm I missing something?

Cheers, Mario

[jeremydmiller]

@nieve @mariotoffia I'm not wild about this, but it doesn't sound too crazy hard either. We'd need to have yet another type of "type forwarding" for the intermediate base classes. The only thing I'd question is whether we made this be an explicitly configured thing like the type hierarchy is today anyway, or try to infer it.

[nieve]

@jeremydmiller @mariotoffia I can take a look at it after we're done with the deleteWhere issue. Personally, I think it should be configurable. Users have to feel some pain to see that what they're doing might be too complicated. It'll also be more performant I believe. my 2p anyway.

[nieve]

Sorry for being a bit slow on this- have started work today, should have something working (though not explicitly configured for now) tomorrow.

[mariotoffia]

Thank you guys for the much appreciated fix!! :)

Cheers, Mario