coldbox-modules / quick

A ColdBox ORM Engine
https://quick.ortusbooks.com
MIT License
23 stars 19 forks source link

Test cases for regression when retrieving child classes through parent #251

Open ryanalbrecht opened 2 months ago

ryanalbrecht commented 2 months ago

Its seems between v6 and v9 there was a major regression. ( edit: I think this might have always been this way)

When using multi-table inheritance and you try to load a child class through a parent entity, quick will generate a query that has a join onto each child table. As each child class will often have their foreign key to the parent named the same, this causes an issue when the child class is instantiated as it could land up with an empty value depending on the order of the joins.

This subsequently causes an exception to be thrown when updating loaded entities as the foreign key will not be present in the child class.

This is a big issue and not 100% how to fix it but I created some tests case for showing the failure and the exception

ryanalbrecht commented 2 months ago

I was playing around with the idea of adding another select in the applyInheritanceJoins() function where you alias the join column. Then in the loadEntity() function you copy the aliased column form the memento into the actual FK variable then delete the alias key. This seems to work however you now have this extra column the query when using retrievingQuery() so it 'feels' wrong.

What are your thoughts?

public any function loadEntity( required struct data ) {
....
  if( !getEntity().isSingleTableInheritance() ){
      //de-alias join column on data struct
      var joincolumn = childClass.get_meta().localMetadata.joincolumn;
      data[joincolumn] = data['#discriminator#_join_column']; //aliased foreign key from query
      structDelete(data, '#discriminator#_join_column');
  }
}
bdw429s commented 2 months ago

This pull request has been mentioned on Ortus Solutions Community. There might be relevant details there:

https://community.ortussolutions.com/t/quick-will-create-a-join-on-each-child-table-when-loading-an-child-through-a-parent/10208/1

ryanalbrecht commented 2 months ago

Another option would be to coalesce the key selections.

  function applyInheritanceJoins() {
    var entity = getEntity();
    // Apply and append any inheritance joins/columns
    if ( entity.hasParentEntity() && !entity.isSingleTableInheritance() ) {
      var parentDefinition = entity.getParentDefinition();
      variables.qb.join(
        parentDefinition.meta.table,
        parentDefinition.meta.table & "." & parentDefinition.key,
        entity.qualifyColumn( entity.keyNames()[ 1 ] )
      );
    } else if ( entity.isDiscriminatedParent() && entity.get_loadChildren() ) {
      var coalesceJoin = {};
      entity
        .getDiscriminations()
        .each( function( discriminator, data ) {
          // only join if this is a polymorphicn association
          if ( !entity.isSingleTableInheritance() ) {
            variables.qb.join(
              data.table,
              getEntity().qualifyColumn( getEntity().keyNames()[ 1 ] ),
              "=",
              data.joincolumn,
              "left outer"
            );
          }

          var join_key = listLast( data.joincolumn, '.' );
          if( !structKeyExists( coalesceJoin, join_key ) ){
            coalesceJoin.insert( join_key, [] );
          }

          data.childColumns.delete( data.joincolumn );
          coalesceJoin[join_key].append( data.joincolumn );
          variables.qb.addSelect( data.childColumns );
        } );

      //for each unique foreign key coalesce it
      coalesceJoin.each( function( key, value ){
        var rawSel = value.len() == 1 ? value.toList() : "COALESCE(#value.toList()#) AS #key#";
        variables.qb.selectRaw( rawSel );
      } );
    }
  }