[RESOLVED] MS SQL - Dynamic SQL

[Ecniv]

Hi,

Situation:
A table of alterations which holds the old and new values (as Varchar).
A request from the client to not show the ID and show instead the interpreted data (ie name of a customer instead of their id).

I thought of just joining the table to the other table via id but had to utilise a convert function on the join to be able to run when a value in the log isnt a number.

I thought of using dynamic sql inside a stored proc to get a table of table names and fields and which field(s) to return. Pull back the log data into a temporary variable/table. Then loop the results, mount the Sql and get the actual data to show then update the temporary table before returning to the app to show in a grid.
I managed to get it to mount an SQL that looks correct, but MS Sql is complaining it cannot find the temporary table variable I defined.

The Questions are:
Is this possible?
When using the EXECUTE sp_executesql @newsql, does this create a separate instance type thing where it cannot see the variable declared already? Or so I need to pass the execution a pointer to the variable, somehow?
Any other ways I could try?

[FunkyDexter]

I'm really not sure I've understood the problem you're trying to solve so perhaps you could expand on that first paragraph. Maybe include some sample data and the desired result.

I've got a couple of thoughts that might be relevant but I'm not sure so ignore them if I've missed the point:-

I thought of just joining the table to the other table via id but had to utilise a convert function on the join to be able to run when a value in the log isn't a number.

There's nothing wrong with that. The only downside is that the join will be non-sargable but there's ways of dealing with that. The quickest and dirtiest is to join without the convert but include an IsNumeric check in the where clause to limit your dataset to record where the key is numeric. This will avoid error on unconvertable value in the join.

Select *
From Table1
Join Table2 on Table1.VarcharKey = Table2.NumericKey
Where IsNumeric(Table1.VarcharKey) = 1

If limiting the dataset like that isn't acceptable then stand up a temp table and populate it with the varchar values of the NumericKey. You can index this which will make the join to your non-numeric key sargable but does have the overhead of having to populate the table in the first place.

I thought of using dynamic sql inside a stored proc to get a table of table names and fields and which field(s) to return

Dynamic SQL is rarely necessary and should be avoided where possible. It's difficult to maintain and vulnerable to injection attacks. Take a look at Information_Schema.Tables and Information_Schema.Columns. The chances are they contain everything you need to write a static query. Dynamic SQL can't always be avoided, though, so you may just have to suck it up as a last resort.

Then loop the results

Loop based solutions should be a last resort. They perform horribly and, like dynamic sql, are rarely necessary. See if you can come up with a set based approach first.

MS Sql is complaining it cannot find the temporary table variable I defined.

There's no such thing as a Temporary Table Variable. There are Temporary Tables and there are Table Variables and they have different scoping rules. We'd need to know exactly what you're doing to tell you why that's failing. Show us code.

When using the EXECUTE sp_executesql @newsql, does this create a separate instance type thing where it cannot see the variable declared already?

That would depend on where the variable was scoped. If it's declared in the stored procedure it would be available to the stored procedure. It would fall out of scope when the stored procedure ended. If it's declared at a higher scope it will exist for the duration of that scope.

I have to say, though, you really haven't been very clear. Explain your problem (rather than your proposed solution) as clearly as possible. And then give is the code for your proposed solution. Chances are there a better solution anyway and, if there's not, seeing your code will give us a fighting chance it.

[Ecniv]

@Funky Dexter: Thanks. A good explanation

I think you got it from my somewhat bad explanation.

I found on the MS site a bit about passing the variable into the spexecute... Which I got working (although why it does work I'm still not sure).
I meant table variable rather than temp table.
I didnt want to loop the results but I havent found a way to get selects of other tables with data from another table controlling which fields to use/return.

CREATE PROC dbo.gridLogAlterations
            @tabela VARCHAR(200),
            @idTabela INT,
            @dt DATETIME,
            @usr VARCHAR(50),
            @acao VARCHAR(1),
            @todos VARCHAR(1) = 'N'
AS              
BEGIN

---------

DECLARE @opTbl TABLE (
  IdLogAlteracao INT,
  Dt DATETIME,
  Tabela VARCHAR(200),
  IdTabela INT,
  NomeCampo VARCHAR(200),
  Usr VARCHAR(100),
  Acao VARCHAR(1),
  VlrAnterior VARCHAR(100),
  VlrAtual VARCHAR(100),
  DestinoTabela VARCHAR(200),
  DestinoCampos VARCHAR(512),
  DestinoPK VARCHAR(512),
  OriginalVlrAnterior VARCHAR(100),
  OriginalVlrAtual VARCHAR(100)
);

INSERT INTO @opTbl( 
       IdLogAlteracao,
       Dt,
       Tabela,
       IdTabela,
       NomeCampo,
       Usr,
       Acao,
       VlrAnterior,
       VlrAtual,
       DestinoTabela,
       DestinoCampos,
       DestinoPK,
       OriginalVlrAnterior,
       OriginalVlrAtual
)
SELECT
       la.IdLogAlteracao,
       la.Dt,
       la.Tabela,
       la.IdTabela,
       la.NomeCampo,
       la.Usr,
       la.Acao,
       la.VlrAnterior,
       la.VlrAtual,
       sq.DestinoTabela,
       sq.DestinoCampos,
       sq.DestinoPK,
       la.VlrAnterior OriginalVlrAnterior,
       la.VlrAtual OriginalVlrAtual
  FROM la
  LEFT JOIN
  (SELECT 
          p.idparametro,
          pv.idparametrovlr,
          Tabela,
          Campo,
          DestinoTabela,
          DestinoCampos,
          DestinoPK
     FROM p
    INNER JOIN pv ON p.idparametro = pv.idparametro
    WHERE p.nomeparam = 'sistema.logalteracoes'
      AND COALESCE((SELECT TOP 1 valor FROM pvi WHERE pvi.idparametrovlr = pv.idparametrovlr AND Nome = 'situacao'),'A') = 'A'
        ) sq ON la.Tabela = sq.Tabela
            AND la.NomeCampo = sq.Campo
  WHERE la.tabela = @tabela
    AND la.idTabela = @idTabela
    AND la.dt BETWEEN @dt AND DATEADD(day,1,@dt)
    AND la.Usr = @usr
    AND la.acao = @acao
    AND NOT la.NomeCampo LIKE '%,'
    
-------------
DECLARE @id INT,
        @vv VARCHAR(512),
        @vn VARCHAR(512),
        @desttbl VARCHAR(200),
        @destcmp VARCHAR(200),
        @destpk VARCHAR(200),
        @sql NVARCHAR(200),
        @novo VARCHAR(200)
        ;
        
DECLARE cur CURSOR
    FOR SELECT 
               IdLogAlteracao,
               VlrAnterior,
               VlrAtual,
               DestinoTabela,
               DestinoCampos,
               DestinoPK
          FROM @opTbl
    FOR UPDATE OF DestinoTabela,DestinoCampos
    ;

OPEN cur;

FETCH NEXT FROM cur 
 INTO @id, @vv, @vn, @desttbl, @destcmp, @destpk;

WHILE @@FETCH_STATUS = 0
  BEGIN
    SET @novo='';
    
    if NOT @vv IS NULL AND COALESCE(@desttbl,'')!=''
      BEGIN
        SET @sql = REPLACE( (N'SELECT @op=('+@destcmp+') FROM '+@desttbl+' WHERE '+@destpk+'=@vv') ,'¬','+'' ''+') ;
        EXECUTE sp_executesql @sql,N'@vv VARCHAR(200),@op VARCHAR(200) OUTPUT',@vv = @vv,@op = @novo OUTPUT
        UPDATE @opTbl SET VlrAnterior = LEFT(@novo,100) WHERE CURRENT OF cur
      END

    if NOT @vn IS NULL AND COALESCE(@desttbl,'')!=''
      BEGIN
        SET @sql = REPLACE( (N'SELECT @op=('+@destcmp+') FROM '+@desttbl+' WHERE '+@destpk+'=@vn') ,'¬','+'' ''+') ;
        EXECUTE sp_executesql @sql,N'@vn VARCHAR(200),@op VARCHAR(200) OUTPUT',@vn = @vn,@op = @novo OUTPUT
        UPDATE @opTbl SET VlrAtual = LEFT(@novo,100) WHERE CURRENT OF cur
      END

    FETCH NEXT FROM cur 
     INTO @id, @vv, @vn, @desttbl, @destcmp, @destpk;
  END


CLOSE cur;
DEALLOCATE cur;

--------------
-- i02 qy itens
IF @todos = 'N'
 SELECT 
        NomeCampo,
        VlrAnterior,
        VlrAtual,
        IdLogAlteracao,
        DtAlteracao = FORMAT(Dt,N'dd/MM/yyyy hh\:mm')
   FROM @opTbl    
ELSE
 SELECT 
        IdLogAlteracao,
        DtAlteracao = FORMAT(Dt,N'dd/MM/yyyy hh\:mm'),
        Tabela,
        IdTabela,
        NomeCampo,
        Usr,
        Acao,
        VlrAnterior,
        VlrAtual,
        DestinoTabela,
        DestinoCampos,
        DestinoPK,
        OriginalVlrAnterior,
        OriginalVlrAtual
   FROM @opTbl    
        
END

This is used inside a custom framework, so parts are limited. In theory only the admins would be adding to this lookup table and shouldnt cause a problem...

[FunkyDexter]

I think you got it from my somewhat bad explanation.

I really haven't. Are you saying you've got some "data dictionary" tables? I.e. tables that describe the other table in your system? And you want to somehow use these data dictionary tables to generate queries against other tables?

1. Show me the sample data from ALL the tables that are reference in your sproc. I believe that's LA, P and PV.
2. I believe LA.DestinoTabela contains the names of other tables in your system, correct? If so, show me the data from a typical table that's referenced in this way.
3. And show me the result you want.

If you give me those three things I've got a chance of helping you. If you don't, I can't.

[Ecniv]

Thanks funky.

I have it working (the above works) but was just wondering if there is a better way.


The Table LA is the log of alterations, holds the table and field altered, old value, new value, type of update, who did it and when.
However, this holds the actual field name. Now the clients dont know/arent interested in the id fields, they want something a little more readable. So for each main table and field that is an id, I need to connect to the other table to get some field(s) to show name of a person (for example) rather than the id.
We have a table for putting in flexible custom parameters (p) with corresponding value groups (pv) which I can hold the main table, id, linked table, fields.
The problem was how to get the linked bits back and run a sub select on them when the fields and tables can change.

Hence the first thought of joins. But with ids being integer and the values held in the linked table being string, ms sql complained.
I could convert and it would work but I felt it would be slower as it has to convert for all ids then check against strings.

Second idea was the dynamic sql to build the sub select. Not the best of ideas but more flexible to build an sql sub select and execute.
This is above.
I had to add in the replace statements to allow more than one returning field in the dynamic sql.
Additional request was to update the fieldname to something else for certain fields.

-----------------
Example
Data in log
idlog, table, field, oldvalue, new value, type, usr, dt
1, companies, idowner, 1,5,'U','the user',2018-09-05 10:00:00

To show in a grid
(filtered by usr, date and type)
Table, field, oldvalue, newvalue
1, Companies, Owner, Mr Jones, Mr Smith

(Where the companies table has an owner id pointing to a table of people).

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


As I said, it is working. But perhaps there would be bigger performance problems or security risk using the dynamic sql. But I couldnt see any other way other than lots of joins or IF statements in a stored proc for each type of table, field link.

Again, thank you for taking the time to look over this.

[FunkyDexter]

When I asked for sample data from all your referenced tables what I meant was: please supply sample data from ALL your reference tables. Apparently capitalising the word ALL and listing the tables wasn't sufficient to make that clear. At this point I don't understand what you were joining out to P and PV so I don't know if the following suggestion is suitable but hopefully it'll help.

First off, I don't believe you're going to be able to avoid dynamic sql for this particular problem. As soon as you need to query on a dynamic table or field name it becomes unavoidable.

This speaks to a deeper problem in your design: your log table is an implementation of the entity attribute value pair anti-pattern. It's evil in relational databases and your typing (strings vs ints) problems are just one of the symptoms of why - data types have no meaning in this design. Further relational databases are simply not designed to support it. All your queries are going to be across a single table which you won't readily be able to index so you're going to have significant blocking problems and poorly performing table scans - it's a perfect storm. Worse, in this case it's going to force dynamic sql on us with all the inherent risks. And because you've got field and table names in there we're stuck with string concatenation which is even worse. More than a perfect storm, it's a perfect tsunami.

I know it's not the thrust of your question so I apologise if I'm hijacking your thread but I see this pattern used a lot and it needs challenging anywhere it raises it's ugly head. It's a fantastic pattern if used in a NoSQL database but in a relational database it's horrible.

Anyway, given that we're stuck with some dynamic sql, we can at least get rid of your loop which should help somewhat with performance:-

--Some Set Up :-
 
Create Table #LA (idLog int, [table] varchar(max), field varchar(max), oldvalue int, newvalue int, type char(1), usr varchar(max), dt datetime)
insert into #LA Values (1, '#companies', 'idowner', 1,5,'U','the user','2018-09-05 10:00:00')
 
Create Table #companies (idowner int, [Value] varchar(max))
Insert into #companies values (1,'smith'),(5,'jones')
 
Declare @Table varchar(max) = '#companies'
Declare @User varchar(max) = 'the user'
Declare @Date DateTime = '2018-09-05 00:00:00'
 
 
--This is what should be in your sproc
Declare @SQL nvarchar(max)
Declare @ParamDef nvarchar(max)
Declare @Field nvarchar(max)
 
Select @Field = Field
From #LA
Where [table] = @Table
 
Set @SQL = 'Select idLog, [table], T1.[Value], T2.[Value]
            From #LA LA
            Join ' + @Table + ' T1
                on LA.oldvalue = T1.' + @Field + '
            Join ' + @Table + ' T2
                on LA.newvalue = T2.' + @Field + '
            Where [table] = @Table
            and usr = @User
            and dt between @Date and DateAdd(Day, 1, @Date)'
 
Set @ParamDef = '@Table varchar(max), @User varchar(max), @Date datetime'
 
execute sp_executesql @Statement = @SQL, @Params = @ParamDef, @Table = @Table, @User = @User, @Date = @Date
 
Drop Table #LA
Drop Table #companies

[Ecniv]

Thanks for the reply.

I cannot show the data as the p and pv tables are flexible values as parameters in another set of tables. They werent a problem here... just how to better connect to the original tables for old and new values and whether there was a better approach.

Not my design, I'm just administrating on top of the coding/tables already there. Appreciate its a poor design and perhaps should be crushed under a boot heel way before it grows, but life is such... can't change it.

I like the look of your sql, seems much cleaner, but I have to ask, the Sql will only do one hit (correct?).

So if I filter by user, date and update type, I am going to retrieve several lines of changes.
Hence the loop connecting each of those returned records.

Also, do you happen to have a link to a better way to log changes of records. Just for personal use in the future.
My personal way is to duplicate the whole record to another table to show all fields changed, but I know this takes a lot of space. Which is why I think they implemented this design.
Which is fine until you need to show the client the changes and they ask for the actual values instead of ids.

Thank you again for your response

[FunkyDexter]

the Sql will only do one hit (correct?)

Nope, it should do multiple rows. Basically anything that meets the where clause :-

            Where [table] = @Table
            and usr = @User
            and dt between @Date and DateAdd(Day, 1, @Date)'

One potential mistake I did just spot: I've assumed that the value of Field will always be the same for a given table. I think that's meant to be the primary key so that should be correct. If it's not then this bit:-
SELECT @FIELD = FIELD
FROM #LA
WHERE [TABLE] = @TABLE

Would be incorrect.

[Ecniv]

Hi

Thanks.

The Field could be any field in that table which has been recorded, rather than just the pk.

Log Alterations table would be :
idLogAlt (pk), tablename, idfktable, fieldname, usrupdated, the action (U,I,D), old value, new value.




You have been very helpful and explained it well.

Thank you for your assistance and for taking the time to post back.

[FunkyDexter]

The Field could be any field in that table which has been recorded, rather than just the pk.

Hmmm, I think you're stuck with the loop as well then. I can't see a way of feeding the Field name into the query without it... which sucks.