glorp icon indicating copy to clipboard operation
glorp copied to clipboard

RelationExpression>>primaryKeyFromDictionary: can't find primaryKey for descriptors with FilteredTypeResolver

Open eMaringolo opened this issue 4 years ago • 0 comments

Introduction

When trying to look into the cache instead of performing a database query, there is a check to verify if the referenced object primary key is already in the cache, to get the value used as index, the primaryKeyFromDictionary: aDictionary is called which will return the value or nil.

If you use a Descriptor that uses a FilteredTypeResolver, the queries you perform will use a IN(...) query together with the pk value.

E.g. If have this hierarchy:

Participant   Type identifier
  \_ Player     'player'
  \_ Team      'team'

So when I perform a read on Participant the query will do SELECT ... FROM Participant WHERE type IN ('player', 'team').

But if in another Descriptor (e.g. for ParticipantRegistration) I have a ReferenceMapping to Participant or any of its subclasses, when performing a query like this:

aSession read: ParticipantRegistration where: [:each | each participant = aParticipant ]

Then the SQL generated will be like the following:

SELECT ... FROM ParticipantRegistration WHERE type IN ('player', 'team') AND participant_id = 10;

Description

Enter RelationExpression>>primaryKeyFromDictionary:.

This method tries to get the actual primary key from the relation expression, but since GLORP is prepared to have multi column primary keys that it concatenates to build keys for different purposes it has a special check to know if the expression is an AND kind of relation, meaning that the PK might be composed by more than one field.

The problem is that this check is ignoring the possibility of expressing something like the FilteringTypeResolver type column AND the actual PK (single value), which is the most common case.

As follows:

RelationExpression>>primaryKeyFromDictionary: aDictionary
	"Given a set of parameters, return a primary key suitable for retrieving our target. 
        Do this only if the expression is for a primary key, and has no other conditions than
        the primary key one.  If the table's primary key is composite, return the array that will 
        be needed with the found values in the right position and nils elsewhere.  
        (If unreferenced primaryKey fields are nillable, this could return a matching key. 
         A query with other values elsewhere will throw away the primary key, returning nil.  One without will "

	| left right field primaryKeyFields |
	relation = #AND
          ifTrue: [
             left := leftChild primaryKeyFromDictionary: aDictionary.
             left isNil ifTrue: [^nil].
             right := rightChild primaryKeyFromDictionary: aDictionary.
             right isNil ifTrue: [^nil].
            ^self assembleCompositePrimaryKeyFrom: left and: right].
	relation = #= ifFalse: [^nil].
        "method continues"

That relation = #AND condition there ignores the case that the #AND relation might be used to specify the filtered type identifier and the actual PK value.

So I quickly changed that to this variation, also checking for the option of having an #IN kind of relation in the left side of the expression:

	| left right field primaryKeyFields |
	relation = #AND
		ifTrue: [ left := leftChild primaryKeyFromDictionary: aDictionary.
			right := rightChild primaryKeyFromDictionary: aDictionary.
			"Assume this might be filtered type resolver."
			^leftChild relation == #IN
				ifTrue: [ right  ]
				ifFalse: [ left isNil | right isNil
						ifTrue: [ nil ]
						ifFalse: [ self assembleCompositePrimaryKeyFrom: left and: right ] ] ].
	relation = #= ifFalse: [^nil].
        "method continues"

This fixed the issue and as a side effect got a tremendous speedup in my program, because now the cache is hit most of the times, avoiding to re-read the object from the database and then update the cache again, to be matched only when reading the object outside of a relation expression (it is, only when reading the object directly).

Comments

I'm creating the issue because a proper test case should be created to consider this bug solved, and to work as a heads-up to anybody already having cache issues with Descriptors using a FilteredTypeResolver .

eMaringolo avatar Aug 12 '20 02:08 eMaringolo