Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

components/webui/client/src/sql-parser/index.ts (4)

50-75: Fix timestamp key detection: normalise case and strip quotes to match UpperCaseCharStream.

UpperCaseCharStream upper-cases everything (including quoted/back‑quoted identifiers), so direct string comparisons miss matches. Normalise once and compare upper‑cased text.

Apply this diff:

 class TimestampKeyChecker extends SqlBaseVisitor<void> {
     hasTimestamp: boolean;

     constructor (timestampKey: string) {
         super();
         this.hasTimestamp = false;

-        // Three possible terminal nodes of the `identifer` context
-        const timestampIdentifiers = new Set([
-            // unquotedIdentifier
-            timestampKey,
-
-            // quotedIdentifier
-            `"${timestampKey}"`,
-
-            // backQuotedIdentifier
-            `\`${timestampKey}\``,
-        ]);
+        // Normalise: strip one layer of quotes/backquotes and upper-case
+        const base = timestampKey.replace(/^["`]|["`]$/g, "");
+        const TK = base.toUpperCase();
+        const timestampIdentifiers = new Set<string>([TK, `"${TK}"`, `\`${TK}\``]);

-        this.visitColumnReference = (ctx: ColumnReferenceContext) => {
-            if (timestampIdentifiers.has(ctx.identifier().getText())) {
+        this.visitColumnReference = (ctx: ColumnReferenceContext) => {
+            const idText = ctx.identifier().getText().toUpperCase();
+            if (timestampIdentifiers.has(idText)) {
                 this.hasTimestamp = true;
             }
         };
     }
 }

---

105-106: Make InvalidBooleanExpressionError carry context (message + name).

Provide a useful message and set the error name.

Apply this diff:

-class InvalidBooleanExpressionError extends Error {
-}
+class InvalidBooleanExpressionError extends Error {
+    constructor (message?: string) {
+        super(message ?? "The WHERE boolean expression must not reference the timestamp column.");
+        this.name = "InvalidBooleanExpressionError";
+    }
+}

---

148-159: Trim/ignore empty booleanExpression and include a descriptive error on violation.

Avoid parsing empty strings; include the key in the thrown message.

Apply this diff:

-    if ("undefined" !== typeof booleanExpression) {
-        const booleanExpressionTree = buildParser(booleanExpression).standaloneBooleanExpression()
+    if ("undefined" !== typeof booleanExpression) {
+        const expr = booleanExpression.trim();
+        if (expr.length === 0) {
+            // no-op
+        } else {
+            const booleanExpressionTree = buildParser(expr).standaloneBooleanExpression()
             .booleanExpression();
 
-        const checker = new TimestampKeyChecker(timestampKey);
+            const checker = new TimestampKeyChecker(timestampKey);
             checker.visit(booleanExpressionTree);
-        if (checker.hasTimestamp) {
-            throw new InvalidBooleanExpressionError();
-        }
+            if (checker.hasTimestamp) {
+                throw new InvalidBooleanExpressionError(
+                    `The WHERE boolean expression must not reference the timestamp column '${timestampKey}'.`
+                );
+            }
 
-        queryString += ` AND (${booleanExpression})`;
+            queryString += ` AND (${expr})`;
+        }
     }

---

206-241: Fix ordering: ORDER BY should use the always-present label table, not the LEFT side’s nullable join key.

Ordering by bucket_count.index yields NULLs for empty buckets and unstable ordering. Order by bucket_timestamp.index.

Apply this diff:

-ORDER BY bucket_count.index
+ORDER BY bucket_timestamp.index

Additional recommended improvements:

• Use centre-of-bin timestamps for clearer charting.
• Use strict bounds (> start AND <= end) so width_bucket yields only 1..bucketCount and you can drop the extra WHERE filter.

Apply this diff to centre the timestamps and tighten bounds:

-    WHERE to_unixtime(${timestampKey}) BETWEEN ${startTimestampFixed} AND ${endTimestampFixed}
+    WHERE to_unixtime(${timestampKey}) > ${startTimestampFixed} AND to_unixtime(${timestampKey}) <= ${endTimestampFixed}
@@
 bucket_timestamp AS (
     SELECT
         index + 1 AS index,
-        ${startTimestampFixed}
-        + (index*(${endTimestampFixed}-${startTimestampFixed})/${bucketCount}) AS timestamp
+        ${startTimestampFixed}
+        + ((index + 0.5) * ((${endTimestampFixed}-${startTimestampFixed})/${bucketCount})) AS timestamp
     FROM
         UNNEST(sequence(0, ${bucketCount - 1}, 1)) AS t(index)
 )

Optional tidy-up (naming): consider renaming “index”→“bucket” throughout to avoid confusion with SQL “INDEX”.

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro