August 29, 2022 at 7:34 pm
This was removed by the editor as SPAM
August 30, 2022 at 6:05 pm
OK, New Code here for critique! Thanks to all. What else can I clean up?
Select cust.fcustno, fcompany, fcreated, cust.fsalespn, Sum(fnamount) as Sales,
Cast(sum(fnamount) *.05 as numeric(10,2)) as commission, 'Existing' as type
from slcdpmx cust
inner join (select fcustno, fnamount, finvdate
from armast where
fcustno in (Select fcustno from armast where finvdate < Case when Month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19060101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19050101', current_timestamp), '19000101')
end)
and fcustno in (Select fcustno from armast where finvdate > Case when month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19010101', current_timestamp), '19000101')
else
Dateadd(year, DateDiff(year,'19000101', current_timestamp), '19000101')
End )
and fcustno not in (Select fcustno from armast where finvdate > Case when Month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19060101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19050101', current_timestamp), '19000101')
End
and finvdate < Dateadd(year, DateDiff(year,'19000101', current_timestamp), '19000101')))
ar
on ar.fcustno = cust.fcustno and finvdate >=
Dateadd(Quarter, DateDiff(Quarter,'19000401', current_timestamp), '19000101')
where fcreated <= Case when Month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19060101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19050101', current_timestamp), '19000101')
End
Group by cust.fcustno, fcompany, fcreated, cust.fsalespn
union
Select cust.fcustno, fcompany, fcreated, cust.fsalespn, sum(fnamount) as sales,
Cast(sum(fnamount)*.05 as numeric(10,2)) as commission, 'New' as type
from slcdpmx cust
inner join armast ar on ar.fcustno = cust.fcustno and finvdate >= Case when month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19010101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19000101', current_timestamp), '19000101')
End
where (fcreated >= Case when month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19010101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19000101', current_timestamp), '19000101')
End)
and (finvdate >= Dateadd(Quarter, DateDiff(Quarter,'19000401', current_timestamp), '19000101'))
Group by cust.fcustno, fcompany, fcreated, cust.fsalespn
order by type, fcreated, fcustno
August 30, 2022 at 8:34 pm
This was removed by the editor as SPAM
August 30, 2022 at 10:35 pm
This is a personal preference, but I find your case structure difficult to parse and comprehend.
Instead of your
where finvdate < Case when Month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19060101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19050101', current_timestamp), '19000101')
end
I prefer the use of DateFromParts instead. I find it much better describes what is going on, namely going to the start of the year 5 og 6 years prior to the current year, depending on whether the current month is January or not:
where finvdate < DateFromParts(Year(current_timestamp)-IIf(Month(current_timestamp)=1, 6, 5), 1, 1)
August 30, 2022 at 11:23 pm
I would go further than that - using better names that make it clear what each date is
with dates as
(select DateFromParts(Year(current_timestamp)-IIf(Month(current_timestamp)=1, 6, 5), 1, 1) as Year_minus_5_or_6
, DateFromParts(Year(current_timestamp)-IIf(Month(current_timestamp)=1, 1, 0), 1, 1) as Year_minus_0_or_1
, DateFromParts(Year(current_timestamp), 1, 1) as Year_start
, Dateadd(Quarter, DateDiff(Quarter,'19000401', current_timestamp), '19000101') as Quarter_year
)
Select cust.fcustno, fcompany, fcreated, cust.fsalespn, Sum(fnamount) as Sales,
Cast(sum(fnamount) *.05 as numeric(10,2)) as commission, 'Existing' as type
from slcdpmx cust
inner join (select fcustno, fnamount, finvdate
from armast
where fcustno in (Select fcustno
from armast
where finvdate < dates.Year_minus_5_or_6
)
and fcustno in (Select fcustno
from armast
where finvdate > dates.Year_minus_0_or_1
)
and fcustno not in (Select fcustno
from armast
where finvdate > dates.Year_minus_5_or_6
and finvdate < dates.Year_start
)
) ar
on ar.fcustno = cust.fcustno
and finvdate >= dates.Quarter_year
where fcreated <= dates.Year_minus_5_or_6
Group by cust.fcustno, fcompany, fcreated, cust.fsalespn
union
Select cust.fcustno, fcompany, fcreated, cust.fsalespn, sum(fnamount) as sales,
Cast(sum(fnamount)*.05 as numeric(10,2)) as commission, 'New' as type
from slcdpmx cust
inner join armast ar
on ar.fcustno = cust.fcustno
and finvdate >= dates.Year_minus_0_or_1
where (fcreated >= dates.Year_minus_0_or_1
)
and (finvdate >= dates.Quarter_year
)
Group by cust.fcustno, fcompany, fcreated, cust.fsalespn
order by type, fcreated, fcustno
August 31, 2022 at 11:28 am
That is very interesting. I did not know you could do WITH in TSQL. That opens up a new avenue. Thanks. Also DateFromParts is something I never knew too.
August 31, 2022 at 2:12 pm
This is a personal preference, but I find your case structure difficult to parse and comprehend.
Instead of your
where finvdate < Case when Month(current_timestamp) = 1 then
Dateadd(year, DateDiff(year,'19060101', current_timestamp), '19000101')
Else
Dateadd(year, DateDiff(year,'19050101', current_timestamp), '19000101')
endI prefer the use of DateFromParts instead. I find it much better describes what is going on, namely going to the start of the year 5 og 6 years prior to the current year, depending on whether the current month is January or not:
where finvdate < DateFromParts(Year(current_timestamp)-IIf(Month(current_timestamp)=1, 6, 5), 1, 1)
I prefer simplifying the code by using the correct reference point. For instance, if you use the previous month instead of the current month as the reference point you can replace
where finvdate < DateFromParts(Year(current_timestamp)-IIf(Month(current_timestamp)=1, 6, 5), 1, 1)
with
where finvdate < DateFromParts(Year(DateAdd(MONTH, -1, current_timestamp)) - 5), 1, 1)
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
August 31, 2022 at 3:02 pm
So the wrong solution to the wrong problem, What you actually need is a table (or tables) that ulimately hold the commission allocation details: SalesMan, Customer, Start, End and link this up to the orders/shipments/payments/credits/returns/bonuses/penalties
As to the original problem, find customers with transactions before X and after Y but with no transactions between X and Y, I would have approached this as three separate CTEs: Select Customers before X, Select customers after Y and Select customers between X and Y
SELECT CTE_Outer AS
(
SELECT * FROM CTE_Before
INTERSECT
SELECT * FROM CTE_Before
)
, CTE_Exclude AS
(
SELECT * FROM CTE_Between
EXCEPT
SELECT * FROM CTE_Outer
)
personally I find it much easier to reason about and you can unit test the three starting CTEs.
September 1, 2022 at 3:26 pm
So the wrong solution to the wrong problem, What you actually need is a table (or tables) that ulimately hold the commission allocation details: SalesMan, Customer, Start, End and link this up to the orders/shipments/payments/credits/returns/bonuses/penalties
As to the original problem, find customers with transactions before X and after Y but with no transactions between X and Y, I would have approached this as three separate CTEs: Select Customers before X, Select customers after Y and Select customers between X and Y
SELECT CTE_Outer AS
(
SELECT * FROM CTE_Before
INTERSECT
SELECT * FROM CTE_Before
)
, CTE_Exclude AS
(
SELECT * FROM CTE_Between
EXCEPT
SELECT * FROM CTE_Outer
)personally I find it much easier to reason about and you can unit test the three starting CTEs.
I still think that querying the same table multiple times is inefficient. I think that having a group by on the customer with amounts (or counts) for ancient history, recent history, and current period where the recent history is zero and the others are not would be better.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
Viewing 9 posts - 16 through 23 (of 23 total)
You must be logged in to reply to this topic. Login to reply