Feat builder

[abnegate]

No description provided.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 550a4101-07c3-4838-8fd9-5a84b0ecb557

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat-builder

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[abnegate]

@greptileai

[greptile-apps]

Greptile Summary

This PR introduces a large query builder subsystem (Builder, dialect subclasses, traits, feature interfaces) alongside a full SQL AST layer: a tokenizer-backed Parser, Serializer, Walker, and Visitor implementations (FilterInjector, TableRenamer, ColumnValidator).

• P1 – FETCH NEXT / singular ROW not parsed: parseSelect only accepts FETCH FIRST n ROWS ONLY; any query using the equally-valid FETCH NEXT n ROWS ONLY (PostgreSQL, DB2, SQL standard) or the singular ROW form throws a parse exception.
• P2 – ColumnValidator wildcard gap: Star (wildcard) expressions pass through visitExpression unchecked, so SELECT * bypasses the column allowlist.
• P2 – dead condition in CTE parsing: !$this->matchKeyword('AS') inside a branch already guarded by current()->type === TokenType::LeftParen is always true and can be removed.

Confidence Score: 4/5

Safe to merge with the FETCH NEXT parsing gap addressed; remaining findings are style/hardening.

One P1 defect (FETCH NEXT parse failure) means valid SQL will throw at runtime. The remaining two findings are P2 and do not block merge on their own, but the P1 warrants a fix before shipping.

src/Query/AST/Parser.php (FETCH clause), src/Query/AST/Visitor/ColumnValidator.php (Star handling)

Security Review

• Column allowlist bypass (ColumnValidator): Star/wildcard expressions are not inspected, allowing SELECT * to expose columns that the allowlist was intended to restrict. Severity depends on how callers use this visitor.

Important Files Changed

Filename	Overview
src/Query/AST/Parser.php	New SQL AST parser (1211 lines); missing FETCH NEXT / singular ROW support, and a dead guard condition in CTE column list parsing
src/Query/AST/Visitor/ColumnValidator.php	New visitor for column allowlist enforcement; Star/wildcard expressions bypass the check, creating a potential security gap
src/Query/Builder.php	Large new abstract Builder (2806 lines); bindings reset correctly on each build()/update()/delete() call, clone logic looks thorough
src/Query/AST/Serializer.php	New AST serializer with correct precedence handling, safe literal escaping, and proper parenthesization for non-commutative operators
src/Query/AST/Walker.php	New AST walker with bottom-up traversal; covers all expression types including Conditional/CaseWhen, subqueries, and CTEs
src/Query/AST/Visitor/FilterInjector.php	New visitor that injects a WHERE condition; correctly documents Walker vs direct-call semantics for subquery scope
src/Query/QuotesIdentifiers.php	New identifier-quoting trait; correctly handles dotted identifiers and wildcard segments

_{Reviews (1): Last reviewed commit: "fix(test): assert MySQL driver error cod..." | Re-trigger Greptile}

[greptile-apps]

FETCH NEXT and singular ROW not handled

The parser only accepts FETCH FIRST n ROWS ONLY. The SQL standard (and PostgreSQL, DB2, etc.) also allows FETCH NEXT n ROWS ONLY and the singular form FETCH FIRST/NEXT n ROW ONLY. Any query using either alternative will throw a parse exception at consumeKeyword('FIRST').

// Suggested fix
if ($this->matchKeyword('FETCH')) {
    $this->advance();
    if (!$this->matchKeyword('FIRST') && !$this->matchKeyword('NEXT')) {
        throw new Exception("Expected FIRST or NEXT after FETCH at position {$this->current()->position}");
    }
    $this->advance(); // consume FIRST or NEXT
    $limit = $this->parseExpression();
    if (!$this->matchKeyword('ROW') && !$this->matchKeyword('ROWS')) {
        throw new Exception("Expected ROW or ROWS at position {$this->current()->position}");
    }
    $this->advance(); // consume ROW or ROWS
    $this->expectIdentifierValue('ONLY');
}

[greptile-apps]

Dead condition in CTE column list guard

When $this->current()->type === TokenType::LeftParen, matchKeyword('AS') always returns false (the current token is a LeftParen, not a Keyword). So !$this->matchKeyword('AS') is unconditionally true here — the conjunction is redundant and misleadingly implies an AS-check that never fires. The actual disambiguation is handled by peekIsColumnList().

Suggested change

	if ($this->current()->type === TokenType::LeftParen) {

[greptile-apps]

Star expressions bypass the column allowlist

visitExpression only inspects Column nodes. A Star (wildcard * or table.*) expression passes through unchecked. If ColumnValidator is used as a security gate to prevent access to specific columns, SELECT * or SELECT t.* will silently bypass the whitelist, potentially exposing columns the caller intended to restrict.

Consider explicitly rejecting (or explicitly allowing) Star nodes.