unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
error-handlingtesting-strategy

Special handling for block comments after an operator

Project: duckdb/duckdb | Language: C++ | Concepts: error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/duckdb/duckdb/pull/25177

The problem

The tokenizer in DuckDB's new parser mishandled SQL block comments (/* ... */) that appeared immediately after an operator (like +, *, /). This caused incorrect tokenization and parsing results, leading to subtle bugs and deviations from PostgreSQL compatibility. Additionally, unterminated block comments (e.g., SELECT 1 /*;) were not reliably detected and reported as syntax errors.

Why it's non-trivial

A naive fix might just treat everything after an operator as a comment or ignore the context, but SQL tokenization rules are subtle:

  • Operators can be multi-character and may abut comments with no whitespace.
  • Block comments can appear in tricky places, and the tokenizer must not merge them into operators.
  • Proper error reporting for unterminated comments is required, matching established SQL behavior (like PostgreSQL).
  • Fixes must not break other parsing scenarios or introduce regressions.

The approach

1. Refactor operator tokenization into a helper function

A new static method is introduced to encapsulate the logic for pushing operator tokens, including PostgreSQL's trimming rule:

+	static void PushOperatorToken(TokenizerBehavior &behavior, idx_t start, idx_t end);

This centralizes and reuses the logic for handling operator tokens.

2. Implement special handling for block comments after operators

In the tokenizer state machine, a new check is added to detect the start of a block comment (/*) while in the OPERATOR state:

+			if (c == '/' && i + 1 < sql.size() && sql[i + 1] == '*') {
+				PushOperatorToken(behavior, last_pos, i);
+				// Go back to STANDARD state so it tokenizes as a block comment
+				state = TokenizeState::STANDARD;
+				last_pos = i;
+				i--;
+				break;
+			}

This ensures that if a block comment starts immediately after an operator, the operator is tokenized first, and then the comment is handled separately.

3. Move operator trimming logic into the helper

The logic for trimming trailing + characters from operators (unless they contain certain special characters) is moved into PushOperatorToken:

+	auto &sql = behavior.sql;
+	auto &tokens = behavior.tokens;
+	// Apply PostgreSQL trimming rule: an operator cannot end in '+' unless
+	// it contains at least one of: ~ ! @ # % ^ & | ` ?
+	idx_t end_pos = end;
+	bool has_special = false;
+	for (idx_t pos = start; pos < end_pos; pos++) {
+		char operator_char = sql[pos];
+		if (operator_char == '~' || operator_char == '!' || operator_char == '@' || operator_char == '#' ||
+		    operator_char == '%' || operator_char == '^' || operator_char == '&' || operator_char == '|' ||
+		    operator_char == '`' || operator_char == '?') {
+			has_special = true;
+			break;
+		}
+	}
+	if (!has_special) {
+		while (end_pos > start && sql[end_pos - 1] == '+') {
+			end_pos--;
+		}
+	}
+	behavior.PushToken(start, end_pos, TokenType::OPERATOR);
+	// Push any trimmed '+' characters as individual tokens
+	for (idx_t pos = end_pos; pos < end; pos++) {
+		tokens.emplace_back(string(1, sql[pos]), pos, TokenType::OPERATOR);
+	}

This matches PostgreSQL's operator tokenization rules.

4. Improve unterminated block comment detection and error reporting

When the tokenizer ends in a MULTI_LINE_COMMENT state, it now pushes an unterminated comment token and throws a descriptive syntax error:

+	case TokenizeState::MULTI_LINE_COMMENT:
+		behavior.PushToken(last_pos, sql.size(), TokenType::COMMENT, true);
+		return false;

And in the parser behavior:

+	if (type == TokenType::COMMENT && unterminated) {
+		auto comment = sql.substr(start, end - start);
+		throw ParserException::SyntaxError(sql, "unterminated /* comment at or near \"" + comment + "\"",
+		                                   optional_idx(start));
+	}

This ensures that SELECT 1 /*; produces a clear syntax error.

5. Add regression and error tests

New test cases are added to verify correct handling of comments after operators and unterminated block comments:

+query III
+SELECT 1+/**/2, 1*/**/2, 1+/* comment */2;
+----
+3	2	3
+
+query I
+SELECT 8//**/2;
+----
+4

And for error cases:

+statement error
+SELECT 1 /*;
+----
+Parser Error: unterminated /* comment at or near "/*;"

Python shell tests also check error codes and stderr output.

From the review thread

carlopi: What should happen with open ended comments? Like: SELECT 1\* ?

Dtenwolde: That's an unterminated comment in the CLI. While using -c strips the comment:

build/release/duckdb -c "select 1/*;"
┌───────┐
│   1   │
│ int32 │
├───────┤
│     1 │
└───────┘

carlopi: Old parsed did:

/opt/homebrew/bin/duckdb -c "select 1/*;"

Parser Error:
unterminated /* comment at or near "/*;"

LINE 1: select 1/*;

that I think I would expect as behaviour

Dtenwolde: I think that makes most sense, I'll adjust the PR to restore that behavior :)

Takeaways

  • Tokenization logic must handle context-sensitive cases, especially with operators and comments, to avoid subtle parsing bugs.
  • Centralizing complex tokenization rules (like operator trimming) in helper functions improves maintainability and reduces duplication.
  • Explicit error handling and user-facing error messages for unterminated constructs (like block comments) are essential for robust parser behavior and developer experience.
  • Regression and edge-case tests are critical when fixing parser/tokenizer bugs to prevent future breakage and to document expected behavior.