FCO / Red

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

Aggregates in e.g. map #514

Closed jonathanstowe closed 1 year ago

jonathanstowe commented 3 years ago

It would be handy to be able to express aggregate functions in a .map, .classify set so one can do something like:

select a, b, max(c)
from foo
group by a, b;

It strikes me as simply providing for methods on the Red::Column that generate the Red::AST::Function for , say, min, max', sum, avg etc.

It would also be good to able to express similar for a related resultseq for

select foo.a, foo.b, max(bar.c)
from foo
join bar on bar.foo_id = foo.id
group by foo.a, foo.b;

I'm not so sure about the syntax for this but something like:

Foo.^all.map({ .a, .b, .bars.max(*.c) })

Or something like that.

FCO commented 3 years ago

I've being thinking on something like

Foo.^all.map({ .a, .b, .bars.c.max })

and also make $foo.bars.c.max be a thing.

I plan to include .max, .sum, .avg on https://github.com/FCO/Red/blob/master/lib/Red/ColumnMethods.pm6

What do you think?

FCO commented 3 years ago

but now thinking... I think you option makes more sense!

and also make $foo.bars.max: *.c a thing

jonathanstowe commented 3 years ago

All good.

and also make $foo.bars.max: *.c a thing

Yeah otherwise you'd need to push the column accessors onto the ResultSeq, which becomes really complicated.

FCO commented 2 years ago

I'm trying to figure out how to do that... the problem is .bars on Foo.^all.map({ .a, .b, .bars.c.max }) (that I agree should be the syntax for doing that) returns a ResultSeq. I was able to make ResultSeq.min return the AST::Function when inside a map and a new ResultSeq when outside it... But that's not enough, the .map should be able to recognise it and handle the joins and the group by. for now that's what I have:

image
FCO commented 2 years ago

Maybe the solution is stop returning ResultSeq when calling @ relationship on type object and create and return a new UndefinedRelationship that would have the aggregate methods.

FCO commented 2 years ago

Maybe this:

Foo.^all.classify({ .a, .b }, :as{ .bars.max(*.c) })

should run:

select a, b, max(c)
from foo
group by a, b;

And:

Foo.^all.map({ .a, .b, .bars.max(*.c) })

should do something like:

SELECT
   a,
   b,
   (
      SELECT
         MAX(c)
      FROM
         foo f
      WHERE
         f.id = id
   ) as max
FROM
   foo
jonathanstowe commented 2 years ago

Yeah that looks sane, as long as the result is a further ResultSeq so it can be filtered, joined or whatever that would work.

FCO commented 2 years ago
image
jonathanstowe commented 2 years ago

Now I have to remember what I was doing when I found this would be useful :rofl:

FCO commented 2 years ago

Sorry for the delay…

jonathanstowe commented 2 years ago

Ah. With Pg, where there is a column alias on the column that is being aggregated it adds the as which is a syntax error

i.e you get something like :

( SELECT
      max("stats_view_refresh".refresh_time as "refresh-time") as "data_1"
   FROM
      "stats_view_refresh" )

And get :

    Unknown Error!!!
    Please, copy this backtrace and open an issue on https://github.com/FCO/Red/issues/new
    Driver: Red::Driver::Pg
    Original error: DB::Pg::Error::FatalError.new(message => "syntax error at or near \"as\"", message-detail => Str, message-hint => Str, context => Str, type => "ERROR", type-localized => "ERROR", state => "42601", statement-position => "1063", internal-position => Str, internal-query => Str, schema => Str, table => Str, column => Str, datatype => Str, constraint => Str, source-file => "scan.l", source-line => "1129", source-function => "scanner_yyerror")

Something like :

diff --git a/lib/Red/Driver/CommonSQL.pm6 b/lib/Red/Driver/CommonSQL.pm6
index 0732450..a2810b1 100644
--- a/lib/Red/Driver/CommonSQL.pm6
+++ b/lib/Red/Driver/CommonSQL.pm6
@@ -472,7 +472,7 @@ multi method translate(Red::AST::DateTimeFunction $_, $context?) {
 multi method translate(Red::AST::Function $_, $context?) {
     my @bind;
     "{ .func }({ .args.map({
-        my ($s, @b) := do given self.translate: $_, $context { .key, .value }
+        my ($s, @b) := do given self.translate: $_, 'func' { .key, .value }
         @bind.append: @b;
         $s
     }).join: ", " })" => @bind

Fixes it.

jonathanstowe commented 2 years ago

The tests pass with the above.

For reference the problem was happening because it was getting the select context from the caller and subsequently gets the translate for Red::Column which adds the alias.

jonathanstowe commented 2 years ago

The above is a PR for the previous diff.

FCO commented 2 years ago

Is that working now?

jonathanstowe commented 2 years ago

I'll test today.

jonathanstowe commented 2 years ago

Yep that all works fine :+1:

jonathanstowe commented 2 years ago

Actually, I think there may be some concurrency problem in there, I'm seeing this intermittently:

