Skip to content

Parse DuckDB's // integer division operator - #482

Open
eddietejeda wants to merge 2 commits into
tobilg:mainfrom
hotdata-dev:feat/duckdb-integer-division
Open

eddietejeda wants to merge 2 commits into
tobilg:mainfrom
hotdata-dev:feat/duckdb-integer-division

Conversation

@eddietejeda

Copy link
Copy Markdown

DuckDB has an integer division operator written as //. Right now, the parser doesn't recognize it. See DuckDB operator list here: https://duckdb.org/docs/lts/sql/functions/numeric

SELECT 7 // 2 AS v        -- DuckDB returns 3

What this does

When the source dialect is DuckDB and a / is followed by another /, the parser now treats the pair as one operator and builds an IntDiv node.

DuckDB -> DuckDB       SELECT 7 // 2 AS v
DuckDB -> PostgreSQL   SELECT DIV(7, 2) AS v     -- same output MySQL's 7 DIV 2 gets today

Nothing else is affected. a / / b isn't valid SQL in any dialect, so there's no existing input this could change the meaning of. The check is also limited to DuckDB as the source.

Tests

tests/duckdb_integer_division.rs covers the round trip, the lowering to other targets, precedence against +, and that a plain / still parses as ordinary division.

@tobilg

tobilg commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Verdict: request changes before merging revision 56fb9c0d.

This addresses a real DuckDB parsing gap. Validation passed for all four new tests, 1,301 library tests, 208 dialect-matrix tests, and formatting. Additional execution probes identified the following concerns.

  1. Preserve DuckDB’s division semantics when converting to other dialects.

    DuckDB’s // behavior depends on operand types. These conversions currently change behavior or produce unsupported SQL:

    DuckDB input Source result Generated target SQL Problem
    7.0 // 2 3.5 PostgreSQL: DIV(7.0, 2) Returns 3
    7 // 0 NULL BigQuery: DIV(7, 0) Raises a division-by-zero error
    7 // 2 3 SQLite: DIV(7, 2) Fails because SQLite has no built-in DIV function

    These are existing IntDiv generation limitations exposed by the newly supported DuckDB syntax. Conversions should preserve the source behavior or produce an explicit unsupported diagnostic when that cannot be established.

    SQLGlot 30.14.0 reproduces the fractional and zero-division conversion problems, so matching its output is insufficient to establish correctness here.

  2. Use the existing tokenizer support for //.

    TokenizerConfig::double_slash_int_div, already used by Vertica, recognizes contiguous // as TokenType::Div. Both multiplication parser paths already support that token.

    The proposed parser lookahead also accepts separated slashes:

    SELECT 7 / / 2;
    SELECT 7 / /* comment */ / 2;

    DuckDB rejects both statements, but this revision accepts them even under strict validation and regenerates them as integer division.

    Please enable the existing tokenizer setting for DuckDB and remove the two new parser branches. This fits the shared architecture, avoids duplicated logic, and matches SQLGlot’s tokenizer-level recognition.

    It also avoids materializing and discarding the second slash token. An allocation probe measured 1,000 additional allocations for 1,000 division expressions with the proposed parser approach compared with the existing tokenizer path. This measures allocation overhead; no latency regression has been established.

  3. Include regression coverage in the routinely executed test suites.

    The new standalone duckdb_integer_division test target is not selected by make test-rust-verify or its CI wrappers. Please move these cases into appropriate existing suites so the normal verification process exercises them.

    Additional coverage should include fractional operands, explicitly typed floating-point operands, zero divisors, separated slashes, and AST precedence. For precedence, 2 + 7 // 2 distinguishes the possible groupings; the current 1 + 7 // 2 example produces the same result under either grouping. An AST assertion should verify that integer division binds more tightly than addition.

    After these changes, please run make test-rust-verify to check for regressions.

This branch has not been deployed

No deployments
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.

2 participants