Table-valued function, dynamic SQL, SProc

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2008 Forums

Transact-SQL (2008)

Table-valued function, dynamic SQL, SProc

Author

Topic

SEFL
Starting Member

8 Posts

Posted - 2011-08-10 : 17:21:50

I'm still learning SQL Server 2008 on the fly as I go (for about 8 months now), and I'm trying to create a table function. This particular function worked when it was inside of a stored procedure; however, I need to call the function several times withi the stored procedure and as a result, I was trying to simplify code and call a function.

And here's where I get stuck. This is the function as it stands right now (edited several times):


ALTER FUNCTION GenerateEventTable
(	
	@Interests_Table InterestTable ReadOnly,
	@Partner_Point geography,
	@Distance decimal
)
RETURNS @Event_Nights TABLE (Event_Night_ID int identity (1, 1), Company nvarchar (255), Latitude float, Longitude float, 
Distance float, Prev_Event_Night_ID int default 0, Idea_Order int default 1)
AS
BEGIN
Declare @Coords geography
Declare @Company nvarchar (255), @Latitude float, @Longitude float, @Event_Night_Distance float, @Event_Night_ID integer, @Sorted_Interest_ID integer
Declare @sql nvarchar (4000), @sql2 nvarchar (4000)

-- generate events
set @sql = 'Declare forward_cursor cursor for Select Top 5 Companies.Company, Companies.Latitude, Companies.Longitude, 
Companies.Coords, @Partner_Point.STDistance([Coords])/1000 as Distance from (Companies inner join Companies_interests on Companies.Company_ID = 
Companies_Interests.Company_ID) inner join @Interests_Table on Companies_Interests.Interest_ID = @Interests_Table.Interest_ID 
where @Partner_Point.STDistance([Coords])/1000 <= @Distance and @Interests_Table.Sorted_Interest_ID = 1 order by @Partner_Point.STDistance([Coords])'

exec @sql

	FETCH NEXT FROM Forward_Cursor INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance
	WHILE @@FETCH_STATUS = 0
		BEGIN
			Insert into @Event_Nights (Company, Latitude, Longitude, Distance) values (@Company, @Latitude, @Longitude, @Event_Night_Distance)
	
			-- insert the second event into the table, calculate the distance from the primary event so that the closest ones show up first.
					
			select @Event_Night_ID = MAX (Event_Night_ID) from @Event_Nights
			set @sql2 = 'Declare forward_cursor_2 cursor for select Top 5 Companies.Company, Companies.Latitude, Companies.Longitude, Companies.Coords, 
			@Coords.STDistance([Coords])/1000 as Distance from (Companies inner join Companies_interests on Companies.Company_ID = Companies_Interests.Company_ID) 
			inner join @Interests_Table on Companies_Interests.Interest_ID = @Interests_Table.Interest_ID where @Coords.STDistance([Coords])/1000 <= @Distance and 
			@Interests_Table.Sorted_Interest_ID = 2 order by @Coords.STDistance([Coords])'
			exec @sql2
			FETCH NEXT FROM Forward_Cursor_2 INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance
			WHILE @@FETCH_STATUS = 0
				BEGIN
					Insert into @Event_Nights (Company, Latitude, Longitude, Distance, Prev_Event_Night_ID, Idea_order) values (@Company, @Latitude, @Longitude, 
					@Event_Night_Distance, @Event_Night_ID, 2)
					FETCH NEXT FROM Forward_Cursor_2 INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance
				END
			FETCH NEXT FROM Forward_Cursor INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance
		END
RETURN 
END
GO

I originally didn't have the SQL statements in variables; however, I kept getting the error "must declare the scalar variable Interests_Table" even though it's clearly being passed as a parameter (I've read a couple of explanations on this, and I still have no idea what the problem is with this...if someone could explain this in plain English along with the workaround if any, I'd appreciate it).

I then put the SQL statements into variables and attempted to use exec sp_executesql @SQL, but then got the error message that I couldn't use stored procedures in a function. I "kind of" get the logic here, but not really.

Then I tried it the way I have it now, and I get this message:

Msg 203, Level 16, State 2, Procedure Retrieve_Date_Night_Ideas, Line 49
The name 'Declare forward_cursor cursor for Select Top 5 Companies.Company, Companies.Latitude, Companies.Longitude, Companies.Coords, @Partner_Point.STDistance([Coords])/1000 as Distance from (Companies inner join Companies_interests on Companies.Company_ID = Companies_Interests.Company_ID) inner join @Interests_Table on Companies_Interests.Interest_ID = @Interests_Table.Interest_ID where @Partner_Point.STDistance([Coords])/1000 <= @Distance and @Interests_Table.Sorted_Interest_ID = 1 order by @Partner_Point.STDistance([Coords])' is not a valid identifier.

