Geekly articles weekly

PVS-Studio besucht Apache Hive

Abbildung 1

In den letzten zehn Jahren war die Open Source-Bewegung einer der Schlüsselfaktoren für die Entwicklung der IT-Branche und ein wichtiger Teil davon. Die Rolle und der Ort von Open Source nehmen nicht nur in Form einer Zunahme quantitativer Indikatoren zu, sondern es ändert sich auch die qualitative Positionierung von Open Source auf dem gesamten IT-Markt. Das mutige Team von PVS-Studio trägt aktiv dazu bei, die Positionen von Open-Source-Projekten zu festigen, versteckte Fehler in einer Vielzahl von Codebasen zu finden und kostenlose Lizenzen für solche Projekte anzubieten. Dieser Artikel ist keine Ausnahme! Heute werden wir über Apache Hive sprechen! Bericht erhalten - es gibt etwas zu sehen!

Über PVS-Studio


Der statische Code-Analysator PVS-Studio existiert seit mehr als 10 Jahren auf dem IT-Markt und ist eine multifunktionale und einfach zu implementierende Softwarelösung. Derzeit unterstützt der Analyzer die Sprachen C, C ++, C #, Java und funktioniert auf den Plattformen Windows, Linux und macOS.

PVS-Studio ist eine kostenpflichtige B2B-Lösung und wird von einer Vielzahl von Teams in verschiedenen Unternehmen eingesetzt. Wenn Sie sehen möchten, wozu der Analysator in der Lage ist, laden Sie das Verteilungskit herunter und fordern Sie hier einen Testschlüssel an .

Wenn Sie ein Open-Source-Geek oder beispielsweise ein Student sind, können Sie eine der kostenlosen Lizenzoptionen von PVS-Studio verwenden.

Über Apache Hive


Das Datenvolumen der letzten Jahre wächst rasant. Standarddatenbanken können die Funktionsfähigkeit bei einer solchen Wachstumsrate der Informationsmenge, die zur Entstehung des Begriffs Big Data und aller damit verbundenen Dinge diente (Verarbeitung, Speicherung und aller sich daraus ergebenden Aktionen mit solchen Datenmengen), nicht mehr aufrechterhalten.

Derzeit gilt Apache Hadoop als eine der grundlegenden Technologien von Big Data. Die Hauptziele dieser Technologie sind die Speicherung, Verarbeitung und Verwaltung großer Datenmengen. Die Hauptkomponenten des Frameworks sind Hadoop Common, HDFS , Hadoop MapReduce und Hadoop YARN . Im Laufe der Zeit hat sich um Hadoop ein ganzes Ökosystem verwandter Projekte und Technologien gebildet, von denen sich viele ursprünglich als Teil des Projekts entwickelten und anschließend unabhängig wurden. Eines dieser Projekte ist Apache Hive .

Apache Hive ist ein verteiltes Data Warehouse. Es verwaltet die in HDFS gespeicherten Daten und bietet eine SQL-basierte Abfragesprache (HiveQL) für die Arbeit mit diesen Daten. Für eine detaillierte Bekanntschaft mit diesem Projekt können Sie die Informationen hier studieren.

Über die Analyse


Die Abfolge der Schritte für die Analyse ist recht einfach und erfordert nicht viel Zeit:

  • Habe Apache Hive mit GitHub ;
  • Ich habe die Anweisungen zum Starten des Java-Analysators verwendet und die Analyse gestartet.
  • Ich erhielt einen Analysatorbericht, analysierte ihn und hob interessante Fälle hervor.

Analyseergebnisse: 1456 Warnungen des hohen und mittleren Konfidenzniveaus (602 bzw. 854) wurden für mehr als 6500 Dateien ausgegeben.

Nicht alle Warnungen sind Fehler. Dies ist eine normale Situation, und vor der regelmäßigen Verwendung des Analysators ist dessen Konfiguration erforderlich. Dann können wir einen relativ geringen Prozentsatz an falsch positiven Ergebnissen erwarten ( Beispiel ).

Unter den Warnungen wurden 407 Warnungen (177 hoch und 230 mittel) pro Testdatei nicht berücksichtigt. Die Diagnoseregel V6022 wurde nicht berücksichtigt (es ist schwierig, Fehlersituationen von korrekten in einem unbekannten Code zu trennen), die bis zu 482 Warnungen enthielt. V6021 mit 179 Warnungen wurde ebenfalls nicht berücksichtigt.

