From a31af1da503aa75723884a4223b901d15e630680 Mon Sep 17 00:00:00 2001 From: Bolek Ziobrowski <26925920+bziobrowski@users.noreply.github.com> Date: Wed, 23 Mar 2022 15:27:54 +0100 Subject: [PATCH] fix(sql): limit implicit string, symbol -> timestamp cast to constants to fix expressions like symbol = int (#1977) --- .../io/questdb/griffin/FunctionParser.java | 8 +- .../questdb/griffin/AbstractGriffinTest.java | 2 +- .../griffin/ImplicitToTimestampCastTest.java | 99 +++++++++++++++++++ .../questdb/griffin/TimestampQueryTest.java | 8 +- 4 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 core/src/test/java/io/questdb/griffin/ImplicitToTimestampCastTest.java diff --git a/core/src/main/java/io/questdb/griffin/FunctionParser.java b/core/src/main/java/io/questdb/griffin/FunctionParser.java index cd1a26444..ba0cedcbd 100644 --- a/core/src/main/java/io/questdb/griffin/FunctionParser.java +++ b/core/src/main/java/io/questdb/griffin/FunctionParser.java @@ -628,16 +628,16 @@ public class FunctionParser implements PostOrderTreeTraversalAlgo.Visitor, Mutab overloadPossible |= argTypeTag == ColumnType.CHAR && sigArgTypeTag == ColumnType.STRING; - // Implicit cast from STRING to TIMESTAMP - overloadPossible |= argTypeTag == ColumnType.STRING && + // Implicit cast from STRING to TIMESTAMP + overloadPossible |= argTypeTag == ColumnType.STRING && arg.isConstant() && sigArgTypeTag == ColumnType.TIMESTAMP && !factory.isGroupBy(); // Implicit cast from STRING to GEOHASH overloadPossible |= argTypeTag == ColumnType.STRING && sigArgTypeTag == ColumnType.GEOHASH && !factory.isGroupBy(); - // Implicit cast from SYMBOL to TIMESTAMP - overloadPossible |= argTypeTag == ColumnType.SYMBOL && + // Implicit cast from SYMBOL to TIMESTAMP + overloadPossible |= argTypeTag == ColumnType.SYMBOL && arg.isConstant() && sigArgTypeTag == ColumnType.TIMESTAMP && !factory.isGroupBy(); overloadPossible |= arg.isUndefined(); diff --git a/core/src/test/java/io/questdb/griffin/AbstractGriffinTest.java b/core/src/test/java/io/questdb/griffin/AbstractGriffinTest.java index e029b65e9..c5c6fe7cb 100644 --- a/core/src/test/java/io/questdb/griffin/AbstractGriffinTest.java +++ b/core/src/test/java/io/questdb/griffin/AbstractGriffinTest.java @@ -870,7 +870,7 @@ public class AbstractGriffinTest extends AbstractCairoTest { int index = factory.getMetadata().getColumnIndexQuiet(expectedTimestamp); Assert.assertTrue("Column " + expectedTimestamp + " can't be found in metadata", index > -1); Assert.assertNotEquals("Expected non-negative value as timestamp index", -1, index); - Assert.assertEquals(index, factory.getMetadata().getTimestampIndex()); + Assert.assertEquals("Timestamp column index", index, factory.getMetadata().getTimestampIndex()); assertTimestampColumnValues(factory, sqlExecutionContext, expectAscendingOrder); } } diff --git a/core/src/test/java/io/questdb/griffin/ImplicitToTimestampCastTest.java b/core/src/test/java/io/questdb/griffin/ImplicitToTimestampCastTest.java new file mode 100644 index 000000000..7add7398d --- /dev/null +++ b/core/src/test/java/io/questdb/griffin/ImplicitToTimestampCastTest.java @@ -0,0 +1,99 @@ +package io.questdb.griffin; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Checks that implicit string/symbol -> timestamp conversion works only for literals. + */ +public class ImplicitToTimestampCastTest extends AbstractGriffinTest { + + @Test + public void testImplicitIntegerToSymbolConversionFails() throws Exception { + try { + assertQuery("", + "select * from balances where cust_id = 1", + "CREATE TABLE balances ( " + + " cust_id SYMBOL, " + + " ts TIMESTAMP " + + ") TIMESTAMP(ts) PARTITION BY DAY;", + "k", false, true, true + ); + Assert.fail("error should be thrown"); + } catch (SqlException e) { + Assert.assertEquals(e.getMessage(), "[37] unexpected argument for function: =. expected args: (STRING,STRING). actual args: (SYMBOL,INT constant)"); + } + } + + @Test + public void testImplicitStringLiteralToTimestampConversionWorks() throws Exception { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = '2022-03-23'", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", null, true, true, false); + } + + @Test + public void testImplicitSymbolLiteralToTimestampConversionWorks() throws Exception { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = cast('2022-03-23' as symbol)", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", null, true, true, false); + } + + @Test + public void testImplicitStringConstExpressionToTimestampCastWorks() throws Exception { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = '2022-03-23' || ' 00:00'", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", null, true, true, false); + } + + @Test + public void testImplicitSymbolConstExpressionToTimestampCastWorks() throws Exception { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = cast(('2022-03-23' || ' 00:00') as symbol)", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", null, true, true, false); + } + + @Test + public void testImplicitNonConstStringExpressionToTimestampConversionFails() throws Exception { + try { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = rnd_str('2022-03-23')", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", null, true, true, false); + Assert.fail("Exception should be thrown"); + } catch (SqlException e) { + Assert.assertEquals(e.getMessage(), "[32] unexpected argument for function: =. expected args: (STRING,STRING). actual args: (TIMESTAMP,STRING)"); + } + } + + @Test + public void testImplicitNonConstSymbolExpressionToTimestampConversionFails() throws Exception { + try { + assertQuery("cust_id\tts\n" + + "abc\t2022-03-23T00:00:00.000000Z\n", + "select * from balances where ts = rnd_symbol('2022-03-23')", + "CREATE TABLE balances as (" + + "select cast('abc' as symbol) as cust_id, cast('2022-03-23' as timestamp) as ts from long_sequence(1) " + + ");", + null, true, true, false); + Assert.fail("Exception should be thrown"); + } catch (SqlException e) { + Assert.assertEquals(e.getMessage(), "[32] unexpected argument for function: =. expected args: (STRING,STRING). actual args: (TIMESTAMP,SYMBOL)"); + } + } + +} diff --git a/core/src/test/java/io/questdb/griffin/TimestampQueryTest.java b/core/src/test/java/io/questdb/griffin/TimestampQueryTest.java index 443b3f7c8..f616b1b9f 100644 --- a/core/src/test/java/io/questdb/griffin/TimestampQueryTest.java +++ b/core/src/test/java/io/questdb/griffin/TimestampQueryTest.java @@ -1162,7 +1162,7 @@ public class TimestampQueryTest extends AbstractGriffinTest { // NOT Between runtime const evaluating to invalid string expected = "min\tmax\n" + "2020-01-01T00:00:00.000000Z\t2020-01-02T23:00:00.000000Z\n"; - assertTimestampTtQuery(expected, "select min(nts), max(nts) from tt where nts not between to_str(now(), 'yyyy-MM-dd') || '-222' and now()"); + assertTimestampTtQuery(expected, "select min(nts), max(nts) from tt where nts not between cast((to_str(now(), 'yyyy-MM-dd') || '-222') as timestamp) and now()"); // Between columns expected = "min\tmax\n" + @@ -1187,7 +1187,7 @@ public class TimestampQueryTest extends AbstractGriffinTest { // between constants expected = "min\tmax\n" + "2020-01-01T00:00:00.000000Z\t2020-01-02T00:00:00.000000Z\n"; - assertTimestampTtQuery(expected, "select min(nts), max(nts) from tt where nts = to_str(nts,'yyyy-MM-dd')"); + assertTimestampTtQuery(expected, "select min(nts), max(nts) from tt where nts = cast( to_str(nts,'yyyy-MM-dd') as timestamp)"); }); } @@ -1273,7 +1273,7 @@ public class TimestampQueryTest extends AbstractGriffinTest { "a\tk\n" + "1970-01-01T00:00:00.040000Z\t1970-01-01T00:00:00.030000Z\n" + "1970-01-01T00:00:00.050000Z\t1970-01-01T00:00:00.040000Z\n", - "select a, k from x where k < a", + "select a, k from x where k < cast(a as timestamp)", "create table x as (select cast(concat('1970-01-01T00:00:00.0', (case when x > 3 then x else x - 1 end), '0000Z') as symbol) a, timestamp_sequence(0, 10000) k from long_sequence(5)) timestamp(k)", "k", null, @@ -1290,7 +1290,7 @@ public class TimestampQueryTest extends AbstractGriffinTest { "a\tdk\tk\n" + "1970-01-01T00:00:00.040000Z\t1970-01-01T00:00:00.030000Z\t1970-01-01T00:00:00.030000Z\n" + "1970-01-01T00:00:00.050000Z\t1970-01-01T00:00:00.040000Z\t1970-01-01T00:00:00.040000Z\n", - "select a, dk, k from x where dk < a", + "select a, dk, k from x where dk < cast(a as timestamp)", "create table x as (select cast(concat('1970-01-01T00:00:00.0', (case when x > 3 then x else x - 1 end), '0000Z') as symbol) a, timestamp_sequence(0, 10000) dk, timestamp_sequence(0, 10000) k from long_sequence(5)) timestamp(k)", "k", null, -- GitLab