Динамическая хранимая процедура новичка T-SQL - как я могу ее улучшить? - PullRequest
1 голос
/ 12 февраля 2010

Я новичок в T-SQL; весь мой опыт - в полностью другой среде базы данных (Openedge). Я научился достаточно, чтобы написать процедуру ниже - но также достаточно, чтобы знать, что я не знаю достаточно!

Эта рутина скоро должна войти в живую среду, и она работает, но я совершенно уверен, что в ней есть ряд с ** к-ов и ошибок, о которых я ничего не знаю.

Процедура копирует данные из таблицы A в таблицу B, заменяя данные в таблице B. Таблицы могут находиться в любой базе данных. Я планирую вызывать эту процедуру несколько раз из другой хранимой процедуры. Разрешения не являются проблемой: подпрограмма будет запущена dba как временное задание.

Могу ли я получить ваши предложения о том, как сделать так, чтобы они соответствовали передовым практикам? Чтобы пуленепробиваемые это?

ALTER PROCEDURE [dbo].[copyTable2Table]
    @sdb        varchar(30),
    @stable     varchar(30),
    @tdb        varchar(30),
    @ttable     varchar(30),
    @raiseerror bit = 1,
    @debug      bit = 0
as
begin
    set nocount on

    declare @source      varchar(65)
    declare @target      varchar(65)
    declare @dropstmt    varchar(100)
    declare @insstmt     varchar(100)
    declare @ErrMsg      nvarchar(4000)
    declare @ErrSeverity int

    set @source      = '[' + @sdb + '].[dbo].[' + @stable + ']'
    set @target      = '[' + @tdb + '].[dbo].[' + @ttable + ']'
    set @dropStmt    = 'drop table ' + @target
    set @insStmt     = 'select * into ' + @target + ' from ' + @source
    set @errMsg      = ''
    set @errSeverity = 0

    if @debug = 1
        print('Drop:' + @dropStmt + '  Insert:' + @insStmt)

    -- drop the target table, copy the source table to the target
    begin try
        begin transaction    
        exec(@dropStmt)
        exec(@insStmt)
        commit
    end try

    begin catch
        if @@trancount > 0
            rollback

        select @errMsg      = error_message(), 
               @errSeverity = error_severity()

    end catch

    -- update the log table
    insert into HHG_system.dbo.copyaudit
        (copytime, copyuser, source, target, errmsg, errseverity)
        values( getdate(), user_name(user_id()), @source, @target, @errMsg, @errSeverity)

    if @debug = 1
        print ( 'Message:' + @errMsg + '  Severity:' + convert(Char, @errSeverity) )

    -- handle errors, return value
    if @errMsg <> ''
    begin 
        if @raiseError = 1
            raiserror(@errMsg, @errSeverity, 1)

        return 1
    end

    return 0
END

Спасибо!

Ответы [ 4 ]

3 голосов
/ 13 февраля 2010

Я говорю здесь с точки зрения Sybase (я не уверен, используете ли вы SQLServer или Sybase), но я подозреваю, что вы обнаружите те же проблемы в любой среде, так что здесь ...

  • Во-первых, я бы повторил комментарии, сделанные в предыдущих ответах о предполагаемом владении dbo таблицами.

  • Тогда я бы уточнил у ваших администраторов баз данных, что этому хранимому процессу будет предоставлено разрешение на удаление таблиц в любой базе данных, кроме tempdb. По моему опыту, администраторы баз данных ненавидят это и редко предоставляют его в качестве опции из-за вероятности катастрофы.

  • Операции DDL, такие как удаление таблицы, допускаются в транзакции только в том случае, если база данных настроена с параметром sp_dboption my_database, "ddl in tran", true. Вообще говоря, действия, выполняемые внутри транзакций с участием DDL, должны быть очень короткими, поскольку они будут блокировать часто используемые системные таблицы, такие как sysobjects, и при этом блокировать ход других процессов сервера данных. Учитывая, что у нас нет возможности узнать, сколько данных необходимо скопировать, это может привести к очень длинной транзакции, которая на некоторое время блокирует действия для всех. Более того, администраторы баз данных должны будут выполнять эту команду в каждой базе данных, которая содержит таблицы, которые могут содержать таблицу «@Target» этого хранимого процесса. Если бы вы использовали транзакцию для drop table, было бы неплохо отделить ее от любой транзакции, обрабатывающей вставку данных.

  • Хотя вы можете выполнять drop table команды в транзакции, если установлена ​​опция ddl in tran, невозможно выполнить select * into внутри транзакции. Поскольку select * into представляет собой комбинацию создания таблицы и вставки, она неявно блокирует базу данных (возможно, на некоторое время, если данных много), если она была выполнена в транзакции.

  • Если в вашей таблице @target есть ограничения внешнего ключа, вы не сможете просто удалить ее, не избавившись от ограничений внешнего ключа.

  • Если у вас есть столбец «id», в котором используется числовой тип идентификатора (часто используемый в качестве функции автонумерации для создания значений для суррогатных первичных ключей), помните, что вы не сможете скопировать значения из столбца 'id' таблицы '@Source' до столбца id таблицы '@Target'.

  • Я бы также проверил размер вашего журнала транзакций в любой возможной базе данных, которая может содержать таблицу «@Target» относительно размера любой возможной таблицы «@Source». Учитывая, что все копирование выполняется за одну транзакцию, вы вполне можете скопировать таблицу, настолько большую, что она уничтожит журнал транзакций в вашем сервере данных prod, что приведет к остановке всех процессов. Я видел людей, использующих чанкинг для достижения этой цели в особенно больших таблицах, но в итоге вам нужно было вставить свои собственные проверки в код, чтобы удостовериться, что вы действительно получили непротиворечивый снимок таблицы.

