tamnd / tamnd/firepanda

S7. The front doors

Open
#312 0 comments 0 reactions 0 assignees View on GitHub
area/python area/sql enhancement security
Dominant language
Mojo
Stars
1
Forks
0
PR merge metrics
PR metrics pending

Description

Part of #304. Depends on S6.

Six ways a query string reaches the engine. They differ in who wrote the string, what names are in scope and how much the caller should be trusted, and the last one is why the capability flag is specified here rather than added after an incident.

This is also the issue that delivers the latency axis, which is the one place where a multiple of ten over the rivals is real.

### Scope

- [ ] `firepanda.sql(query, **params)` with implicit capture of the caller's frames
- [ ] `df.sql(query)`, with the frame bound to `self`
- [ ] `df.query(expr)` and `df.eval(expr)` for pandas parity
- [ ] Parameters in three forms, positional, numbered and named
- [ ] The prepared statement cache
- [ ] The CLI, interactive and non interactive
- [ ] `enable_external_access`
- [ ] The GIL split around planning and execution, which is #204 for this path

### Implicit capture

`fp.sql("SELECT ... FROM customers JOIN orders ...")` where `customers` and `orders` are Python locals and nothing was registered. That is DuckDB's behaviour and it is most of why `duckdb.sql()` feels good, so it is worth copying, and implicit capture that is not specified becomes a bug report.

Resolution order is explicit registrations, then the caller's locals, then the caller's globals. Only DataFrame and Series are captured, and a name bound to anything else is skipped rather than raising, so a local called `orders` that is a list does not shadow a registered frame. Capture is by borrow and not by copy, with the Python object kept alive for the call. Capture is opt out, because a library calling `sql()` on behalf of its own caller does not want that caller's locals in scope. A name in both a registration and the caller's locals resolves to the registration, explicitly. Two names differing only in case is ambiguous and raises, because identifiers fold down.

### query() and eval() are not SQL

These are pandas API surface, so they are governed by `docs/specs/06-pandas-parity.md` and not by DuckDB compatibility. It is the one place in this milestone where we implement a dialect that is not DuckDB's.

pandas expressions are Python syntax. `@` for locals, backticks for column names that are not identifiers, `and` and `or` and `not`, `in` and `not in`, chained comparisons, and a `parser` argument that changes precedence for `&` and `|`. That cannot route through the SQL parser and pretending otherwise would break both, so it gets a small dedicated parser producing the same bound expressions and lowering to the same nodes. A few hundred lines over a closed grammar that has not changed in years, sharing everything below the AST.

One deliberate difference, documented rather than hidden. pandas with numexpr computes in float64 in places the dtypes would not suggest, and silently falls back to Python for unsupported expressions. We use firepanda's own type rules throughout, and that goes in the divergence table on the row where it bites.

### Parameters are values, never text

Bound after parsing into a parameter node and substituted at execution. There is no string interpolation anywhere in the implementation and no API that accepts one, which makes injection structurally impossible on this path rather than discouraged. That property is worth more than any documentation about it. Parameter types are inferred from where the parameter is used, so `a > ?` against an INTEGER column binds an integer and a string argument raises a conversion error naming the parameter.

### The prepared statement cache

Keyed on the exact query text plus a catalog generation counter plus the values of the settings that affect binding. The value is the bound and optimized plan. Parameters are not part of the key, which is the entire point, because a loop over a parameterized query then parses once.

Soundness rests on two earlier rules. Parsing is a pure function of text and grammar, from S1, and binding depends only on the catalog, from S3. The generation counter increments on any registration or schema change and invalidates the cache wholesale rather than selectively, because a REPL registers rarely and queries constantly, and a precise invalidation scheme is a correctness risk bought for no measurable benefit.

This is shared mutable state on the hot path, so it goes in the file that holds every atomic in the library and it gets cases in the concurrency stress suite.

### The capability flag

`enable_external_access`. When false, the table functions that reach the filesystem refuse with a clear error naming the setting rather than a file not found. That covers `read_csv`, `read_parquet`, `read_json`, `glob`, `COPY TO` and the replacement scan that turns `FROM 'file.parquet'` into one of them.

Defaults by door. `fp.sql()` and `df.sql()` permissive, because the caller is the program and the program can already open files. The CLI with `-c` or piped stdin restrictive, because the string may have been assembled by a script. ADBC restrictive, because by construction the query came from another process.

The setting is one way within a session. It can be turned off and not back on, for the same reason DuckDB locks it, which is that a query able to turn it back on would make it decorative.

### The GIL split

The release goes around planning and execution, not around capture. Capture reads the caller's frame dictionaries and must hold the GIL, and execution touches only firepanda's own memory and must not.

Getting that boundary wrong fails silently, as a performance non result rather than an error, and running several `fp.sql()` calls under `concurrent.futures` is a thing users try immediately.

### Exit criteria

- [ ] `sql("SELECT a, sum(b) FROM t GROUP BY a")` over a ten thousand row frame is under 100 microseconds end to end, warm, returning a frame
- [ ] Under 20 microseconds of that is parse, bind, plan and optimize
- [ ] A repeat execution of a seen statement reaches the physical plan in under 2 microseconds
- [ ] The latency suite reports p50, p99 and max rather than a mean, because a 5 microsecond median with a 2 millisecond p99 is not a low latency front end
- [ ] Several concurrent `fp.sql()` calls scale, measured rather than assumed
- [ ] The concurrency stress suite covers the cache under concurrent registration

### Depends on

S6.

Contributor guide

Open the contributing guide

Research direction

Start with the S6 dependency and docs/specs/06-pandas-parity.md, then trace the fp.sql(), df.sql(), CLI, and ADBC entry points described here. Run the latency suite and concurrency stress suite while mapping the prepared-statement cache and capability-setting paths. Done means the listed query forms, parameter handling, access rules, GIL boundary, latency targets, and concurrent-cache cases are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
pandas, python
Domain
api, cli, data, performance, security, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
18/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.