feat(ddl): disallow inserting by position, require source columns to be subset of target columns by NickCrews · Pull Request #11776 · ibis-project/ibis

NickCrews · 2025-11-21T19:50:33Z

After slowly picking apart the prerequisites for this PR, I think it is finally focused enough to actually tackle! Resolves #11731

This is breaking, but probably not for a huge fraction of users (???). Currently, on main, this works, aligning by position:

con.create_table("table", {"a": "int64", "b": "int64"})
con.insert("table", [{"a bogus column name": 4, "foobar": 5}])

After this PR, the above errors, which I think it is good.

Currently, on main, this (inserting data with anonymous column names) works, and after this PR it will continue to work. Originally in this PR I dropped support for this, hence you can see some comments in the thread below. But after that discussion we decided to keep supporting this.

con.insert("table", [(1,2), (3,4)])

A summary of the changes are

refactoring memtable() to have an internal variant, _memtable(), that ALSO provides analysis of whether the given data had column names vs was unnamed. eg [(1,2), (3,4)] is unnamed. The public API doesn't change, but we use this internal version to infer the columns during con.insert("table", [(1,2), (3,4)])
moved the test for the above from the duckdb backend to a more general test file so all backends get the test
added a _ensure_table_to_insert() utility method to the base SQL backend that all subclasses can use so they can share the verification and coercion logic.

NickCrews · 2025-11-21T19:53:40Z

@deepyaman this is related to your upsert PR, if you want to give it a look. I don't think there is any ordering needed between the two, but I do think it would be great if this PR was included in the same release as that PR, so that the .upsert() API never goes through a breaking change.

NickCrews · 2026-01-27T22:45:12Z

ibis/backends/duckdb/tests/test_client.py

    assert t.count().execute() == 2

-    con.insert(name, [(1,), (2,)])
-    assert t.count().execute() == 4


This actually is still supported if we keep the second commit, I just forgot to un-delete it.

This is now tested across all backends in test_insert_overwrite_from_list() in ibis/backends/tests/test_client.py

NickCrews · 2026-01-27T22:47:27Z

@cpcloud can I get confirmation this is the right idea before I do the rebase and fixup the tests?

gforsyth

Hey @NickCrews -- I think this is a good change/fix. I DO think we should keep positional inserts for tables without column names -- it does make everything messier, but it's a nice convenience.

I haven't taken a deep reviewing pass on this yet, but I think this is the right approach.

ibis/expr/api.py

ibis/backends/sql/__init__.py

NickCrews · 2026-02-18T03:18:17Z

Thanks for the reviews @gforsyth and @deepyaman , I'll get this fixed up per your suggestions!

ibis/backends/impala/ddl.py

NickCrews · 2026-02-19T18:15:23Z

This PR has gotten quite large. If you want, I can split it up a bit. I think the lowest hanging fruit would be to fix the impala DDL compiler first in a separate PR, that doesn't require any of the .insert() changes.

Then, potentially could split out the typing changes, adding the IntoMemtable TypeAlias.

Then, could actually get to the .insert() stuff.

NickCrews · 2026-02-20T17:50:34Z

@gforsyth and @deepyaman this is ready for your review (or telling me to split it into smaller PRs) whenever you get some time. Thanks! I'm also happy to video chat about any of this if that is easier at all.

gforsyth · 2026-02-20T19:23:43Z

Hey @NickCrews -- I think your suggestion to break up this PR is a good one.

@gforsyth

