Skip to content

docs(readme): sync with code, document AST layer and correct dialect claims - #15

Merged
abnegate merged 1 commit into
mainfrom
readme-query-sync-e93776
Aug 13, 2026
Merged

docs(readme): sync with code, document AST layer and correct dialect claims#15
abnegate merged 1 commit into
mainfrom
readme-query-sync-e93776

Conversation

@abnegate

Copy link
Copy Markdown
Member

Summary

The README had drifted from the code. Most of this is correcting claims that would actively mislead a reader, not just filling gaps.

Every documented output in the new and corrected content was produced by executing the code against src/.

Broken or wrong

Claim Reality
Badges point at tests.yml / integration.yml Neither exists. Repo has ci.yml, baseline.yml, linter.yml, static-analysis.yml
Aggregations example emits HAVING `total` > ? Emits HAVING COUNT(*) > ? — the builder expands the aggregate alias, since SQL forbids aliases in HAVING
Write keywords include RENAME, REPLACE, LOAD, MERGE, EXECUTE, HANDLER, IMPORT All classify as Type::Unknown. The real one it omitted is LOCK
SAVEPOINT / RELEASE / SETTransactionEnd They return Type::Transaction, a case the README never mentioned
MySQL parser recognizes COM_INIT_DB, COM_QUIT Neither constant exists
PostgreSQL parser handles X and startup messages Both fall through to Unknown; B and E are handled but were undocumented

Overstated UnsupportedException claims

Worse than omissions, because the documented failure mode was loud and the real one is silent:

  • Column-level ttl() is silently ignored outside ClickHouse.
  • MongoDB silently drops CHECK constraints and generated columns; SERIAL maps to int.
  • SQLite does support foreign keys inline via foreignKey($column) — only the ALTER-based form is absent. The inline form was undocumented entirely.
  • ClickHouse procedures/triggers are absent from the class, so calling them is a fatal Error, not a catchable exception.

Dialect drift

MariaDB was missing from five capability lists it inherits from MySQL. Upsert was attributed to Builder\SQL, which does not implement it. SQLite was missing from String Aggregates. vector was "PostgreSQL only" but exists on ClickHouse and MongoDB too. Partitions were claimed on ClickHouse, which has only its own partitionBy(). The Feature Matrix is rebuilt from the actual implements clauses.

Added

  • New section: SQL Tokenizer and ASTsrc/Query/AST/ and src/Query/Tokenizer/ had zero coverage. Tokenizer, recursive-descent parser, dialect serializers, Walker/Visitor with the three shipped visitors, and toAst()/fromAst() round-trip.
  • ExecutorssetExecutor(), execute(), Statement::withExecutor().
  • Materialized views (carrying the interface's verbatim-body security caveat), renameIndex(), createDatabase/dropDatabase, analyzeTable(), inline foreignKey(), srid()/dimensions(), Query::distinct/fingerprint/getCursorQueries, the full date-helper set, NullsPosition ordering, ParsedQuery::$timeBuckets.

Two code issues surfaced while verifying

Documented as-is here; both are worth separate fixes:

  1. Query::elemMatch() is a dead factory. It serializes fine but no builder compiles it — all six throw. Documented as usable only by consumers implementing their own Compiler.
  2. groupByTimeBucket on MongoDB silently drops the bucket instead of rejecting it. Scoped the docs to ClickHouse.

Test plan

  • composer test — 5299 tests, 12289 assertions, all pass
  • composer lint — pass
  • No source files touched; README only

🤖 Generated with Claude Code

…claims

The README had drifted from the code in ways that would mislead a reader
rather than merely omit detail.

Broken or wrong:
- CI badges pointed at tests.yml/integration.yml, neither of which exists;
  the repo has ci.yml, baseline.yml, linter.yml, static-analysis.yml.
- The aggregations example claimed `HAVING \`total\` > ?`. The builder
  expands an aggregate alias back to its expression, so it emits
  `HAVING COUNT(*) > ?` -- standard SQL forbids aliases in HAVING.
- The wire-protocol section listed seven write keywords that actually
  classify as Unknown (RENAME, REPLACE, LOAD, MERGE, EXECUTE, HANDLER,
  IMPORT) while omitting the real one (LOCK); put SAVEPOINT/RELEASE/SET
  under TransactionEnd when they return Type::Transaction; omitted the
  Transaction case entirely; named COM_INIT_DB/COM_QUIT, which do not
  exist; and claimed the PostgreSQL parser handles X and startup
  messages, which fall through to Unknown, while omitting B and E.

Overstated UnsupportedException claims -- these are worse than omissions
because the documented failure mode was loud and the real one is silent:
- Column-level ttl() is ignored outside ClickHouse, not rejected.
- MongoDB drops CHECK constraints and generated columns silently, and
  maps SERIAL to int rather than throwing.
- SQLite supports foreign keys inline via foreignKey($column); only the
  ALTER-based form is absent. The inline form was undocumented.
- ClickHouse procedures/triggers are absent from the class, so calling
  them is a fatal error, not a catchable exception.

Dialect drift: MariaDB was missing from five capability lists it inherits
from MySQL; Upsert was attributed to Builder\SQL, which does not
implement it; SQLite was missing from String Aggregates; vector was
"PostgreSQL only" but also exists on ClickHouse and MongoDB; partitions
were claimed on ClickHouse, which has only its own partitionBy(). The
Feature Matrix is rebuilt from the actual implements clauses.

The AST and tokenizer subsystems had no coverage at all, so this adds a
section for them, plus the executor pattern, materialized views,
renameIndex, database and analyze DDL, and the Query factories and
modifiers that were missing.

Query::elemMatch() is documented as compilable only by a custom Compiler,
since every bundled builder rejects it, and groupByTimeBucket is scoped
to ClickHouse.

Every documented output in the new and corrected content was produced by
executing the code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📊 Coverage

Metric Covered Ratio
Lines 91.83% 7473 / 8138
Methods 84.29% 1105 / 1311
Classes 65.53% 135 / 206

Full per-file breakdown in the job summary.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR comprehensively updates the README to match the library’s current APIs and dialect behavior.

  • Corrects feature matrices, parser classifications, generated SQL examples, and unsupported-operation claims.
  • Documents executors, schema capabilities, dialect-specific behavior, and additional query helpers.
  • Adds substantial tokenizer, AST, serializer, visitor, and builder round-trip documentation.

Confidence Score: 5/5

The documentation-only PR appears safe to merge with no actionable inaccuracies identified.

The changed API examples and capability statements checked against source and tests match the implemented interfaces, method signatures, traversal behavior, and builder round-trip contract.

Important Files Changed

Filename Overview
README.md Updates and expands public API documentation; the investigated examples, capability claims, and AST behavior align with the implementation.

Fix All in Greploop

Reviews (1): Last reviewed commit: "docs(readme): sync with code, document A..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant