Skip to content

Commit a5bc6ea

Browse files
committed
fix: route sharded DML through ShardMap
INSERT/UPDATE/DELETE used a private abs(k)%n hash while SELECT prune used ShardMap (FNV-1a/RANGE/LIST), so point lookups after an engine INSERT could miss with no error. Route all DML through ShardMap, fail closed on missing/non-literal keys and shard-key UPDATEs, and cover HASH/RANGE/LIST INSERT-then-SELECT.
1 parent b20c1cc commit a5bc6ea

5 files changed

Lines changed: 313 additions & 189 deletions

File tree

AGENTS.md

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,66 @@
1-
# Repository Guidelines
1+
# Agent notes
22

3-
## Project Structure & Module Organization
4-
Core parser headers live in `include/sql_parser/` and parser implementations in `src/sql_parser/`. SQL engine, remote execution, and transaction interfaces live in `include/sql_engine/` with implementations in `src/sql_engine/`. Tests are in `tests/`, mostly as focused `test_<area>.cpp` files plus `corpus_test.cpp` for large parser corpora. Developer tools live in `tools/`, automation scripts in `scripts/`, benchmark reports in `docs/benchmarks/`, and vendored dependencies in `third_party/`.
3+
Trust the `Makefile` over prose. Extension recipes live in `CLAUDE.md`. `docs/superpowers/` is historical, not current behavior.
54

6-
## Build, Test, and Development Commands
7-
Use the `Makefile` as the source of truth:
5+
## Layout
86

9-
- `make all` builds `libsqlparser.a` and runs the full GoogleTest suite.
10-
- `make test` rebuilds `run_tests` and executes all tests locally.
11-
- `make lib` builds just the static library.
12-
- `make build-sqlengine` builds the interactive CLI as `./sqlengine`.
13-
- `make build-corpus-test` builds `./corpus_test` for external SQL corpus validation.
14-
- `make bench` runs the benchmark binary; use it for parser or executor performance changes.
15-
- `make clean` removes generated objects and binaries.
7+
- Parser: header-only templates in `include/sql_parser/` except `src/sql_parser/{arena,parser}.cpp`
8+
- Engine: headers in `include/sql_engine/` (`operators/`, `functions/`, `rules/`); compiled files are the explicit `ENGINE_SRCS` list
9+
- High-level API: `Session<D>` (`include/sql_engine/session.h`) — parse → plan → optimize → distribute → execute
10+
- Production remote path: `ThreadSafeMultiRemoteExecutor`, not the single-connection executors
11+
- All shard routing (SELECT prune and DML) goes through `ShardMap`. Do not add a private hash in the planner.
12+
- Backend URL / shard-spec parsing: `tool_config_parser` — do not add another copy in tools
13+
- Do not edit `third_party/`
1614

17-
## Coding Style & Naming Conventions
18-
This repository is C++17 with warnings enabled via `-Wall -Wextra`. Match the existing style: 4-space indentation, opening braces on the same line, and concise comments only where the code is not obvious. Use `PascalCase` for types, `snake_case` for functions and methods, `UPPER_SNAKE_CASE` for include guards and macros, and keep file names module-oriented such as `parser.cpp`, `distributed_txn.h`, and `test_select.cpp`. There is no repo-wide formatter config outside vendored code, so follow surrounding files closely.
15+
## Commands
1916

20-
## Testing Guidelines
21-
Tests use GoogleTest through `tests/test_main.cpp`. Add coverage in the nearest existing `test_<feature>.cpp`, or create a new file with that pattern if the area is new. Prefer small, focused `TEST` or `TEST_F` cases that mirror the production module name. Run `make test` before opening a PR; for grammar or dialect work, also run `make build-corpus-test`.
17+
```bash
18+
make all # libsqlparser.a + full GoogleTest suite
19+
make lib
20+
make test # rebuild ./run_tests and run it
21+
./run_tests --gtest_filter='*WindowFunc*'
22+
make build-sqlengine # ./sqlengine
23+
make build-corpus-test # ./corpus_test
24+
make mysql-server engine-stress bench-distributed
25+
make bench # -O2; release+corpus report: bash scripts/run_benchmarks.sh report.md
26+
make test-pg-compat # committed PG18 gate (needs PG_COMPAT_CACHE / libpg_query)
27+
```
2228

