You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2021/09/22 12:58:39 UTC
[tinkerpop] branch TINKERPOP-2610 created (now b89cc74)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2610
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at b89cc74 TINKERPOP-2610 Fixed inconsistency in NumberHelper return types
This branch includes the following new commits:
new b89cc74 TINKERPOP-2610 Fixed inconsistency in NumberHelper return types
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2610 Fixed inconsistency in
NumberHelper return types
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2610
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit b89cc748f628470ff32b9cd1ca91ae413e7d4089
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Wed Sep 22 08:56:48 2021 -0400
TINKERPOP-2610 Fixed inconsistency in NumberHelper return types
It is now possible to return byte/short explicitly rather than promoting to int.
---
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.6.x.asciidoc | 39 ++++++++++++++++++
.../tinkerpop/gremlin/util/NumberHelper.java | 16 ++++----
.../tinkerpop/gremlin/util/NumberHelperTest.java | 48 ++++++----------------
4 files changed, 60 insertions(+), 44 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7564dae..77ab152 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -36,6 +36,7 @@ limitations under the License.
* Packaged Gherkin tests and data as standalone package as a convenience distribution.
* Bumped to Apache Hadoop 3.3.1.
* Bumped to Apache Spark 3.1.2.
+* Changed `NumberHelper` to properly cast to `byte` and `short` rather than default coercing to `Integer`.
* Modified some driver defaults (maximum content length, pool size, maximum in process) to be more consistent with one another.
== TinkerPop 3.5.0 (The Sleeping Gremlin: No. 18 Entr'acte Symphonique)
diff --git a/docs/src/upgrade/release-3.6.x.asciidoc b/docs/src/upgrade/release-3.6.x.asciidoc
index 8c0f7ab..80626f8 100644
--- a/docs/src/upgrade/release-3.6.x.asciidoc
+++ b/docs/src/upgrade/release-3.6.x.asciidoc
@@ -58,6 +58,45 @@ containers which should work more cleanly out of the box.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-2534[TINKERPOP-2534]
+==== Short and Byte
+
+Numeric operations around `short` and `byte` have not behaved quite like `int` and `long`. Here is an example of a
+`sum` operation with `sack()`:
+
+[source,text]
+----
+gremlin> g.withSack((short) 2).inject((short) 1, (int) 2).sack(sum).sack()
+==>3
+==>4
+gremlin> g.withSack((short) 2).inject((short) 1, (int) 2).sack(sum).sack().collect{it.class}
+==>class java.lang.Integer
+==>class java.lang.Integer
+gremlin> g.withSack((short) 2).inject((short) 1, (long) 2).sack(sum).sack().collect{it.class}
+==>class java.lang.Integer
+==>class java.lang.Long
+gremlin> g.withSack((short) 2).inject((short) 1,(byte) 2).sack(sum).sack().collect{it.class}
+==>class java.lang.Integer
+==>class java.lang.Integer
+----
+
+Note that the type returned for the the `sum` should be the largest type encountered in the operation, thus if a
+`long + int` would return `long` or a `byte + int` would return `int`. The last example above shows inconsistency in
+this rule when dealing with types `short` and `byte` which simply promote them to `int`.
+
+For 3.6.0, that inconsistency is resolved and may be a breaking change should code be relying on the integer promotion.
+
+[source,text]
+----
+gremlin> g.withSack((short) 2).inject((short) 1,(byte) 2).sack(sum).sack().collect{it.class}
+==>class java.lang.Short
+==>class java.lang.Short
+gremlin> g.withSack((byte) 2).inject((byte) 1,(byte) 2).sack(sum).sack().collect{it.class}
+==>class java.lang.Byte
+==>class java.lang.Byte
+----
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2610[TINKERPOP-2610]
+
==== Groovy in gremlin-driver
The `gremlin-driver` module no longer depends on `groovy` or `groovy-json`. It became an `<optional>` dependency in
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
index 8293195..db756ec 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/NumberHelper.java
@@ -29,10 +29,10 @@ import java.util.function.BiFunction;
public final class NumberHelper {
static final NumberHelper BYTE_NUMBER_HELPER = new NumberHelper(
- (a, b) -> a.byteValue() + b.byteValue(),
- (a, b) -> a.byteValue() - b.byteValue(),
- (a, b) -> a.byteValue() * b.byteValue(),
- (a, b) -> a.byteValue() / b.byteValue(),
+ (a, b) -> (byte) (a.byteValue() + b.byteValue()),
+ (a, b) -> (byte) (a.byteValue() - b.byteValue()),
+ (a, b) -> (byte) (a.byteValue() * b.byteValue()),
+ (a, b) -> (byte) (a.byteValue() / b.byteValue()),
(a, b) -> {
if (isNumber(a)) {
if (isNumber(b)) {
@@ -56,10 +56,10 @@ public final class NumberHelper {
(a, b) -> Byte.compare(a.byteValue(), b.byteValue()));
static final NumberHelper SHORT_NUMBER_HELPER = new NumberHelper(
- (a, b) -> a.shortValue() + b.shortValue(),
- (a, b) -> a.shortValue() - b.shortValue(),
- (a, b) -> a.shortValue() * b.shortValue(),
- (a, b) -> a.shortValue() / b.shortValue(),
+ (a, b) -> (short) (a.shortValue() + b.shortValue()),
+ (a, b) -> (short) (a.shortValue() - b.shortValue()),
+ (a, b) -> (short) (a.shortValue() * b.shortValue()),
+ (a, b) -> (short) (a.shortValue() / b.shortValue()),
(a, b) -> {
if (isNumber(a)) {
if (isNumber(b)) {
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
index 01ad1ca..5b32e19 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/util/NumberHelperTest.java
@@ -128,17 +128,12 @@ public class NumberHelperTest {
@Test
public void shouldAddAndReturnCorrectType() {
-
- // NOTE: The smallest possible number type for return values is Integer; this seems to be a JVM thing and has
- // nothing to do with the actual NumberHelper implementation
-
- // the above NOTE isn't quite true - TINKERPOP-2610
assertEquals((byte) 1, add((byte) 1, (Byte) null));
assertNull(add((Byte) null, (byte) 1));
// BYTE
- assertEquals(2, add((byte) 1, (byte) 1));
- assertEquals(2, add((byte) 1, (short) 1));
+ assertEquals((byte) 2, add((byte) 1, (byte) 1));
+ assertEquals((short) 2, add((byte) 1, (short) 1));
assertEquals(2, add((byte) 1, 1));
assertEquals(2L, add((byte) 1, 1L));
assertEquals(2F, add((byte) 1, 1F));
@@ -147,7 +142,7 @@ public class NumberHelperTest {
assertEquals(BigDecimal.ONE.add(BigDecimal.ONE), add((byte) 1, BigDecimal.ONE));
// SHORT
- assertEquals(2, add((short) 1, (short) 1));
+ assertEquals((short)2, add((short) 1, (short) 1));
assertEquals(2, add((short) 1, 1));
assertEquals(2L, add((short) 1, 1L));
assertEquals(2F, add((short) 1, 1F));
@@ -191,17 +186,12 @@ public class NumberHelperTest {
@Test
public void shouldSubtractAndReturnCorrectType() {
-
- // NOTE: The smallest possible number type for return values is Integer; this seems to be a JVM thing and has
- // nothing to do with the actual NumberHelper implementation
-
- // the above NOTE isn't quite true - TINKERPOP-2610
assertEquals((byte) 1, sub((byte) 1, (Byte) null));
assertNull(sub((Byte) null, (byte) 1));
// BYTE
- assertEquals(0, sub((byte) 1, (byte) 1));
- assertEquals(0, sub((byte) 1, (short) 1));
+ assertEquals((byte) 0, sub((byte) 1, (byte) 1));
+ assertEquals((short) 0, sub((byte) 1, (short) 1));
assertEquals(0, sub((byte) 1, 1));
assertEquals(0L, sub((byte) 1, 1L));
assertEquals(0F, sub((byte) 1, 1F));
@@ -210,7 +200,7 @@ public class NumberHelperTest {
assertEquals(BigDecimal.ZERO, sub((byte) 1, BigDecimal.ONE));
// SHORT
- assertEquals(0, sub((short) 1, (short) 1));
+ assertEquals((short) 0, sub((short) 1, (short) 1));
assertEquals(0, sub((short) 1, 1));
assertEquals(0L, sub((short) 1, 1L));
assertEquals(0F, sub((short) 1, 1F));
@@ -254,17 +244,12 @@ public class NumberHelperTest {
@Test
public void shouldMultiplyAndReturnCorrectType() {
-
- // NOTE: The smallest possible number type for return values is Integer; this seems to be a JVM thing and has
- // nothing to do with the actual NumberHelper implementation
-
- // the above NOTE isn't quite true - TINKERPOP-2610
assertEquals((byte) 1, mul((byte) 1, (Byte) null));
assertNull(mul((Byte) null, (byte) 1));
// BYTE
- assertEquals(1, mul((byte) 1, (byte) 1));
- assertEquals(1, mul((byte) 1, (short) 1));
+ assertEquals((byte) 1, mul((byte) 1, (byte) 1));
+ assertEquals((short) 1, mul((byte) 1, (short) 1));
assertEquals(1, mul((byte) 1, 1));
assertEquals(1L, mul((byte) 1, 1L));
assertEquals(1F, mul((byte) 1, 1F));
@@ -273,7 +258,7 @@ public class NumberHelperTest {
assertEquals(BigDecimal.ONE, mul((byte) 1, BigDecimal.ONE));
// SHORT
- assertEquals(1, mul((short) 1, (short) 1));
+ assertEquals((short) 1, mul((short) 1, (short) 1));
assertEquals(1, mul((short) 1, 1));
assertEquals(1L, mul((short) 1, 1L));
assertEquals(1F, mul((short) 1, 1F));
@@ -317,17 +302,12 @@ public class NumberHelperTest {
@Test
public void shouldDivideAndReturnCorrectType() {
-
- // NOTE: The smallest possible number type for return values is Integer; this seems to be a JVM thing and has
- // nothing to do with the actual NumberHelper implementation
-
- // the above NOTE isn't quite true - TINKERPOP-2610
assertEquals((byte) 1, div((byte) 1, (Byte) null));
assertNull(div((Byte) null, (byte) 1));
// BYTE
- assertEquals(1, div((byte) 1, (byte) 1));
- assertEquals(1, div((byte) 1, (short) 1));
+ assertEquals((byte) 1, div((byte) 1, (byte) 1));
+ assertEquals((short) 1, div((byte) 1, (short) 1));
assertEquals(1, div((byte) 1, 1));
assertEquals(1L, div((byte) 1, 1L));
assertEquals(1F, div((byte) 1, 1F));
@@ -336,7 +316,7 @@ public class NumberHelperTest {
assertEquals(BigDecimal.ONE, div((byte) 1, BigDecimal.ONE));
// SHORT
- assertEquals(1, div((short) 1, (short) 1));
+ assertEquals((short) 1, div((short) 1, (short) 1));
assertEquals(1, div((short) 1, 1));
assertEquals(1L, div((short) 1, 1L));
assertEquals(1F, div((short) 1, 1F));
@@ -380,10 +360,6 @@ public class NumberHelperTest {
@Test
public void shouldDivideForceFloatingPointAndReturnCorrectType() {
-
- // NOTE: The smallest possible number type for return values is Integer; this seems to be a JVM thing and has
- // nothing to do with the actual NumberHelper implementation
-
// BYTE
assertEquals(1F, div((byte) 1, (byte) 1, true));
assertEquals(1F, div((byte) 1, (short) 1, true));