Repository navigation
duckdb: fix unknown column error for the rowid pseudo-column - #4678
Open
RyoKusnadi wants to merge 1 commit into
Open
RyoKusnadi wants to merge 1 commit into
RyoKusnadi wants to merge 1 commit into
Conversation
Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: On the DuckDB engine,
rowidfails withunknown column "rowid"in a SELECT list, asp.rowid, in WHERE, and in ORDER BY. DuckDB accepts all of these.How it happens: Any DuckDB query that uses
rowidon a table fromschema.sqlhits it. The table has no declaredrowidcolumn, and the engine only knew declared columns.Steps to reproduce: Use the schema and queries from the issue (
SELECT rowid, id FROM plan,WHERE rowid = $1) withengine: duckdb, then runsqlc generate.Expected: Code is generated, with
rowidas a nullable BIGINT (whatDESCRIBEreports).Actual:
unknown column "rowid".Cause:
convertCreateTableInfoininternal/engine/duckdb/convert.goonly emitted the declared columns. SQLite's fts5 hidden columns already useColumnDef.IsHiddenfor "resolves by name, not in*or models". The DuckDB side never used it.Fix: Add a hidden nullable
bigintrowidcolumn to everyCREATE TABLE. Star expansion and the generated model stay the same. If the table declares its ownrowidcolumn, that one wins and nothing is added.Scope:
CREATE TABLE ... ASand views get norowid. I haven't checked whether DuckDB exposes it for them. I did not touch #4677 (table alias as a value) or #4659 (table functions).Testing: New case
internal/endtoend/testdata/duckdb_rowid(select, qualified, WHERE, ORDER BY,SELECT *skips rowid, user-declaredrowid). It needs no database. Before the fix it fails with the four errors from the issue. After:go test -C internal/endtoend -run 'TestReplay/base' .passes.go vetandgofmtare clean on the duckdb package. I did not run the full suite or the tests that need PostgreSQL/MySQL or a live DuckDB.Environment: fix/duckdb-rowid-column @ 5d94812, go1.27.1, darwin/arm64.
Fixes #4676