Просто мысль - если это используется для получения снимков, как насчет BCP? Это может быть использовано для выгрузки содержимого таблицы, давая вам снимок, который вы ищете. Если вы используете опцию -c, вы даже получите ее в удобочитаемой форме.

Всего наилучшего, Стюарт

1 голос
/ 12 февраля 2010

Эта строка кажется немного опасной:

set @dropStmt    = 'drop table ' + @target

Что, если целевой таблицы не существует?

Я бы попытался это как-то защитить - что-то вроде:

set @dropStmt = 
    'if object_id(' + @target + ') IS NOT NULL   DROP TABLE ' + @target

Таким образом, только если вызов OBJECT_ID(tablename) не возвращает NULL (что означает: таблица не существует) и таблица гарантированно существует, выполните оператор DROP TABLE.

0 голосов
/ 17 февраля 2010

Я считаю весь процесс, который вы написали, ужасно опасным. Даже если это выполняется из базы данных, а не пользователем, динамический SQL - плохая практика! В базах данных, использующих это, возможность делать это с любой таблицей в любое время опасно и будет запрещено в базах данных, с которыми я работаю. Слишком легко случайно сбросить неправильные таблицы! Также невозможно правильно протестировать все возможные значения, с которыми может работать sp, так что это может быть и код с ошибками, и вы не узнаете, пока он не будет в работе.

Кроме того, при отбрасывании и повторном создании с помощью select into у вас не должно быть индексов или ограничений ключа или вещей, которые вам необходимы для обеспечения производительности и целостности данных. BAD BAD IDEA в целом (хорошо, если это просто промежуточные таблицы какого-либо типа, но не для чего-либо еще).

Это задача для служб SSIS. Мы сохраняем наши пакеты служб SSIS и отправляем их в Subversion, как и все остальное. Мы можем сделать для них различия (это просто файлы XML), и мы можем сказать, что работает на prod и какую конфигурацию мы используем.

Вы не должны удалять и воссоздавать таблицы, если они не являются относительно небольшими. Вам следует обновить существующие записи, удалить ненужные записи и добавлять только новые. Если у вас есть миллион записей и 27000 изменились, 10 были удалены, а 3000 новые, зачем удалять и вставлять все 1 000 000 записей. Это бесполезно расходует ресурсы сервера, может вызвать проблемы с блокировкой и блокировкой, а также может создать проблемы, если пользователи смотрят на таблицы во время запуска, а данные внезапно исчезают и для возврата требуется несколько минут. Пользователи сходит с ума по этому поводу.

0 голосов
/ 12 февраля 2010

Во-первых, замените весь код как

set @source = '[' + @sdb + '].[dbo].[' + @stable + ']'

с кодом типа

set @source = QuoteName(@sdb) + '.[dbo].' + QuoteName(@stable)

Во-вторых, ваша процедура предполагает, что все объекты принадлежат dbo - это может быть не так.

В-третьих, ваши имена переменных слишком короткие - 30 символов - 128 - это длина sysname.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...