Am Ende blieb jedoch eine ausreichende Anzahl von Warnungen übrig. Und da ich den Analysator nicht konfiguriert habe, gibt es wieder falsch positive Ergebnisse. Es macht keinen Sinn, eine große Anzahl von Warnungen in einem Artikel zu beschreiben :). Überlegen Sie, was mir aufgefallen ist und interessant erschien.

Vorgegebene Bedingungen


Die Diagnoseregel V6007 ist der Rekordhalter unter allen verbleibenden Warnungen des Analysators. Etwas mehr als 200 Warnungen !!! Einige sind harmlos, andere misstrauisch, während andere echte Fehler sind! Schauen wir uns einige davon an.

V6007 Ausdruck 'key.startsWith ("hplsql.")' Ist immer wahr. Exec.java (675)

void initOptions() { .... if (key == null || value == null || !key.startsWith("hplsql.")) { // <= continue; } else if (key.compareToIgnoreCase(Conf.CONN_DEFAULT) == 0) { .... } else if (key.startsWith("hplsql.conn.init.")) { .... } else if (key.startsWith(Conf.CONN_CONVERT)) { .... } else if (key.startsWith("hplsql.conn.")) { .... } else if (key.startsWith("hplsql.")) { // <= .... } }

Ein ziemlich langes Wenn-Sonst-Wenn-Konstrukt! Der Analysator schwört auf das letzte if (key.startsWith ("hplsql.")) Und zeigt seine Wahrheit an, wenn das Programm dieses Codefragment erreicht. Wenn Sie sich den Anfang des if-else-if-Konstrukts ansehen, ist die Prüfung bereits abgeschlossen. Und falls unsere Zeile nicht mit dem Teilstring "hplsql" begann. Dann sprang die Ausführung des Codes sofort zur nächsten Iteration.

V6007 Der Ausdruck 'columnNameProperty.length () == 0' ist immer falsch. 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>() : ....; .... } } } .... }

Wenn Sie die Zeichenfolgenlängen von columnNameProperty mit Null vergleichen, wird immer false zurückgegeben . Dies liegt daran, dass unser Vergleich getestet wird ! Strings.isNullOrEmpty (columnNameProperty) . Wenn der Status des Programms unseren fraglichen Zustand erreicht, ist die Spalte columnNameProperty garantiert nicht Null und nicht leer.

Dies gilt auch für die Spalte columnTypeProperty . Warnzeile unten:

  • V6007 Der Ausdruck 'columnTypeProperty.length () == 0' ist immer falsch. OrcRecordUpdater.java (239)

V6007 Der Ausdruck 'colOrScalar1.equals ("Column")' ist immer falsch. 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")) // <= { .... } else if (colOrScalar1.equals("Col") && colOrScalar1.equals("Scalar")) { .... } else if (colOrScalar1.equals("Scalar") && colOrScalar1.equals("Column")) { .... }

}}

Hier ist ein triviales Kopieren und Einfügen. Es stellte sich heraus, dass die Zeile colOrScalar1 gleichzeitig unterschiedlichen Werten entsprechen sollte, und dies ist unmöglich. Anscheinend sollte die Variable colOrScalar1 links und colOrScalar2 rechts überprüft werden.

Weitere ähnliche Warnungen in den folgenden Zeilen:

  • V6007 Der Ausdruck 'colOrScalar1.equals ("Scalar")' ist immer falsch. GenVectorCode.java (3475)
  • V6007 Der Ausdruck 'colOrScalar1.equals ("Column")' ist immer falsch. GenVectorCode.java (3486)

Infolgedessen werden niemals Aktionen im if-else-if-Konstrukt ausgeführt.

Einige andere Warnungen für den V6007 :

  • V6007 Der Ausdruck 'Zeichen == null' ist immer falsch. RandomTypeUtil.java (43)
  • V6007 Der Ausdruck 'writeIdHwm> 0' ist immer falsch. TxnHandler.java (1603)
  • V6007 Ausdruck 'fields.equals ("*")' ist immer wahr. Server.java (983)
  • V6007 Ausdruck 'currentGroups! = Null' ist immer wahr. GenericUDFCurrentGroups.java (90)
  • V6007 Der Ausdruck 'this.wh == null' ist immer falsch. New gibt eine Referenz ungleich Null zurück. StorageBasedAuthorizationProvider.java (93), StorageBasedAuthorizationProvider.java (92)
  • usw...