This is really starting to get confusing and frustrating, and I get the feeling the answer is pretty stupid and simple but I'm missing it completely. Any help would be appreciated.

edit: reformatted code to fit window

Lumbago
Norsk Yak Master

3271 Posts

Posted - 2011-08-11 : 03:28:21

I don't mean to put you off or be discouraging or anything but there are several design flaws here that could lead to numerous problems. Two things to notice first: 1) cursors are the devil, stay as far away from them as you can. Nesting them is even worse 2) dynamic sql should be avoided. Putting them both inside a table-valued function is...well...pretty bad to tell you the truth.

Now, see if this will do it for you:

ALTER FUNCTION GenerateEventTable
(	
	@Interests_Table InterestTable ReadOnly,
	@Partner_Point geography,
	@Distance decimal
)
RETURNS @Event_Nights TABLE (
	Event_Night_ID int identity (1, 1), 
	Company nvarchar (255), 
	Latitude float, 
	Longitude float, 
	Distance float, 
	Prev_Event_Night_ID int default 0, 
	Idea_Order int default 1)
AS
BEGIN

	DECLARE @Event_Night_ID int

	Insert into @Event_Nights (Company, Latitude, Longitude, Distance)
	Select Top 5 Companies.Company, Companies.Latitude, Companies.Longitude, Companies.Coords, 
		@Partner_Point.STDistance([Coords])/1000 as Distance 
	from (Companies 
		inner join Companies_interests 
			on Companies.Company_ID = Companies_Interests.Company_ID) 
		inner join @Interests_Table 
			on Companies_Interests.Interest_ID = @Interests_Table.Interest_ID 
	where @Partner_Point.STDistance([Coords])/1000 <= @Distance 
		and @Interests_Table.Sorted_Interest_ID = 1 
	order by @Partner_Point.STDistance([Coords])


	select @Event_Night_ID = MAX(Event_Night_ID) from @Event_Nights

	Insert into @Event_Nights (Company, Latitude, Longitude, Distance, Prev_Event_Night_ID, Idea_order) 
	select Top 5 Companies.Company, Companies.Latitude, Companies.Longitude, Companies.Coords, 
		@Coords.STDistance([Coords])/1000 as Distance, @Event_Night_ID, 2
	from (Companies 
		inner join Companies_interests 
			on Companies.Company_ID = Companies_Interests.Company_ID) 
		inner join @Interests_Table 
			on Companies_Interests.Interest_ID = @Interests_Table.Interest_ID 
	where @Coords.STDistance([Coords])/1000 <= @Distance 
		and @Interests_Table.Sorted_Interest_ID = 2 
	order by @Coords.STDistance([Coords])

RETURN 
END
GO

- Lumbago
My blog-> http://thefirstsql.com/2011/07/08/how-to-find-gaps-in-identity-columns-at-the-speed-of-light/

Lumbago
Norsk Yak Master

3271 Posts

Posted - 2011-08-11 : 10:30:37

Crap, my suggestion for the 2nd isn't gonna work. You'd have to greate a join instead to insert the correct EvenetNightID. Unfortunately I'm on my ipad at the moment so typing out a better working solution is a mess. I'll giveit a go later probably...

- Lumbago
My blog-> http://thefirstsql.com/2011/07/08/how-to-find-gaps-in-identity-columns-at-the-speed-of-light/

robvolk
Most Valuable Yak

15732 Posts

Posted - 2011-08-12 : 11:00:19

Here's a version that uses only a single cursor and works (based on the sample data I generated):

ALTER FUNCTION GenerateEventTable
(	
	@Interests_Table InterestTable ReadOnly,
	@Partner_Point geography,
	@Distance DECIMAL
)
RETURNS @Event_Nights TABLE (Event_Night_ID INT IDENTITY(1,1), Company NVARCHAR(255), Latitude FLOAT, Longitude FLOAT, 
Distance FLOAT, Prev_Event_Night_ID INT DEFAULT 0, Idea_Order INT DEFAULT 1) AS
BEGIN
DECLARE @Coords geography, @Company NVARCHAR (255), @Latitude FLOAT, @Longitude FLOAT, 
@Event_Night_Distance FLOAT, @Event_Night_ID INT