Wed, 18 May 2022 06:11:47 +0000 [Debug] YNAP::Hesabu::Notifier notify-session : Sending Upload of STO_MRP_Stock_Misalignment_20220518.csv completed for hwA16dL0nxYMFpoiJpb3j2fb2gOPc4bm2i5q2EE9HcHblu3iilWWGShFJTtLERa6
No such method 'has-one-relationships' for invocant of type
'Perl6::Metamodel::ClassHOW'
  in method ast at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 410
  in method agg at /usr/share/perl6/site/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 6
  in method max at /usr/share/perl6/site/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 17
  in method view-needs-refreshing at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 41
  in method refresh-view-if-required at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 46
  in block  at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 16
  in block  at /usr/share/perl6/site/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 201
  in block  at /usr/share/perl6/site/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 197

Wed, 18 May 2022 06:11:49 +0000 [Error] YNAP::Hesabu::HTTP::Logger  : [ERROR] 500 /chart-data - 10.228.129.103
Wed, 18 May 2022 06:26:21 +0000 [Info] YNAP::Hesabu::Route::Upload  : Upload of STO_NAP_Stock_Misalignment_20220518.csv completed
Wed, 18 May 2022 06:26:21 +0000 [Debug] YNAP::Hesabu::Notifier notify-session : Sending Upload of STO_NAP_Stock_Misalignment_20220518.csv completed for hwA16dL0nxYMFpoiJpb3j2fb2gOPc4bm2i5q2EE9HcHblu3iilWWGShFJTtLERa6
No such method 'has-one-relationships' for invocant of type
'Perl6::Metamodel::ClassHOW'
  in method ast at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 410
  in method iterator at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 95
  in method Seq at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 101
  in method do-it at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 106
  in method head at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 286
  in method elems at /usr/share/perl6/site/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 304
  in method view-needs-refreshing at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 42
  in method refresh-view-if-required at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 46
  in block  at /hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 16
  in block  at /usr/share/perl6/site/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 201
  in block  at /usr/share/perl6/site/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 197

Wed, 18 May 2022 06:26:23 +0000 [Error] YNAP::Hesabu::HTTP::Logger  : [ERROR] 500 /chart-data - 10.228.129.103

I'm digging in to see if I can replicate outside the web app.

jonathanstowe commented 2 years ago

It seems to boil down to the $.of of the final ResultSeq not being set correctly, though I'm not sure how this is happening.

jonathanstowe commented 2 years ago

FWIW, I'm pretty certain that it is some concurrency issue, I've fixed it from being a problem in my application by wrapping the offending code in a Lock, not ideal but it works. Will still look at the underlying issue.

FCO commented 2 years ago

Reproduced:

image
FCO commented 2 years ago

@jonathanstowe could you please confirm if the race condition was fixed, please?

FCO commented 2 years ago

This seems to be fixed to me...

jonathanstowe commented 2 years ago

I'm still seeing the same thing:

No such method 'has-one-relationships' for invocant of type
'Perl6::Metamodel::ClassHOW'
  in method ast at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 419
  in method agg at /home/jonathan/.raku/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 6
  in method max at /home/jonathan/.raku/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 17
  in method view-needs-refreshing at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 41
  in method refresh-view-if-required at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 46
  in block  at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 16
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 201
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 197

No such method 'has-one-relationships' for invocant of type
'Perl6::Metamodel::ClassHOW'
  in method ast at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 419
  in method agg at /home/jonathan/.raku/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 6
  in method max at /home/jonathan/.raku/sources/9EDF974E0AD8B044AC097AD680D062754CF70314 (Red::ResultSeqMethods) line 17
  in method view-needs-refreshing at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 41
  in method refresh-view-if-required at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 46
  in block  at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 16
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 201
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 197

No such method 'has-one-relationships' for invocant of type
'Perl6::Metamodel::ClassHOW'
  in method ast at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 419
  in method iterator at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 102
  in method Seq at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 109
  in method do-it at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 115
  in method head at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 295
  in method elems at /home/jonathan/.raku/sources/14541EFEF63E0E567F36634F7459B1B64EF650D6 (Red::ResultSeq) line 313
  in method view-needs-refreshing at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 42
  in method refresh-view-if-required at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 46
  in block  at /home/jonathan/working/NAP/hesabu/lib/YNAP/Hesabu/Route/Data.rakumod (YNAP::Hesabu::Route::Data) line 16
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 201
  in block  at /home/jonathan/.raku/sources/BB802F84B227D34EB41F3FA87E22BD753F6E4033 (Cro::HTTP::Router) line 197

Sorry it took so long to get back, been a bit busy...

FCO commented 2 years ago

Does t/71-agg-methods.t work for you?

jonathanstowe commented 2 years ago

Yeah, that's fine, I think that this may be harder to reproduce than it appears. I'll try and knock something up that I can share with you, the failing code is a bit complex.

FCO commented 1 year ago

Please, reopen if you find any problem