On January 27th, this site will be read-only as we migrate to Oracle Forums for an improved community experience. You will not be able to initiate activity until January 30th, when you will be able to use this site as normal.

    Forum Stats

  • 3,889,534 Users
  • 2,269,755 Discussions
  • 7,916,775 Comments

Discussions

Dynamic sql in procedure for inserting

Rafal S
Rafal S Member Posts: 15
edited Jul 2, 2020 6:41AM in SQL & PL/SQL

Dear Oracle Family

I am struggling with a problem below:

I want users to provide information in stored proc like user_id,country_id ,category_id and based on that i would like to run insert(or merge) statement.

So i was thinking that if i generate string as 'insert into test_table values user_id,country,country_id' and then execute immediate that it would work  but its not.

I tried to play a bit with bind variables but without any luck ;/

below You can see only select statement as try to see how it would work.

Additionally my second requirement is that sometimes users can provide group of countries for ex EUROPE(i have mapping from europe to countries in other table )-but not sure how to touch that also so i focused on 1 country per execute for now.

Thank You for any tips !!!

CREATE or replace PROCEDURE add_user_test (user_id in VARCHAR2,country in VARCHAR2,category_id integer ) ASsqlrun varchar2(1024) :='select '||user_id||' from dual '; BEGIN   -- dbms_output.put_line(sqlrun);   EXECUTE IMMEDIATE sqlrun ;END;exec add_subscriptions_test('sas')
Tagged:
BEDERafal S

Best Answer

  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 5:23AM Answer ✓

    I ask again, what about a simple merge ?

    BEGIN   MERGE INTO TableWithSubscriptions c     using ( select v_user                      as out_user_id,                    get_aff_id    (v_affiliate) as out_affiliate_id,                    get_category_id(v_category) as out_category_id,                    v_frequency                 as out_frequency               from dual ) s        ON (  c.user_id     = s.out_user_id           and c.affiliate   = s.out_affiliate_id          and c.category_id = s.out_category_id )    when not matched then          insert (user_id,     affiliate,        category_id,     add_comment, add_subcomment, action_item_change, active, notification_frequency, last_modified)          VALUES (out_user_id, out_affiliate_id, out_category_id, 1,           1,              1,                  1,      out_frequency,          systimestamp);end;

    btw don't you need a when-matched-clause ?

    BEDERafal S
«1