…fiers and strings (#11942) Before, it was inserting by position of the passed in table. I started this in #11776, but then that got too big, so this is just this bugfix for impala. Now the groundwork is laid for the rest of that PR. @gforsyth, this is ready for review as soon as it passes CI. Thanks!

… of target columns

NickCrews · 2026-02-26T18:14:21Z

ibis/backends/tests/test_client.py

    [
        param([{"a": 1, "b": 2}], [{"b": 22, "a": 11}], id="column order reversed"),
        param([{"a": 1, "b": 2}], [{"a": 11, "b": 22}], id="column order matching"),
-        param(


this is now tested in test_insert_overwrite_from_list

NickCrews · 2026-02-26T18:16:08Z

Boom, finally this is simplified and ready to go, whenever you get the chance @gforsyth @deepyaman @cpcloud

github-actions bot added tests Issues or PRs related to tests sql Backends that generate SQL labels Nov 21, 2025

NickCrews added the breaking change Changes that introduce an API break at any level label Nov 21, 2025

NickCrews changed the title ~~feat(ddl): disallow inserting by position, require source columns to be ubset of target columns~~ feat(ddl): disallow inserting by position, require source columns to be subset of target columns Nov 21, 2025

NickCrews force-pushed the insert-only-by-name branch from d67de9c to 6fe21f7 Compare January 25, 2026 20:38

github-actions bot added the duckdb The DuckDB backend label Jan 25, 2026

NickCrews force-pushed the insert-only-by-name branch 2 times, most recently from c9dd811 to d74fdf0 Compare January 25, 2026 21:04

github-actions bot added sqlite The SQLite backend clickhouse The ClickHouse backend snowflake The Snowflake backend labels Jan 26, 2026

NickCrews force-pushed the insert-only-by-name branch from da91ef7 to 036ecea Compare January 26, 2026 05:22

NickCrews commented Jan 27, 2026

View reviewed changes

NickCrews force-pushed the insert-only-by-name branch from 036ecea to 1b324bb Compare February 4, 2026 03:04

github-actions bot added the impala The Apache Impala backend label Feb 4, 2026

gforsyth reviewed Feb 17, 2026

View reviewed changes

ibis/expr/api.py Outdated Show resolved Hide resolved

ibis/expr/api.py Outdated Show resolved Hide resolved

ibis/backends/sql/__init__.py Outdated Show resolved Hide resolved

NickCrews force-pushed the insert-only-by-name branch from 1b324bb to 8052056 Compare February 19, 2026 07:04

gforsyth reviewed Feb 19, 2026

View reviewed changes

ibis/backends/impala/ddl.py Outdated Show resolved Hide resolved

NickCrews force-pushed the insert-only-by-name branch from b523876 to 305f9fd Compare February 19, 2026 22:50

NickCrews mentioned this pull request Feb 22, 2026

fix(impala): generate INSERT with explicit column order, quote identifiers and strings #11942

Merged

NickCrews mentioned this pull request Feb 25, 2026

feat: add IntoMemtable TypeAlias #11953

Merged

NickCrews marked this pull request as draft February 25, 2026 17:44

NickCrews force-pushed the insert-only-by-name branch from 305f9fd to 8eb38c7 Compare February 25, 2026 19:17

NickCrews mentioned this pull request Feb 25, 2026

fix(snowflake): fix return type of SnowflakeBackend.get_schema() #11954

Merged

feat(ddl): in .insert()/.upsert() require source columns to be subset…

7ae6006

… of target columns

NickCrews force-pushed the insert-only-by-name branch from 8eb38c7 to 7ae6006 Compare February 26, 2026 16:53

NickCrews marked this pull request as ready for review February 26, 2026 18:10

NickCrews commented Feb 26, 2026

View reviewed changes

Conversation

NickCrews commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NickCrews commented Nov 21, 2025

Uh oh!

NickCrews Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

NickCrews Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

NickCrews commented Jan 27, 2026

Uh oh!

gforsyth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NickCrews commented Feb 18, 2026

Uh oh!

Uh oh!

NickCrews commented Feb 19, 2026

Uh oh!

NickCrews commented Feb 20, 2026

Uh oh!

gforsyth commented Feb 20, 2026

Uh oh!

NickCrews Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

NickCrews commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NickCrews commented Nov 21, 2025 •

edited

Loading