quick icon indicating copy to clipboard operation
quick copied to clipboard

Test cases for regression when retrieving child classes through parent

Open ryanalbrecht opened this issue 1 year ago • 4 comments

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 avatar Jun 13 '24 19:06 ryanalbrecht

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');
  }
}

ryanalbrecht avatar Jun 14 '24 13:06 ryanalbrecht

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

bdw429s avatar Jun 17 '24 14:06 bdw429s

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 );
      } );
    }
  }

ryanalbrecht avatar Jun 17 '24 16:06 ryanalbrecht

This is awesome @ryanalbrecht. Nicely done. I hope it gets added to the next Quick version.

homestar9 avatar Nov 04 '24 18:11 homestar9