laravel-trend
laravel-trend copied to clipboard
sum() did not return right value.
I've a datetime
column named date
, I want to get sum of total
using perDay
and perMonth
, but it seems not giving the right value.
Please take a look at this specific line of code: https://github.com/Flowframe/laravel-trend/blob/dd00920b628e6e9655aeb6c59921248ee8ff0937/src/Trend.php#L94
Imo, it should be like this:
->groupByRaw($this->getSqlDate())
because while using perMonth or perDay, it no grouping rows by the formatted column like date_format(date, '%Y-%m')
instead it group using column like date
.
In this case, rows with same same day or month not summing up the total
.
Edit:
Here is the query of given example above using perDay
method:
select
date_format(date, '%Y-%m-%d') as date,
sum(total) as aggregate
from `sales` where `date` between '2022-05-23 00:00:00' and '2022-05-30 23:59:59' group by `date` order by `date` asc
if I have 2 records within the same day but different hour, it will return 2 record and not summed up the total.
Info: Laravel 9, MySQL, Apache
I am sorry for my bad English. Hope you all can understand what I am trying to say. Thank you !
I haven’t tested it, but I suspect it’s related to #8 and issues when the table has a field named ‘date’. I’ve submitted a PR awaiting approval: https://github.com/Flowframe/laravel-trend/pull/13
Ah, I have noticed that. Maybe it because I have a column named date
so when the query builder trying to group by 'date' it refers to my column named date
instead of the alias.
Hey there! This issue was solved? I think the sum() function is not working:
Hello. I have encountered the same problem.
PR #28 allows us the change the alias used for aggregating. However, when I set the alias as such:
$trend->dateAlias = 'aggregation_date';
return $trend->dateColumn('date')
->between(
start: now()->sub($this->filter, 1),
end: now(),
)
I get an errorException Undefined property: stdClass::$date
It seems to me like a field might have been overlooked when the dateAlias feature got introduced. The aggregate()
method passes its values through the mapValuesToDates()
method before returning, which does the following:
$values = $values->map(fn ($value) => new TrendValue(
date: $value->date,
aggregate: $value->aggregate,
));
It looks like this method presumes the presence of a date
property in the values collection, even though the alias might have changed.
Am I using dateAlias
incorrectly, or is this indeed unintended?
This could be fixed - if a fix is in order - like so:
date: $value->{$this->dateAlias},
I'm sorry if I am completely mistaken or this is not the right place for mentioning it. I hope it is useful in some way.
Thank you for your work.