23-
## Commit & Pull Request Guidelines
24-
Recent history uses short conventional prefixes such as `feat:`, `fix:`, `test:`, `docs:`, and `chore:`. Keep commit titles imperative and specific, for example `feat: add UTC normalization for PgSQL timestamps`. PRs should target `main`, explain parser/engine behavior changes, list the commands you ran, and link related issues. Include benchmark or corpus-test notes when performance or SQL coverage changes. Do not commit generated `.o` files, binaries, or benchmark artifacts.
29+
New `tests/test_*.cpp` must be appended to `TEST_SRCS`. New `src/sql_engine/*.cpp` must be appended to `ENGINE_SRCS`. Otherwise they never build.
30+
31+
No repo formatter. C++17, `-Wall -Wextra`. Match neighboring files. Includes: `"sql_parser/..."`, `"sql_engine/..."`.
32+
33+
macOS needs client libs: `brew install mysql-client postgresql zstd`, then
34+
`LIBRARY_PATH=/opt/homebrew/lib make all MYSQL_CFLAGS="-I/opt/homebrew/opt/mysql-client/include"`.
35+
Tests and tools link libmysqlclient + libpq even when no live backend is used.
36+
37+
## Parser gotchas
38+
39+
- Dialect is compile-time: `Parser<Dialect::MySQL>` / `Parser<Dialect::PostgreSQL>`. One `Parser` per thread (non-copyable).
40+
- `parse(sql, len)` takes an explicit length. `StringRef` views the input — keep the SQL buffer alive until you are done with the AST.
41+
- `parser.reset()` rewinds the arena; AST and emitter output are invalid after reset.
42+
- Keyword lookup is a hash table from `keywords_mysql.h` / `keywords_pgsql.h`. Keep those arrays alphabetically sorted. New keywords also need `token.h`, and usually `is_keyword_as_identifier()` in `expression_parser.h` plus `is_alias_start()` in `table_ref_parser.h`.
43+
- Classifier switch: `classify_and_dispatch()` in `src/sql_parser/parser.cpp`.
44+
- Status is `OK` / `PARTIAL` / `ERROR`. `PARTIAL` can still have a usable AST (e.g. multi-assign SET with one bad element). Do not treat PARTIAL as a hard failure without checking the AST.
45+
46+
## Tests
47+
48+
Default gate: `make test`. Add coverage in the nearest `tests/test_<area>.cpp`.
49+
50+
Live-backend tests `GTEST_SKIP` when unreachable:
51+
- MySQL `127.0.0.1:13306` root/test/testdb — `scripts/start_test_backends.sh`
52+
- PostgreSQL `127.0.0.1:15432` postgres/test/testdb — same script
53+
- `test_single_backend_txn.cpp` / `test_distributed_txn.cpp` skip unless `MYSQL_TEST_HOST` is set
54+
55+
`scripts/start_test_backends.sh` and `scripts/start_sharding_demo.sh` both bind **13306** — do not run them together.
56+
57+
`make test-sqlengine` drives `./sqlengine` and **fails loudly** (exit 2) if containers are missing. Start them first:
58+
- in-memory: no backend
59+
- single: `scripts/setup_single_backend.sh` (port 13308)
60+
- sharded: `scripts/start_sharding_demo.sh` (13306 + 13307)
61+
62+
Corpus is not in-tree. `./corpus_test <mysql|pgsql> [files...]`. Full download: `scripts/run_benchmarks.sh`. CI runs `make all` plus a corpus subset. For grammar/dialect work, also build `corpus_test`.
63+
64+
## Commits / PRs
65+
66+
Conventional prefixes (`feat:`, `fix:`, `test:`, `docs:`, `chore:`, `build:`). PRs target `main`. Do not commit `*.o`, `libsqlparser.a`, `run_tests`, `sqlengine`, `corpus_test`, `run_bench*`, or benchmark artifacts.

0 commit comments

Comments
 (0)