Mauvaise formation de requête SQL dans fonction spip_pg_groupby #3410

Open
opened 6 years ago by miros · 4 comments
miros commented 6 years ago

SPIP version 3.0.7

Résumé :
Mauvais parsing de requête SQL qui exclue l'alias de table précédent le nom de champs dans GROUP BY tel que :
SELECT A.id_rubrique AS id_rubrique, R.lang AS lang
FROM spip_rubriques AS A, spip_rubriques AS R
WHERE A.id_parent = R.id_rubrique AND A.langue_choisie != 'oui' AND R.lang<>'' AND R.lang<>A.lang
GROUP BY id_rubrique,lang
^^^^^^^^^^^^^^^

Détail :
Dans le fichier écrire/req/pg.php à la fonction spip_pg_groupby, la ligne :
$v = preg_replace('/^.\sAS\s+(\w+)\s$/i','\1', $v);
ne retient que la partie de droite présumant que la partie de gauche est une fonction appliquée par exemple à un champs et qui serait aliasé tel que : SELECT UNE_FONCTION(champs) AS alias_champs, mais ce parsing néglige qu'il puisse s'agir d'un simple aliasing d'un champs existant tel que SELECT A.id_rubrique AS id_rubrique, R.lang AS lang

Solution suggérée :
Remplacer la ligne :
$v = preg_replace('/^.\sAS\s+(\w+)\s$/i','\1', $v);
par :
if( preg_match( '/^(\w+.\w+)\s+AS\s+/i', $v, $matches ) )
{
$v = $matches[1];
}
else
{
$v = preg_replace('/^.\sAS\s+(\w+)\s$/i','\1', $v);
}

Nota bene :
Faire du parsing de requête SQL est une mavaise idée, car elles peuvent être considérablement compliquée et je ne suis pas sûr que SPIP ait vocation à devenir un parser de requête SQL.
Le mieux serait peut-être de faire appel à des fonctions qui renverraient la requête SQL écrite de toute pièce à la main et adapté pour le driver SQL utilisé, plutôt que d'utiliser des fonctions qui génèrent du SQL dynamiquement, à moins que cela soit vraiment nécessaire. Cela aurait le mérite de mieux pouvoir optimiser les requêtes SQL par des spécialistes SQL.
Par exemple, dans le cas ci-dessus, il serait préférable d'avoir une fonction telle que sql_rubrique_lang() et qui renverrait :
SELECT A.id_rubrique AS id_rubrique, R.lang AS lang
FROM spip_rubriques AS A, spip_rubriques AS R
WHERE A.id_parent = R.id_rubrique AND A.langue_choisie != 'oui' AND R.lang<>'' AND R.lang<>A.lang
GROUP BY A.id_rubrique,R.lang

SPIP version 3.0.7 Résumé : Mauvais parsing de requête SQL qui exclue l'alias de table précédent le nom de champs dans GROUP BY tel que : SELECT A.id_rubrique AS id_rubrique, R.lang AS lang FROM spip_rubriques AS A, spip_rubriques AS R WHERE A.id_parent = R.id_rubrique AND A.langue_choisie != 'oui' AND R.lang<>'' AND R.lang<>A.lang GROUP BY id_rubrique,lang ^^^^^^^^^^^^^^^ Détail : Dans le fichier écrire/req/pg.php à la fonction spip_pg_groupby, la ligne : $v = preg_replace('/^.*\sAS\s+(\w+)\s*$/i','\\1', $v); ne retient que la partie de droite présumant que la partie de gauche est une fonction appliquée par exemple à un champs et qui serait aliasé tel que : SELECT UNE_FONCTION(champs) AS alias_champs, mais ce parsing néglige qu'il puisse s'agir d'un simple aliasing d'un champs existant tel que SELECT A.id_rubrique AS id_rubrique, R.lang AS lang Solution suggérée : Remplacer la ligne : $v = preg_replace('/^.*\sAS\s+(\w+)\s*$/i','\\1', $v); par : if( preg_match( '/^(\w+\.\w+)\s+AS\s+/i', $v, $matches ) ) { $v = $matches[1]; } else { $v = preg_replace('/^.*\sAS\s+(\w+)\s*$/i','\\1', $v); } Nota bene : Faire du parsing de requête SQL est une mavaise idée, car elles peuvent être considérablement compliquée et je ne suis pas sûr que SPIP ait vocation à devenir un parser de requête SQL. Le mieux serait peut-être de faire appel à des fonctions qui renverraient la requête SQL écrite de toute pièce à la main et adapté pour le driver SQL utilisé, plutôt que d'utiliser des fonctions qui génèrent du SQL dynamiquement, à moins que cela soit vraiment nécessaire. Cela aurait le mérite de mieux pouvoir optimiser les requêtes SQL par des spécialistes SQL. Par exemple, dans le cas ci-dessus, il serait préférable d'avoir une fonction telle que sql_rubrique_lang() et qui renverrait : SELECT A.id_rubrique AS id_rubrique, R.lang AS lang FROM spip_rubriques AS A, spip_rubriques AS R WHERE A.id_parent = R.id_rubrique AND A.langue_choisie != 'oui' AND R.lang<>'' AND R.lang<>A.lang GROUP BY A.id_rubrique,R.lang
Owner

Le support de PG reste buggué et experimental…
Version cible mise à 3.0

Le support de PG reste buggué et experimental… **Version cible mise à 3.0**
b_b commented 6 years ago
Owner

Je repasse le ticket en mode "normal", je ne sais pas si quelqu'un aura le courage de s'occuper de PG (qui n'est plus disponible par défaut en 3.1).

Je repasse le ticket en mode "normal", je ne sais pas si quelqu'un aura le courage de s'occuper de PG (qui n'est plus disponible par défaut en 3.1).
b_b commented 5 years ago
Owner
There is no content yet.
b_b commented 4 years ago
Owner

Version cible mise à 90. PostgreSQL

**Version cible mise à 90. PostgreSQL**
cy.altern added the
PostgreSQL
label 1 week ago
cy.altern removed this from the (deleted) milestone 1 week ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.