-- generate events
DECLARE forward_cursor CURSOR STATIC FORWARD_ONLY FOR 
SELECT TOP 5 C.Company, C.Latitude, C.Longitude, C.Coords, @Partner_Point.STDistance([Coords])/1000 AS Distance 
FROM Companies C 
INNER JOIN Companies_interests CI ON C.Company_ID = CI.Company_ID
INNER JOIN @Interests_Table IT ON CI.Interest_ID = IT.Interest_ID 
WHERE @Partner_Point.STDistance([Coords])/1000 <= @Distance AND IT.Sorted_Interest_ID = 1 
ORDER BY @Partner_Point.STDistance([Coords])

OPEN forward_cursor 
FETCH NEXT FROM Forward_Cursor INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance

WHILE @@FETCH_STATUS = 0
BEGIN
	INSERT INTO @Event_Nights (Company,Latitude,Longitude,Distance) VALUES (@Company,@Latitude,@Longitude,@Event_Night_Distance)
	SELECT @Event_Night_ID=SCOPE_IDENTITY()

	;WITH CTE(Company, Latitude, Longitude, Coords, Distance) AS (
		SELECT TOP 5 C.Company, C.Latitude, C.Longitude, C.Coords, @Coords.STDistance([Coords])/1000 AS Distance
		FROM Companies C 
		INNER JOIN Companies_interests CI ON C.Company_ID = CI.Company_ID
		INNER JOIN @Interests_Table IT ON CI.Interest_ID = IT.Interest_ID 
		WHERE @Coords.STDistance([Coords])/1000 <= @Distance AND IT.Sorted_Interest_ID = 2 
		ORDER BY @Coords.STDistance([Coords]))
	INSERT INTO @Event_Nights (Company, Latitude, Longitude, Distance, Prev_Event_Night_ID, Idea_order) 
	SELECT Company, Latitude, Longitude, Distance, @Event_Night_ID, 2 FROM CTE

	FETCH NEXT FROM Forward_Cursor INTO @Company, @Latitude, @Longitude, @Coords, @Event_Night_Distance
END
CLOSE forward_cursor
DEALLOCATE forward_cursor

RETURN 
END
GO

The main problems with the original were:

1. Need to alias table variables, can't use @variable.Column references (see IT.Interest_ID). I aliased all the tables to simplify the code.
2. Dynamic SQL cannot access variables from the parent session, you'd have to incorporate their values into the SQL statement. And it's not needed here anyway.
3. As Lumbago mentioned, dynamic SQL is not recommended, and cursors are not recommended, so dynamic SQL cursors are...shudder.

I was working on a version to eliminate both cursors but couldn't get the results to match using 2 common-table expressions. I'd recommend having 2 versions (one cursor based, one not) that you can compare results against.

I'd also recommend changing this to be a stored procedure instead of a function. Using this in a JOIN or CROSS APPLY could hinder performance the way it's currently written, and I don't see that you'd use it that way anyhow.

SEFL
Starting Member

8 Posts

Posted - 2011-08-13 : 20:32:31

Okay, I ended up solving this an entirely different way without modifying much of my code. Forgot to post back. My bad.

I don't get a few things, though:

1) People say cursors are bad, but I've never seen an explanation for this. Why do people say this?

2) Same question with dynamic SQL. Again...why do people say this?

Keep in mind that my background isn't with SQL as such, but with classic ASP and learning ASP.net as well.

robvolk
Most Valuable Yak

15732 Posts

Posted - 2011-08-14 : 07:30:06

Regarding cursors, see this post: http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=47319

Dynamic SQL isn't bad per se, but since it's building a SQL command and executing it, you have to take special care against SQL injection attacks, syntax errors, and permission issues. And if you use a lot of dynamic SQL that's written a certain way, you can lower overall performance because SQL Server will cache a lot of ad-hoc, 1-time plans. It should be avoided as much as possible.

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-08-14 : 14:29:56

erm. Also as far as I know you can't use dynamic sql inside functions....

Functions have to be deterministic and as such are prevented from doing a bunch of stuff.

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

Lumbago
Norsk Yak Master

3271 Posts

Posted - 2011-08-15 : 01:57:07

SEFL: the nature of a cursor is that it operates on a row-by-row basis, but the database works in a set based manner which means that it doesn't really care if you're working on 1 row or on 100 000, it's the dataset that counts. Thus, the amount of resources used working on 1 row is "the same" as working on 100 000 rows (leaving out ram and IO of course). Imagine pouring a glass of milk; using a cursor would mean pouring the glass one drop at a time, a set based manner would be to pour it all in one go.

- Lumbago
My blog-> http://thefirstsql.com/2011/07/08/how-to-find-gaps-in-identity-columns-at-the-speed-of-light/

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources