November 15, 2023 at 3:21 pm
HI All.
Iam looking for some support where i have been stuck since few days while converting the below PL SQL logic into Microsoft SQL Server Stored procedure logic.
===========================================
================ Pl SQL SP ===================
===========================================
procedure SP_CoCASA_FollowUp(
p_FollowUp_ID IN int,
p_err_code OUT number,
p_err_msg OUT varchar2,
p_CoCASA_cursor OUT t_cursor
)
as r_results t_cursor;
ccsql varchar2(3500); endsql varchar2(6500); mmrvsql varchar2(2500); hhsql varchar2(500); cptsql varchar2(500);
FluStartDate date; FluEndDate date; v_AsOfDate date; v_site integer; v_SAge integer;
begin
select distinct data_pulled_as_of, site_id, beginning_age into v_AsOfDate, v_site, v_SAge from aimsdba.cocasa_followup where cocasa_followup_id = p_FollowUp_ID;
--get date range for flu
if to_char(v_AsOfDate,'mm') <= 6 then
FluStartDate := to_date('07/01' || (to_char(v_AsOfDate,'yyyy')-2),'mm/dd/yyyy');
FluEndDate := to_date('06/30' || (to_char(v_AsOfDate,'yyyy')-1),'mm/dd/yyyy');
else
FluStartDate := to_date('07/01' || (to_char(v_AsOfDate,'yyyy')-1),'mm/dd/yyyy');
FluEndDate := v_AsOfDate;
end if;
--making sure string are empty
ccsql := '';
mmrvsql :='';
hhsql := '';
cptsql := '';
endsql := '';
--Retrieve details
ccsql := 'select p.patient_id as PatientID, ';
ccsql := ccsql || '(select ds.data_source_nbr from aimsdba.pat_data_source ds where patient_id = p.patient_id ';
--Do not return patients with temporary CHR numbers
ccsql := ccsql || ' and not (ds.data_source_nbr like ''' || 'T%' || ''' and ds.data_source_id = 658)
and p.patient_id = ds.patient_id(+) ';
if v_site <> -1 then
--if site selected, only return patients from that site
ccsql := ccsql || ' and ds.site_id = ' || v_site;
end if;
--fields selected, Y for chickenpox if present, and using generic CPT code for that vaccine
ccsql := ccsql || ' and rownum = 1) as ChartNbr, p.first_name as First_Name, substr(p.middle_name,1,1) as M_Initial,
p.last_name as Last_Name, to_char(p.dob, ''' || 'mm/dd/yyyy' || ''') as dob, ';
--cpt has it's own section because it will be treated differently for MMRV and HepB-Hib
cptsql := '(case
when vh.vaccine_id = 51 then
(select CVX_code from aimsdba.vaccine_cpt_xref where vaccine_id = 45 and rownum = 1)
else
(select CVX_code from aimsdba.vaccine_cpt_xref where vaccine_id = vh.vaccine_id and rownum = 1)
end) CVX_code,';
endsql := ccsql || cptsql || 'to_char(vh.vaccination_date, ''' || 'mm/dd/yyyy' || ''') as VaccinationDate,
(select ''' || 'Y' || ''' from aimsdba.disease_history where patient_id = p.patient_id and antigen_id IN (31) and rownum=1) as Chickenpox,
(case
when gender_id = 309 then 1
when gender_id = 310 then 2
else 98
end) gender,
(case
when p.status_id = 1200 or p.status_id = 1201 or p.status_id = 408 or p.status_id = 1949 then ''' || 'Patient inactivated at state level' || '''
else ''' || '' || '''
end) status_change,
(case (select status_id from aimsdba.pat_data_source where patient_id = p.patient_id and site_id = ' || v_site || ' and rownum <=1 )
when 1200 then ''' || 'Patient inactivated at site level' || '''
when 1201 then ''' || 'Patient inactivated at site level' || '''
when 408 then ''' || 'Patient inactivated at site level' || '''
when 1949 then ''' || 'Patient inactivated at site level' || '''
else ''' || '' || '''
end) site_status_change
from aimsdba.patient p,
aimsdba.vaccine_history vh, aimsdba.vaccine v ';
--different vaccines/vaccine families returned for adolescents and children - only return flu for specified range
if v_SAge < 132 then
endsql := endsql || 'where (vh.vaccine_id IN (select vaccine_id from aimsdba.vaccine_family_xref VF
WHERE family_id IN (629, 560, 1203, 556, 558, 634, 638, 627, 632, 562, 563, 631, 3118))
OR (vh.vaccine_id in (select vaccine_id from aimsdba.vaccine_family_xref VF
WHERE family_id = 637) and vaccination_date >= ' || '''' || FluStartDate || '''' || ' and
vaccination_date <= ' || '''' || FluEndDate || '''))';
else
endsql := endsql || 'where (vh.cvx_code in (138,139,113,9,115,45,44,43,8,42,21,108,114,136,62,118,137,3,32,51,94)
OR (vh.vaccine_id in (select vaccine_id from aimsdba.vaccine_family_xref VF
WHERE family_id = 637) and vaccination_date >= ' || '''' || FluStartDate || '''' || ' and
vaccination_date <= ' || '''' || FluEndDate || ''')) ' ;
end if;
endsql := endsql || 'and p.patient_id = vh.patient_id
and vh.vaccine_id = v.vaccine_id ';
endsql := endsql || ' and p.patient_id in (select patient_id from aimsdba.cocasa_followup cc where cocasa_followup_id = ' || p_FollowUp_ID || '
and cc.patient_id not in (select patient_id from aimsdba.pat_data_source where site_id = cc.site_id and patient_Id = cc.patient_Id and status_Id in (1200,1201,408,1949)))';
--special handling of MMRV for adolescents
if v_SAge >= 132 then
mmrvsql := 'UNION ' || ccsql || 'vc.cvx_code, to_char(vh.vaccination_date, ''' || 'mm/dd/yyyy' || ''') as VaccinationDate,
(select ''' || 'Y' || ''' from aimsdba.disease_history where patient_id = p.patient_id and antigen_id IN (31) and rownum=1) as Chickenpox,
(case
when gender_id = 309 then 1
when gender_id = 310 then 2
else 98
end) gender,
(case
when p.status_id = 1200 or p.status_id = 1201 or p.status_id = 408 or p.status_id = 1949 then ''' || 'Patient inactivated at state level' || '''
else ''' || '' || '''
end) status_change,
(case (select status_id from aimsdba.pat_data_source where patient_id = p.patient_id and site_id = ' || v_site || ' and rownum <=1)
when 1200 then ''' || 'Patient inactivated at site level' || '''
when 1201 then ''' || 'Patient inactivated at site level' || '''
when 408 then ''' || 'Patient inactivated at site level' || '''
when 1949 then ''' || 'Patient inactivated at site level' || '''
else ''' || '' || '''
end) site_status_change
from aimsdba.patient p, aimsdba.vaccine_history vh, aimsdba.vaccine_cpt_xref vc
where vh.vaccine_id = 94 and vc.vaccine_id in (3,21) and p.patient_id = vh.patient_id
and p.patient_id in (select patient_id from aimsdba.cocasa_followup cc where cocasa_followup_id = ' || p_FollowUp_ID ||
' and cc.patient_id not in (select patient_id from aimsdba.pat_data_source where site_id = cc.site_id and patient_Id = cc.patient_Id and status_Id in (1200,1201,408,1949)))';
endsql := endsql || mmrvsql;
end if;
endsql := endsql || 'order by PatientID, last_name, dob, cvx_code, vaccinationdate';
execute immediate 'begin OPEN :c for ' || endsql ||'; end;' using r_results;
p_CoCASA_cursor := r_results;
p_err_code := 1;
p_err_msg := 'Success';
exception
when others then
p_err_code := sqlcode;
p_err_msg := substr(sqlerrm,1,250);
end;
I have converted this Pl SQL syntax as shown below but stuck with few errors while executing' it
===============================================
================Converted SQL SP ==================
===============================================
ALTER PROCEDURE [AIMSDBA].[P_COCASA_SP_COCASA_FOLLOWUP]
@P_FOLLOWUP_ID INT,
@P_ERR_CODE INT OUTPUT,
@P_ERR_MSG VARCHAR(250) OUTPUT
-- ,@P_COCASA_CURSOR VARCHAR(4000) OUTPUT
AS
BEGIN
BEGIN TRY
DECLARE @R_RESULTS CURSOR;
DECLARE @ccsql VARCHAR(3500), @ENDSQL VARCHAR(6500), @MMRVSQL VARCHAR(2500), @HHSQL VARCHAR(500), @CPTSQL VARCHAR(500);
DECLARE @FLUSTARTDATE DATE, @FLUENDDATE DATE, @V_ASOFDATE DATE, @V_SITE INT, @V_SAGE INT;
SELECT DISTINCT @V_ASOFDATE = DATA_PULLED_AS_OF,
@V_SITE = SITE_ID,
@V_SAGE = BEGINNING_AGE
FROM AIMSDBA.COCASA_FOLLOWUP
WHERE COCASA_FOLLOWUP_ID = @P_FOLLOWUP_ID;
-- GET DATE RANGE FOR FLU
IF MONTH(@V_ASOFDATE) <= 6
BEGIN
SET @FLUSTARTDATE = DATEFROMPARTS(YEAR(@V_ASOFDATE) - 2, 7, 1);
SET @FLUENDDATE = DATEFROMPARTS(YEAR(@V_ASOFDATE) - 1, 6, 30);
END
ELSE
BEGIN
SET @FLUSTARTDATE = DATEFROMPARTS(YEAR(@V_ASOFDATE) - 1, 7, 1);
SET @FLUENDDATE = @V_ASOFDATE;
END;
--SELECT @FLUSTARTDATE AS FLUSTARTDATE ,@FLUENDDATE AS FLUENDDATE
-- SQLINES DEMO *** are empty
SET @ccsql = '';
SET @MMRVSQL ='';
SET @HHSQL = '';
SET @CPTSQL = '';
SET @ENDSQL = '';
SET @ccsql = 'SELECT P.PATIENT_ID AS PATIENTID, ';
SET @ccsql = ISNULL(@CCSQL, '') + '(SELECT DS.DATA_SOURCE_NBR FROM AIMSDBA.PAT_DATA_SOURCE DS WHERE PATIENT_ID = P.PATIENT_ID ';
--DO NOT RETURN PATIENTS WITH TEMPORARY CHR NUMBERS
SET @ccsql = ISNULL(@CCSQL, '') + ' AND NOT (DS.DATA_SOURCE_NBR LIKE ''' + 'T%' + ''' AND DS.DATA_SOURCE_ID = 658)
AND P.PATIENT_ID = DS.PATIENT_ID(+) ';
IF @V_SITE <> -1 BEGIN
-- SQLINES DEMO *** ONLY RETURN PATIENTS FROM THAT SITE
SET @ccsql = ISNULL(@CCSQL, '') + ' AND DS.SITE_ID = ' + ISNULL(@V_SITE, '');
END
-- SQLINES DEMO *** FOR CHICKENPOX IF PRESENT, AND USING GENERIC CPT CODE FOR THAT VACCINE
SET @ccsql = ISNULL(@CCSQL, '') + ' AND ROW_NUMBER() OVER (ORDER BY @V_SITE) AS ROWNUM) AS CHARTNBR, P.FIRST_NAME AS FIRST_NAME, SUBSTR(P.MIDDLE_NAME,1,1) AS M_INITIAL,
P.LAST_NAME AS LAST_NAME, TO_CHAR(P.DOB, ''' + 'MM/DD/YYYY' + ''') AS DOB, ';
-- SQLINES DEMO *** ECTION BECAUSE IT WILL BE TREATED DIFFERENTLY FOR MMRV AND HEPB-HIB
SET @CPTSQL ='(CASE
WHEN VH.VACCINE_ID = 51 THEN
(SELECT CPT_CODE FROM AIMSDBA.VACCINE_CPT_XREF WHERE VACCINE_ID = 45 AND ROWNUM = 1 AND CPT_CODE NOT IN (11111,11112))
ELSE
(SELECT CPT_CODE FROM AIMSDBA.VACCINE_CPT_XREF WHERE VACCINE_ID = VH.VACCINE_ID AND ROWNUM = 1 AND CPT_CODE NOT IN (11111,11112))
END) CPT_CODE, ';
SET @ENDSQL = COALESCE(@CCSQL, '') + COALESCE(@CPTSQL, '') + 'CONVERT(VARCHAR(10), VH.VACCINATION_DATE, 101) AS VACCINATIONDATE,
(SELECT ''Y'' FROM AIMSDBA.DISEASE_HISTORY WHERE PATIENT_ID = P.PATIENT_ID AND ANTIGEN_ID IN (31) LIMIT 1) AS CHICKENPOX,
(CASE
WHEN GENDER_ID = 309 THEN 1
WHEN GENDER_ID = 310 THEN 2
ELSE 98
END) AS GENDER,
(CASE
WHEN P.STATUS_ID = 1200 OR P.STATUS_ID = 1201 OR P.STATUS_ID = 408 OR P.STATUS_ID = 1949 THEN ''PATIENT INACTIVATED AT STATE LEVEL''
ELSE ''''
END) AS STATUS_CHANGE,
(CASE (SELECT STATUS_ID FROM AIMSDBA.PAT_DATA_SOURCE WHERE PATIENT_ID = P.PATIENT_ID AND SITE_ID = ' + ISNULL(CONVERT(VARCHAR, @V_SITE), '') + ')
WHEN 1200 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 1201 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 408 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 1949 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
ELSE ''' + '' + '''
END) SITE_STATUS_CHANGE
FROM AIMSDBA.PATIENT P,
AIMSDBA.VACCINE_HISTORY VH, AIMSDBA.VACCINE V ';
--DIFFERENT VACCINES/VACCINE FAMILIES RETURNED FOR ADOLESCENTS AND CHILDREN - ONLY RETURN FLU FOR SPECIFIED RANGE
IF @V_SAGE < 156 BEGIN
SET @ENDSQL = @ENDSQL + 'WHERE VH.VACCINE_ID IN (SELECT VACCINE_ID FROM AIMSDBA.VACCINE_FAMILY_XREF VF
WHERE FAMILY_ID IN (629, 560, 1203, 556, 558, 634, 638, 627, 632, 562, 563, 3118, 631)
OR (VH.VACCINE_ID IN (SELECT VACCINE_ID FROM AIMSDBA.VACCINE_FAMILY_XREF VF
WHERE FAMILY_ID = 637) AND VACCINATION_DATE >= ''' + CONVERT(VARCHAR, @FLUSTARTDATE, 120) + ''' AND
VACCINATION_DATE <= ''' + CONVERT(VARCHAR, @FLUENDDATE, 120) + '''))';
END
ELSE
BEGIN
SET @ENDSQL = ISNULL(@ENDSQL, '') + 'WHERE VH.VACCINE_ID IN (9, 99, 45, 21, 97, 102, 108, 3)
OR (VH.VACCINE_ID IN (SELECT VACCINE_ID FROM AIMSDBA.VACCINE_FAMILY_XREF VF
WHERE FAMILY_ID = 637) AND VACCINATION_DATE >= ''' + CONVERT(VARCHAR, @FLUSTARTDATE, 120) + ''' AND
VACCINATION_DATE <= ''' + CONVERT(VARCHAR, @FLUENDDATE, 120) + ''')';
END
SET @ENDSQL = @ENDSQL + 'AND P.PATIENT_ID = VH.PATIENT_ID
AND VH.VACCINE_ID = V.VACCINE_ID ';
SET @ENDSQL = @ENDSQL + 'AND P.PATIENT_ID IN (SELECT PATIENT_ID FROM AIMSDBA.COCASA_FOLLOWUP WHERE COCASA_FOLLOWUP_ID = ' + CAST(@P_FOLLOWUP_ID AS VARCHAR) + ')';
IF @V_SAGE >= 156
BEGIN
SET @MMRVSQL = 'UNION ' + @ccsql + 'VC.CPT_CODE, CONVERT(VARCHAR, VH.VACCINATION_DATE, 101) AS VACCINATIONDATE,
CASE
WHEN GENDER_ID = 309 THEN 1
WHEN GENDER_ID = 310 THEN 2
ELSE 98
END AS GENDER,
CASE
WHEN P.STATUS_ID = 1200 OR P.STATUS_ID = 1201 OR P.STATUS_ID = 408 OR P.STATUS_ID = 1949 THEN ''' + 'PATIENT INACTIVATED AT STATE LEVEL' + '''
ELSE ''
END AS STATUS_CHANGE,
(CASE (SELECT STATUS_ID FROM AIMSDBA.PAT_DATA_SOURCE WHERE PATIENT_ID = P.PATIENT_ID AND SITE_ID = ' + ISNULL(@V_SITE, '') + ')
WHEN 1200 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 1201 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 408 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
WHEN 1949 THEN ''' + 'PATIENT INACTIVATED AT SITE LEVEL' + '''
ELSE ''' + '' + '''
END) SITE_STATUS_CHANGE
FROM AIMSDBA.PATIENT P, AIMSDBA.VACCINE_HISTORY VH, AIMSDBA.VACCINE_CPT_XREF VC
WHERE VH.VACCINE_ID = 94 AND VC.VACCINE_ID IN (3,21) AND P.PATIENT_ID = VH.PATIENT_ID
AND P.PATIENT_ID IN (SELECT PATIENT_ID FROM AIMSDBA.COCASA_FOLLOWUP WHERE COCASA_FOLLOWUP_ID = ' + CAST(@P_FOLLOWUP_ID AS VARCHAR) + ')'
SET @ENDSQL = @ENDSQL + @MMRVSQL;
END;
SET @ENDSQL = @ENDSQL + 'ORDER BY PATIENTID, LAST_NAME, DOB, CPT_CODE, VACCINATIONDATE';
PRINT @endsql;
EXECUTE sp_executesql @ENDSQL, N'@C CURSOR OUTPUT', @C = @R_RESULTS OUTPUT;
--SET @P_COCASA_CURSOR = @R_RESULTS;
SET @P_ERR_CODE = 1;
SET @P_ERR_MSG = 'SUCCESS';
RETURN;
END TRY
BEGIN CATCH
SET @P_ERR_CODE = ERROR_NUMBER();
SET @P_ERR_MSG = LEFT(ERROR_MESSAGE(), 250);
END CATCH;
END;
But while executing's the above procedure started seeing below errors.
@P_ERR_CODE @P_ERR_MSG
245 Conversion failed when converting the varchar value 'SELECT P.PATIENT_ID AS PATIENTID, (SELECT DS.DATA_SOURCE_NBR FROM AIMSDBA.PAT_DATA_SOURCE DS WHERE PATIENT_ID = P.PATIENT_ID AND NOT (DS.DATA_SOURCE_NBR LIKE 'T%' AND DS.DATA_SOURCE_ID = 658) A
Please help to address to fix, i have tried all possible ways but no luck since from couple of days.
Thank you all.
Pap.P
November 15, 2023 at 7:39 pm
VERY quick look at that, you have a bunch of dynamic SQL which is always a bit risky, so my opinion, I would re-write the whole thing without the dynamic SQL.
But the error is plain as day - Conversion failed when converting the varchar value. WHY this is happening we cannot say with any certainty as you are truncating the end of the error message!! WHY, oh why are you truncating the error message down to 250 characters? If you truncate it down to 250 characters you don't know the full error. Best practice with ANY coding language is to NEVER truncate error messages. The only exception I can think of is if you know that a certain point in the technical error message contains a friendly error message and you can substring that to get the friendly error message out of the mess. But even then, that would only apply to specific error codes, so you'd still want to keep the full error and compare it to the error code...
My guess is that it is failing to convert the VARCHAR value to a CURSOR, but I don't know for sure because you truncated the error message!
My last question for you is why are you doing this in dynamic SQL? Is there a reason you NEED dynamic SQL? As a general rule, I try to avoid dynamic SQL except in very specific scenarios and those scenarios are usually due to some administrative task and then the dynamic SQL is being generated with "safe" input. What I mean by safe input is that nobody can do SQL injection on it. If I am not 100% certain that malicious SQL injection is impossible on my dynamic query, then I will refuse to let it hit production. At my current workplace, back when I was in IT (14 years back), I was doing some testing of an app developed by our IS team. It used a table for storing usernames and hashed passwords. When the app started, you had to enter your credentials. I put in my username as:
' or 1=1 '
and then any password I felt like as it didn't matter and guess what? the app let me in! Permissions were all screwed up as it couldn't figure out who I was, but it let me in. Now, what if I had tossed in something like ";DROP TABLE" instead? Since I wasn't authenticated (it was an IIS app using a shared account) and I doubt that the developer would have used an admin level account, I would expect the drop to fail, but I could snoop around to make some guesses about what permissions it had and what if by some dumb chance, they had it running as sa and I could shut down the instance with some malformed SQL? or I create a job to shut down the instance that runs every minute? or I just go ahead and make a new backdoor account that is full sysadmin? Dynamic SQL is RISKY stuff...
I think I narrowed down the issue! I am pretty sure (90% sure) you can't have a cursor as an output to a dynamic SQL call, and even if you CAN, I am 99.9999% sure that isn't the return type you are looking for. I think what you are trying to do is return the results to a table, but I am not 100% certain that is your end goal.
Converting code from one language (PLSQL) to another (SQL Server) is not just simply copy-pasting the code and then fixing syntax. The BEST way to do it is to make sure you understand the requirements and then build your query from scratch so you can make sure to use the appropriate data types and syntax. And I am not sure what best practices are with PLSQL, but in SQL Server, for best performance, I avoid cursors as much as I can. They have their place, but it is not as often as I've seen them used. Same thing with dynamic SQL, but that is more to reduce risk of bad actors than for performance.
Now with the above being said, I would STRONGLY encourage you to use the "code" button when posting code as it is a TON easier to read and I would try to use some sort of code formatting because your code is incredibly hard to decipher when it is just a bunch of text like that. Having it in code blocks is a TON easier to read.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
November 15, 2023 at 8:16 pm
as Brian said you should not be trying to convert as is but rather understand the requirements and do fresh and better code targetting SQL Server.
for the particular error you have - your issue is "+ isnull(@V_SITE, '')" - as @V_SITE is a int, SQL is trying to convert the remaining string to a int, which obviously fails.
you need to convert @V_SITE to a varchar(20) and only then concatenate it.
you also have a few other issues
and potentially a few other issues. adding to the fact that the whole SQL while it can behave well in Oracle, in SQL Server it would behave rather bad for any type of volumes even after you fix the above issues.
November 15, 2023 at 9:27 pm
VERY quick look at that, you have a bunch of dynamic SQL which is always a bit risky, so my opinion, I would re-write the whole thing without the dynamic SQL.
But the error is plain as day - Conversion failed when converting the varchar value. WHY this is happening we cannot say with any certainty as you are truncating the end of the error message!! WHY, oh why are you truncating the error message down to 250 characters? If you truncate it down to 250 characters you don't know the full error. Best practice with ANY coding language is to NEVER truncate error messages. The only exception I can think of is if you know that a certain point in the technical error message contains a friendly error message and you can substring that to get the friendly error message out of the mess. But even then, that would only apply to specific error codes, so you'd still want to keep the full error and compare it to the error code...
My guess is that it is failing to convert the VARCHAR value to a CURSOR, but I don't know for sure because you truncated the error message!
My last question for you is why are you doing this in dynamic SQL? Is there a reason you NEED dynamic SQL? As a general rule, I try to avoid dynamic SQL except in very specific scenarios and those scenarios are usually due to some administrative task and then the dynamic SQL is being generated with "safe" input. What I mean by safe input is that nobody can do SQL injection on it. If I am not 100% certain that malicious SQL injection is impossible on my dynamic query, then I will refuse to let it hit production. At my current workplace, back when I was in IT (14 years back), I was doing some testing of an app developed by our IS team. It used a table for storing usernames and hashed passwords. When the app started, you had to enter your credentials. I put in my username as:
' or 1=1 '
and then any password I felt like as it didn't matter and guess what? the app let me in! Permissions were all screwed up as it couldn't figure out who I was, but it let me in. Now, what if I had tossed in something like ";DROP TABLE" instead? Since I wasn't authenticated (it was an IIS app using a shared account) and I doubt that the developer would have used an admin level account, I would expect the drop to fail, but I could snoop around to make some guesses about what permissions it had and what if by some dumb chance, they had it running as sa and I could shut down the instance with some malformed SQL? or I create a job to shut down the instance that runs every minute? or I just go ahead and make a new backdoor account that is full sysadmin? Dynamic SQL is RISKY stuff...
I think I narrowed down the issue! I am pretty sure (90% sure) you can't have a cursor as an output to a dynamic SQL call, and even if you CAN, I am 99.9999% sure that isn't the return type you are looking for. I think what you are trying to do is return the results to a table, but I am not 100% certain that is your end goal.
Converting code from one language (PLSQL) to another (SQL Server) is not just simply copy-pasting the code and then fixing syntax. The BEST way to do it is to make sure you understand the requirements and then build your query from scratch so you can make sure to use the appropriate data types and syntax. And I am not sure what best practices are with PLSQL, but in SQL Server, for best performance, I avoid cursors as much as I can. They have their place, but it is not as often as I've seen them used. Same thing with dynamic SQL, but that is more to reduce risk of bad actors than for performance.
Now with the above being said, I would STRONGLY encourage you to use the "code" button when posting code as it is a TON easier to read and I would try to use some sort of code formatting because your code is incredibly hard to decipher when it is just a bunch of text like that. Having it in code blocks is a TON easier to read.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
November 16, 2023 at 2:37 pm
Thank you Mr. Brian Gale and much appreciated your recommendations looking forward while posting any code .
As per suggestions given by fellow teammates we are "truncating the error message down to 250 characters", I will take this case to them to review back and address the user friendly error message displaying.
Thank you for all your value feedback and coding standard which make me aware as an entry level coder in this area.
Thank you.
PAP
November 16, 2023 at 2:39 pm
Thank you frederico_fonseca.
Let me check those area which was suggested by you to address this issue.
Thank you.
PaP
November 16, 2023 at 4:51 pm
Fed? Brain? I hate to think what you would call me.
November 16, 2023 at 4:58 pm
Sorry for typos'Phil Parkin. Now this has been corrected.
November 16, 2023 at 5:02 pm
Welcome to the dev world then! When I was new to SQL, one of the hardest things I had to wrap my brain around was "row based operations" vs "column based operations". I came into SQL with a background in C#, so my early SQL code was riddled with loops as I couldn't wrap my brain around column based operations. After being a DBA for years (10+ years), I get frustrated that C# can't do column based operations without a loop. Once you get into the "column based operations" mindset, you will start to wonder where you would need a loop in SQL! There ARE use cases, but they are VERY limited.
Early in my development career with my current employer (thinking it was around 2009-ish), I worked to convert some functionality out of an old VB6 app into C#. Risk was low because if my new app didn't work, they could fall back onto the old VB6 app. Translating the logic from VB6 to C# was messy and my code was not very good as I was trying to make C# work like VB6. So I had REALLY poor design decisions per the language and created a TON of technical debt. So throughout the years, as time permitted, we added features to the app, and worked to clean up the code to make it more maintainable and more of an OOP approach to the code. Something to note here - VB6 as an IDE was end of EXTENDED support in 2008, so we were late to getting that change out the door. Technically, VB6 applications are still supported in Windows 11!
I learned quickly that translating from VB6 to C# was not trivial and even though SOME of it was copy-pasted code, the copy-pasted code did not perform well and it was hard to support. Same thing going from PLSQL to TSQL. SOME of it will copy-paste across without issue, while other things will fail due to mismatched syntax. A MUCH better approach is to take small chunks of the code and translate it across. Break the code up into small chunks on BOTH the PLSQL side and the SQL Server side and make sure that the small chunks produce the same results. I would also see if you CAN remove the dynamic SQL if possible, but if you can't, just make sure that you have some mechanism in place to do some sanitization on the SQL before you run it.
The other thing I would do is NOT execute the dynamic SQL while you are in the development and testing stage - instead print the SQL out and review the result to see if it is what you would expect.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply