Skip to content

fix attaching an Iceberg database through a REST catalog - #89

Merged
adsharma merged 1 commit into
LadybugDB:mainfrom
ericyuanhui:main_iceberg
Sep 24, 2026
Merged

adsharma merged 1 commit into
LadybugDB:mainfrom
ericyuanhui:main_iceberg

Conversation

@ericyuanhui

Copy link
Copy Markdown
Contributor

Problem: When attaching an Iceberg database through a REST catalog, DuckDB may initially report only a placeholder column named __ with type UNKNOWN in information_schema.columns, rather than the table’s actual schema. Ladybug previously used this information to register the external table, causing ATTACH to fail because UNKNOWN is unsupported. Subsequent column-name lookups could also mistake __ for a real column.

Fix: When the only reported column is __ / UNKNOWN, run SELECT * FROM ... LIMIT 0 and use the result’s actual column names and types to register the external table and resolve subsequent column-name lookups. This query returns no data rows but triggers schema resolution. The Iceberg documentation is also updated to clarify the roles of the REST catalog and embedded DuckDB, as well as how to configure an unauthenticated REST endpoint.

@adsharma

Copy link
Copy Markdown
Contributor

Thanks for fixing the Iceberg REST __/UNKNOWN placeholder issue — the fallback to SELECT * … LIMIT 0 is the right direction. A few points to address before merging:

1. Broken identifier quoting (duckdb/src/catalog/duckdb_catalog.cpp:324)

"SELECT * FROM \"{}\".{}.{} LIMIT 0", catalogName, schemaName, tableName

Only the catalog is double-quoted; schema/table are interpolated bare with no "-escaping. This breaks on mixed-case / spaces / reserved words / embedded quotes, and is inconsistent with the attached_duckdb_database.h fallback, which reuses the caller-quoted tableName. Please quote all three identifiers and escape " → "" (and note the surrounding information_schema query has the same single-quote issue — '{}' without escapeSingleQuotes, even though that helper already exists in the header).

2. Inconsistent guard (duckdb/src/include/storage/attached_duckdb_database.h:99)

if (parts.size() == 3 && result->RowCount() == 1 && ...

getTableInfo fires the fallback for any placeholder table, but getTableColumnNames only for 3-part refs. The README explicitly supports 2-part refs (namespace.table, auto-prefixed with iceberg_catalog), so a 2-part lookup hitting the placeholder still returns ["__"]. Please apply the fallback uniformly or explain why 3-part-only is sufficient.

3. Unhandled failure of the fallback query

DuckDBConnector::executeQuery throws on error (duckdb_connector.cpp). If the fallback SELECT * … LIMIT 0 fails (permissions, missing table, transient REST error), ATTACH now throws instead of returning false / skipping. The skipUnsupportedTable path catches BinderException from type conversion but not from the fallback SELECT itself. Please wrap the fallback or confirm throw-on-fallback is intended.

4. Duplicated magic check with different column orders

The RowCount == 1 && UNKNOWN / __ predicate is duplicated in two files with swapped column orders (data_type,column_name vs column_name,data_type). The indices are correct as written, but this is fragile — a future column reorder breaks silently. Please extract a small isPlaceholderSchema() helper (or at minimum comment the column order at each site).

Signed-off-by: ericyuanhui <285521263@qq.com>
@ericyuanhui

Copy link
Copy Markdown
Contributor Author

Thanks for fixing the Iceberg REST __/UNKNOWN placeholder issue — the fallback to SELECT * … LIMIT 0 is the right direction. A few points to address before merging:

1. Broken identifier quoting (duckdb/src/catalog/duckdb_catalog.cpp:324)

"SELECT * FROM \"{}\".{}.{} LIMIT 0", catalogName, schemaName, tableName

Only the catalog is double-quoted; schema/table are interpolated bare with no "-escaping. This breaks on mixed-case / spaces / reserved words / embedded quotes, and is inconsistent with the attached_duckdb_database.h fallback, which reuses the caller-quoted tableName. Please quote all three identifiers and escape " → "" (and note the surrounding information_schema query has the same single-quote issue — '{}' without escapeSingleQuotes, even though that helper already exists in the header).

2. Inconsistent guard (duckdb/src/include/storage/attached_duckdb_database.h:99)

if (parts.size() == 3 && result->RowCount() == 1 && ...

getTableInfo fires the fallback for any placeholder table, but getTableColumnNames only for 3-part refs. The README explicitly supports 2-part refs (namespace.table, auto-prefixed with iceberg_catalog), so a 2-part lookup hitting the placeholder still returns ["__"]. Please apply the fallback uniformly or explain why 3-part-only is sufficient.

3. Unhandled failure of the fallback query

DuckDBConnector::executeQuery throws on error (duckdb_connector.cpp). If the fallback SELECT * … LIMIT 0 fails (permissions, missing table, transient REST error), ATTACH now throws instead of returning false / skipping. The skipUnsupportedTable path catches BinderException from type conversion but not from the fallback SELECT itself. Please wrap the fallback or confirm throw-on-fallback is intended.

4. Duplicated magic check with different column orders

The RowCount == 1 && UNKNOWN / __ predicate is duplicated in two files with swapped column orders (data_type,column_name vs column_name,data_type). The indices are correct as written, but this is fragile — a future column reorder breaks silently. Please extract a small isPlaceholderSchema() helper (or at minimum comment the column order at each site).

Thanks for your questions and suggestions. All of them have been incorporated into the current commit.

@adsharma

Copy link
Copy Markdown
Contributor

Thank you!

@adsharma
adsharma merged commit 2d5f07a into LadybugDB:main Sep 24, 2026
2 checks passed
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