Answers

  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 3:19AM

    Hi Rafal,

    the question is not how you manage to do something like that you asked above.

    The question is "why do you want to do it with execute immediate" ?

    What are your requirements ?

    If you code with execute immediate you need an output of statement for you own.

    What do you want to show us with your example ?

    It may produce a statement like this

         select scott from dual

    What do you expect from a query like this ?

    It can work if do it in this way.

        sqlrun varchar2(1024) :='select '''||user_id||''' from dual ';  

    But what will be the expected result ?

    regards

    Kay

  • Rafal S
    Rafal S Member Posts: 15
    edited Jul 2, 2020 3:32AM

    What i would like to do is to insert data based on what users are gonna put into procedure variables

    So lets say someone would exec add_user_test('username123','Poland','Finance','Daily') then i would like to insert this values into a table (or merge) .

    so in a table would look like :

    username|Ireland|finance|daily

    There is also possiblity that someone would put  ('username123','Europe','AllDepartments','Daily') so i would like to insert this data for all europe countries (i have them mapped in a table) and all departments (also mapped)

    so in a table would look like :

    username123|Ireland|Finance|daily

    username123|France|Sales|daily

    username123|Spain|Marketing|daily

    I made some progress while i posted it so it looks like that for now  :

    create or replace procedure my_procedure    ( v_user varchar2    ,v_country varchar2    , v_category varchar2    ,v_frequency varchar2)AUTHID CURRENT_USER ASbegin       execute immediate 'INSERT INTO z_TEMPO (user_id,aff_id,category_id,notification_frequency)                       Select :user_out, get_aff_id(:id_out),get_category_id(:category_out),:frequency_out                       FROM dual' using v_user, v_country,v_category,v_frequency;end my_procedure;
  • BEDE
    BEDE Oracle Developer Bucharest, RomaniaMember Posts: 2,484 Gold Trophy
    edited Jul 2, 2020 3:43AM

    That's worst practice possible. That is what is called SQL injection and is generally to be avoided.

    And also, avoid dynamic SQL as much as you can because it may lead to parsing errors at runtime, which are quite hard to debug. The only practical way to debug dynamic SQL is by logging into a table the SQL statement itself plus the values for the bind variables used, that logging being performed on exception.

    Why not use plain SQL, not dynamic? Could you give me a good reason.

    I usually use dynamic SQL to execute some DDL.

    Then, dynamic SQL is to be used when the table name in some SQL statement must vary. But, by different database and application design, such cases can be avoided in most of the situations.

  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 3:44AM

    What about a simple insert like this

    INSERT INTO z_TEMPO (user_id, aff_id, category_id, notification_frequency) values ( v_user, get_aff_id(v_country), get_category_id(v_category), v_frequency );

    I think you don't need an execute immediate.

    It looks like there may be a only handfull of different types of insert statements to do what you want.

    Don't use dynamic sql if you can avoid it. And most the times that is possible.

  • Rafal S
    Rafal S Member Posts: 15
    edited Jul 2, 2020 3:50AM

    ok i am fine with avoiding exec immediate but how then my procedure would look like ?

    I would like to still have possibility to exec like that :

    execute my_procedure('username123','Spain','Accounting','Daily')

    Also what could be good approach when i would like to run this over all European Countries for example ?

    Thank You for this valuable lessons !!!

  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 4:18AM

    You have to define your requirements.

    It seems your procedure will have 4 parameters.

    What should happen with an explicit username, what happens if you have a wildcard or something like "all_users" ? The same for the other parameters

    That may leed to a simple insert of one row or some other queries/loops/procedures to generate more rows to insert.

    It will be same work if you use dynamic sql.

  • Rafal S
    Rafal S Member Posts: 15
    edited Jul 2, 2020 5:00AM

    i want to first explore possibility of adding only one user to one country(affiliate) and one category.

    So i have now (and its working).Any tips how to rewrite so be more safe appreciated :

    CREATE OR REPLACE PROCEDURE ADD_SUBSCRIPTION     ( v_user varchar2    ,v_affiliate varchar2    , v_category varchar2    ,v_frequency varchar2)     AS  plsql_block   VARCHAR2(4000);     BEGINplsql_block := 'MERGE INTO TableWithSubscriptions c using  ( select :out_user as out_user_id,get_aff_id(:out_affiliate) as out_affiliate_id,get_category_id(:out_category) as out_category_id,:out_frequency as out_frequency from dual  ) s ON ( c.user_id = s.out_user_id  and out_affiliate_id=affiliate              and c.category_id = s.out_category_id )     when not matched then insert (user_id, affiliate, category_id, add_comment, add_subcomment, action_item_change, active, notification_frequency, last_modified)                    VALUES (out_user_id, out_affiliate_id,out_category_id,1, 1, 1, 1, out_frequency, systimestamp)';EXECUTE IMMEDIATE plsql_block USING v_user,v_affiliate,v_category,v_frequency;dbms_output.put_line (plsql_block);END ADD_SUBSCRIPTION;exec add_subscription('username123','France','Acounting','Daily')
  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 5:23AM Answer ✓

    I ask again, what about a simple merge ?

    BEGIN   MERGE INTO TableWithSubscriptions c     using ( select v_user                      as out_user_id,                    get_aff_id    (v_affiliate) as out_affiliate_id,                    get_category_id(v_category) as out_category_id,                    v_frequency                 as out_frequency               from dual ) s        ON (  c.user_id     = s.out_user_id           and c.affiliate   = s.out_affiliate_id          and c.category_id = s.out_category_id )    when not matched then          insert (user_id,     affiliate,        category_id,     add_comment, add_subcomment, action_item_change, active, notification_frequency, last_modified)          VALUES (out_user_id, out_affiliate_id, out_category_id, 1,           1,              1,                  1,      out_frequency,          systimestamp);end;

    btw don't you need a when-matched-clause ?

    BEDERafal S
  • Rafal S
    Rafal S Member Posts: 15
    edited Jul 2, 2020 6:16AM

    i copied the simple merge provided and it worked just fine !

    ps. yes i will add when marched clause also

    Would this approach be safer than binding variables ?

    Can You explain to me why or at least point me to the right article ?

    Thank You ! This forum is amazing !

  • KayK
    KayK Bonn, GermanyMember Posts: 1,743 Bronze Crown
    edited Jul 2, 2020 6:39AM

    There is no need for binding variables.

    Your procedures is called with some IN-parameters.

    And within your procedures you use them as local parameters.

    You can use the binding variables if you use execute immediate. That's better than use the content of local parameters as literals.

    That can decrease the parsing time.

    But the rule is: if you can use plain sql then USE plain sql and NO execute immediate stuff ;-)