Fix literal subscripts shifting by one under dialect="duckdb" #239

Description

Every literal subscript in a query transpiled with dialect="duckdb" is silently shifted by one. No join, no INTERSECTS, and no genomic operator is required — it affects any query that subscripts an array or string.

conn.execute('CREATE TABLE t (arr INTEGER[], nm VARCHAR)')
conn.execute("INSERT INTO t VALUES ([7,8,9], 'abcdef')")

conn.execute(giql.transpile("SELECT arr[1] FROM t", tables=["t"]))                    # -> 7
conn.execute(giql.transpile("SELECT arr[1] FROM t", tables=["t"], dialect="duckdb"))  # -> 8

The emitted SQL is SELECT t.arr[2] AS v FROM t AS t. The literal 1 became 2.

Observed across the affected and unaffected spellings, on one row with arr = [7,8,9] and nm = 'abcdef':

Three things make this worse than a plain off-by-one. It is silent — no error, no warning, just a different value. At the end of an array it becomes data loss rather than a wrong element, since arr[3] returns NULL. And it depends on how the index is spelled: t.arr[1] shifts while the equivalent t.arr[t.start - 99] and list_extract(t.arr, 1) do not, because only a literal index is rewritten.

Slices, struct field access, and the function-call forms are unaffected. dialect=None and dialect="datafusion" are unaffected.

This is not a coordinate-system question

Worth recording, because the off-by-one invites the reading and it is wrong. The shift is identical under both coordinate systems:

They cannot interact mechanically. Table.coordinate_system is applied only where interval predicates are built — canonical_start / canonical_end wrap an endpoint column in text such as (a."start" - 1) at src/giql/expanders/intersects_duckdb.py:1889-1891, plus the decanonical_* sites in disjoin.py and nearest.py. That is arithmetic on endpoint columns and never touches a Bracket. INDEX_OFFSET applies to exp.Bracket literal indices at generation time. GIQL also never constructs a Bracket itself, so the only ones in play are the user's own.

Conflating them would also be wrong on its own terms. coordinate_system describes how a table's interval columns are encoded, a property of the data; array subscripting is SQL language semantics, a property of the query. coordinate_system is per-Table while INDEX_OFFSET is per-dialect and global to the query, so SELECT a.arr[1], b.arr[1] FROM peaks a JOIN genes b ... over a 0-based peaks and a 1-based genes would need two index origins in one SELECT list, which sqlglot cannot express.

Expected behavior

arr[1] means the same element on every target. A query transpiled with dialect="duckdb" returns the same rows as the same query transpiled with dialect=None and run on the same DuckDB connection.

Root cause

The query is parsed with one dialect and serialized with another, and the two disagree on array index origin.

GIQLDialect (src/giql/dialect.py:35) extends sqlglot's base Dialect and never declares INDEX_OFFSET, so it inherits 0. transpile() serializes with ast.sql(dialect=target.sqlglot_dialect) (src/giql/transpile.py:217), and DuckDBTarget.sqlglot_dialect is "duckdb", whose INDEX_OFFSET is 1. sqlglot's generator then applies the difference:

def bracket_offset_expressions(self, expression, index_offset=None):
    return apply_index_offset(
        expression.this,
        expression.expressions,
        (index_offset or self.dialect.INDEX_OFFSET) - expression.args.get("offset", 0),
        dialect=self.dialect,
    )

The GIQL parser stamps no offset on the Bracket, so the shift evaluates to 1 - 0 = 1 on every literal index.

INDEX_OFFSET appears nowhere in this repository, and src/giql/dialect.py has two commits, neither concerning indexing. The 0 was never a decision — it is an inherited sqlglot default that nothing has examined.

The generic and DataFusion targets escape the shift only because sqlglot_dialect is None there, giving 0 - 0 = 0. That is correct by accident rather than by construction: None resolves to sqlglot's base Dialect, which is not an engine but sqlglot's internal normalization convention, and its 0-based array origin matches none of the engines GIQL targets.

DuckDB                1-based    (INDEX_OFFSET 1)
DataFusion            1-based    (verified on datafusion 54.0.0: make_array(7,8,9)[1] -> 7)
Postgres              1-based
sqlglot base Dialect  0-based    <- inherited by GIQLDialect

Proposed fix

sqlglot's model is that a dialect declares the truth about its own syntax: the parser normalizes from it and each generator denormalizes to its own. There is no pass-through idiom, only declaring correctly. GIQLDialect currently declares that GIQL's input is 0-based, which is false — GIQL's input is SQL, and users write arr[1] meaning the first element. The defect is in the declaration, not in the conversion.

Two lines, both of which stop the declaration lying:

# src/giql/dialect.py
class GIQLDialect(Dialect):
    """Generic SQL dialect with genomic spatial operators."""

    INDEX_OFFSET = 1

# src/giql/transpile.py:217
sql = ast.sql(dialect=target.sqlglot_dialect or GIQLDialect)

The second makes the generic and DataFusion paths an identity transpile — same dialect in, same dialect out — instead of routing through a base dialect that models no engine. Without it, declaring GIQLDialect 1-based would fix DuckDB and regress those two paths to arr[0].

Validation

Both lines were prototyped and the full suite run:

8 failed, 2292 passed, 90 xfailed

All eight failures are the subscript tests added in #235, and they fail in the expected direction: with the offset corrected a subscript round-trips faithfully, so the IEJoin's rebuild gate accepts it and the projection engages rather than declining. Nothing else moved. Generic-path output was also diffed across twelve queries spanning EXCEPT, FILTER, window functions, CASE, struct / array / map literals, slices, lambdas, CTEs and DATE_TRUNC, with zero differences.

Tests that must be updated when this lands, since they pin the current decline:

• TestTranspileDuckDBIEJoinProjectionContract::test_transpile_should_decline_subscript_projection — four cases; delete or invert, as the shape becomes rebuildable.
• TestTranspileDuckDBIEJoinProjectionContract::test_transpile_should_hold_its_published_projection_contract[a.arr[1]-False] — flip the expectation to True.
• TestTranspileDuckDBIEJoinUnsupportedProjectionFallback::test_transpile_should_decline_unsupported_projection_to_naive_plan — drop the three subscript entries.

docs/transpilation/performance.rst also loses the subscript case from its fall-back list.

INDEX_OFFSET is not the only knob sqlglot normalizes between a read and a write dialect. The identity-transpile change closes the class for the generic path, but the DuckDB path parses with one dialect and generates with another by design, so the round-trip fidelity gate in _is_rebuildable_scalar should stay after this lands.

Context

Found while remediating the round-1 review of #235, which recorded this as finding B1 and attributed it to the scalar-expression projection rebuild that PR introduced. That attribution is wrong. Declining the IEJoin does not avoid the shift, because dialect="duckdb" serializes the whole statement with DuckDB's generator regardless of which plan wins — a bare SELECT arr[1] FROM t with no join at all already returns the second element. The defect predates #235 and is not specific to the IEJoin path.

#235 does close the door on the projection rebuild becoming a second source of it: _is_rebuildable_scalar now verifies that DuckDB's generator round-trips a projection subtree before rebuilding it, so a subscript projection declines.