Au cours des dix dernières années, le mouvement open source a été l'un des facteurs clés du développement de l'industrie informatique et une partie importante de celui-ci. Le rôle et la place de l'open source sont non seulement renforcés par la croissance des indicateurs quantitatifs, mais il y a aussi un changement de son positionnement qualitatif sur le marché informatique dans son ensemble. Sans rester les bras croisés, l'équipe courageuse de PVS-Studio contribue activement à consolider les positions des projets open source, à trouver des bogues cachés dans d'énormes épaisseurs de bases de code et à offrir des licences gratuites pour de tels projets. Cet article ne fait pas exception! Aujourd'hui, nous allons parler d'Apache Hive! Rapport reçu - il y a quelque chose à voir!
À propos de PVS-Studio
L' analyseur de code statique
PVS-Studio existe depuis plus de 10 ans sur le marché informatique et est une solution logicielle multifonctionnelle et facile à mettre en œuvre. À l'heure actuelle, l'analyseur prend en charge les langages C, C ++, C #, Java et fonctionne sur les plates-formes Windows, Linux et macOS.
PVS-Studio est une solution B2B payante et est utilisée par un grand nombre d'équipes dans diverses entreprises. Si vous voulez voir de quoi l'analyseur est capable, téléchargez le kit de distribution et demandez une clé d'essai
ici .
Si vous êtes un geek open source ou, par exemple, vous êtes un étudiant, vous pouvez utiliser l'une des
options de licence gratuites
de PVS-Studio.
À propos d'Apache Hive
Le volume de données ces dernières années augmente à grande vitesse. Les bases de données standard ne peuvent plus maintenir l'opérabilité à un tel taux de croissance de la quantité d'informations qui a servi à l'émergence du terme Big Data et de tout ce qui y est associé (traitement, stockage et toutes les actions qui en découlent avec de tels volumes de données).
Actuellement,
Apache Hadoop est considéré comme l'une des technologies fondamentales du Big Data. Les principaux objectifs de cette technologie sont le stockage, le traitement et la gestion de gros volumes de données. Les principaux composants du framework sont Hadoop Common,
HDFS ,
Hadoop MapReduce ,
Hadoop YARN . Au fil du temps, tout un écosystème de projets et de technologies connexes s'est formé autour de Hadoop, dont beaucoup se sont développés initialement dans le cadre du projet, puis sont devenus indépendants. Un de ces projets est
Apache Hive .
Apache Hive est un entrepôt de données distribué. Il gère les données stockées dans HDFS et fournit un langage de requête basé sur SQL (HiveQL) pour travailler avec ces données. Pour une connaissance détaillée de ce projet, vous pouvez étudier les informations
ici .
À propos de l'analyse
La séquence des étapes de l'analyse est assez simple et ne nécessite pas beaucoup de temps:
- Got Apache Hive avec GitHub ;
- J'ai utilisé les instructions pour démarrer l'analyseur Java et commencé l'analyse;
- J'ai reçu un rapport d'analyseur, je l'ai analysé et j'ai mis en évidence des cas intéressants.
Résultats de l'analyse: 1456 avertissements du niveau de confiance élevé et moyen (602 et 854, respectivement) ont été émis pour plus de 6 500 fichiers.
Tous les avertissements ne sont pas des erreurs. Il s'agit d'une situation normale et avant une utilisation régulière de l'analyseur, sa configuration est requise. On peut alors s'attendre à un pourcentage assez faible de faux positifs (
exemple ).
Parmi les avertissements, 407 avertissements (177 élevé et 230 moyen) par fichier de test n'ont pas été pris en compte. La règle de diagnostic
V6022 n'a pas
été prise en compte (il est difficile de séparer les situations erronées des situations correctes dans un code inconnu), qui
contenait jusqu'à 482 avertissements.
Le V6021 avec 179 avertissements n'a pas été pris en compte non plus.
Au final, tout de même, un nombre suffisant d'avertissements est resté. Et comme je n'ai pas configuré l'analyseur, parmi eux, il y a encore des faux positifs. Cela n'a aucun sens de décrire un grand nombre d'avertissements dans un article :). Considérez ce qui a attiré mon attention et m'a semblé intéressant.
Conditions prédéterminées
La règle de diagnostic
V6007 est le détenteur d'enregistrement parmi tous les avertissements restants de l'analyseur. Un peu plus de 200 avertissements !!! Certains, comme, inoffensifs, certains sont suspects, tandis que d'autres sont de véritables erreurs! Jetons un coup d'œil à certains d'entre eux.
V6007 Expression 'key.startsWith ("hplsql.")' Est toujours vrai. Exec.java (675)
void initOptions() { .... if (key == null || value == null || !key.startsWith("hplsql.")) {
Une construction if-else-if assez longue! L'analyseur jure au dernier
if (key.startsWith ("hplsql.")) , Indiquant sa vérité si le programme atteint ce fragment de code. En effet, si vous regardez le tout début de la construction if-else-if, la vérification est déjà terminée. Et au cas où notre ligne ne commencerait pas par la sous-chaîne
«hplsql». , puis l'exécution du code est immédiatement passée à l'itération suivante.
V6007 L' expression 'columnNameProperty.length () == 0' est toujours fausse. OrcRecordUpdater.java (238)
private static TypeDescription getTypeDescriptionFromTableProperties(....) { .... if (tableProperties != null) { final String columnNameProperty = ....; final String columnTypeProperty = ....; if ( !Strings.isNullOrEmpty(columnNameProperty) && !Strings.isNullOrEmpty(columnTypeProperty)) { List<String> columnNames = columnNameProperty.length() == 0 ? new ArrayList<String>() : ....; List<TypeInfo> columnTypes = columnTypeProperty.length() == 0 ? new ArrayList<TypeInfo>() : ....; .... } } } .... }
La comparaison des longueurs de chaîne de
columnNameProperty avec zéro renvoie toujours
false . En effet, notre comparaison est en cours de test
! Strings.isNullOrEmpty (columnNameProperty) . Si l'état du programme atteint notre condition en question, alors la ligne
columnNameProperty est garantie d'être non nulle et non vide.
Cela est également vrai pour la ligne
columnTypeProperty . Ligne d'avertissement ci-dessous:
- V6007 L'expression 'columnTypeProperty.length () == 0' est toujours fausse. OrcRecordUpdater.java (239)
V6007 L' expression 'colOrScalar1.equals ("Column")' est toujours fausse. GenVectorCode.java (3469)
private void generateDateTimeArithmeticIntervalYearMonth(String[] tdesc) throws Exception { .... String colOrScalar1 = tdesc[4]; .... String colOrScalar2 = tdesc[6]; .... if (colOrScalar1.equals("Col") && colOrScalar1.equals("Column"))
}
Voici un copier-coller trivial. Il s'est avéré que la ligne
colOrScalar1 devrait être égale à différentes valeurs en même temps, ce qui est impossible. Apparemment, la variable
colOrScalar1 doit être vérifiée à gauche et
colOrScalar2 à droite.
Des avertissements plus similaires dans les lignes ci-dessous:
- V6007 L'expression 'colOrScalar1.equals ("Scalar")' est toujours fausse. GenVectorCode.java (3475)
- V6007 L'expression 'colOrScalar1.equals ("Column")' est toujours fausse. GenVectorCode.java (3486)
Par conséquent, aucune action de la construction if-else-if ne sera jamais exécutée.
Quelques autres avertissements pour le
V6007 :
- V6007 L'expression "caractères == null" est toujours fausse. RandomTypeUtil.java (43)
- V6007 L'expression 'writeIdHwm> 0' est toujours fausse. TxnHandler.java (1603)
- V6007 L'expression 'fields.equals ("*")' est toujours vraie. Server.java (983)
- V6007 L'expression 'currentGroups! = Null' est toujours vraie. GenericUDFCurrentGroups.java (90)
- V6007 L'expression 'this.wh == null' est toujours fausse. Nouveau retourne une référence non nulle. StorageBasedAuthorizationProvider.java (93), StorageBasedAuthorizationProvider.java (92)
- et ainsi de suite ...
NPE
V6008 Déréférence nulle potentielle de 'dagLock'. QueryTracker.java (557), QueryTracker.java (553)
private void handleFragmentCompleteExternalQuery(QueryInfo queryInfo) { if (queryInfo.isExternalQuery()) { ReadWriteLock dagLock = getDagLock(queryInfo.getQueryIdentifier()); if (dagLock == null) { LOG.warn("Ignoring fragment completion for unknown query: {}", queryInfo.getQueryIdentifier()); } boolean locked = dagLock.writeLock().tryLock(); ..... } }
A attrapé le zéro objet, s'est engagé et ... a continué à travailler. Cela conduit au fait qu'après vérification de l'objet, un déréférencement de l'objet zéro se produit. Tristesse!
Très probablement, dans le cas d'une référence nulle, vous devez immédiatement quitter la fonction ou lever une exception spéciale.
V6008 Déréférence nulle de 'buffer' dans la fonction 'unlockSingleBuffer'. MetadataCache.java (410), MetadataCache.java (465)
private boolean lockBuffer(LlapBufferOrBuffers buffers, ....) { LlapAllocatorBuffer buffer = buffers.getSingleLlapBuffer(); if (buffer != null) {
Et encore une fois un NPE potentiel. Si le programme atteint la méthode
unlockSingleBuffer , l'objet
tampon sera nul. Disons que c'est arrivé! Examinons la méthode
unlockSingleBuffer et immédiatement sur la première ligne, nous voyons que notre objet est déréférencé. Nous y voilà!
N'a pas suivi le changement
V6034 Le décalage de la valeur de 'bitShiftsInWord - 1' peut être incompatible avec la taille du type: 'bitShiftsInWord - 1' = [-1 ... 30]. UnsignedInt128.java (1791)
private void shiftRightDestructive(int wordShifts, int bitShiftsInWord, boolean roundUp) { if (wordShifts == 0 && bitShiftsInWord == 0) { return; } assert (wordShifts >= 0); assert (bitShiftsInWord >= 0); assert (bitShiftsInWord < 32); if (wordShifts >= 4) { zeroClear(); return; } final int shiftRestore = 32 - bitShiftsInWord;
Décalage possible de -1. Si, par exemple,
wordShifts == 3 et
bitShiftsInWord == 0 viennent à l'entrée de la méthode en question, alors 1 << -1 se produira dans la ligne spécifiée. Est-ce prévu?
V6034 Le décalage de la valeur de 'j' peut être incompatible avec la taille du type: 'j' = [0 ... 63]. IoTrace.java (272)
public void logSargResult(int stripeIx, boolean[] rgsToRead) { .... for (int i = 0, valOffset = 0; i < elements; ++i, valOffset += 64) { long val = 0; for (int j = 0; j < 64; ++j) { int ix = valOffset + j; if (rgsToRead.length == ix) break; if (!rgsToRead[ix]) continue; val = val | (1 << j);
Dans la ligne spécifiée, la variable
j peut prendre une valeur dans la plage [0 ... 63]. Pour cette raison, le calcul de la valeur de
val dans la boucle peut ne pas se produire comme le développeur le souhaitait. Dans l'expression
(1 << j), l' unité est de type
int et, en la déplaçant de 32 ou plus, nous allons au-delà des limites du admissible. Pour remédier à la situation, vous devez écrire
((long) 1 << j) .
Enthousiaste à propos de la journalisation
V6046 Format incorrect. Un nombre différent d'éléments de format est attendu. Arguments non utilisés: 1, 2. StatsSources.java (89)
private static ImmutableList<PersistedRuntimeStats> extractStatsFromPlanMapper (....) { .... if (stat.size() > 1 || sig.size() > 1) { StringBuffer sb = new StringBuffer(); sb.append(String.format( "expected(stat-sig) 1-1, got {}-{} ;",
Lors du formatage d'une chaîne via
String.format (), le développeur a confondu la syntaxe. Conclusion: les paramètres passés ne sont pas entrés dans la chaîne résultante. Je peux supposer que dans la tâche précédente, le développeur a travaillé sur la journalisation, d'où il a emprunté la syntaxe.
A volé l'exception
V6051 L'utilisation de l'instruction 'return' dans le bloc 'finalement' peut entraîner la perte d'exceptions non gérées. ObjectStore.java (9080)
private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... committed = commitTransaction(); return result; } catch (Exception ex) { LOG.error("Error retrieving statistics via jdo", ex); if (ex instanceof MetaException) { throw (MetaException) ex; } throw new MetaException(ex.getMessage()); } finally { if (!committed) { rollbackTransaction(); return Lists.newArrayList(); } } }
Renvoyer quelque chose du bloc finally est une très mauvaise pratique, et avec cet exemple, nous verrons cela.
Dans le bloc
try , la demande est formée et le stockage est accessible. Par défaut, la variable validée a la valeur
false et ne change son état qu'après toutes les actions réussies dans le bloc
try . Cela signifie que si une exception se produit, notre variable sera toujours
fausse .
Le bloc de
capture a pris une exception, légèrement corrigé et a jeté plus loin. Et quand vient le temps du bloc
finalement, l' exécution entre dans une condition à partir de laquelle nous renvoyons la liste vide vers l'extérieur. Que nous coûte ce retour? Mais cela vaut le fait que toutes les exceptions interceptées ne seront jamais levées et traitées de manière appropriée. Toutes ces exceptions qui sont indiquées dans la signature de la méthode ne seront jamais jetées et simplement déroutantes.
Un avertissement similaire:
- V6051 L'utilisation de l'instruction 'return' dans le bloc 'finalement' peut entraîner la perte d'exceptions non gérées. ObjectStore.java (808)
... autre
V6009 La fonction 'compareTo' reçoit un argument impair. Un objet 'o2.getWorkerIdentity ()' est utilisé comme argument de sa propre méthode. LlapFixedRegistryImpl.java (244)
@Override public List<LlapServiceInstance> getAllInstancesOrdered(....) { .... Collections.sort(list, new Comparator<LlapServiceInstance>() { @Override public int compare(LlapServiceInstance o1, LlapServiceInstance o2) { return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity());
Copiez-collez, négligence, précipitation et bien d'autres raisons pour faire cette stupide erreur. Lors de la vérification de projets open source, des erreurs de ce type sont assez courantes. Il y a même un
article entier à ce sujet.
V6020 Divisez par zéro. La plage des valeurs du dénominateur du «diviseur» comprend zéro. SqlMathUtil.java (265)
public static long divideUnsignedLong(long dividend, long divisor) { if (divisor < 0L) { return (compareUnsignedLong(dividend, divisor)) < 0 ? 0L : 1L; } if (dividend >= 0) {
Ici, tout est assez simple. Un certain nombre de contrôles n'ont pas mis en garde contre une division par 0.
Plus d'avertissements:
- V6020 Mod par zéro. La plage des valeurs du dénominateur du «diviseur» comprend zéro. SqlMathUtil.java (309)
- V6020 Divisez par zéro. La plage des valeurs du dénominateur du «diviseur» comprend zéro. SqlMathUtil.java (276)
- V6020 Divisez par zéro. La plage des valeurs du dénominateur du «diviseur» comprend zéro. SqlMathUtil.java (312)
V6030 La méthode située à droite du '|' L'opérateur sera appelé quelle que soit la valeur de l'opérande gauche. Peut-être vaut-il mieux utiliser '||'. OperatorUtils.java (573)
public static Operator<? extends OperatorDesc> findSourceRS(....) { .... List<Operator<? extends OperatorDesc>> parents = ....; if (parents == null | parents.isEmpty()) {
Au lieu de l'opérateur logique || a écrit un opérateur au niveau du bit |. Cela signifie que le côté droit sera exécuté quel que soit le résultat du côté gauche. Une telle faute de frappe, dans le cas de
parents == null , conduira immédiatement à un NPE dans la prochaine sous-expression logique.
V6042 La
compatibilité de l'expression avec le type «A» est vérifiée, mais elle est convertie en type «B». VectorColumnAssignFactory.java (347)
public static VectorColumnAssign buildObjectAssign(VectorizedRowBatch outputBatch, int outColIndex, PrimitiveCategory category) throws HiveException { VectorColumnAssign outVCA = null; ColumnVector destCol = outputBatch.cols[outColIndex]; if (destCol == null) { .... } else if (destCol instanceof LongColumnVector) { switch(category) { .... case LONG: outVCA = new VectorLongColumnAssign() { .... } .init(.... , (LongColumnVector) destCol); break; case TIMESTAMP: outVCA = new VectorTimestampColumnAssign() { .... }.init(...., (TimestampColumnVector) destCol);
Les classes en
question sont LongColumnVector étend ColumnVector et
TimestampColumnVector étend ColumnVector . La vérification de la
propriété LongColumnVector de notre objet
destCol nous indique clairement que l'objet de cette classe se trouvera à l'intérieur de l'instruction conditionnelle. Malgré cela, nous diffusons sur
TimestampColumnVector ! Comme vous pouvez le voir, ces classes sont différentes, sans compter leur parent commun. En conséquence -
ClassCastException .
Tout de même peut être dit à propos de la conversion de type en
IntervalDayTimeColumnVector :
- V6042 La compatibilité de l'expression avec le type «A» est vérifiée, mais elle est convertie en type «B». VectorColumnAssignFactory.java (390)
V6060 La référence «var» a été utilisée avant d'être vérifiée par rapport à null. Var.java (402), Var.java (395)
@Override public boolean equals(Object obj) { if (getClass() != obj.getClass()) {
Une étrange comparaison d'un objet
var avec
null après le déréférencement s'est produite. Dans ce contexte,
var et
obj sont le même objet (
var = (Var) obj ). La vérification de
null implique qu'un objet nul peut venir. Et dans le cas d'
égaux (null), nous obtenons immédiatement sur la première ligne NPE au lieu du
faux attendu. Hélas, il y a un chèque, mais pas là.
Moments suspects similaires en utilisant l'objet avant la vérification:
- V6060 La référence «valeur» a été utilisée avant d'être vérifiée par rapport à null. ParquetRecordReaderWrapper.java (168), ParquetRecordReaderWrapper.java (166)
- V6060 La référence 'defaultConstraintCols' a été utilisée avant d'être vérifiée par rapport à null. HiveMetaStore.java (2539), HiveMetaStore.java (2530)
- V6060 La référence 'projIndxLst' a été utilisée avant d'être vérifiée par rapport à null. RelOptHiveTable.java (683), RelOptHiveTable.java (682)
- V6060 La référence «oldp» a été utilisée avant d'être vérifiée par rapport à null. ObjectStore.java (4343), ObjectStore.java (4339)
- et ainsi de suite ...
Conclusion
Quiconque était un peu intéressé par le Big Data, il a à peine raté l'importance d'Apache Hive. Le projet est populaire et assez volumineux, et dans sa composition compte plus de 6500 fichiers de code source (* .java). Le code a été écrit par de nombreux développeurs depuis de nombreuses années et, par conséquent, l'analyseur statique a quelque chose à trouver. Cela confirme une fois de plus que l'analyse statique est extrêmement importante et utile dans le développement de projets de moyenne et grande envergure!
Remarque De telles vérifications ponctuelles démontrent les capacités d'un analyseur de code statique, mais sont une manière complètement erronée de l'utiliser. Cette idée est présentée plus en détail
ici et
ici . Utilisez l'analyse régulièrement!
Lors de la vérification de la ruche, un nombre suffisant de défauts et de moments suspects ont été détectés. Si cet article attire l'attention de l'équipe de développement Apache Hive, nous serons heureux de contribuer à cette tâche difficile.
Il est impossible d'imaginer Apache Hive sans Apache Hadoop, il est donc probable que la licorne de PVS-Studio y regarde aussi. Mais c'est tout pour aujourd'hui, mais pour l'instant
téléchargez l' analyseur et vérifiez vos propres projets.
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Maxim Stefanov.
PVS-Studio visite Apache Hive .