Skip to content

Query on subclass entity ("Table-per-subclass with discriminator" mapping) uses subclass' table alias with base class' key column name in join to collection in a dynamic component #2094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gmcelhanon opened this issue Mar 30, 2019 · 0 comments

Comments

@gmcelhanon
Copy link

gmcelhanon commented Mar 30, 2019

We have the following mappings for an abstract "EducationOrganization" with a derived "School".

Note that the PK column for EducationOrganization is EducationOrganizationId, but in the derived entities, the key column name changes to match the derived entity name -- SchoolId for School, LocalEducationAgencyId for LocalEducationAgency, etc.

  <!-- Class definition -->
  <class name="EducationOrganization" table="EducationOrganization" lazy="false" abstract="true" discriminator-value="NULL">
    <id name="EducationOrganizationId" column="EducationOrganizationId" type="int">
      <generator class="assigned" />
    </id>

    <!-- Discriminator -->
    <discriminator column="Discriminator" type="string" />
    ...

    <!-- Table-per-subclass, using discriminator -->
    <subclass name="School" lazy="false" discriminator-value="edfi.School" >
      <join table="School" >
          <key>
            <column name="SchoolId" />
          </key>

          <!-- PK properties -->
          <property name="SchoolId" column="SchoolId" type="int" not-null="true" insert="false" update="false"/>

          <!-- Properties -->
          ...

          <!-- Collections -->
          <set name="SchoolCategories" cascade="all-delete-orphan" inverse="true" lazy="false">
            <key>
              <column name="SchoolId" />
            </key>
            <one-to-many class="SchoolCategory" />
          </set>

          <dynamic-component name="Extensions">
            <set name="Sample" cascade="all-delete-orphan" inverse="true" lazy="false">
              <key>
                <column name="SchoolId" />
              </key>
              <one-to-many class="SchoolExtension" />
            </set>
          </dynamic-component>
      </join>
    </subclass>
  </class>  

When we execute an HQL query like the following:

from School a left join fetch a.SchoolCategories b where a.Id IN (:ids)

... the following FROM clause is generated (with correct joins):

from edfi.EducationOrganization school0_ 
    inner join edfi.School school0_1_ 
        on school0_.EducationOrganizationId=school0_1_.SchoolId 
    left outer join edfi.SchoolCategory schoolcate1_ 
        on school0_.EducationOrganizationId=schoolcate1_.SchoolId

Note that the "left outer join" to SchoolCategory is performed from the base entity (EducationOrganization aka school0_) using EducationOrganizationId.

However, when we execute an HQL query on the dynamic component's collection, like the following:

from School a left join fetch a.Extensions.Sample b where a.Id IN (:ids)

... the following FROM clause is generated (SQL join clause is invalid -- joins SchoolExtension using the derived table's alias with the base table's PK column name):

from edfi.EducationOrganization school0_ 
    inner join edfi.School school0_1_ 
        on school0_.EducationOrganizationId=school0_1_.SchoolId 
    left outer join sample.SchoolExtension sample1_ 
        on school0_1_.EducationOrganizationId=sample1_.SchoolId

We get the following exception in this case:

System.Data.SqlClient.SqlException (0x80131904): Invalid column name 'EducationOrganizationId'.

Note that this time, the "left outer join" to SchoolExtension is performed using the concrete entity's alias (School aka school0_1_), but still using EducationOrganizationId (the base table's primary key).

Previously we had mapped this relationship as "Table per subclass" without discriminators -- and it worked correctly (using the base entity's alias of school0_ for the joins):

<class ...>
  <joined-subclass>
    ...
  </joined-subclass>
</class>

After debugging NHibernate, here is what I have found:

  • With the "Table per subclass" mapping (without discriminator), it uses the JoinedSubclassEntityPersister.
  • With the "Table per subclass, using discriminator" mapping, it uses the SingleTableEntityPersister.
  • The alias used in the join is built by the ToColumns method in the BasicEntityPropertyMapping class, which looks like this:
    // class BasicEntityPropertyMapping
    public override string[] ToColumns(string alias, string propertyName)
    {
      return base.ToColumns(
        this.persister.GenerateTableAlias(
          alias, 
          this.persister.GetSubclassPropertyTableNumber(propertyName)), 
        propertyName);
    }

As you can see in the method below (from AbstractEntityPersister), if the persister's GetSubclassPropertyTableNumber returns a 0, the alias will be the root alias (i.e. the EducationOrganization table). If the return value is 1, the alias will be school0_1_ (the School table). (The property passed in is always the base table's key -- EducationOrganizationId.)

    // class AbstractEntityPersister
    public string GenerateTableAlias(string rootAlias, int tableNumber)
    {
      if (tableNumber == 0)
        return rootAlias;
      StringBuilder stringBuilder = new StringBuilder().Append(rootAlias);
      if (!rootAlias.EndsWith("_"))
        stringBuilder.Append('_');
      return stringBuilder.Append(tableNumber).Append('_').ToString();
    }

