You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by sz...@apache.org on 2019/10/18 09:19:07 UTC
[hive] branch master updated: HIVE-22356: CacheTag's compareTo()
produces wrong result for edge cases (Adam Szita, reviewed by Peter Vary)
This is an automated email from the ASF dual-hosted git repository.
szita pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 5c91d32 HIVE-22356: CacheTag's compareTo() produces wrong result for edge cases (Adam Szita, reviewed by Peter Vary)
5c91d32 is described below
commit 5c91d324f22c2ae47e234e76a9bc5ee1a71e6a70
Author: Adam Szita <sz...@cloudera.com>
AuthorDate: Wed Oct 16 14:35:11 2019 +0200
HIVE-22356: CacheTag's compareTo() produces wrong result for edge cases (Adam Szita, reviewed by Peter Vary)
---
.../hive/llap/cache/TestCacheContentsTracker.java | 49 ++++++++++++++++++++++
.../org/apache/hadoop/hive/common/io/CacheTag.java | 39 +++++++++++++++--
2 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestCacheContentsTracker.java b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestCacheContentsTracker.java
index 1d242e0..03e66a8 100644
--- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestCacheContentsTracker.java
+++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestCacheContentsTracker.java
@@ -83,6 +83,55 @@ public class TestCacheContentsTracker {
assertEquals(EXPECTED_CACHE_STATE_AFTER_EVICTION, sb.toString());
}
+
+ /**
+ * Tests CacheTag.compareTo().
+ */
+ @Test
+ public void testCacheTagComparison() {
+
+ // Comparing with null
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename"), null);
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename", "p1=v1"), null);
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2"), null);
+
+ // Comparing similar constructs
+ compareViceVersa(0, cacheTagBuilder("dbname.tablename"),
+ cacheTagBuilder("dbname.tablename"));
+ compareViceVersa(0, cacheTagBuilder("dbname.tablename", "p1=v1"),
+ cacheTagBuilder("dbname.tablename", "p1=v1"));
+ compareViceVersa(0, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2"),
+ cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2"));
+
+ // Comparing structs of different lengths
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2", "p3=v3"),
+ cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2"));
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2"),
+ cacheTagBuilder("dbname.tablename", "p1=v1"));
+ compareViceVersa(1, cacheTagBuilder("dbname.tablename", "p1=v1"),
+ cacheTagBuilder("dbname.tablename"));
+
+ // Comparing different constructs with same length
+ compareViceVersa(-1, cacheTagBuilder("dbname.tablename", "p1=v1"),
+ cacheTagBuilder("dbname.tablenamf", "p1=v0"));
+ compareViceVersa(-1, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v3"),
+ cacheTagBuilder("dbname.tablenamf", "p1=v1", "p2=v2"));
+ compareViceVersa(-25, cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2a"),
+ cacheTagBuilder("dbname.tablename", "p1=v1", "p2=v2z"));
+ compareViceVersa(-1, cacheTagBuilder("dbname.tablenameAA", "p1=v1", "p2=v2"),
+ cacheTagBuilder("dbname.tablenameBB", "p1=v1", "p2=v2"));
+
+ }
+
+ private static void compareViceVersa(int expected, CacheTag a, CacheTag b) {
+ if (a != null) {
+ assertEquals(expected, a.compareTo(b));
+ }
+ if (b != null) {
+ assertEquals(-1 * expected, b.compareTo(a));
+ }
+ }
+
private static LlapCacheableBuffer createMockBuffer(long size, CacheTag cacheTag) {
LlapCacheableBuffer llapCacheableBufferMock = mock(LlapCacheableBuffer.class);
diff --git a/storage-api/src/java/org/apache/hadoop/hive/common/io/CacheTag.java b/storage-api/src/java/org/apache/hadoop/hive/common/io/CacheTag.java
index 623c181..47730cc 100644
--- a/storage-api/src/java/org/apache/hadoop/hive/common/io/CacheTag.java
+++ b/storage-api/src/java/org/apache/hadoop/hive/common/io/CacheTag.java
@@ -46,6 +46,9 @@ public abstract class CacheTag implements Comparable<CacheTag> {
@Override
public int compareTo(CacheTag o) {
+ if (o == null) {
+ return 1;
+ }
return tableName.compareTo(o.tableName);
}
@@ -152,6 +155,9 @@ public abstract class CacheTag implements Comparable<CacheTag> {
@Override
public int compareTo(CacheTag o) {
+ if (o == null) {
+ return 1;
+ }
if (o instanceof SinglePartitionCacheTag || o instanceof MultiPartitionCacheTag) {
return -1;
} else {
@@ -200,14 +206,21 @@ public abstract class CacheTag implements Comparable<CacheTag> {
@Override
public int compareTo(CacheTag o) {
+ if (o == null) {
+ return 1;
+ }
if (o instanceof TableCacheTag) {
return 1;
} else if (o instanceof MultiPartitionCacheTag) {
return -1;
}
SinglePartitionCacheTag other = (SinglePartitionCacheTag) o;
- return super.compareTo(o) +
- partitionDesc.toString().compareTo(other.partitionDesc.toString());
+ int tableNameDiff = super.compareTo(other);
+ if (tableNameDiff != 0) {
+ return tableNameDiff;
+ } else {
+ return partitionDesc.compareTo(other.partitionDesc);
+ }
}
@Override
@@ -235,12 +248,30 @@ public abstract class CacheTag implements Comparable<CacheTag> {
@Override
public int compareTo(CacheTag o) {
+ if (o == null) {
+ return 1;
+ }
if (o instanceof TableCacheTag || o instanceof SinglePartitionCacheTag) {
return 1;
}
MultiPartitionCacheTag other = (MultiPartitionCacheTag) o;
- return super.compareTo(o) +
- partitionDesc.toString().compareTo(other.partitionDesc.toString());
+ int tableNameDiff = super.compareTo(other);
+ if (tableNameDiff != 0) {
+ return tableNameDiff;
+ } else {
+ int sizeDiff = partitionDesc.size() - other.partitionDesc.size();
+ if (sizeDiff != 0) {
+ return sizeDiff;
+ } else {
+ for (int i = 0; i < partitionDesc.size(); ++i) {
+ int partDiff = partitionDesc.get(i).compareTo(other.partitionDesc.get(i));
+ if (partDiff != 0) {
+ return partDiff;
+ }
+ }
+ return 0;
+ }
+ }
}
@Override