fix(sql): treat zero char as empty string literal (#1550)

core/src/main/java/io/questdb/griffin/FunctionParser.java

@@ -419,7 +419,7 @@ public class FunctionParser implements PostOrderTreeTraversalAlgo.Visitor {
 
             if (len == 2) {
                 // empty
-                return CharConstant.ZERO;
+                return StrConstant.EMPTY;
             }
             return new StrConstant(tok);
         }

core/src/main/java/io/questdb/griffin/engine/functions/bool/InCharFunctionFactory.java

@@ -34,6 +34,7 @@ import io.questdb.griffin.SqlExecutionContext;
 import io.questdb.griffin.engine.functions.BooleanFunction;
 import io.questdb.griffin.engine.functions.UnaryFunction;
 import io.questdb.griffin.engine.functions.constants.BooleanConstant;
+import io.questdb.griffin.engine.functions.constants.CharConstant;
 import io.questdb.std.IntHashSet;
 import io.questdb.std.IntList;
 import io.questdb.std.ObjList;
@@ -64,6 +65,12 @@ public class InCharFunctionFactory implements FunctionFactory {
             Function func = args.getQuick(i);
             if (ColumnType.isChar(func.getType())) {
                 set.add(func.getChar(null));
+            } else if (ColumnType.isString(func.getType())) {
+                // Implicitly cast empty string literal ('') to zero char
+                if (func.getStrLen(null) != 0) {
+                    throw SqlException.$(argPositions.getQuick(i), "CHAR constant expected");
+                }
+                set.add(CharConstant.ZERO.getChar(null));
             } else {
                 throw SqlException.$(argPositions.getQuick(i), "CHAR constant expected");
             }

core/src/main/java/io/questdb/griffin/engine/functions/eq/NullIfCharCharFunctionFactory.java → core/src/main/java/io/questdb/griffin/engine/functions/conditional/NullIfCharFunctionFactory.java

@@ -22,7 +22,7 @@
  *
  ******************************************************************************/
 
-package io.questdb.griffin.engine.functions.eq;
+package io.questdb.griffin.engine.functions.conditional;
 
 import io.questdb.cairo.CairoConfiguration;
 import io.questdb.cairo.sql.Function;
@@ -34,7 +34,7 @@ import io.questdb.griffin.engine.functions.CharFunction;
 import io.questdb.std.IntList;
 import io.questdb.std.ObjList;
 
-public class NullIfCharCharFunctionFactory implements FunctionFactory {
+public class NullIfCharFunctionFactory implements FunctionFactory {
     @Override
     public String getSignature() {
         return "nullif(AA)";
@@ -42,7 +42,6 @@ public class NullIfCharCharFunctionFactory implements FunctionFactory {
 
     @Override
     public Function newInstance(int position, ObjList<Function> args, IntList argPositions, CairoConfiguration configuration, SqlExecutionContext sqlExecutionContext) {
-
         Function chrFunc1 = args.getQuick(0);
         Function chrFunc2 = args.getQuick(1);
 

core/src/main/java/io/questdb/griffin/engine/functions/catalogue/NullIfIFunctionFactory.java → core/src/main/java/io/questdb/griffin/engine/functions/conditional/NullIfIFunctionFactory.java

@@ -22,7 +22,7 @@
  *
  ******************************************************************************/
 
-package io.questdb.griffin.engine.functions.catalogue;
+package io.questdb.griffin.engine.functions.conditional;
 
 import io.questdb.cairo.CairoConfiguration;
 import io.questdb.cairo.sql.Function;
@@ -70,7 +70,7 @@ public class NullIfIFunctionFactory implements FunctionFactory {
         @Override
         public int getInt(Record rec) {
             final int val = value.getInt(rec);
-            return val != Numbers.INT_NaN ? val : replacement;
+            return val == replacement ? Numbers.INT_NaN : val;
         }
     }
 }

core/src/test/java/io/questdb/griffin/engine/functions/str/LengthSymbolVFunctionFactoryTest.java → core/src/main/java/io/questdb/griffin/engine/functions/conditional/NullIfStrFunctionFactory.java

@@ -22,33 +22,81 @@
  *
  ******************************************************************************/
 
-package io.questdb.griffin.engine.functions.str;
+package io.questdb.griffin.engine.functions.conditional;
 
+import io.questdb.cairo.CairoConfiguration;
+import io.questdb.cairo.sql.Function;
+import io.questdb.cairo.sql.Record;
 import io.questdb.griffin.FunctionFactory;
-import io.questdb.griffin.SqlException;
-import io.questdb.griffin.engine.AbstractFunctionFactoryTest;
-import org.junit.Test;
+import io.questdb.griffin.SqlExecutionContext;
+import io.questdb.griffin.engine.functions.BinaryFunction;
+import io.questdb.griffin.engine.functions.StrFunction;
+import io.questdb.std.Chars;
+import io.questdb.std.IntList;
+import io.questdb.std.ObjList;
 
-public class LengthSymbolVFunctionFactoryTest extends AbstractFunctionFactoryTest {
-
-    @Test
-    public void testEmpty() throws SqlException {
-        call("").andAssert(0);
+public class NullIfStrFunctionFactory implements FunctionFactory {
+    @Override
+    public String getSignature() {
+        return "nullif(SS)";
     }
 
-    @Test
-    public void testNull() throws SqlException {
-        call((Object) null).andAssert(-1);
+    @Override
+    public Function newInstance(
+            int position,
+            ObjList<Function> args,
+            IntList argPositions,
+            CairoConfiguration configuration,
+            SqlExecutionContext sqlExecutionContext
+    ) {
+        Function strFunc1 = args.getQuick(0);
+        Function strFunc2 = args.getQuick(1);
+        return new Func(strFunc1, strFunc2);
     }
 
-    @Test
-    public void testSimple() throws SqlException {
-        call("xyz").andAssert(3);
-    }
+    private static class Func extends StrFunction implements BinaryFunction {
+        private final Function strFunc1;
+        private final Function strFunc2;
 
-    @Override
-    protected FunctionFactory getFunctionFactory() {
-        return new LengthSymbolFunctionFactory();
-    }
+        public Func(Function strFunc1, Function strFunc2) {
+            this.strFunc1 = strFunc1;
+            this.strFunc2 = strFunc2;
+        }
+
+        @Override
+        public Function getLeft() {
+            return strFunc1;
+        }
+
+        @Override
+        public Function getRight() {
+            return strFunc2;
+        }
 
-}
+        @Override
\ No newline at end of file
+        public CharSequence getStr(Record rec) {
+            CharSequence cs1 = strFunc1.getStr(rec);
+            if (cs1 == null) {
+                return null;
+            }
+            CharSequence cs2 = strFunc2.getStr(rec);
+            if (cs2 == null) {
+                return null;
+            }
+            return Chars.equals(cs1, cs2) ? null : cs1;
+        }
+
+        @Override
+        public CharSequence getStrB(Record rec) {
+            CharSequence cs1 = strFunc1.getStrB(rec);
+            if (cs1 == null) {
+                return null;
+            }
+            CharSequence cs2 = strFunc2.getStrB(rec);
+            if (cs2 == null) {
+                return null;
+            }
+            return Chars.equals(cs1, cs2) ? null : cs1;
+        }
+    }
+}

core/src/main/java/io/questdb/griffin/engine/functions/constants/StrConstant.java

@@ -31,6 +31,7 @@ import io.questdb.std.Chars;
 
 public class StrConstant extends StrFunction implements ConstantFunction {
     public static final StrConstant NULL = new StrConstant(null);
+    public static final StrConstant EMPTY = new StrConstant("");
 
     private final String value;
     private final int length;

core/src/main/java/module-info.java

@@ -126,7 +126,9 @@ open module io.questdb {
             io.questdb.griffin.engine.functions.eq.EqStrGeoHashFunctionFactory,
 
             //nullif
-            io.questdb.griffin.engine.functions.eq.NullIfCharCharFunctionFactory,
+            io.questdb.griffin.engine.functions.conditional.NullIfCharFunctionFactory,
+            io.questdb.griffin.engine.functions.conditional.NullIfIFunctionFactory,
+            io.questdb.griffin.engine.functions.conditional.NullIfStrFunctionFactory,
 
 //                   '<' operator
             io.questdb.griffin.engine.functions.lt.LtDoubleVVFunctionFactory,
@@ -542,7 +544,6 @@ open module io.questdb {
             io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetPartKeyDefFunctionFactory,
             io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetSITExprFunctionFactory,
             io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetSIExprFunctionFactory,
-            io.questdb.griffin.engine.functions.catalogue.NullIfIFunctionFactory,
             io.questdb.griffin.engine.functions.catalogue.FormatTypeFunctionFactory,
             io.questdb.griffin.engine.functions.catalogue.ProcCatalogueFunctionFactory,
             io.questdb.griffin.engine.functions.catalogue.RangeCatalogueFunctionFactory,

core/src/main/resources/META-INF/services/io.questdb.griffin.FunctionFactory

@@ -58,8 +58,10 @@ io.questdb.griffin.engine.functions.eq.EqGeoHashGeoHashFunctionFactory
 io.questdb.griffin.engine.functions.eq.EqGeoHashStrFunctionFactory
 io.questdb.griffin.engine.functions.eq.EqStrGeoHashFunctionFactory
 
-#nullif
-io.questdb.griffin.engine.functions.eq.NullIfCharCharFunctionFactory
+# nullif
+io.questdb.griffin.engine.functions.conditional.NullIfCharFunctionFactory
+io.questdb.griffin.engine.functions.conditional.NullIfStrFunctionFactory
+io.questdb.griffin.engine.functions.conditional.NullIfIFunctionFactory
 
 # '<' operator
 io.questdb.griffin.engine.functions.lt.LtDoubleVVFunctionFactory
@@ -520,7 +522,6 @@ io.questdb.griffin.engine.functions.catalogue.CurrentSchemaFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetPartKeyDefFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetSIExprFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.PrefixedPgGetSITExprFunctionFactory
-io.questdb.griffin.engine.functions.catalogue.NullIfIFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.FormatTypeFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.ProcCatalogueFunctionFactory
 io.questdb.griffin.engine.functions.catalogue.RangeCatalogueFunctionFactory

core/src/test/java/io/questdb/cutlass/pgwire/PGJobContextTest.java

@@ -3062,10 +3062,10 @@ nodejs code:
                         "qdb", null, null, null
                 )) {
                     assertResultSet(
-                            "TABLE_CAT[VARCHAR],TABLE_SCHEM[VARCHAR],TABLE_NAME[VARCHAR],TABLE_TYPE[VARCHAR],REMARKS[VARCHAR],TYPE_CAT[CHAR],TYPE_SCHEM[CHAR],TYPE_NAME[CHAR],SELF_REFERENCING_COL_NAME[CHAR],REF_GENERATION[CHAR]\n" +
-                                    "pg_catalog,pg_catalog,pg_class,SYSTEM TABLE,null,null,null,null,null,null\n" +
-                                    "public,public,test,TABLE,null,null,null,null,null,null\n" +
-                                    "public,public,test2,TABLE,null,null,null,null,null,null\n",
+                            "TABLE_CAT[VARCHAR],TABLE_SCHEM[VARCHAR],TABLE_NAME[VARCHAR],TABLE_TYPE[VARCHAR],REMARKS[VARCHAR],TYPE_CAT[VARCHAR],TYPE_SCHEM[VARCHAR],TYPE_NAME[VARCHAR],SELF_REFERENCING_COL_NAME[VARCHAR],REF_GENERATION[VARCHAR]\n" +
+                                    "pg_catalog,pg_catalog,pg_class,SYSTEM TABLE,null,,,,,\n" +
+                                    "public,public,test,TABLE,null,,,,,\n" +
+                                    "public,public,test2,TABLE,null,,,,,\n",
                             sink,
                             rs
                     );

core/src/test/java/io/questdb/griffin/engine/functions/cast/CastTest.java

@@ -1385,31 +1385,6 @@ public class CastTest extends AbstractGriffinTest {
         );
     }
 
-    @Test
-    public void testCharToStrConstZero() throws Exception {
-        assertQuery(
-                "a\n",
-                "select a from tab",
-                "create table tab (a string)",
-                null,
-                "insert into tab select cast('' as string) from long_sequence(10)",
-                "a\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n" +
-                        "\n",
-                true,
-                true,
-                true
-        );
-    }
-
     @Test
     public void testCharToStrSort() throws Exception {
         assertQuery(
@@ -4574,6 +4549,31 @@ public class CastTest extends AbstractGriffinTest {
         );
     }
 
+    @Test
+    public void testStrConstZeroToChar() throws Exception {
+        assertQuery(
+                "a\n",
+                "select a from tab",
+                "create table tab (a char)",
+                null,
+                "insert into tab select cast('' as char) from long_sequence(10)",
+                "a\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n" +
+                        "\n",
+                true,
+                true,
+                true
+        );
+    }
+
     @Test
     public void testStrToDate() throws Exception {
         assertQuery(

core/src/test/java/io/questdb/griffin/engine/functions/conditional/NullIfFunctionFactoryTest.java

+/*******************************************************************************
+ *     ___                  _   ____  ____
+ *    / _ \ _   _  ___  ___| |_|  _ \| __ )
+ *   | | | | | | |/ _ \/ __| __| | | |  _ \
+ *   | |_| | |_| |  __/\__ \ |_| |_| | |_) |
+ *    \__\_\\__,_|\___||___/\__|____/|____/
+ *
+ *  Copyright (c) 2014-2019 Appsicle
+ *  Copyright (c) 2019-2022 QuestDB
+ *
+ *  Licensed 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 io.questdb.griffin.engine.functions.conditional;
+
+import io.questdb.griffin.AbstractGriffinTest;
+import org.junit.Test;
+
+public class NullIfFunctionFactoryTest extends AbstractGriffinTest {
+
+    @Test
+    public void testStrSimple() throws Exception {
+        assertQuery(
+                "str1\tstr2\tnullif\n" +
+                        "cat\tcat\t\n" +
+                        "dog\t\t\n" +
+                        "\t\t\n" +
+                        "\tdog\t\n" +
+                        "cat\tdog\tcat\n" +
+                        "dog\t\t\n" +
+                        "dog\tdog\t\n" +
+                        "dog\tcat\tdog\n" +
+                        "cat\tdog\tcat\n" +
+                        "cat\tdog\tcat\n",
+                "select str1,str2,nullif(str1,str2) from x",
+                "create table x as (" +
+                        "select rnd_str('cat','dog',NULL) as str1\n" +
+                        ", rnd_str('cat','dog',NULL) as str2\n" +
+                        "from long_sequence(10)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
+    }
+
+    @Test
+    public void testCharSimple() throws Exception {
+        assertQuery(
+                "ch1\tch2\tnullif\n" +
+                        "V\tT\tV\n" +
+                        "J\tW\tJ\n" +
+                        "C\tP\tC\n" +
+                        "S\tW\tS\n" +
+                        "H\tY\tH\n" +
+                        "R\tX\tR\n" +
+                        "P\tE\tP\n" +
+                        "H\tN\tH\n" +
+                        "R\tX\tR\n" +
+                        "G\tZ\tG\n" +
+                        "S\tX\tS\n" +
+                        "U\tX\tU\n" +
+                        "I\tB\tI\n" +
+                        "B\tT\tB\n" +
+                        "G\tP\tG\n" +
+                        "G\tW\tG\n" +
+                        "F\tF\t\n" +
+                        "Y\tU\tY\n" +
+                        "D\tE\tD\n" +
+                        "Y\tY\t\n",
+                "select ch1,ch2,nullif(ch1,ch2) from x",
+                "create table x as (" +
+                        "select rnd_char() as ch1\n" +
+                        ", rnd_char() as ch2\n" +
+                        "from long_sequence(20)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
+    }
+
+    @Test
+    public void testIntSimple() throws Exception {
+        assertQuery(
+                "int\tnullif\n" +
+                        "4\t4\n" +
+                        "2\t2\n" +
+                        "5\tNaN\n" +
+                        "2\t2\n" +
+                        "4\t4\n",
+                "select int,nullif(int,5) from x",
+                "create table x as (" +
+                        "select rnd_int(1,5,0) as int\n" +
+                        "from long_sequence(5)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
+    }
+
+    @Test
+    public void testIntConstNull() throws Exception {
+        assertQuery(
+                "nullif1\tnullif2\n" +
+                        "NaN\t5\n",
+                "select nullif(null,5) nullif1, nullif(5,null) nullif2",
+                null,
+                null,
+                true,
+                false,
+                true
+        );
+    }
+
+}

core/src/test/java/io/questdb/griffin/engine/functions/eq/NullIfCharCharFunctionFactoryTest.java → core/src/test/java/io/questdb/griffin/engine/functions/str/LengthFunctionFactoryTest.java

@@ -22,27 +22,84 @@
  *
  ******************************************************************************/
 
-package io.questdb.griffin.engine.functions.eq;
+package io.questdb.griffin.engine.functions.str;
 
-import io.questdb.griffin.FunctionFactory;
-import io.questdb.griffin.SqlException;
-import io.questdb.griffin.engine.AbstractFunctionFactoryTest;
+import io.questdb.griffin.AbstractGriffinTest;
 import org.junit.Test;
 
-public class NullIfCharCharFunctionFactoryTest extends AbstractFunctionFactoryTest {
+public class LengthFunctionFactoryTest extends AbstractGriffinTest {
 
     @Test
-    public void testNullIfWhenDifferent() throws SqlException {
-        call('A', 'B').andAssert('A');
+    public void testStrSimple() throws Exception {
+        assertQuery(
+                "str\tlength\n" +
+                        "abc\t3\n" +
+                        "\t0\n" +
+                        "x\t1\n" +
+                        "\t-1\n" +
+                        "x\t1\n",
+                "select str,length(str) from x",
+                "create table x as (" +
+                        "select rnd_str('abc','x','',NULL) as str\n" +
+                        "from long_sequence(5)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
     }
 
     @Test
-    public void testNullIfWhenEquals() throws SqlException {
-        call('A', 'A').andAssert(Character.MIN_VALUE);
+    public void testSymbolSimple() throws Exception {
+        assertQuery(
+                "sym\tlength\n" +
+                        "WC\t2\n" +
+                        "\t-1\n" +
+                        "EH\t2\n" +
+                        "\t-1\n" +
+                        "EH\t2\n" +
+                        "SWH\t3\n" +
+                        "T\t1\n" +
+                        "T\t1\n" +
+                        "T\t1\n" +
+                        "\t-1\n",
+                "select sym,length(sym) from x",
+                "create table x as (" +
+                        "select rnd_symbol(5,1,3,5) as sym\n" +
+                        "from long_sequence(10)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
     }
 
-    @Override
-    protected FunctionFactory getFunctionFactory() {
-        return new NullIfCharCharFunctionFactory();
+    @Test
+    public void testBinSimple() throws Exception {
+        assertQuery(
+                "bin\tlength\n" +
+                        "00000000 41 1d\t2\n" +
+                        "00000000 8a 17 fa d8\t4\n" +
+                        "00000000 ce f1\t2\n" +
+                        "\t-1\n" +
+                        "00000000 91\t1\n" +
+                        "00000000 db f3 04 1b\t4\n" +
+                        "00000000 de a0\t2\n" +
+                        "\t-1\n" +
+                        "00000000 15 68\t2\n" +
+                        "00000000 af 19 c4 95\t4\n",
+                "select bin,length(bin) from x",
+                "create table x as (" +
+                        "select rnd_bin(1,5,5) as bin\n" +
+                        "from long_sequence(10)" +
+                        ")",
+                null,
+                true,
+                false,
+                true
+        );
     }
-}
+
\ No newline at end of file
+}

core/src/test/java/io/questdb/griffin/engine/functions/str/StrPosFunctionFactoryTest.java

@@ -30,7 +30,7 @@ import org.junit.Test;
 public class StrPosFunctionFactoryTest extends AbstractGriffinTest {
 
     @Test
-    public void testVarStr() throws Exception {
+    public void testStrVar() throws Exception {
         assertQuery(
                 "substr\tstr\tstrpos\n" +
                         "XYZ\tABC XYZ XYZ\t5\n" +
@@ -62,7 +62,7 @@ public class StrPosFunctionFactoryTest extends AbstractGriffinTest {
     }
 
     @Test
-    public void testVarStrConstSubstr() throws Exception {
+    public void testStrVarConst() throws Exception {
         assertQuery(
                 "str\tstrpos\n" +
                         "ABC XYZ XYZ\t5\n" +
@@ -83,7 +83,7 @@ public class StrPosFunctionFactoryTest extends AbstractGriffinTest {
     }
 
     @Test
-    public void testVarChar() throws Exception {
+    public void testCharVar() throws Exception {
         assertQuery(
                 "substr\tstr\tstrpos\n" +
                         "T\tTEST\t1\n" +
@@ -115,7 +115,7 @@ public class StrPosFunctionFactoryTest extends AbstractGriffinTest {
     }
 
     @Test
-    public void testVarCharConstSubstr() throws Exception {
+    public void testCharVarConst() throws Exception {
         assertQuery(
                 "str\tstrpos\n" +
                         "ABC XYZ XYZ\t3\n" +
@@ -153,8 +153,7 @@ public class StrPosFunctionFactoryTest extends AbstractGriffinTest {
     public void testConstantEmptyString() throws Exception {
         assertQuery(
                 "pos1\tpos2\tpos3\n" +
-                        // TODO(puzpuzpuz): fix the empty string literal, so that the result is "0\t1\t1\n"
-                        "NaN\tNaN\tNaN\n",
+                        "0\t1\t1\n",
                 "select strpos('','a') pos1, strpos('a',cast('' as string)) pos2, strpos('','') pos3",
                 null,
                 null,