Both persisters (JoinedSubclassEntityPersister and SingleTableEntityPersister) use the GetSubclassPropertyTableNumber method inherited from AbstractEntityPersister:

    // class AbstractEntityPersister
    public virtual int GetSubclassPropertyTableNumber(string propertyPath)
    {
      string propertyName = StringHelper.Root(propertyPath);
      IType type = this.propertyMapping.ToType(propertyName);
      if (type.IsAssociationType)
      {
        IAssociationType associationType = (IAssociationType) type;
        if (associationType.UseLHSPrimaryKey)
          return 0;
        if (type.IsCollectionType)
          propertyName = associationType.LHSPropertyName;
      }
      int i = System.Array.IndexOf<string>(this.SubclassPropertyNameClosure, propertyName);
      if (i != -1)
        return this.GetSubclassPropertyTableNumber(i);
      return 0;
    }

It is clear that the intent (from these 2 methods) in AbstractEntityPersister is that the "root" table will have an index of 0. Properties that represent associations are explicitly given the 0 to join using the base class.

However, there is a discrepancy between the two persisters with how the tables are ordered in their internal arrays.

For the JoinedSubclassEntityPersister, I see the following in the subclassTableNameClosure field:

index name
0 edfi.School
1 edfi.EducationOrganization

For the SingleTableEntityPersister, I see the following in the subclassTableNameClosure table:

index name
0 edfi.EducationOrganization
1 edfi.School

Note that they are reversed, and I think that based on the intent of having the "root" table in position 0, that the JoinedSubclassEntityPersister actually has it wrong/backwards (but ironically it is this implementation that works because there is another problem GetSubclassPropertyTableNumber method which happens to "correct" this problem).

When the GetSubclassPropertyTableNumber is invoked, here are the results:

a.SchoolCategories a.Extensions.Sample
JoinedSubclassEntityPersister 0 (it's an association) 0 (School's table index)
SingleTableEntityPersister 0 (it's an association) 1 (School's table index)

Remember, the return value has the following impact on the generated JOIN:

TableNumber Resulting Join
0 on school0_.EducationOrganizationId=sample1_.SchoolId
1 on school0_1_.EducationOrganizationId=sample1_.SchoolId

Property path SchoolCategories is an association, so it always returns 0 and is joined correctly using school0_.EducationOrganizationId.

Property path Extensions.Sample (which actually represents an association) is trimmed by the method to the root value of Extensions. This trimmed path represents a component, so the property lookup proceeds, as follows:

  • On the JoinedSubclassEntityPersister, this column is found in the "School" table, which is in the 0 position of the table name closure, and so the "correct" value of 0 is returned.
  • On the SingleTableEntityPersister, this column is found in the "School" table (correctly), which is in the 1 position of the table name closure, and so the "incorrect" value of 1 is returned.

Essentially, I believe there are 2 problems here:

  • JoinedSubclassEntityPersister seems to have the table order reversed in its table name closure, so that the derived table is in the first position instead of the "root" table.
  • The GetSubclassPropertyTableNumber method assumes that the provided property path should always be trimmed to the "root" before evaluation. However, our dynamic component (Extensions) can have associations (Sample), and those associations should be evaluated for what they are rather than just the containing property for the component. As associations, they would be given the "root" alias, but as members of a component, it depends on the position of the derived table in the closure.

In the end, when evaluating the a.Extensions.Sample property path, the JoinedSubclassEntityPersister seems to return the correct value of 0 because these two (perceived) bugs cancel each other out. The method returns 0 because that is where the Extensions property is found (on the "edfi.School" table in position 0).

While in the SingleTableEntityPersister, the method returns 1 because it is evaluating the Extensions component rather than the Extensions.Sample association. What we really need here is to do the join on the association Extensions.Sample, and so a 0 should be returned.

Here is my suggested fix for the GetSubclassPropertyTableNumber method:

            string propertyName = StringHelper.Root(propertyPath);
            IType type = this.propertyMapping.ToType(propertyName);

            // BEGIN SUGGESTED CODE
            // If root path is a component, revert to using the full property path
            if (type.IsComponentType)
            {
                propertyName = propertyPath;
                type = this.propertyMapping.ToType(propertyPath);
            }
            // END SUGGESTED CODE
            
            if (type.IsAssociationType)
            {

As for the discrepancy in the order of the entries of the table name closures between JoinedSubclassEntityPersister and the SingleTableEntityPersister, I don't have any suggestions -- but it doesn't look right to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant