From 1dbbc0415dafd1ff28dbdb174d99955b3291a789 Mon Sep 17 00:00:00 2001 From: Bereng Date: Wed, 22 Jul 2020 12:15:12 +0200 Subject: [PATCH] CASSANDRA-15896 NULL/Empty UUID json string representation fix --- .../cassandra/db/marshal/AbstractType.java | 3 +- .../cassandra/db/marshal/DecimalType.java | 7 +- .../cassandra/db/marshal/DoubleType.java | 2 + .../cassandra/db/marshal/FloatType.java | 2 + .../cassandra/db/marshal/InetAddressType.java | 7 +- .../cassandra/db/marshal/Int32Type.java | 3 +- .../cassandra/db/marshal/IntegerType.java | 3 +- .../apache/cassandra/db/marshal/LongType.java | 3 +- .../cassandra/db/marshal/ShortType.java | 3 +- .../cassandra/db/marshal/TimestampType.java | 7 +- .../cassandra/cql3/EmptyValuesTest.java | 240 ++++++++++++++++++ .../cassandra/db/marshal/UUIDTypeTest.java | 8 + 12 files changed, 278 insertions(+), 10 deletions(-) create mode 100644 test/unit/org/apache/cassandra/cql3/EmptyValuesTest.java diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java b/src/java/org/apache/cassandra/db/marshal/AbstractType.java index a15dd48b0dcd..b14af70a4b46 100644 --- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java +++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java @@ -26,6 +26,7 @@ import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Objects; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -150,7 +151,7 @@ public String getString(ByteBuffer bytes) */ public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return '"' + getSerializer().deserialize(buffer).toString() + '"'; + return '"' + Objects.toString(getSerializer().deserialize(buffer), "") + '"'; } /* validate that the byte array is a valid sequence for the type we are supposed to be comparing */ diff --git a/src/java/org/apache/cassandra/db/marshal/DecimalType.java b/src/java/org/apache/cassandra/db/marshal/DecimalType.java index 17d91d3d47a6..43b2711816df 100644 --- a/src/java/org/apache/cassandra/db/marshal/DecimalType.java +++ b/src/java/org/apache/cassandra/db/marshal/DecimalType.java @@ -19,6 +19,7 @@ import java.math.BigDecimal; import java.nio.ByteBuffer; +import java.util.Objects; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.Constants; @@ -71,9 +72,9 @@ public Term fromJSONObject(Object parsed) throws MarshalException { try { - return new Constants.Value(getSerializer().serialize(new BigDecimal(parsed.toString()))); + return new Constants.Value(fromString(Objects.toString(parsed))); } - catch (NumberFormatException exc) + catch (NumberFormatException | MarshalException exc) { throw new MarshalException(String.format("Value '%s' is not a valid representation of a decimal value", parsed)); } @@ -82,7 +83,7 @@ public Term fromJSONObject(Object parsed) throws MarshalException @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return getSerializer().deserialize(buffer).toString(); + return Objects.toString(getSerializer().deserialize(buffer), "\"\""); } public CQL3Type asCQL3Type() diff --git a/src/java/org/apache/cassandra/db/marshal/DoubleType.java b/src/java/org/apache/cassandra/db/marshal/DoubleType.java index d39059b63ef7..17d9cf52a0f0 100644 --- a/src/java/org/apache/cassandra/db/marshal/DoubleType.java +++ b/src/java/org/apache/cassandra/db/marshal/DoubleType.java @@ -86,6 +86,8 @@ public Term fromJSONObject(Object parsed) throws MarshalException public String toJSONString(ByteBuffer buffer, int protocolVersion) { Double value = getSerializer().deserialize(buffer); + if (value == null) + return "\"\""; // JSON does not support NaN, Infinity and -Infinity values. Most of the parser convert them into null. if (value.isNaN() || value.isInfinite()) return "null"; diff --git a/src/java/org/apache/cassandra/db/marshal/FloatType.java b/src/java/org/apache/cassandra/db/marshal/FloatType.java index 58f5702f8d61..0ff02a3afcf4 100644 --- a/src/java/org/apache/cassandra/db/marshal/FloatType.java +++ b/src/java/org/apache/cassandra/db/marshal/FloatType.java @@ -85,6 +85,8 @@ public Term fromJSONObject(Object parsed) throws MarshalException public String toJSONString(ByteBuffer buffer, int protocolVersion) { Float value = getSerializer().deserialize(buffer); + if (value == null) + return "\"\""; // JSON does not support NaN, Infinity and -Infinity values. Most of the parser convert them into null. if (value.isNaN() || value.isInfinite()) return "null"; diff --git a/src/java/org/apache/cassandra/db/marshal/InetAddressType.java b/src/java/org/apache/cassandra/db/marshal/InetAddressType.java index 7ffb9c744170..393f09e501ba 100644 --- a/src/java/org/apache/cassandra/db/marshal/InetAddressType.java +++ b/src/java/org/apache/cassandra/db/marshal/InetAddressType.java @@ -73,10 +73,15 @@ public Term fromJSONObject(Object parsed) throws MarshalException } } + private String toString(InetAddress inet) + { + return inet != null ? inet.getHostAddress() : ""; + } + @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return '"' + getSerializer().deserialize(buffer).getHostAddress() + '"'; + return '"' + toString(getSerializer().deserialize(buffer)) + '"'; } public CQL3Type asCQL3Type() diff --git a/src/java/org/apache/cassandra/db/marshal/Int32Type.java b/src/java/org/apache/cassandra/db/marshal/Int32Type.java index 770a76d480eb..9ac7d5b32b37 100644 --- a/src/java/org/apache/cassandra/db/marshal/Int32Type.java +++ b/src/java/org/apache/cassandra/db/marshal/Int32Type.java @@ -18,6 +18,7 @@ package org.apache.cassandra.db.marshal; import java.nio.ByteBuffer; +import java.util.Objects; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.Constants; @@ -97,7 +98,7 @@ public Term fromJSONObject(Object parsed) throws MarshalException @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return getSerializer().deserialize(buffer).toString(); + return Objects.toString(getSerializer().deserialize(buffer), "\"\""); } public CQL3Type asCQL3Type() diff --git a/src/java/org/apache/cassandra/db/marshal/IntegerType.java b/src/java/org/apache/cassandra/db/marshal/IntegerType.java index 8f4ba442c6cb..f65856e72df1 100644 --- a/src/java/org/apache/cassandra/db/marshal/IntegerType.java +++ b/src/java/org/apache/cassandra/db/marshal/IntegerType.java @@ -19,6 +19,7 @@ import java.math.BigInteger; import java.nio.ByteBuffer; +import java.util.Objects; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.Constants; @@ -165,7 +166,7 @@ public Term fromJSONObject(Object parsed) throws MarshalException @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return getSerializer().deserialize(buffer).toString(); + return Objects.toString(getSerializer().deserialize(buffer), "\"\""); } @Override diff --git a/src/java/org/apache/cassandra/db/marshal/LongType.java b/src/java/org/apache/cassandra/db/marshal/LongType.java index 8a1528a572b0..d63229f314eb 100644 --- a/src/java/org/apache/cassandra/db/marshal/LongType.java +++ b/src/java/org/apache/cassandra/db/marshal/LongType.java @@ -18,6 +18,7 @@ package org.apache.cassandra.db.marshal; import java.nio.ByteBuffer; +import java.util.Objects; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.Constants; @@ -99,7 +100,7 @@ public Term fromJSONObject(Object parsed) throws MarshalException @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return getSerializer().deserialize(buffer).toString(); + return Objects.toString(getSerializer().deserialize(buffer), "\"\""); } @Override diff --git a/src/java/org/apache/cassandra/db/marshal/ShortType.java b/src/java/org/apache/cassandra/db/marshal/ShortType.java index 482fd810b7c8..22f134a3bd5d 100644 --- a/src/java/org/apache/cassandra/db/marshal/ShortType.java +++ b/src/java/org/apache/cassandra/db/marshal/ShortType.java @@ -18,6 +18,7 @@ package org.apache.cassandra.db.marshal; import java.nio.ByteBuffer; +import java.util.Objects; import org.apache.cassandra.cql3.CQL3Type; import org.apache.cassandra.cql3.Constants; @@ -77,7 +78,7 @@ public Term fromJSONObject(Object parsed) throws MarshalException @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return getSerializer().deserialize(buffer).toString(); + return Objects.toString(getSerializer().deserialize(buffer), "\"\""); } @Override diff --git a/src/java/org/apache/cassandra/db/marshal/TimestampType.java b/src/java/org/apache/cassandra/db/marshal/TimestampType.java index 45b08d97a7f1..a2d92ba23104 100644 --- a/src/java/org/apache/cassandra/db/marshal/TimestampType.java +++ b/src/java/org/apache/cassandra/db/marshal/TimestampType.java @@ -87,10 +87,15 @@ public Term fromJSONObject(Object parsed) throws MarshalException } } + private String toString(Date date) + { + return date != null ? TimestampSerializer.getJsonDateFormatter().format(date) : ""; + } + @Override public String toJSONString(ByteBuffer buffer, int protocolVersion) { - return '"' + TimestampSerializer.getJsonDateFormatter().format(TimestampSerializer.instance.deserialize(buffer)) + '"'; + return '"' + toString(TimestampSerializer.instance.deserialize(buffer)) + '"'; } @Override diff --git a/test/unit/org/apache/cassandra/cql3/EmptyValuesTest.java b/test/unit/org/apache/cassandra/cql3/EmptyValuesTest.java new file mode 100644 index 000000000000..6c42a592789d --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/EmptyValuesTest.java @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.cql3; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; +import java.util.regex.Pattern; + +import org.apache.commons.io.IOUtils; + +import org.junit.Assert; +import org.junit.Test; + +import com.datastax.driver.core.ResultSet; +import com.datastax.driver.core.Row; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.marshal.BytesType; +import org.apache.cassandra.db.marshal.DecimalType; +import org.apache.cassandra.db.marshal.DoubleType; +import org.apache.cassandra.db.marshal.FloatType; +import org.apache.cassandra.db.marshal.InetAddressType; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.marshal.LongType; +import org.apache.cassandra.db.marshal.ShortType; +import org.apache.cassandra.db.marshal.SimpleDateType; +import org.apache.cassandra.db.marshal.TimeType; +import org.apache.cassandra.db.marshal.TimeUUIDType; +import org.apache.cassandra.db.marshal.TimestampType; +import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.db.marshal.UUIDType; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.transport.Server; +import org.apache.cassandra.utils.ByteBufferUtil; + +import static org.junit.Assume.assumeTrue; + +public class EmptyValuesTest extends CQLTester +{ + private void verify(String emptyValue) throws Throwable + { + UntypedResultSet result = execute("SELECT * FROM %s"); + UntypedResultSet.Row row = result.one(); + Assert.assertTrue(row.getColumns().stream().anyMatch(c -> c.name.toString().equals("v"))); + Assert.assertEquals(0, row.getBytes("v").remaining()); + + ResultSet resultNet = executeNet(Server.CURRENT_VERSION, "SELECT * FROM %s"); + Row rowNet = resultNet.one(); + Assert.assertTrue(rowNet.getColumnDefinitions().contains("v")); + Assert.assertEquals(0, rowNet.getBytesUnsafe("v").remaining()); + + ResultSet jsonNet = executeNet(Server.CURRENT_VERSION, "SELECT JSON * FROM %s"); + Row jsonRowNet = jsonNet.one(); + Assert.assertTrue(jsonRowNet.getString("[json]"), jsonRowNet.getString("[json]").matches(".*\"v\"\\s*:\\s*\"" + Pattern.quote(emptyValue) + "\".*")); + + ColumnFamilyStore cfs = getCurrentColumnFamilyStore(); + ByteArrayOutputStream buf = new ByteArrayOutputStream(); + for (SSTableReader ssTable : cfs.getLiveSSTables()) + { + try (PrintStream out = new PrintStream(buf, true)) + { + ProcessBuilder pb = new ProcessBuilder("tools/bin/sstabledump", ssTable.getFilename()); + Process process = pb.start(); + process.waitFor(); + IOUtils.copy(process.getInputStream(), buf); + } + catch (Throwable t) + { + Assert.fail(t.getClass().getName()); + } + } + + String outString = new String(buf.toByteArray(), StandardCharsets.UTF_8); + Assert.assertTrue(outString, outString.contains("{ \"name\" : \"v\", \"value\" : \"" + emptyValue + "\" }")); + } + + private void verifyPlainInsert(String emptyValue) throws Throwable + { + execute("TRUNCATE %s"); + + // In most cases we cannot insert empty value when we do not bind variables + // This is due to the current implementation of org.apache.cassandra.cql3.Constants.Literal.testAssignment + // execute("INSERT INTO %s (id, v) VALUES (1, '" + emptyValue + "')"); + execute("INSERT INTO %s (id, v) VALUES (1, ?)", ByteBufferUtil.EMPTY_BYTE_BUFFER); + flush(); + + verify(emptyValue); + } + + private void verifyJsonInsert(String emptyValue) throws Throwable + { + execute("TRUNCATE %s"); + execute("INSERT INTO %s JSON '{\"id\":\"1\",\"v\":\"" + emptyValue + "\"}'"); + flush(); + + verify(emptyValue); + } + + @Test + public void testEmptyInt() throws Throwable + { + assumeTrue(Int32Type.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v INT)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyText() throws Throwable + { + assumeTrue(UTF8Type.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v TEXT)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyTimestamp() throws Throwable + { + assumeTrue(TimestampType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v TIMESTAMP)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyUUID() throws Throwable + { + assumeTrue(UUIDType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v UUID)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyInetAddress() throws Throwable + { + assumeTrue(InetAddressType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v INET)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyLong() throws Throwable + { + assumeTrue(LongType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v BIGINT)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyBytes() throws Throwable + { + assumeTrue(BytesType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v BLOB)"); + verifyJsonInsert("0x"); + verifyPlainInsert(""); + } + + @Test + public void testEmptyDate() throws Throwable + { + assumeTrue(SimpleDateType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v DATE)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyDecimal() throws Throwable + { + assumeTrue(DecimalType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v DECIMAL)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyDouble() throws Throwable + { + assumeTrue(DoubleType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v DOUBLE)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyFloat() throws Throwable + { + assumeTrue(FloatType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v FLOAT)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptySmallInt() throws Throwable + { + assumeTrue(ShortType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v SMALLINT)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyTime() throws Throwable + { + assumeTrue(TimeType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v TIME)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } + + @Test + public void testEmptyTimeUUID() throws Throwable + { + assumeTrue(TimeUUIDType.instance.isEmptyValueMeaningless()); + String table = createTable("CREATE TABLE %s (id INT PRIMARY KEY, v TIMEUUID)"); + verifyJsonInsert(""); + verifyPlainInsert(""); + } +} diff --git a/test/unit/org/apache/cassandra/db/marshal/UUIDTypeTest.java b/test/unit/org/apache/cassandra/db/marshal/UUIDTypeTest.java index 335860c6ffe5..74e5ef39f29a 100644 --- a/test/unit/org/apache/cassandra/db/marshal/UUIDTypeTest.java +++ b/test/unit/org/apache/cassandra/db/marshal/UUIDTypeTest.java @@ -31,6 +31,7 @@ import org.junit.Test; import junit.framework.Assert; +import org.apache.cassandra.transport.Server; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.UUIDGen; import org.slf4j.Logger; @@ -43,6 +44,13 @@ public class UUIDTypeTest UUIDType uuidType = new UUIDType(); + @Test //CASSANDRA-15896 + public void testToJsonEmptyValue() + { + String res = uuidType.toJSONString(uuidType.fromJSONObject("").bindAndGet(null), Server.CURRENT_VERSION); + assertEquals("\"\"", res); + } + @Test public void testRandomCompare() {