Discussion :

[alex_972]

Bonjour,

Etant débutant en PL/SQL, j'aimerais avoir votre avis sur mon code.
Des optimisations sont-elles possibles ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
CREATE OR REPLACE PACKAGE BODY pkg_test IS
 
  PROCEDURE query_test (table1 IN VARCHAR2, table2 IN VARCHAR2) IS
 
query_max VARCHAR2(100);
	max_q   VARCHAR2(100);
	collection_query  VARCHAR2(3500);
 
  BEGIN
 	query_max := 'SELECT max(num) FROM ' || table2;
EXECUTE IMMEDIATE query_max  INTO max_q;
 
collection_query := 'INSERT INTO ' || table2 || ' (SELECT num FROM '|| table1 ||' WHERE num > :1)';    
	EXECUTE IMMEDIATE collection_query USING max_q;
 
  END;
 
END;

Je n'ai pas réussi à la faire fonctionner en remplaçant le premier EXECUTE IMMEDIATE par une requête sous la forme SELECT.. INTO.. FROM table2.
table2 me pose problème dans ce cas.

Merci d'avance

[skuatamad]

Je n'ai pas réussit à la faire fonctionner en remplaçant le premier EXECUTE IMMEDIATE par une requête sous la forme SELECT.. INTO.. FROM table2.

C'est normal le INTO se fait bien dans le EXECUTE IMMEDIATE.
Par contre je ne sais pas dans quel contexte cette procédure est utilisée mais la concaténation de chaines de caractères représente un risque d'injection SQL.
Comme pour les noms de tables, colonnes, il n'est bien évidemment pas possible de binder, il faut alors utiliser le package DBMS_ASSERT, par exemple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

query_max := 'SELECT max(num) FROM ' || dbms_assert.simple_sql_name(table2);

Sinon perso j'aurais mis le SELECT MAX directement dans la requête INSERT.
Et si num est bien un numérique, il ne faut pas le fetch dans un varchar2 afin d'éviter des conversions implicites.

[alex_972]

Merci skuatamad pour ta réponse.

Je ne connaissais pas DBMS_ASSERT. Y a-t-il d'autre chose à savoir sur l'injection SQL lors de la programmation en PL/SQL ?

Pour le select dans l'Execute immediate, c'est ce que j'ai fait au début mais je me demandais si ce n'étais pas plus propre comme çà.

Concernant la colonne num, c'est un timestamp. (la requete originale fait une bonne centaine de ligne

[alex_972]

Est-il possible d'appliquer dbms_assert.simple_sql_name() une seule fois sur la variable de ma procédure ? Ca me permettrait d'éviter d'utiliser cette fonction X fois.

Quelque chose dans ce genre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
CREATE OR REPLACE PACKAGE BODY pkg_test IS
 
  PROCEDURE query_test (table1 IN VARCHAR2, table2 IN VARCHAR2) IS

        table2 VARCHAR2(100) := dbms_assert.simple_sql_name(table2);

	max_q   VARCHAR2(100);
	collection_query  VARCHAR2(3500);
 
  BEGIN
 	
EXECUTE IMMEDIATE  'SELECT max(num) FROM ' || table2  INTO max_q;
 
collection_query := 'INSERT INTO ' || table2 || ' (SELECT num FROM '|| table1 ||' WHERE num > :1)';    
	EXECUTE IMMEDIATE collection_query USING max_q;
 
  END;
 
END;

Là, j'ai une belle erreur duplicate field.

[skuatamad]

Y a-t-il d'autre chose à savoir sur l'injection SQL lors de la programmation en PL/SQL ?

Utiliser USING pour les valeurs des colonnes.

Concernant la colonne num, c'est un timestamp.

Alors max_q devrait être déclaré comme un timestamp.

Est-il possible d'appliquer dbms_assert.simple_sql_name() une seule fois sur la variable de ma procédure

En passant par une variable :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
is
l_table2 VARCHAR2(30);
...
begin
l_table2 := dbms_assert.simple_sql_name(table2);
...

30 c'est suffisant. En passant on préfixe souvent les paramètres par un p_ donc PROCEDURE query_test (p_table1 IN VARCHAR2, p_table2 IN VARCHAR2)

[alex_972]

Utiliser USING pour les valeurs des colonnes.

Peux tu etre plus explicite ?

SI le prefix p_ est utilisé pour les paramètres de procédure, je suppose que l'on utilise f_ pour les fonctions. Mais à quoi correspond le préfix l_?