NPE


V6008 Mögliche Null-Dereferenzierung von '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(); ..... } }

Das Null-Objekt gefangen, verpfändet und ... weiter gearbeitet. Dies führt dazu, dass nach Überprüfung des Objekts eine Dereferenzierung des Nullobjekts auftritt. Traurigkeit!

Im Falle einer Nullreferenz sollten Sie die Funktion höchstwahrscheinlich sofort beenden oder eine spezielle Ausnahme auslösen.

V6008 Null-Dereferenzierung von 'Puffer' in der Funktion 'UnlockSingleBuffer'. MetadataCache.java (410), MetadataCache.java (465)

 private boolean lockBuffer(LlapBufferOrBuffers buffers, ....) { LlapAllocatorBuffer buffer = buffers.getSingleLlapBuffer(); if (buffer != null) { // <= return lockOneBuffer(buffer, doNotifyPolicy); } LlapAllocatorBuffer[] bufferArray = buffers.getMultipleLlapBuffers(); for (int i = 0; i < bufferArray.length; ++i) { if (lockOneBuffer(bufferArray[i], doNotifyPolicy)) continue; for (int j = 0; j < i; ++j) { unlockSingleBuffer(buffer, true); // <= } .... } .... } .... private void unlockSingleBuffer(LlapAllocatorBuffer buffer, ....) { boolean isLastDecref = (buffer.decRef() == 0); // <= if (isLastDecref) { .... } }

Und wieder eine potenzielle NPE. Wenn das Programm die Methode lockerSingleBuffer erreicht, ist das Pufferobjekt Null. Sagen wir, es ist passiert! Schauen wir uns die Methode lockerSingleBuffer an und sehen sofort in der ersten Zeile, dass unser Objekt dereferenziert ist. Hier sind wir!

Verfolgte die Schicht nicht


V6034 Die Verschiebung um den Wert 'bitShiftsInWord - 1' kann mit der Größe des Typs nicht übereinstimmen: '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; // check this because "123 << 32" will be 123. final boolean noRestore = bitShiftsInWord == 0; final int roundCarryNoRestoreMask = 1 << 31; final int roundCarryMask = (1 << (bitShiftsInWord - 1)); // <= .... }

Möglicher Versatz um -1. Wenn beispielsweise wordShifts == 3 und bitShiftsInWord == 0 zur Eingabe der betreffenden Methode kommen, wird 1 << -1 in der angegebenen Zeile angezeigt . Ist das geplant?

V6034 Die Verschiebung um den Wert 'j' kann mit der Größe des Typs nicht übereinstimmen: '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); // <= } .... } .... }

In der angegebenen Zeile kann die Variable j einen Wert im Bereich [0 ... 63] annehmen. Aus diesem Grund erfolgt die Berechnung des Werts von val in der Schleife möglicherweise nicht wie vom Entwickler beabsichtigt. Im Ausdruck (1 << j) ist die Einheit vom Typ int , und wenn wir sie von 32 oder mehr verschieben, gehen wir über die Grenzen des Zulässigen hinaus. Um Abhilfe zu schaffen, müssen Sie ((lang) 1 << j) schreiben.

Begeistert von der Protokollierung


V6046 Falsches Format. Eine andere Anzahl von Formatelementen wird erwartet. Nicht verwendete Argumente: 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 {}-{} ;", // <= stat.size(), sig.size() )); .... } .... if (e.getAll(OperatorStats.IncorrectRuntimeStatsMarker.class).size() > 0) { LOG.debug( "Ignoring {}, marked with OperatorStats.IncorrectRuntimeStatsMarker", sig.get(0) ); continue; } .... }

Beim Formatieren eines Strings über String.format () hat der Entwickler die Syntax verwirrt. Fazit: Die übergebenen Parameter sind nicht in die resultierende Zeichenfolge gelangt. Ich kann davon ausgehen, dass der Entwickler in der vorherigen Aufgabe an der Protokollierung gearbeitet hat, von wo er die Syntax ausgeliehen hat.

Die Ausnahme gestohlen


V6051 Die Verwendung der Anweisung 'return' im Block 'finally' kann zum Verlust nicht behandelter Ausnahmen führen. ObjectStore.java (9080)

 private List<MPartitionColumnStatistics> getMPartitionColumnStatistics(....) throws NoSuchObjectException, MetaException { boolean committed = false; try { .... /*some actions*/ 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(); } } }

Etwas aus dem finally-Block zurückzugeben ist eine sehr schlechte Praxis, und mit diesem Beispiel werden wir dies sehen.

Im try- Block wird die Anforderung gebildet und auf den Speicher zugegriffen. Die festgeschriebene Variable ist standardmäßig false und ändert ihren Status erst, nachdem alle Aktionen im try- Block erfolgreich abgeschlossen wurden. Dies bedeutet, dass unsere Variable im Falle einer Ausnahme immer falsch ist . Catch Block hat eine Ausnahme abgefangen, leicht korrigiert und weiter geworfen. Und wenn die Zeit für den finally- Block gekommen ist , tritt die Ausführung in eine Bedingung ein, von der aus wir die leere Liste nach außen zurückgeben. Was kostet uns diese Rücksendung? Es ist jedoch die Tatsache wert, dass alle abgefangenen Ausnahmen niemals verworfen und in angemessener Weise verarbeitet werden. Alle Ausnahmen, die in der Methodensignatur angegeben sind, werden niemals weggeworfen und sind einfach verwirrend.

Eine ähnliche Warnung:

  • V6051 Die Verwendung der Anweisung 'return' im Block 'finally' kann zum Verlust nicht behandelter Ausnahmen führen. ObjectStore.java (808)

... andere


V6009 Die Funktion 'compareTo' empfängt ein ungerades Argument. Ein Objekt 'o2.getWorkerIdentity ()' wird als Argument für seine eigene Methode verwendet. 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()); // <= } }); .... }

