Добро пожаловать в форум, Guest  >>   Войти | Регистрация | Поиск | Правила | В избранное | Подписаться
Все форумы / Microsoft SQL Server Новый топик    Ответить
 Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Коллеги, добрый вечер!

Если кому-то не лень покопаться в чужом коде под вечер, подскажите пожалуйста, что я делаю неправильно, не по Best Practice, и т.п. Мне нужно стороннее мнение для работы над ошибками.

Кстати, в моем коде команда RAISERROR не переводит выполнение в блок Catch, т.к. severity = 5. Но если я ставлю 10 и более, то при вызове этой хранимки из другой и возникновении этого события, родительская хранимка останавливается (и остаются висеть курсоры и транзакции). Как можно Вызвать ошибку, переехать в catch и аккуратно завершить выполнение, не руша родительскую хранимку?

USE [Budget]
GO
/****** Object:  StoredProcedure [dbo].[Import_Data]    Script Date: 03.07.2013 17:25:22 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
-- =============================================
-- =============================================
ALTER PROCEDURE [dbo].[Import_Data] 
	@ExcelFileFQN nvarchar(max) 
AS
BEGIN
SET NOCOUNT ON;	-- SET NOCOUNT ON added to prevent extra result sets from interfering with SELECT statements.
-------------------------------------------------------------------
-- Объявляем переменные
DECLARE @SQL nvarchar(max)
DECLARE @table_name nvarchar(50)
DECLARE @column_name nvarchar(50)
DECLARE @datatype varchar(50)
DECLARE @IsDateUnique as tinyint -- Будет показывать, есть ли уже данные с такой датой в базе
-------------------------------------------------------------------

SET @IsDateUnique = 0 -- По умолчанию данных там нет 

IF @ExcelFileFQN IS NULL
	BEGIN
		PRINT 'You have to set ExcelFileFQN to a Fully-Qualified file path to the Excel file you are trying to import'
		RETURN;
	END

-------------------------------------------------------------------
-- На случай, если у нас каким-то мифическим образом не завершенная транзакция с таким именем
IF @@TRANCOUNT > 0 
	ROLLBACK TRANSACTION Insert_Data

BEGIN TRY
-------------------------------------------------------------------
-- Проверяем наличие данных в базе. Если данных нет, возвращается 0 строк - переменная не меняется.
-- Если данные есть - вернётся 1, и выполнение скрипта будет прервано

	SET @SQL = 
	'DECLARE @DS smalldatetime, @KP int
	SELECT TOP 1 @KP =CAST([КодПредпр] as int), @DS=MAX(CAST([ДатаСохр] as smalldatetime)) FROM OPENROWSET(''Microsoft.ACE.OLEDB.12.0'', ''Excel 12.0; Database='+@ExcelFileFQN+';Extended Properties=HDR;'', [Data_1_01]) AS T GROUP BY [КодПредпр]

	SELECT @IsDateUniqueOUT=IsNull(1, 0) 
	FROM Data_1_01
	WHERE [КодПредпр] = @KP AND CAST([ДатаСохр] as smalldatetime) = @DS
	GROUP BY [КодПредпр], [ДатаСохр]'

	EXEC sp_executesql @SQL, N'@IsDateUniqueOUT int OUTPUT', @IsDateUniqueOUT=@IsDateUnique OUTPUT;

	IF @IsDateUnique > 0 
		BEGIN 
			RAISERROR ('This file seems to be already imported! There are entries in tables with same Company ID and SaveDate!
			
			', 5, 1);
		--	THROW 50000,'This file seems to be already imported! There are entries in tables with same SaveDate.', 1;
		END 

BEGIN TRANSACTION Insert_Data

-- =====================================
-- Здесь мы пройдём по каждой таблице, и сгенерируем код, 
-- который будет конвертировать типы данных из файла Excel в те, которые у нас в таблицах, 
-- и вставлять данные в них. При этом в случае возникновении ошибки отменяется вставка данных из всего файла,
-- что позволит потом не тратить время на "чистку" базы
-- =====================================

-------------------------------------------------------------------
-- Список наших таблиц
DECLARE Cur CURSOR FOR 
SELECT Name FROM sys.tables WHERE type='U' AND Name like 'Data%'
-------------------------------------------------------------------
OPEN Cur
FETCH NEXT FROM Cur INTO @table_name

WHILE @@FETCH_STATUS = 0 
BEGIN
SET @SQL = 'INSERT INTO ' + @table_name + '
SELECT '
-------------------------------------------------------------------
--Список колонок в каждой таблице с типом данных, преобразованным так, что можно использовать для декларирования в дальнейшем коде
	DECLARE Cur_Col CURSOR FOR 
	SELECT 
	c.name as cName 
	, CASE dt.name 
		WHEN 'nvarchar' 
			THEN dt.name+'('+
							CASE c.max_length 
								WHEN -1 THEN 'max' 
								ELSE CAST(c.max_length/2 as varchar(3)) 
								END 
						+')'
		ELSE dt.name
		END		as dType
	FROM sys.tables t join sys.columns c on t.object_id=c.object_id join sys.types dt ON c.system_type_id=dt.system_type_id
	WHERE t.name = @table_name and dt.name <> 'sysname' ORDER BY c.column_id
-------------------------------------------------------------------

	OPEN Cur_Col
	FETCH NEXT FROM Cur_Col INTO @column_name, @datatype 
	WHILE @@FETCH_STATUS = 0 
		BEGIN
-------------------------------------------------------------------
-- Для первой колонки генерируем другой код, т.к. там не должно быть запятой в начале.
		IF RTRIM(SUBSTRING(@SQL, LEN(RTRIM(@SQL))-5,7))='SELECT' 
			SET @SQL += '
CAST([' +@column_name+'] as '+@datatype+') as ['+@column_name+']
'
		ELSE
			SET @SQL += ', CAST([' +@column_name+'] as '+@datatype+') as ['+@column_name+']
'
-------------------------------------------------------------------
			FETCH NEXT FROM Cur_Col INTO @column_name, @datatype -- цикл
		END
	CLOSE Cur_Col
	DEALLOCATE Cur_Col
-- Вставляем источник данных (файл Excel)
	SET @SQL += '
FROM OPENROWSET(''Microsoft.ACE.OLEDB.12.0'', ''Excel 12.0; Database='+@ExcelFileFQN+';Extended Properties=HDR;'', ['+@table_name+']) as t	'

-------------------------------------------------------------------
-- Запуск сгенерированного кода на исполнение
	EXEC sp_executesql @SQL
-------------------------------------------------------------------
	PRINT '			... table ' + @table_name + ' has been successfully imported into database!'
	FETCH NEXT FROM Cur INTO @table_name -- цикл

END

CLOSE Cur
DEALLOCATE Cur

-------------------------------------------------------------------
-- Если все вставки завершились без ошибок, одобряем транзакцию, и данные действительно появляются в таблице. Обеспечивается целостность.
	IF @@TRANCOUNT > 0 COMMIT TRANSACTION Insert_Data;
	PRINT '
FILE SUCCESSFULLY IMPORTED : '+@ExcelFileFQN +'!
	
	'
END TRY

BEGIN CATCH
-------------------------------------------------------------------
--Возникла ошибка.
	PRINT 'Current transaction count: ' + Cast(@@TRANCOUNT as varchar(2)) -- Сколько открыто транзакций (должна быть 1)
	PRINT @SQL -- Печатаем текущий код SQL
	
	IF @@TRANCOUNT > 0 ROLLBACK TRANSACTION Insert_Data; -- Отменяем все изменения
	THROW; -- Ошибка в область сообщений
-------------------------------------------------------------------
END CATCH

END


Сообщение было отредактировано: 3 июл 13, 19:37
3 июл 13, 17:37    [14518217]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
SELECT @@VERSION

Microsoft SQL Server 2012 (SP1) - 11.0.3000.0 (Intel X86)
Oct 19 2012 13:43:21
Copyright (c) Microsoft Corporation
Express Edition on Windows NT 6.1 <X86> (Build 7601: Service Pack 1)
3 июл 13, 17:37    [14518224]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Неужели никто не ответит?
4 июл 13, 13:51    [14522158]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
super-code
Member

Откуда:
Сообщений: 244
Eugene_p1, мои пять копеек.

IF @ExcelFileFQN IS NULL
	BEGIN
		PRINT 'You have to set ExcelFileFQN to a Fully-Qualified file path to the Excel file you are trying to import'
		RETURN;
	END


Как Вы себе представляете клиентское приложение получит эту строчку. Нужен raiserror, с кодом ошибки, или какой-то код возврата.
4 июл 13, 14:00    [14522212]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Minamoto
Member

Откуда: Москва
Сообщений: 1162
Eugene_p1
Кстати, в моем коде команда RAISERROR не переводит выполнение в блок Catch, т.к. severity = 5. Но если я ставлю 10 и более, то при вызове этой хранимки из другой и возникновении этого события, родительская хранимка останавливается (и остаются висеть курсоры и транзакции). Как можно Вызвать ошибку, переехать в catch и аккуратно завершить выполнение, не руша родительскую хранимку?

Уберите THROW из блока CATCH.
Он передает пойманную ошибку выше - в родительскую хранимку. Если нужно завершать выполнение дочерней процедуры, но не завершать выполнение родительской, в блоке TRY оставьте severity = 10, а в CATCH вместо THROW поставьте RAISERROR с severity = 5.
4 июл 13, 14:00    [14522214]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Glory
Member

Откуда:
Сообщений: 104751
А какой смысл конвертирования полей из FROM OPENROWSET, если при INSERT и так будет конвертация ?
4 июл 13, 14:10    [14522282]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Glory,

Задумывался для обнаружения ошибок на начальном этапе.

Раз можно обойтись без этого, подумаю, как реализовать.
4 июл 13, 14:46    [14522603]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Minamoto,

Спасибо!
4 июл 13, 14:47    [14522609]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
super-code
Eugene_p1, мои пять копеек.

IF @ExcelFileFQN IS NULL
	BEGIN
		PRINT 'You have to set ExcelFileFQN to a Fully-Qualified file path to the Excel file you are trying to import'
		RETURN;
	END


Как Вы себе представляете клиентское приложение получит эту строчку. Нужен raiserror, с кодом ошибки, или какой-то код возврата.


super-code,

А я могу применть RAISERROR вне блока TRY...CATCH ?
4 июл 13, 14:49    [14522638]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Glory
Member

Откуда:
Сообщений: 104751
Eugene_p1
Задумывался для обнаружения ошибок на начальном этапе.

В смысле ? Чем в данном случае ошибка явного convert-а "начальнее" неявного конверта самого insert ?
Тогда уж правильнее создаваь из результата OPENROWSET промежточную таблицу.
В которой и проверять ошибки.
По крайней мере будет понятно, в каких именно строках ошибка. А не общее cant convert to <type> from <type> due ..
4 июл 13, 14:51    [14522664]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Glory,

Спасибо в любом случае - может, упрощу код.

Сильно заморачиваться тоже не имеет смысла, т.к. файл excel залочен, формат полей задан, можно только вбить данные.

Но опыт, полученный при решении этих задач, был очень полезен!
4 июл 13, 16:24    [14523391]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Гость333
Member

Откуда:
Сообщений: 3683
Eugene_p1
-------------------------------------------------------------------
-- На случай, если у нас каким-то мифическим образом не завершенная транзакция с таким именем
IF @@TRANCOUNT > 0 
	ROLLBACK TRANSACTION Insert_Data

А где тут проверка на "такое имя" транзакции? Попытка отката произойдёт при любом вызове процедуры, в случае, если открыта какая-либо транзакция (не обязательно именованная).

USE tempdb;
GO
CREATE PROCEDURE dbo.Test_Tran
AS
-- На случай, если у нас каким-то мифическим образом не завершенная транзакция с таким именем
IF @@TRANCOUNT > 0 
	ROLLBACK TRANSACTION Insert_Data
GO

BEGIN TRANSACTION;
EXEC dbo.Test_Tran;
COMMIT TRANSACTION;
GO
DROP PROCEDURE dbo.Test_Tran;
GO


Msg 6401, Level 16, State 1, Procedure Test_Tran, Line 5
Cannot roll back Insert_Data. No transaction or savepoint of that name was found.


Если же убрать имя транзакции, то будет такая ошибка:
USE tempdb;
GO
CREATE PROCEDURE dbo.Test_Tran
AS
-- На случай, если у нас каким-то мифическим образом не завершенная транзакция с таким именем
IF @@TRANCOUNT > 0 
	ROLLBACK TRANSACTION -- Insert_Data
GO

BEGIN TRANSACTION;
EXEC dbo.Test_Tran;
COMMIT TRANSACTION;
GO
DROP PROCEDURE dbo.Test_Tran;
GO


Msg 266, Level 16, State 2, Procedure Test_Tran, Line 0
Transaction count after EXECUTE indicates a mismatching number of BEGIN and COMMIT statements. Previous count = 1, current count = 0.
Msg 3902, Level 16, State 1, Line 4
The COMMIT TRANSACTION request has no corresponding BEGIN TRANSACTION.
4 июл 13, 16:31    [14523444]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Гость333,

В первой версии скрипта нужно BEGIN TRANSACTION Insert_Data;
У Вас не именованная транзакция открывается.
4 июл 13, 16:49    [14523569]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Гость333
Member

Откуда:
Сообщений: 3683
Eugene_p1
В первой версии скрипта нужно BEGIN TRANSACTION Insert_Data;

Кому нужно? Вот другой разработчик пишет свою хранимую процедуру. В ней открывается транзакция с другим именем. Или, как у меня, неименованная транзакция. Производятся какие-то действия. Дальше этот разработчик хочет вызвать вашу процедуру Import_Data. Но нельзя, ибо "No transaction or savepoint of that name was found".

А если транзакцию с именем Insert_Data откроет, то всё ещё хуже — откатятся все изменения, выполненные в первоначальной транзакции, плюс Transaction count after EXECUTE будет mismatching.
4 июл 13, 17:48    [14523917]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Гость333
Eugene_p1
В первой версии скрипта нужно BEGIN TRANSACTION Insert_Data;

Кому нужно? Вот другой разработчик пишет свою хранимую процедуру. В ней открывается транзакция с другим именем. Или, как у меня, неименованная транзакция. Производятся какие-то действия. Дальше этот разработчик хочет вызвать вашу процедуру Import_Data. Но нельзя, ибо "No transaction or savepoint of that name was found".

А если транзакцию с именем Insert_Data откроет, то всё ещё хуже — откатятся все изменения, выполненные в первоначальной транзакции, плюс Transaction count after EXECUTE будет mismatching.


Ваша правда!
Что посоветуете?
5 июл 13, 12:48    [14527027]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Уважаемые коллеги, нужен совет!

Вкратце - этот запрос используется для импорта стандартизированной формы в базу.

Сейчас возникла необходимость консолидировать имеющиеся данные по всем предприятиям. То есть - суммировать все ячейки одинаковых строк.

Порядок строк в форме одинаков, но в базе теряется, т.к. единого номера строки в форме нет, а поле КодСтроки запросто может быть NULL - такую уж форму сконструировали.

Как мне лучше пронумеровать строки во время импорта, чтобы при выводе я получил такой же порядок, как в исходной форме?

Попробовал ROW_NUMBER() OVER(ORDER BY), но так тут же сбивается оригинальный порядок.

Суммируются колонки от 4 и до конца, остальные имеют одинаковые значения во всех формах.

SELECT 
CAST([КодПредпр] as int) as [КодПредпр]
, CAST([ДатаСохр] as smalldatetime) as [ДатаСохр]
, CAST([КодВерсииБюдж] as int) as [КодВерсииБюдж]
, CAST([Depth] as int) as [Depth]
, CAST([idx] as nvarchar(10)) as [idx]
, CAST([F6] as int) as [F6]
, CAST([F7] as int) as [F7]
, CAST([F8] as int) as [F8]
, CAST([F9] as int) as [F9]
, CAST([F10] as int) as [F10]
, CAST([КодРег] as int) as [КодРег]
, CAST([1] as nvarchar(10)) as [1]
, CAST([2] as nvarchar(255)) as [2]
, CAST([3] as nvarchar(255)) as [3]
, CAST([4] as float) as [4]
, CAST([5] as float) as [5]
--[... еще 93 такие же строчки съели мыши...]
, CAST([98] as float) as [98]
, ROW_NUMBER() OVER(ORDER BY [КодПредпр])
FROM OPENROWSET('Microsoft.ACE.OLEDB.12.0', 'Excel 12.0; Database=D:\Budget\Import\Приложение_16_Налоговые бюджеты_01072013-1530 без защиты.xlsm;Extended Properties=HDR;', [Data_1_011]) as t	
5 июл 13, 13:06    [14527180]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
КодСтроки = idx
5 июл 13, 13:07    [14527187]     Ответить | Цитировать Сообщить модератору
 Re: Покритикуйте код  [new]
Eugene_p1
Member

Откуда: Москва
Сообщений: 295
Актуально, подскажите пожалуйста, кто может.
12 июл 13, 13:17    [14557538]     Ответить | Цитировать Сообщить модератору
Все форумы / Microsoft SQL Server Ответить