GROUP BY

21 Jul 2005 9:13 PM

James T.

Hello!

I am using following dynamic stored procedure to return search results.
Evrything works fine... Now I would like to group results by IsFeatured
field, but I am receiving error message: "Column 'Table1.Id' is invalid in
the select list because it is not contained in either an aggregate function
or the GROUP BY clause."

Please help me!

James

---------------------------------------------------

CREATE PROCEDURE [_Search]

@PriceMin       Money = 0,
@PriceMax         Money = 0,
@DistanceMin      int = 0,
@DistanceMax      int = 0,
@HighwayIds      varchar(50) = NULL

AS

DECLARE @sql       nvarchar(4000),
@paramlist         nvarchar(4000)

SELECT @sql = 'SELECT
       Table1.* FROM [Table1]'

       IF @HighwayIds IS NOT NULL
       SELECT @sql = @sql + 'JOIN IntlistToTable(@HighwayIds) S ON
Table1.HighwayId = S.number '

       SELECT @sql = @sql + 'WHERE 1 = 1 '

       IF (@PriceMin <> 0) AND (@PriceMax <> 0)
       SELECT @sql = @sql + ' AND (Price Between @PriceMin And @PriceMax)'

       IF (@PriceMin = 0) AND (@PriceMax <> 0)
       SELECT @sql = @sql + ' AND (Price Between 0 And @PriceMax)'

       IF (@DistanceFromMKADMin <> 0) AND (@DistanceFromMKADMax <> 0)
       SELECT @sql = @sql + ' AND (Distance Between @DistanceMin And
@DistanceMax)'

       IF (@DistanceFromMKADMin = 0) AND (@DistanceFromMKADMax <> 0)
       SELECT @sql = @sql + ' AND (Distance Between 0 And @DistanceMax)'

       SELECT @sql = @sql + ' And (IsActive = 1)'

       SELECT @paramlist = '@PriceMin               Money,
                           @PriceMax                Money,
                           @DistanceMin     int,
                           @DistanceMax     int,
                           @HighwayIds              varchar(50)'

   EXEC sp_executesql @sql, @paramlist, @PriceMin, @PriceMax, @DistanceMin,
@DistanceMax, @HighwayIds

22 Jul 2005 12:00 AM

--CELKO--

Please post DDL, so that people do not have to guess what the keys,
constraints, Declarative Referential Integrity, data types, etc. in
your schema are. Sample data is also a good idea, along with clear
specifications. It is very hard to debug code when you do not let us
see it.

Try using SET instead of SELECT in assignments, so it will look like
SQL/PSM.

Why do you write such procedural code? Think about this for two
seconds:

IF (@price_min <> 0) AND (@price_max <> 0)
SELECT @sql = @sql + ' AND (price BETWEEN @price_min AND @price_max)'
IF (@price_min = 0) AND (@price_max <> 0)
SELECT @sql = @sql + ' AND (price BETWEEN 0 AND @price_max)'

Have you ever worked with a compiled language like SQL before? I see
that you confuse fields and columns, so you are VERY new to SQL.

Did you notice that you used the proprietary MONEY data type that has
funny arithmetic results? Then you use INTEGER in the predicates
(above) to force a type conversion. You give a procedure a useless
name with a leading underscore to destroy portability and create
maintenance problems. In the United States we have highway numbers
and not Highwayid; always use the standard terminology.

Of course you never try to pass a table as a list. You would load a
table with the highway numbers.   Did you mean somethign like this?

CREATE PROCEDURE SearchForFoobars -- better name?
(@price_MIN DECIMAL (12,4) = 0.00,
@price_MAX DECIMAL (12,4) = 0.00,
@distance_MIN INTEGER = 0,
@distance_MAX INTEGER = 0)

AS
BEGIN - Validate parameters
IF @price_min < 0.00 RAISERROR (..);
IF @price_max < 0.00 RAISERROR (..);
IF @distance_min < 0 RAISERROR (..);
IF @distance_max < 0 RAISERROR (..);

SELECT Table1.* -- never use * in production code
FROM Table1, -- need a real name
WHERE Table1.highway_nbr
       IN (SELECT highway_nbr FROM HighwayList)
   AND price BETWEEN @price_min AND @price_max
   AND distance BETWEEN @distance_min AND @distance_max
   AND is_active = 1; -- flags! Just like assembly language
END;

Another alternative would be to put the highways into the parameter
list.

CREATE PROCEDURE SearchForFoobars
(..h1 INTEGER, h2 INTEGER,.., hn INTEGER)

...
WHERE Table1.highway_nbr
       IN (SELECT COALESCE(h1, 0)
            UNION ALL COALESCE(h2, 0)
            ..
           SELECT COALESCE(hn, 0)) AS S(highway_nbr)

Now the proc can be compiled and maintained by a SQL programmer.

22 Jul 2005 7:30 AM

Damien

Hi Joe,

A bit unrelated to the original post, but seeing your discussion of the
parameters to be passed to the stored proc, and a perceived deficiency
in my view recently:

Is there (or has there ever been proposed) a means of applying
constraints to parameters of procedures? By which I mean, a great many
stored procs get passed primary keys for particular tables, numeric
parameters which must fit within certain ranges, etc. I'd love to be
able to have:

Create Procedure Blah
@BorisPK int,
@LowerRange int,
@HigherRange int
as
...
go
alter procedure Blah add constraint
Blah_BorisPK foreign key
(
    @BorisPK
)
references Boris
(
    PK
)
go
alter procedure Blah add constraint
Blah_RangeWellDefined check
(
    @LowerRange > 0 and
    @HigherRange < 100 and
    @LowerRange <= @HigherRange
)
go

The constraint system seems so good, I'd love to use it in more places
(and avoid having to write lots of checks within my SP). So, has this
ever been considered?

Damien