Kopieren-Einfügen, Nachlässigkeit, Eile und viele andere Gründe, um diesen dummen Fehler zu machen. Bei der Überprüfung von Open Source-Projekten treten häufig Fehler dieser Art auf. Es gibt sogar einen ganzen Artikel darüber.

V6020 Durch Null teilen. Der Bereich der 'Divisor'-Nennerwerte umfasst Null. SqlMathUtil.java (265)

 public static long divideUnsignedLong(long dividend, long divisor) { if (divisor < 0L) { /*some comments*/ return (compareUnsignedLong(dividend, divisor)) < 0 ? 0L : 1L; } if (dividend >= 0) { // Both inputs non-negative return dividend / divisor; // <= } else { .... } }

Hier ist alles ganz einfach. Eine Reihe von Überprüfungen warnte nicht vor einer Division durch 0.

Weitere Warnungen:

  • V6020 Mod um Null. Der Bereich der 'Divisor'-Nennerwerte umfasst Null. SqlMathUtil.java (309)
  • V6020 Durch Null teilen. Der Bereich der 'Divisor'-Nennerwerte umfasst Null. SqlMathUtil.java (276)
  • V6020 Durch Null teilen. Der Bereich der 'Divisor'-Nennerwerte umfasst Null. SqlMathUtil.java (312)

V6030 Die Methode rechts neben dem '|' Der Operator wird unabhängig vom Wert des linken Operanden aufgerufen. Vielleicht ist es besser, '||' zu verwenden. OperatorUtils.java (573)

 public static Operator<? extends OperatorDesc> findSourceRS(....) { .... List<Operator<? extends OperatorDesc>> parents = ....; if (parents == null | parents.isEmpty()) { // reached end eg TS operator return null; } .... }

Anstelle des logischen Operators || schrieb einen bitweisen Operator |. Dies bedeutet, dass die rechte Seite unabhängig vom Ergebnis der linken Seite ausgeführt wird. Ein solcher Tippfehler führt im Fall von Eltern == null sofort zu einer NPE im nächsten logischen Unterausdruck.

V6042 Der Ausdruck wird auf Kompatibilität mit Typ 'A' geprüft, aber in Typ 'B' umgewandelt. 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); // <= break; case DATE: outVCA = new VectorLongColumnAssign() { .... } .init(...., (LongColumnVector) destCol); break; case INTERVAL_YEAR_MONTH: outVCA = new VectorLongColumnAssign() { .... }.init(...., (LongColumnVector) destCol); break; case INTERVAL_DAY_TIME: outVCA = new VectorIntervalDayTimeColumnAssign() { .... }.init(...., (IntervalDayTimeColumnVector) destCol);// <= break; default: throw new HiveException(....); } } else if (destCol instanceof DoubleColumnVector) { .... } .... else { throw new HiveException(....); } return outVCA; }

Die fraglichen Klassen sind LongColumnVector erweitert ColumnVector und TimestampColumnVector erweitert ColumnVector . Wenn Sie unser destCol- Objekt auf LongColumnVector- Besitz überprüfen , wird deutlich, dass sich das Objekt dieser Klasse in der bedingten Anweisung befindet. Trotzdem werfen wir auf TimestampColumnVector ! Wie Sie sehen können, sind diese Klassen unterschiedlich und zählen nicht ihre gemeinsamen Eltern. Als Ergebnis - ClassCastException .

Gleiches gilt für die Typkonvertierung in IntervalDayTimeColumnVector :

  • V6042 Der Ausdruck wird auf Kompatibilität mit Typ 'A' geprüft, aber in Typ 'B' umgewandelt. VectorColumnAssignFactory.java (390)

V6060 Die ' var' -Referenz wurde verwendet, bevor sie gegen null verifiziert wurde. Var.java (402), Var.java (395)

 @Override public boolean equals(Object obj) { if (getClass() != obj.getClass()) { // <= return false; } Var var = (Var)obj; if (this == var) { return true; } else if (var == null || // <= var.value == null || this.value == null) { return false; } .... }

Ein seltsamer Vergleich eines var- Objekts mit null nach der Dereferenzierung ist aufgetreten. In diesem Zusammenhang sind var und obj dasselbe Objekt ( var = (Var) obj ). Das Überprüfen auf Null impliziert, dass ein Nullobjekt kommen kann. Und im Fall von equals (null) erhalten wir sofort die erste Zeile NPE anstelle der erwarteten falschen . Leider gibt es einen Scheck, aber nicht da.

Ähnliche verdächtige Momente mit dem Objekt vor der Prüfung:

  • V6060 Die 'Wert'-Referenz wurde verwendet, bevor sie gegen Null verifiziert wurde. ParquetRecordReaderWrapper.java (168), ParquetRecordReaderWrapper.java (166)
  • V6060 Die Referenz 'defaultConstraintCols' wurde verwendet, bevor sie gegen null verifiziert wurde. HiveMetaStore.java (2539), HiveMetaStore.java (2530)
  • V6060 Die Referenz 'projIndxLst' wurde verwendet, bevor sie gegen null verifiziert wurde. RelOptHiveTable.java (683), RelOptHiveTable.java (682)
  • V6060 Die 'oldp'-Referenz wurde verwendet, bevor sie gegen null verifiziert wurde. ObjectStore.java (4343), ObjectStore.java (4339)
  • usw...

Fazit


Wer sich ein wenig für Big Data interessierte, vermisste die Bedeutung von Apache Hive kaum. Das Projekt ist beliebt und ziemlich groß und hat in seiner Zusammensetzung mehr als 6500 Quellcodedateien (* .java). Der Code wurde von vielen Entwicklern seit vielen Jahren geschrieben und daher hat der statische Analysator etwas zu finden. Dies bestätigt einmal mehr, dass die statische Analyse bei der Entwicklung mittlerer und großer Projekte äußerst wichtig und nützlich ist!

Hinweis Solche einmaligen Überprüfungen demonstrieren die Fähigkeiten eines statischen Code-Analysators, sind jedoch eine völlig falsche Verwendung. Diese Idee wird hier und hier ausführlicher vorgestellt. Verwenden Sie die Analyse regelmäßig!

Bei der Überprüfung des Bienenstocks wurde eine ausreichende Anzahl von Fehlern und verdächtigen Momenten festgestellt. Wenn dieser Artikel die Aufmerksamkeit des Apache Hive-Entwicklungsteams auf sich zieht, werden wir gerne zu dieser schwierigen Aufgabe beitragen.

Es ist unmöglich, sich Apache Hive ohne Apache Hadoop vorzustellen, daher ist es wahrscheinlich, dass das Einhorn von PVS-Studio auch dort aussehen wird. Aber das ist alles für heute, aber jetzt laden Sie den Analysator herunter und überprüfen Sie Ihre eigenen Projekte.



Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Maxim Stefanov. PVS-Studio besucht Apache Hive .

Source: https://habr.com/ru/post/de463759/

More articles:


All Articles