Skip to content

Commit

Permalink
IGNITE-23704: Incorrect equal implementation in TemporalNativeType an…
Browse files Browse the repository at this point in the history 
…d VarlenNativeType
  • Loading branch information
@lowka
lowka committed Nov 21, 2024
1 parent ca0d2b5 commit 97bd8e2
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 70 deletions.
23 changes: 23 additions & 0 deletions modules/core/src/main/java/org/apache/ignite/internal/type/TemporalNativeType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;

import java.util.Objects;
import org.apache.ignite.internal.tostring.S;

/**
Expand Down Expand Up @@ -95,6 +96,28 @@ public String displayName() {
return format("{}({})", super.displayName(), precision);
}

/** {@inheritDoc} */
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
if (!super.equals(o)) {
return false;
}
TemporalNativeType that = (TemporalNativeType) o;
return precision == that.precision;
}

/** {@inheritDoc} */
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), precision);
}

/** {@inheritDoc} */
@Override
public String toString() {
Expand Down
24 changes: 24 additions & 0 deletions modules/core/src/main/java/org/apache/ignite/internal/type/VarlenNativeType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;

import java.util.Objects;
import org.apache.ignite.internal.tostring.S;

/**
Expand Down Expand Up @@ -59,6 +60,29 @@ public int length() {
return len;
}

/** {@inheritDoc} */
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
if (!super.equals(o)) {
return false;
}
VarlenNativeType that = (VarlenNativeType) o;
return len == that.len;
}

/** {@inheritDoc} */
@Override
public int hashCode() {
return Objects.hash(super.hashCode(), len);
}

/** {@inheritDoc} */
@Override
public String toString() {
return S.toString(VarlenNativeType.class.getSimpleName(), "name", spec(), "len", len);
Expand Down
67 changes: 67 additions & 0 deletions modules/core/src/test/java/org/apache/ignite/internal/type/NativeTypeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,25 @@

package org.apache.ignite.internal.type;

import static org.apache.ignite.internal.type.NativeTypes.BOOLEAN;
import static org.apache.ignite.internal.type.NativeTypes.BYTES;
import static org.apache.ignite.internal.type.NativeTypes.DATE;
import static org.apache.ignite.internal.type.NativeTypes.DOUBLE;
import static org.apache.ignite.internal.type.NativeTypes.FLOAT;
import static org.apache.ignite.internal.type.NativeTypes.INT16;
import static org.apache.ignite.internal.type.NativeTypes.INT32;
import static org.apache.ignite.internal.type.NativeTypes.INT64;
import static org.apache.ignite.internal.type.NativeTypes.INT8;
import static org.apache.ignite.internal.type.NativeTypes.STRING;
import static org.apache.ignite.internal.type.NativeTypes.UUID;
import static org.apache.ignite.internal.type.NativeTypes.blobOf;
import static org.apache.ignite.internal.type.NativeTypes.datetime;
import static org.apache.ignite.internal.type.NativeTypes.decimalOf;
import static org.apache.ignite.internal.type.NativeTypes.stringOf;
import static org.apache.ignite.internal.type.NativeTypes.time;
import static org.apache.ignite.internal.type.NativeTypes.timestamp;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand Down Expand Up @@ -141,4 +147,65 @@ public void invalidTemporalTypes() {
public void compareVarlenTypesByDesc() {
assertTrue(BYTES.compareTo(STRING) < 0);
}

@Test
public void basicTypeEquality() {
assertEquals(BOOLEAN, new NativeType(NativeTypeSpec.BOOLEAN, 1));

assertEquals(INT8, new NativeType(NativeTypeSpec.INT8, 1));
assertEquals(INT16, new NativeType(NativeTypeSpec.INT16, 2));
assertEquals(INT32, new NativeType(NativeTypeSpec.INT32, 4));
assertEquals(INT64, new NativeType(NativeTypeSpec.INT64, 8));

assertEquals(FLOAT, new NativeType(NativeTypeSpec.FLOAT, 4));
assertEquals(DOUBLE, new NativeType(NativeTypeSpec.DOUBLE, 8));

assertEquals(DATE, new NativeType(NativeTypeSpec.DATE, 3));
assertEquals(UUID, new NativeType(NativeTypeSpec.UUID, 16));
}

@Test
public void temporalTypeEquality() {
assertEquals(time(0), time(0));
assertNotEquals(time(1), time(0));
assertNotEquals(time(0), time(1));

assertEquals(timestamp(1), timestamp(1));
assertEquals(timestamp(0), timestamp(0));
assertNotEquals(timestamp(1), timestamp(0));
assertNotEquals(timestamp(0), timestamp(1));

assertEquals(datetime(1), datetime(1));
assertEquals(datetime(0), datetime(0));
assertNotEquals(datetime(1), datetime(0));
assertNotEquals(datetime(0), datetime(1));
}

@Test
public void decimalTypeEquality() {
assertEquals(decimalOf(10, 2), decimalOf(10, 2));
assertNotEquals(decimalOf(10, 2), decimalOf(10, 3));
}

@Test
public void stringTypeEquality() {
assertEquals(stringOf(10), stringOf(10));
assertEquals(stringOf(2), stringOf(2));

assertNotEquals(stringOf(10), stringOf(5));
assertNotEquals(stringOf(5), stringOf(10));
assertNotEquals(STRING, stringOf(10));
assertNotEquals(stringOf(10), STRING);
}

@Test
public void blobTypeEquality() {
assertEquals(blobOf(10), blobOf(10));
assertEquals(blobOf(2), blobOf(2));

assertNotEquals(blobOf(10), blobOf(5));
assertNotEquals(blobOf(5), blobOf(10));
assertNotEquals(BYTES, blobOf(10));
assertNotEquals(blobOf(10), BYTES);
}
}
4 changes: 2 additions & 2 deletions ...es/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/TypeUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ public class TypeUtilsTest extends BaseIgniteAbstractTest {
private static final BaseTypeSpec INT64 = RowSchemaTypes.nativeType(NativeTypes.INT64);
private static final BaseTypeSpec FLOAT = RowSchemaTypes.nativeType(NativeTypes.FLOAT);
private static final BaseTypeSpec DOUBLE = RowSchemaTypes.nativeType(NativeTypes.DOUBLE);
private static final BaseTypeSpec STRING = RowSchemaTypes.nativeType(NativeTypes.STRING);
private static final BaseTypeSpec BYTES = RowSchemaTypes.nativeType(NativeTypes.BYTES);
private static final BaseTypeSpec STRING = RowSchemaTypes.nativeType(NativeTypes.stringOf(65536));
private static final BaseTypeSpec BYTES = RowSchemaTypes.nativeType(NativeTypes.blobOf(65536));
private static final BaseTypeSpec UUID = RowSchemaTypes.nativeType(NativeTypes.UUID);

@Test
Expand Down
167 changes: 99 additions & 68 deletions modules/table/src/test/java/org/apache/ignite/internal/table/InteropOperationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -58,6 +57,7 @@
import org.apache.ignite.internal.tx.impl.HeapLockManager;
import org.apache.ignite.internal.tx.impl.WaitDieDeadlockPreventionPolicy;
import org.apache.ignite.internal.type.NativeType;
import org.apache.ignite.internal.type.NativeTypeSpec;
import org.apache.ignite.internal.type.NativeTypes;
import org.apache.ignite.sql.IgniteSql;
import org.apache.ignite.table.KeyValueView;
Expand Down Expand Up @@ -389,38 +389,54 @@ private Tuple createTuple(int id, boolean nulls) {
String colName = col.name();
NativeType type = col.type();

if (NativeTypes.BOOLEAN.equals(type)) {
res.set(colName, id % 2 == 0);
} else if (NativeTypes.INT8.equals(type)) {
res.set(colName, (byte) id);
} else if (NativeTypes.INT16.equals(type)) {
res.set(colName, (short) id);
} else if (NativeTypes.INT32.equals(type)) {
res.set(colName, id);
} else if (NativeTypes.INT64.equals(type)) {
res.set(colName, (long) id);
} else if (NativeTypes.FLOAT.equals(type)) {
res.set(colName, (float) id);
} else if (NativeTypes.DOUBLE.equals(type)) {
res.set(colName, (double) id);
} else if (NativeTypes.BYTES.equals(type)) {
res.set(colName, String.valueOf(id).getBytes(StandardCharsets.UTF_8));
} else if (NativeTypes.STRING.equals(type)) {
res.set(colName, String.valueOf(id));
} else if (NativeTypes.UUID.equals(type)) {
res.set(colName, new UUID(0L, (long) id));
} else if (NativeTypes.DATE.equals(type)) {
res.set(colName, LocalDate.ofYearDay(2021, id));
} else if (NativeTypes.time(0).equals(type)) {
res.set(colName, LocalTime.ofSecondOfDay(id));
} else if (NativeTypes.datetime(6).equals(type)) {
res.set(colName, LocalDateTime.ofEpochSecond(id, 0, ZoneOffset.UTC));
} else if (NativeTypes.timestamp(6).equals(type)) {
res.set(colName, Instant.ofEpochSecond(id));
} else if (NativeTypes.decimalOf(5, 2).equals(type)) {
res.set(colName, BigDecimal.valueOf(id * 100).movePointLeft(2));
} else {
fail("Unable to fullfill value of type " + type);
switch (type.spec()) {
case INT8:
res.set(colName, (byte) id);
break;
case INT16:
res.set(colName, (short) id);
break;
case INT32:
res.set(colName, id);
break;
case INT64:
res.set(colName, (long) id);
break;
case FLOAT:
res.set(colName, (float) id);
break;
case DOUBLE:
res.set(colName, (double) id);
break;
case DECIMAL:
res.set(colName, BigDecimal.valueOf(id * 100).movePointLeft(2));
break;
case UUID:
res.set(colName, new UUID(0L, id));
break;
case STRING:
res.set(colName, String.valueOf(id));
break;
case BYTES:
res.set(colName, String.valueOf(id).getBytes(StandardCharsets.UTF_8));
break;
case DATE:
res.set(colName, LocalDate.ofYearDay(2021, id));
break;
case TIME:
res.set(colName, LocalTime.ofSecondOfDay(id));
break;
case DATETIME:
res.set(colName, LocalDateTime.ofEpochSecond(id, 0, ZoneOffset.UTC));
break;
case TIMESTAMP:
res.set(colName, Instant.ofEpochSecond(id));
break;
case BOOLEAN:
res.set(colName, id % 2 == 0);
break;
default:
throw new IllegalArgumentException("Unexpected type: " + type);
}
}

Expand Down Expand Up @@ -449,43 +465,58 @@ private void validateTuple(int id, Tuple t, boolean nulls) {

String colName = col.name();
NativeType type = col.type();

if (NativeTypes.BOOLEAN.equals(type)) {
assertEquals(expected.booleanValue(colName), t.booleanValue(colName));
} else if (NativeTypes.INT8.equals(type)) {
assertEquals(expected.byteValue(colName), t.byteValue(colName));
} else if (NativeTypes.INT16.equals(type)) {
assertEquals(expected.shortValue(colName), t.shortValue(colName));
} else if (NativeTypes.INT32.equals(type)) {
assertEquals(expected.intValue(colName), t.intValue(colName));
} else if (NativeTypes.INT64.equals(type)) {
assertEquals(expected.longValue(colName), t.longValue(colName));
} else if (NativeTypes.FLOAT.equals(type)) {
assertEquals(expected.floatValue(colName), t.floatValue(colName));
} else if (NativeTypes.DOUBLE.equals(type)) {
assertEquals(expected.doubleValue(colName), t.doubleValue(colName));
} else if (NativeTypes.BYTES.equals(type)) {
assertArrayEquals((byte[]) expected.value(colName), (byte[]) t.value(colName));
} else if (NativeTypes.STRING.equals(type)) {
assertEquals(expected.stringValue(colName), t.stringValue(colName));
} else if (NativeTypes.UUID.equals(type)) {
assertEquals(expected.uuidValue(colName), t.uuidValue(colName));
} else if (NativeTypes.DATE.equals(type)) {
assertEquals(expected.dateValue(colName), t.dateValue(colName));
} else if (NativeTypes.time(0).equals(type)) {
assertEquals(expected.timeValue(colName), t.timeValue(colName));
} else if (NativeTypes.datetime(6).equals(type)) {
assertEquals(expected.datetimeValue(colName), t.datetimeValue(colName));
} else if (NativeTypes.timestamp(6).equals(type)) {
assertEquals(expected.timestampValue(colName), expected.timestampValue(colName));
} else if (NativeTypes.decimalOf(5, 2).equals(type)) {
assertEquals((BigDecimal) expected.value(colName), t.value(colName));
} else {
fail("Unable to validate value of type " + type);
NativeTypeSpec typeSpec = type.spec();

switch (typeSpec) {
case BOOLEAN:
assertEquals(expected.booleanValue(colName), t.booleanValue(colName));
break;
case INT8:
assertEquals(expected.byteValue(colName), t.byteValue(colName));
break;
case INT16:
assertEquals(expected.shortValue(colName), t.shortValue(colName));
break;
case INT32:
assertEquals(expected.intValue(colName), t.intValue(colName));
break;
case INT64:
assertEquals(expected.longValue(colName), t.longValue(colName));
break;
case FLOAT:
assertEquals(expected.floatValue(colName), t.floatValue(colName));
break;
case DOUBLE:
assertEquals(expected.doubleValue(colName), t.doubleValue(colName));
break;
case BYTES:
assertArrayEquals(expected.value(colName), (byte[]) t.value(colName));
break;
case STRING:
assertEquals(expected.stringValue(colName), t.stringValue(colName));
break;
case UUID:
assertEquals(expected.uuidValue(colName), t.uuidValue(colName));
break;
case DATE:
assertEquals(expected.dateValue(colName), t.dateValue(colName));
break;
case TIME:
assertEquals(expected.timeValue(colName), t.timeValue(colName));
break;
case DATETIME:
assertEquals(expected.datetimeValue(colName), t.datetimeValue(colName));
break;
case TIMESTAMP:
assertEquals(expected.timestampValue(colName), t.timestampValue(colName));
break;
case DECIMAL:
assertEquals((BigDecimal) expected.value(colName), t.value(colName));
break;
default:
throw new IllegalArgumentException("Expected type: " + type);
}
}

assertTrue(!nulls ^ expected.equals(t), "nulls = " + nulls + ", id = " + id);
}

/**
Expand Down

0 comments on commit 97bd8e2

Please sign in to comment.