FCO / Red

A WiP ORM for Raku
Artistic License 2.0
70 stars 27 forks source link

Unexpected behaviour of parameterised clause in `join-model` #571

Closed jonathanstowe closed 8 months ago

jonathanstowe commented 8 months ago

Given an example that uses a parameterised clause in the &on argument to join-model:

use Red;

my $*RED-DB = database "SQLite";
my $*RED-DEBUG = True;

model Bar::Foo is table('foo') {
    has Str $.a is column;
    has Str $.b is column;
    has Str $.c is column;
    has Date $.foo-date is column = Date.today;

    method get-foos(Date $date) {
        my Date $today = $date;
        my Date $yesterday = $today.earlier(days => 1);
        self.^rs.grep(*.foo-date eq $yesterday ).join-model( self, -> $a, $b {  $a.a eq $b.a   && $a.b eq $b.b && $a.c eq $b.c  && $b.foo-date eq $today }, name => "foo_2" )
    }
}

Bar::Foo.^create-table;

Bar::Foo.get-foos(Date.today).elems;
Bar::Foo.get-foos(Date.today.earlier(days => 1)).elems;

You'll get:

SQL : CREATE TABLE "foo" (
   a varchar(255) NOT NULL ,
   b varchar(255) NOT NULL ,
   c varchar(255) NOT NULL ,
   foo_date varchar(255) NOT NULL 
)
BIND: []
SQL : SELECT
   count('*') as "data_1"
FROM
   "foo"
    INNER JOIN "foo" as foo_2 ON "foo".b = "foo_2".b AND "foo".c = "foo_2".c AND "foo".a = "foo_2".a AND "foo_2".foo_date = ?
WHERE
   "foo".foo_date = ?
LIMIT 1
BIND: ["2024-02-20", "2024-02-19"]
SQL : SELECT
   count('*') as "data_1"
FROM
   "foo"
    INNER JOIN "foo" as foo_2 ON "foo".a = "foo_2".a AND "foo".b = "foo_2".b AND "foo".c = "foo_2".c AND "foo_2".foo_date = ?
WHERE
   "foo".foo_date = ?
LIMIT 1
BIND: ["2024-02-20", "2024-02-18"]

As you can see in the second invocation it has retained the value of $today from the first invocation. This is consistent how ever many times it is run in the same process. Obviously this would be surprising in a long running application.

This can be avoided by moving the parameterised clause into a subsequent filter:

use Red;

my $*RED-DB = database "SQLite";
my $*RED-DEBUG = True;

model Bar::Foo is table('foo') {
    has Str $.a is column;
    has Str $.b is column;
    has Str $.c is column;
    has Date $.foo-date is column = Date.today;

    method get-foos(Date $date) {
        my Date $today = $date;
        my Date $yesterday = $today.earlier(days => 1);
        self.^rs.grep(*.foo-date eq $yesterday ).join-model( self, -> $a, $b {  $a.a eq $b.a   && $a.b eq $b.b && $a.c eq $b.c  }, name => "foo_2" ).grep(-> $v { $v.foo-date eq $today })
    }
}

Bar::Foo.^create-table;

Bar::Foo.get-foos(Date.today).elems;
Bar::Foo.get-foos(Date.today.earlier(days => 1)).elems;

Giving:

SQL : CREATE TABLE "foo" (
   a varchar(255) NOT NULL ,
   b varchar(255) NOT NULL ,
   c varchar(255) NOT NULL ,
   foo_date varchar(255) NOT NULL 
)
BIND: []
SQL : SELECT
   count('*') as "data_1"
FROM
   "foo"
    INNER JOIN "foo" as foo_2 ON "foo".a = "foo_2".a AND "foo".b = "foo_2".b AND "foo".c = "foo_2".c
WHERE
   "foo".foo_date = ? AND "foo_2".foo_date = ?
LIMIT 1
BIND: ["2024-02-19", "2024-02-20"]
SQL : SELECT
   count('*') as "data_1"
FROM
   "foo"
    INNER JOIN "foo" as foo_2 ON "foo".b = "foo_2".b AND "foo".a = "foo_2".a AND "foo".c = "foo_2".c
WHERE
   "foo".foo_date = ? AND "foo_2".foo_date = ?
LIMIT 1
BIND: ["2024-02-18", "2024-02-19"]

I'm not actually sure whether this is a real bug or that my expectations are incorrect, but I do think that it should at least be documented somewhere to avoid some scratching of heads.

FCO commented 8 months ago

Thanks. To me it looks like a bug. I'll try to take a look at it tomorrow.

jonathanstowe commented 8 months ago

I had a quick look to try and understand what was going on and didn't find anything obvious, but I was in a bit of a hurry to fix the problem it was causing in the application. Lazily in the tests I only ran the method once.

FCO commented 8 months ago

It seems the problem is the alias generated being cached (https://github.com/FCO/Red/blob/master/lib/MetamodelX/Red/Model.rakumod#L255). I'm trying to discover a better cache key to avoid that. If I can't find it, do you think it would be better to not cache at all? (I commented that line and that worked...)

FCO commented 8 months ago

Ok... It seems to work now. Could you confirm, please?

jonathanstowe commented 8 months ago

Yep that's all good now :+1: