lock behaviour

[filt3rek]

Hej Jonas,

I hope you're in a good mood because I've another headacke for you... I don't know if it's really a bug or maybe just a behaviour that should be more explicitly described, I'll try to explain with the minimalest example that I could do. Let's say we have that :

This C class is just used to make the bug rise : As noted, this get_as function must be called (I think it's because of the A references...)

class C extends sys.db.Object {
	var id	: SId;

	@:skip
	public var as	(get,null)	: List<A>;

	public function new() {
		super();
	}

	function get_as(){
		return A.manager.search( $c == this );		// <--- If not called, all is ok even if lock == false
	}
}

This A class has a special hxSerialize and hxUnserialize function to get a custom serialization, which consist of having a field called xfields that will just have the fields to serialize. And in get_bs() I do a call to upper class in order to rise the bug

class A extends sys.db.Object{
	@:skip
	public var xfields	(get,null): Array<String>;
	
	var id	: SId;
	
	@:relation( cID )
	public var c	: C;

	@:skip
	public var bs	(get,null)	: Array<B>;

	public function new() {
		super();
	}

	function get_bs(){
		var tmp	= c.as;		// <------- to get the call, then the bug
		return bs	= Lambda.array( B.manager.search( $a == this ) );
	}

	//

	function get_xfields(){
		if( xfields == null ) xfields	= [ "id", "c" ];
		return xfields;
	}

	@:keep
	public function hxSerialize( s : haxe.Serializer ) {
		s.serialize( xfields.join( "," ) );
		for ( f in xfields ) {
			s.serialize( Reflect.getProperty( this, f ) );
		}
	}

	@:keep
	public function hxUnserialize( u : haxe.Unserializer ) {
		var sxfields	: String = u.unserialize();
		xfields = sxfields.split( "," );
		for ( f in xfields ) {
			Reflect.setProperty( this, f, u.unserialize() );
		}
	}
}

Nothing special here...

class B extends sys.db.Object {
	var id	: SId;
	
	@:relation( aID )
	public var a	: A;

	public function new() {
		super();
	}
}

And now the Main class (I'm sorry I tried your quick statements to create the table without success so I've done it using INSERT...:

class Main {
    static function main(){
        haxe.Serializer.USE_CACHE = true;
	sys.db.Manager.cnx = sys.db.Sqlite.open(":memory:");
	sys.db.Manager.cnx.request("CREATE TABLE A AS SELECT 1 id, 1 cID");
        sys.db.Manager.cnx.request("INSERT INTO `A` (`id`, `cID`) VALUES (2, 1), (3, 1)");
	sys.db.Manager.cnx.request("CREATE TABLE B AS SELECT 1 id, 1 aID");
        sys.db.Manager.cnx.request("CREATE TABLE C AS SELECT 1 id");
        
        var list	= Lambda.array( A.manager.unsafeObjects( "SELECT * FROM A WHERE cID = 1", false ) );	// <---- If lock == false then bug
	for( o in list ){
		o.xfields.push( "bs" );
		trace( o.xfields );
	}
	var ser	= Serializer.run( list );
	var userList : Array<A>	= Unserializer.run( ser );

	for( o in userList ){
		trace( o.xfields );
	}
    }
}

I'm sorry for this "swamp" but I would like to understand what is going on here.

As you'll see in the traces, if lock is true, then the unserialized objects have all their xfields filled, but if lock is false, only one of the unserialized objects will have the bs string added to its xfields. Then it's interesting to note that if there is no that var tmp = c.as; // <------- to get the call, then the bug in the A class, all is working fine even if lock is false.

If you could explain me what is going on, if it's a normal behaviour or if it's a bug please ?

Thanks Jonas !

[ConstNW]

https://github.com/HaxeFoundation/record-macros/blob/14ab3ed0497c045492e91578bdd7da75629c962a/src/sys/db/Manager.hx#L680-L709

[filt3rek]

Hej ConstNW,

I've seen that function and I undeerstand but what is confusing me globally is that there is no clear description of what impact lock has on everything.

I mean, there is lock field that, as the name suggest, should be used for DB operations, but instead it has also impact on cache, which is normal but in my case (and another ones I think) it ends in a strange and unexpected behaviour.

It would be great if we could make it more clear or maybe find a solution to not "override" an object that is yet used, by its cached version, which reinitialize all. Could something like that be possible ?

[jonasmalacofilho]

@ConstNW,

Thank you for tracking this down.

---

@filt3rek,

I remembered having looked into this before... #38.

In essence: all fields are currently cleared, and so far I have been reluctant to change this behavior.

The argument for clearing all fields, including @:skip ones, is that they may depend on other fields that are in the database. During a lock(), the consistency between @:skip fields and database fields may be lost, and there is no way for the caller to check this... so it's better to start fresh.

Additionally, I'm also concerned about backwards compatibility; in particular, (for code that expects the current behavior) allowing this loss of consistency would lead to silent and very surprising bugs.

That said, thinking about this for a second time after this long, I have to admit I'm also surprised by this behavior for something called @:skip. In fact, the current behavior would be better described as @:cache, or something like that. So maybe we should look into changing this.

Still, we should probably go through and specify all metas and APIs before experimenting with new behavior. (And @:skip should be replaced, not repurposed).

Which leads me to an important question: who are currently the heaviest users of record-macros? We may want their input when deciding about behavior that was previously ambiguous. (And it would be nice to have some extra sets of hands too).

[filt3rek]

Hej Jonas !

Thanks again !

I didn't remeber but yes in fact I've already wrote about the "strange behaviour" objects like in the thread you mention and globally I've meet many times "bugs" working with macro-records due to this same thing.

I understand that some fields depends on fields that are from DB, but some don't ( especially when explicitly tagged as @:skip) and the developper should care by himself about an unexpected result if he wants to play with his no db field. So once again IMHO these @:skip fields shouldn't be cleared.

Now I understand also the backwards compatibility problem and I'm also curious to see who use record-macros, because I know I use it very often and "hardly" and I feel a bit "penalized" for others :rofl:

Then I could also fork the manager and make my behaviour, depending of how it finish here maybe I'll do that.

At the end, I want you to know that I'm ready to help, "give hand" if I can, don't hesitate to tell me.

Thanks for all !