fix(griffin): validating column name in timestamp() clause. Simplified use of...

fix(griffin): validating column name in timestamp() clause. Simplified use of timestamp by minimizing static SQL checks in favour of dynamic checks. This would enable SQL functions come up with their own timestamp columns and SQL error checking still working.

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

@@ -442,16 +442,6 @@ public class SqlCodeGenerator {
                     final RecordMetadata masterMetadata = master.getMetadata();
                     final RecordMetadata slaveMetadata = slave.getMetadata();
 
-                    if (joinType == QueryModel.JOIN_ASOF || joinType == QueryModel.JOIN_SPLICE) {
-                        if (masterMetadata.getTimestampIndex() == -1) {
-                            throw SqlException.$(slaveModel.getJoinKeywordPosition(), "left side of time series join has no timestamp");
-                        }
-
-                        if (slaveMetadata.getTimestampIndex() == -1) {
-                            throw SqlException.$(slaveModel.getJoinKeywordPosition(), "right side of time series join has no timestamp");
-                        }
-                    }
-
                     switch (joinType) {
                         case QueryModel.JOIN_CROSS:
                             return new CrossJoinRecordCursorFactory(
@@ -461,6 +451,7 @@ public class SqlCodeGenerator {
                                     masterMetadata.getColumnCount()
                             );
                         case QueryModel.JOIN_ASOF:
+                            validateBothTimestamps(slaveModel, masterMetadata, slaveMetadata);
                             processJoinContext(index == 1, slaveModel.getContext(), masterMetadata, slaveMetadata);
                             if (slave.isRandomAccessCursor() && !fullFatJoins) {
                                 if (listColumnFilterA.size() > 0 && listColumnFilterB.size() > 0) {
@@ -504,6 +495,7 @@ public class SqlCodeGenerator {
                             masterAlias = null;
                             break;
                         case QueryModel.JOIN_SPLICE:
+                            validateBothTimestamps(slaveModel, masterMetadata, slaveMetadata);
                             processJoinContext(index == 1, slaveModel.getContext(), masterMetadata, slaveMetadata);
                             if (slave.isRandomAccessCursor() && master.isRandomAccessCursor() && !fullFatJoins) {
                                 master = createSpliceJoin(
@@ -573,6 +565,16 @@ public class SqlCodeGenerator {
         }
     }
 
+    private void validateBothTimestamps(QueryModel slaveModel, RecordMetadata masterMetadata, RecordMetadata slaveMetadata) throws SqlException {
+        if (masterMetadata.getTimestampIndex() == -1) {
+            throw SqlException.$(slaveModel.getJoinKeywordPosition(), "left side of time series join has no timestamp");
+        }
+
+        if (slaveMetadata.getTimestampIndex() == -1) {
+            throw SqlException.$(slaveModel.getJoinKeywordPosition(), "right side of time series join has no timestamp");
+        }
+    }
+
     @NotNull
     private RecordCursorFactory generateLatestByQuery(
             QueryModel model,
@@ -904,6 +906,10 @@ public class SqlCodeGenerator {
     @NotNull
     private RecordCursorFactory generateSampleBy(QueryModel model, SqlExecutionContext executionContext, ExpressionNode sampleByNode) throws SqlException {
         final RecordCursorFactory factory = generateSubQuery(model, executionContext);
+        // we require timestamp
+        if (factory.getMetadata().getTimestampIndex() == -1) {
+            throw SqlException.$(model.getSampleBy().position, "base query does not provide dedicated TIMESTAMP column");
+        }
         final ObjList<ExpressionNode> sampleByFill = model.getSampleByFill();
         final TimestampSampler timestampSampler = TimestampSamplerFactory.getInstance(sampleByNode.token, sampleByNode.position);
 
@@ -1061,7 +1067,10 @@ public class SqlCodeGenerator {
         if (timestamp == null) {
             timestampIndex = metadata.getTimestampIndex();
         } else {
-            timestampIndex = metadata.getColumnIndex(timestamp.token);
+            timestampIndex = metadata.getColumnIndexQuiet(timestamp.token);
+            if (timestampIndex == -1) {
+                throw SqlException.invalidColumn(timestamp.position, timestamp.token);
+            }
         }
         for (int i = 0; i < selectColumnCount; i++) {
             final QueryColumn queryColumn = model.getColumns().getQuick(i);

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

@@ -2240,11 +2240,6 @@ class SqlOptimiser {
         // sample by clause should be promoted to all of the models as well as validated
         final ExpressionNode sampleBy = baseModel.getSampleBy();
         if (sampleBy != null) {
-            final ExpressionNode timestamp = baseModel.getTimestamp();
-            if (timestamp == null) {
-                throw SqlException.$(sampleBy.position, "TIMESTAMP column is not defined");
-            }
-
             // move sample by to group by model
             groupByModel.moveSampleByFrom(baseModel);
         }

core/src/main/java/io/questdb/griffin/model/QueryModel.java

@@ -276,11 +276,6 @@ public class QueryModel implements Mutable, ExecutionModel, AliasTranslator, Sin
             final CharSequence name = columnNames.getQuick(i);
             this.aliasToColumnMap.put(name, name);
         }
-        // do not copy timestamp from nested model to join models
-        // join type will determine timestamp
-        if (this.timestamp == null && other.timestamp != null && joinModels.size() < 2) {
-            this.timestamp = other.timestamp;
-        }
     }
 
     public void clearColumnMapStructs() {

core/src/test/java/io/questdb/cairo/TableModel.java

@@ -155,9 +155,13 @@ public class TableModel implements TableStructure, Closeable {
     }
 
     public TableModel timestamp() {
+        return timestamp("timestamp");
+    }
+
+    public TableModel timestamp(CharSequence name) {
         assert timestampIndex == -1;
         timestampIndex = columnNames.size();
-        col("timestamp", ColumnType.TIMESTAMP);
+        col(name, ColumnType.TIMESTAMP);
         return this;
     }
 }

core/src/test/java/io/questdb/griffin/SqlParserTest.java

@@ -89,6 +89,16 @@ public class SqlParserTest extends AbstractGriffinTest {
         );
     }
 
+    @Test
+    public void testSelectWildcardAndNotTimestamp() {
+        assertSyntaxError(
+                "select * from (select x from tab1) timestamp(y)",
+                45,
+                "Invalid column",
+                modelOf("tab1").col("x", ColumnType.INT).col("y", ColumnType.TIMESTAMP)
+        );
+    }
+
     @Test
     public void testAsOfJoin() throws SqlException {
         assertQuery(
@@ -3687,7 +3697,7 @@ public class SqlParserTest extends AbstractGriffinTest {
     @Test
     public void testOrderByWithSampleBy2() throws SqlException {
         assertQuery(
-                "select-group-by a, sum(b) sum from ((select-group-by a, sum(b) b from ((select-choose a, b, t from (tab) order by t) _xQdbA3) timestamp (t) sample by 10m) _xQdbA1) timestamp (t) order by a",
+                "select-group-by a, sum(b) sum from ((select-group-by a, sum(b) b from ((select-choose a, b, t from (tab) order by t) _xQdbA3) timestamp (t) sample by 10m) _xQdbA1) order by a",
                 "select a, sum(b) from (select a,sum(b) b from (tab order by t) timestamp(t) sample by 10m order by t) order by a",
                 modelOf("tab")
                         .col("a", ColumnType.INT)
@@ -3696,6 +3706,18 @@ public class SqlParserTest extends AbstractGriffinTest {
         );
     }
 
+    @Test
+    public void testOrderByWithSampleBy3() throws SqlException {
+        assertQuery(
+                "select-group-by a, sum(b) sum from ((select-group-by a, sum(b) b from ((select-choose a, b, t from (tab timestamp (t)) order by t) _xQdbA3) sample by 10m) _xQdbA1) order by a",
+                "select a, sum(b) from (select a,sum(b) b from (tab order by t) sample by 10m order by t) order by a",
+                modelOf("tab")
+                        .col("a", ColumnType.INT)
+                        .col("b", ColumnType.INT)
+                        .timestamp("t")
+        );
+    }
+
     @Test
     public void testOuterJoin() throws Exception {
         assertQuery(
@@ -4036,7 +4058,7 @@ public class SqlParserTest extends AbstractGriffinTest {
     public void testSampleByUndefinedTimestamp() {
         assertSyntaxError("select x,sum(y) from tab sample by 2m",
                 35,
-                "TIMESTAMP column is not defined",
+                "base query does not provide dedicated TIMESTAMP column",
                 modelOf("tab")
                         .col("x", ColumnType.INT)
                         .col("y", ColumnType.INT)