In den letzten zehn Jahren war die Open-Source-Bewegung einer der Haupttreiber für die Entwicklung der IT-Branche und ihre entscheidende Komponente. Die Rolle von Open-Source-Projekten wird nicht nur quantitativ, sondern auch qualitativ immer wichtiger, was das Konzept ihrer Positionierung auf dem IT-Markt im Allgemeinen verändert. Unser mutiges PVS-Studio-Team sitzt nicht untätig und beteiligt sich aktiv an der Stärkung der Präsenz von Open-Source-Software, indem es versteckte Fehler in den enormen Tiefen der Codebasen findet und den Autoren solcher Projekte kostenlose Lizenzoptionen anbietet. Dieser Artikel ist nur ein weiterer Teil dieser Aktivität! Heute werden wir über Apache Hive sprechen. Ich habe den Bericht - und es gibt Dinge, die es wert sind, angeschaut zu werden.
Über PVS-Studio
Der statische Code-Analysator
PVS-Studio , den es seit mehr als 10 Jahren gibt, ist eine multifunktionale und einfach zu integrierende Softwarelösung. Derzeit unterstützt es C, C ++, C # und Java und läuft unter Windows, Linux und MacOS.
PVS-Studio ist eine kostenpflichtige B2B-Lösung, die von zahlreichen Teams in einer Reihe von Unternehmen eingesetzt wird. Wenn Sie den Analysator ausprobieren möchten, besuchen Sie diese
Seite , um die Distribution herunterzuladen und einen Testschlüssel anzufordern.
Wenn Sie ein Open-Source-Geek oder beispielsweise ein Student sind, können Sie eine unserer kostenlosen
Lizenzoptionen nutzen .
Über Apache Hive
Die Datenmenge ist in den letzten Jahren enorm gewachsen. Die Standarddatenbanken können dieses schnelle Wachstum nicht mehr bewältigen. Daher kommt der Begriff Big Data zusammen mit anderen verwandten Begriffen (wie Verarbeitung, Speicherung und anderen Vorgängen mit Big Data).
Apache Hadoop gilt derzeit als eine der wegweisenden Big Data-Technologien. Die Hauptaufgaben sind das Speichern, Verarbeiten und Verwalten großer Datenmengen. Die Hauptkomponenten des Frameworks sind Hadoop Common,
HDFS ,
Hadoop MapReduce und
Hadoop YARN . Im Laufe der Zeit hat sich rund um Hadoop ein großes Ökosystem verwandter Projekte und Technologien entwickelt, von denen viele ursprünglich als Teil des Projekts begannen und sich dann auf den Weg machten, um unabhängig zu werden.
Apache Hive ist einer von ihnen.
Apache Hive ist ein verteiltes Data Warehouse. Es verwaltet die in HDFS gespeicherten Daten und stellt die auf SQL (HiveQL) basierende Abfragesprache bereit, um diese Daten zu verarbeiten. Weitere Details zum Projekt finden Sie
hier .
Ausführen der Analyse
Es dauerte nicht viel Mühe oder Zeit, um die Analyse zu starten. Hier ist mein Algorithmus:
- Heruntergeladener Apache Hive von GitHub ;
- Lesen Sie die Anleitung zum Starten des Java-Analysators und starten Sie die Analyse.
- Erhielt den Bericht des Analysators, studierte ihn und schrieb die interessantesten Fälle auf.
Die Analyseergebnisse lauten wie folgt: 1456 Warnungen vor hohen und mittleren Werten (602 bzw. 854) in über 6500 Dateien.
Nicht alle Warnungen beziehen sich auf echte Fehler. Das ist ganz normal; Sie müssen die Einstellungen des Analysators anpassen, bevor Sie ihn regelmäßig verwenden können. Danach erwarten Sie normalerweise eine relativ geringe Rate an Fehlalarmen (
Beispiel ).
Ich habe die 407 Warnungen (177 High- und 230 Medium-Level), die von den Testdateien ausgelöst wurden, weggelassen. Ich habe auch die
V6022- Diagnose ignoriert (da Sie nicht zuverlässig zwischen fehlerhaften und korrekten Fragmenten unterscheiden können, wenn Sie mit dem Code nicht vertraut sind), die bis zu 482 Mal ausgelöst wurde. Ich habe auch nicht die 179 Warnungen untersucht, die von der
V6021- Diagnose generiert wurden.
Am Ende hatte ich immer noch genug Warnungen, und da ich die Einstellungen nicht angepasst habe, gibt es immer noch einen gewissen Prozentsatz an Fehlalarmen. Es macht einfach keinen Sinn, zu viele Warnungen in einen Artikel wie diesen aufzunehmen :). Also reden wir nur darüber, was mir aufgefallen ist und neugierig genug ausgesehen hat.
Vorgegebene Bedingungen
Unter den für diese Analyse untersuchten Diagnosen enthält
V6007 einen Datensatz für die Anzahl der ausgegebenen Warnungen. Etwas mehr als 200 Nachrichten !!! Einige sehen harmlos aus, andere sind misstrauisch und andere sind doch echte Käfer! 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.")) {
Das ist ein ziemlich langwieriges Wenn-Sonst-Wenn-Konstrukt! Dem Analysator gefällt die Bedingung im letzten
if (key.startsWith ("hplsql.")) Nicht. Wenn die Ausführung sie erreicht, bedeutet dies, dass sie wahr ist. Wenn Sie sich die erste Zeile dieses gesamten if-else-if-Konstrukts ansehen, werden Sie
feststellen, dass es bereits die entgegengesetzte Prüfung enthält. Wenn die Zeichenfolge also nicht mit
"hplsql" beginnt. Die Ausführung springt 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>() : ....; .... } } } .... }
Der Vergleich der
Länge der Zeichenfolge
columnNameProperty mit Null gibt immer
false zurück . Dies liegt daran, dass dieser Vergleich der
Prüfung! Strings.isNullOrEmpty (columnNameProperty) folgt. Wenn die Ausführung unseren Zustand erreicht, bedeutet dies, dass die Zeichenfolge
columnNameProperty sicherlich weder null noch leer ist.
Gleiches gilt für die Zeichenfolge
columnTypeProperty eine Zeile später:
- 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"))
Das gute alte Copy-Paste. Aus Sicht der aktuellen Logik kann der String
colOrScalar1 zwei verschiedene Werte gleichzeitig haben, was unmöglich ist. Offensichtlich sollten die Prüfungen die Variable
colOrScalar1 links und
colOrScalar2 rechts haben.
Ähnliche Warnungen einige Zeilen weiter unten:
- 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 wird dieses if-else-if-Konstrukt niemals etwas tun.
Noch ein paar
V6007- Warnungen:
- 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)
- und so weiter ...
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(); ..... } }
Ein Null-Objekt wird abgefangen, protokolliert und ... das Programm läuft einfach weiter. Infolgedessen folgt auf die Prüfung eine Nullzeiger-Dereferenzierung. Autsch!
Die Entwickler müssen tatsächlich gewollt haben, dass das Programm die Funktion beendet oder eine spezielle Ausnahme auslöst, wenn eine Nullreferenz abgerufen wird.
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) {
Ein weiteres potenzielles NPE. Wenn die Ausführung die Methode
lockerSingleBuffer erreicht, bedeutet dies, dass das
Pufferobjekt null ist. Angenommen, das ist passiert! Wenn Sie sich die Methode
lockerSingleBuffer ansehen , werden Sie gleich in der ersten Zeile feststellen, wie unser Objekt dereferenziert wird. Gotcha!
Eine wilde Verschiebung
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;
Dies ist eine mögliche Verschiebung um -1. Wenn die Methode beispielsweise mit
wordShifts == 3 und
bitShiftsInWord == 0 aufgerufen wird,
endet die gemeldete Zeile mit 1 << -1. Ist das ein geplantes Verhalten?
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] haben. Aus diesem Grund kann die Berechnung des Werts von
val in der Schleife auf unerwartete Weise ausgeführt werden. Im Ausdruck
(1 << j) ist der Wert 1 vom Typ
int . Wenn Sie ihn also um 32 Bit und mehr verschieben, überschreiten Sie die Grenzen des Typbereichs. Dies kann durch Schreiben von
((lang) 1 << j) behoben werden.
Sich von der Protokollierung mitreißen lassen
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 {}-{} ;",
Beim Schreiben des Codes zum Formatieren der Zeichenfolge mit
String.format () hat der Entwickler eine falsche Syntax verwendet. Infolgedessen haben es die übergebenen Parameter nie in die resultierende Zeichenfolge geschafft. Ich vermute, dass der Entwickler vor dem Schreiben an der Protokollierung gearbeitet hat, von wo er die Syntax ausgeliehen hat.
Eine gestohlene Ausnahme
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 { .... 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(); } } }
Es ist eine sehr schlechte Praxis, etwas aus dem
finally- Block zurückzugeben, und dieses Beispiel zeigt anschaulich, warum.
Im
try- Block bildet das Programm eine Anforderung und greift auf den Speicher zu. Die
festgeschriebene Variable hat standardmäßig den Wert
false und ändert ihren Status erst, nachdem alle vorherigen Aktionen im
try- Block erfolgreich ausgeführt wurden. Wenn eine Ausnahme ausgelöst wird, ist diese Variable immer
falsch . Der
catch- Block fängt die Ausnahme ab, passt sie ein wenig an und wirft sie auf. Wenn also der
finally- Block an der Reihe ist, gibt die Ausführung die Bedingung ein, aus der eine leere Liste zurückgegeben wird. Was kostet uns diese Rücksendung? Nun, es kostet uns zu verhindern, dass eine gefangene Ausnahme nach außen geworfen wird, wo sie richtig gehandhabt werden kann. Keine der in der Signatur der Methode angegebenen Ausnahmen wird jemals ausgelöst. Sie sind einfach irreführend.
Eine ähnliche Diagnosemeldung:
- V6051 Die Verwendung der Anweisung 'return' im Block 'finally' kann zum Verlust nicht behandelter Ausnahmen führen. ObjectStore.java (808)
Verschiedenes
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());
Es kann eine Reihe von Ursachen geben, die zu solch einem dummen Fehler führen: Kopieren, Einfügen, Nachlässigkeit, Eile und so weiter. Wir sehen solche Fehler oft in Open-Source-Projekten und haben 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) { return (compareUnsignedLong(dividend, divisor)) < 0 ? 0L : 1L; } if (dividend >= 0) {
Dieser ist ziemlich trivial. Eine Reihe von Überprüfungen war hilflos, um die Division durch Null abzuwenden.
Noch ein paar 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()) {
Der Programmierer hat den bitweisen Operator | geschrieben anstelle des logischen ||. Dies bedeutet, dass der rechte Teil ausgeführt wird, unabhängig vom Ergebnis des linken. Wenn
parent == null ist , wird dieser Tippfehler direkt im nächsten logischen Unterausdruck mit einem NPE enden.
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);
Wir interessieren uns für die Klassen
LongColumnVector erweitert ColumnVector und
TimestampColumnVector erweitert ColumnVector . Die Überprüfung, dass das
destCol- Objekt eine Instanz von
LongColumnVector ist, legt ausdrücklich nahe, dass es sich um ein Objekt dieser Klasse handelt, das im Hauptteil der bedingten Anweisung behandelt wird. Trotzdem wird es immer noch in
TimestampColumnVector umgewandelt ! Wie Sie sehen können, handelt es sich um verschiedene Klassen, mit der Ausnahme, dass sie von demselben übergeordneten Element abgeleitet sind. Als Ergebnis erhalten wir eine
ClassCastException .
Gleiches gilt für das Casting 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()) {
Hier sehen Sie eine seltsame Überprüfung des
var- Objekts auf
null, nachdem die Dereferenzierung bereits aufgetreten ist. In diesem Zusammenhang sind
var und
obj dasselbe Objekt (
var = (Var) obj ). Das Vorhandensein der
Nullprüfung impliziert, dass das übergebene Objekt null sein kann. Das Aufrufen von
equals (null) führt also zu einer NPE anstelle der erwarteten
false direkt in der ersten Zeile. Ja, der Scheck
ist da, aber leider ist er am falschen Ort.
Einige andere ähnliche Fälle, in denen ein Objekt vor der Prüfung verwendet wird:
- 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)
- und so weiter ...
Fazit
Wenn Sie sich jemals für Big Data interessiert haben, wenn auch nur ein bisschen, dann können Sie kaum übersehen, wie wichtig Apache Hive ist. Dies ist ein beliebtes und ziemlich großes Projekt, das aus über 6500 Quelldateien (* .java) besteht. Viele Entwickler haben es über viele Jahre geschrieben, was bedeutet, dass ein statischer Analysator dort viele Dinge zu finden hat. Es zeigt nur noch einmal, dass statische Analysen bei der Entwicklung mittlerer und großer Projekte äußerst wichtig und nützlich sind!
Hinweis Einmalige Überprüfungen wie die hier durchgeführte sind gut geeignet, um die Fähigkeiten des Analysators zu demonstrieren, stellen jedoch ein völlig unangemessenes Szenario für die Verwendung dar. Diese Idee wird
hier und
hier weiter ausgeführt . Die statische Analyse ist regelmäßig anzuwenden!
Diese Überprüfung von Hive ergab einige Mängel und verdächtige Fragmente. Wenn die Autoren von Apache Hive auf diesen Artikel stoßen, helfen wir Ihnen gerne bei der schwierigen Aufgabe, das Projekt zu verbessern.
Sie können sich Apache Hive ohne Apache Hadoop nicht vorstellen, daher könnte das Einhorn von PVS-Studio auch diesem einen Besuch abstatten. Aber das ist alles für heute. In der Zwischenzeit lade ich Sie ein, den Analysator
herunterzuladen und Ihre eigenen Projekte zu überprüfen.