Skip to content

Commit 328bd4a

Browse files
ReeceGodingReeceGoding
committed
Moved all validation of the regression mode parameters to be before we build up the table variable. Fixes #611 and is very similar to #602.
1 parent 6715663 commit 328bd4a

1 file changed

Lines changed: 141 additions & 139 deletions

File tree

‎sp_QuickieStore/sp_QuickieStore.sql‎

Lines changed: 141 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,147 @@ BEGIN
516516
@sort_order_is_a_wait = 1;
517517
END;
518518

519+
/*
520+
We also validate regression mode super early.
521+
We need to do this here so we can build @ColumnDefinitions correctly.
522+
It also lets us fail fast, if needed.
523+
*/
524+
DECLARE
525+
@regression_mode bit;
526+
527+
/*
528+
Set @regression_mode if the given arguments indicate that
529+
we are checking for regressed queries.
530+
Also set any default parameters for regression mode while we're at it.
531+
*/
532+
IF @regression_baseline_start_date IS NOT NULL
533+
BEGIN
534+
SELECT
535+
@regression_mode = 1,
536+
@regression_comparator =
537+
ISNULL(@regression_comparator, 'absolute'),
538+
@regression_direction =
539+
ISNULL(@regression_direction, 'regressed');
540+
END;
541+
542+
/*
543+
Error out if the @regression parameters do not make sense.
544+
*/
545+
IF
546+
(
547+
@regression_baseline_start_date IS NULL
548+
AND
549+
(
550+
@regression_baseline_end_date IS NOT NULL
551+
OR @regression_comparator IS NOT NULL
552+
OR @regression_direction IS NOT NULL
553+
)
554+
)
555+
BEGIN
556+
RAISERROR('@regression_baseline_start_date is mandatory if you have specified any other @regression_ parameter.', 11, 1) WITH NOWAIT;
557+
RETURN;
558+
END;
559+
560+
/*
561+
Error out if the @regression_baseline_start_date and
562+
@regression_baseline_end_date are incompatible.
563+
We could try and guess a sensible resolution, but
564+
I do not think that we can know what people want.
565+
*/
566+
IF
567+
(
568+
@regression_baseline_start_date IS NOT NULL
569+
AND @regression_baseline_end_date IS NOT NULL
570+
AND @regression_baseline_start_date >= @regression_baseline_end_date
571+
)
572+
BEGIN
573+
RAISERROR('@regression_baseline_start_date has been set greater than or equal to @regression_baseline_end_date.
574+
This does not make sense. Check that the values of both parameters are as you intended them to be.', 11, 1) WITH NOWAIT;
575+
RETURN;
576+
END;
577+
578+
/*
579+
Validate @regression_comparator.
580+
*/
581+
IF
582+
(
583+
@regression_comparator IS NOT NULL
584+
AND @regression_comparator NOT IN ('relative', 'absolute')
585+
)
586+
BEGIN
587+
RAISERROR('The regression_comparator (%s) you chose is so out of this world that I''m using ''absolute'' instead', 10, 1, @regression_comparator) WITH NOWAIT;
588+
589+
SELECT
590+
@regression_comparator = 'absolute';
591+
END;
592+
593+
/*
594+
Validate @regression_direction.
595+
*/
596+
IF
597+
(
598+
@regression_direction IS NOT NULL
599+
AND @regression_direction NOT IN ('regressed', 'worse', 'improved', 'better', 'magnitude', 'absolute')
600+
)
601+
BEGIN
602+
RAISERROR('The regression_direction (%s) you chose is so out of this world that I''m using ''regressed'' instead', 10, 1, @regression_direction) WITH NOWAIT;
603+
604+
SELECT
605+
@regression_direction = 'regressed';
606+
END;
607+
608+
/*
609+
Error out if we're trying to do regression mode with 'recent'
610+
as our @sort_order. How could that ever make sense?
611+
*/
612+
IF
613+
(
614+
@regression_mode = 1
615+
AND @sort_order = 'recent'
616+
)
617+
BEGIN
618+
RAISERROR('Your @sort_order is ''recent'', but you are trying to compare metrics for two time periods.
619+
If you can imagine a useful way to do that, then make a feature request.
620+
Otherwise, either stop specifying any @regression_ parameters or specify a different @sort_order.', 11, 1) WITH NOWAIT;
621+
RETURN;
622+
END;
623+
624+
/*
625+
Error out if we're trying to do regression mode with 'plan count by hashes'
626+
as our @sort_order. How could that ever make sense?
627+
*/
628+
IF
629+
(
630+
@regression_mode = 1
631+
AND @sort_order = 'plan count by hashes'
632+
)
633+
BEGIN
634+
RAISERROR('Your @sort_order is ''plan count by hashes'', but you are trying to compare metrics for two time periods.
635+
This is probably not useful, since our method of comparing two time period relies on only checking query hashes that are in both time periods.
636+
If you can imagine a useful way to do that, then make a feature request.
637+
Otherwise, either stop specifying any @regression_ parameters or specify a different @sort_order.', 11, 1) WITH NOWAIT;
638+
RETURN;
639+
END;
519640

641+
/*
642+
Error out if @regression_comparator tells us to use division,
643+
but @regression_direction tells us to take the modulus.
644+
It doesn't make sense to specifically ask us to remove the sign
645+
of something that doesn't care about it.
646+
*/
647+
IF
648+
(
649+
@regression_comparator = 'relative'
650+
AND @regression_direction IN ('absolute', 'magnitude')
651+
)
652+
BEGIN
653+
RAISERROR('Your @regression_comparator is ''relative'', but you have asked for an ''absolute'' or ''magnitude'' @regression_direction. This is probably a mistake.
654+
Your @regression_direction tells us to take the absolute value of our result of comparing the metrics in the current time period to the baseline time period,
655+
but your @regression_comparator is telling us to use division to compare the two time periods. This is unlikely to produce useful results.
656+
If you can imagine a useful way to do that, then make a feature request. Otherwise, either change @regression_direction to another value
657+
(e.g. ''better'' or ''worse'') or change @regression_comparator to ''absolute''.', 11, 1) WITH NOWAIT;
658+
RETURN;
659+
END;
520660

521661
/*
522662
These are the tables that we'll use to grab data from query store
@@ -1764,7 +1904,6 @@ DECLARE
17641904
@work_end_utc time(0),
17651905
@regression_baseline_start_date_original datetimeoffset(7),
17661906
@regression_baseline_end_date_original datetimeoffset(7),
1767-
@regression_mode bit,
17681907
@regression_where_clause nvarchar(max),
17691908
@column_sql nvarchar(max),
17701909
@param_name nvarchar(100),
@@ -1847,138 +1986,6 @@ SELECT
18471986
)
18481987
);
18491988

1850-
/*
1851-
Set @regression_mode if the given arguments indicate that
1852-
we are checking for regressed queries.
1853-
*/
1854-
IF @regression_baseline_start_date IS NOT NULL
1855-
BEGIN
1856-
SELECT
1857-
@regression_mode = 1;
1858-
END;
1859-
1860-
/*
1861-
Error out if the @regression parameters do not make sense.
1862-
*/
1863-
IF
1864-
(
1865-
@regression_baseline_start_date IS NULL
1866-
AND
1867-
(
1868-
@regression_baseline_end_date IS NOT NULL
1869-
OR @regression_comparator IS NOT NULL
1870-
OR @regression_direction IS NOT NULL
1871-
)
1872-
)
1873-
BEGIN
1874-
RAISERROR('@regression_baseline_start_date is mandatory if you have specified any other @regression_ parameter.', 11, 1) WITH NOWAIT;
1875-
RETURN;
1876-
END;
1877-
1878-
/*
1879-
Error out if the @regression_baseline_start_date and
1880-
@regression_baseline_end_date are incompatible.
1881-
We could try and guess a sensible resolution, but
1882-
I do not think that we can know what people want.
1883-
*/
1884-
IF
1885-
(
1886-
@regression_baseline_start_date IS NOT NULL
1887-
AND @regression_baseline_end_date IS NOT NULL
1888-
AND @regression_baseline_start_date >= @regression_baseline_end_date
1889-
)
1890-
BEGIN
1891-
RAISERROR('@regression_baseline_start_date has been set greater than or equal to @regression_baseline_end_date.
1892-
This does not make sense. Check that the values of both parameters are as you intended them to be.', 11, 1) WITH NOWAIT;
1893-
RETURN;
1894-
END;
1895-
1896-
1897-
/*
1898-
Validate @regression_comparator.
1899-
*/
1900-
IF
1901-
(
1902-
@regression_comparator IS NOT NULL
1903-
AND @regression_comparator NOT IN ('relative', 'absolute')
1904-
)
1905-
BEGIN
1906-
RAISERROR('The regression_comparator (%s) you chose is so out of this world that I''m using ''absolute'' instead', 10, 1, @regression_comparator) WITH NOWAIT;
1907-
1908-
SELECT
1909-
@regression_comparator = 'absolute';
1910-
END;
1911-
1912-
/*
1913-
Validate @regression_direction.
1914-
*/
1915-
IF
1916-
(
1917-
@regression_direction IS NOT NULL
1918-
AND @regression_direction NOT IN ('regressed', 'worse', 'improved', 'better', 'magnitude', 'absolute')
1919-
)
1920-
BEGIN
1921-
RAISERROR('The regression_direction (%s) you chose is so out of this world that I''m using ''regressed'' instead', 10, 1, @regression_direction) WITH NOWAIT;
1922-
1923-
SELECT
1924-
@regression_direction = 'regressed';
1925-
END;
1926-
1927-
/*
1928-
Error out if we're trying to do regression mode with 'recent'
1929-
as our @sort_order. How could that ever make sense?
1930-
*/
1931-
IF
1932-
(
1933-
@regression_mode = 1
1934-
AND @sort_order = 'recent'
1935-
)
1936-
BEGIN
1937-
RAISERROR('Your @sort_order is ''recent'', but you are trying to compare metrics for two time periods.
1938-
If you can imagine a useful way to do that, then make a feature request.
1939-
Otherwise, either stop specifying any @regression_ parameters or specify a different @sort_order.', 11, 1) WITH NOWAIT;
1940-
RETURN;
1941-
END;
1942-
1943-
/*
1944-
Error out if we're trying to do regression mode with 'plan count by hashes'
1945-
as our @sort_order. How could that ever make sense?
1946-
*/
1947-
IF
1948-
(
1949-
@regression_mode = 1
1950-
AND @sort_order = 'plan count by hashes'
1951-
)
1952-
BEGIN
1953-
RAISERROR('Your @sort_order is ''plan count by hashes'', but you are trying to compare metrics for two time periods.
1954-
This is probably not useful, since our method of comparing two time period relies on only checking query hashes that are in both time periods.
1955-
If you can imagine a useful way to do that, then make a feature request.
1956-
Otherwise, either stop specifying any @regression_ parameters or specify a different @sort_order.', 11, 1) WITH NOWAIT;
1957-
RETURN;
1958-
END;
1959-
1960-
1961-
/*
1962-
Error out if @regression_comparator tells us to use division,
1963-
but @regression_direction tells us to take the modulus.
1964-
It doesn't make sense to specifically ask us to remove the sign
1965-
of something that doesn't care about it.
1966-
*/
1967-
IF
1968-
(
1969-
@regression_comparator = 'relative'
1970-
AND @regression_direction IN ('absolute', 'magnitude')
1971-
)
1972-
BEGIN
1973-
RAISERROR('Your @regression_comparator is ''relative'', but you have asked for an ''absolute'' or ''magnitude'' @regression_direction. This is probably a mistake.
1974-
Your @regression_direction tells us to take the absolute value of our result of comparing the metrics in the current time period to the baseline time period,
1975-
but your @regression_comparator is telling us to use division to compare the two time periods. This is unlikely to produce useful results.
1976-
If you can imagine a useful way to do that, then make a feature request. Otherwise, either change @regression_direction to another value
1977-
(e.g. ''better'' or ''worse'') or change @regression_comparator to ''absolute''.', 11, 1) WITH NOWAIT;
1978-
RETURN;
1979-
END;
1980-
1981-
19821989
/*
19831990
Set the _original variables, as we have
19841991
for other values that would break inside
@@ -2893,7 +2900,6 @@ END;
28932900

28942901
/*
28952902
As above, but for @regression_baseline_start_date and @regression_baseline_end_date.
2896-
We set the other @regression_ variables while we're at it.
28972903
*/
28982904
IF @regression_mode = 1
28992905
BEGIN
@@ -2914,11 +2920,7 @@ We set both _date_original variables earlier.
29142920
MINUTE,
29152921
@utc_minutes_difference,
29162922
@regression_baseline_end_date_original
2917-
),
2918-
@regression_comparator =
2919-
ISNULL(@regression_comparator, 'absolute'),
2920-
@regression_direction =
2921-
ISNULL(@regression_direction, 'regressed');
2923+
);
29222924
END;
29232925

29242926
/*

0 commit comments

Comments
 (0)