pavansai1997 opened a new pull request, #76921:
URL: https://github.com/apache/airflow/pull/76921
Since #76363, `AdbcHook.insert_rows` loads rows with the driver's bulk
ingest by default (`fast_executemany=True`). The batch is built in
`target_fields` order and passed to `cursor.adbc_ingest(table, mode="append")`,
which sends no column list. DuckDB's ADBC driver appends ingested columns by
position, so when `target_fields` is not every table column in table order:
- reordered columns are written to the wrong columns, and the task succeeds;
- a subset of columns fails with `InternalError: incorrect column count in
AppendDataChunk`, which the `NotSupportedError` fallback does not catch.
Reproduced through `AdbcHook.insert_rows` with DuckDB 2.0.0 and a table
`people(first_name, last_name, note DEFAULT 'n/a')`, inserting John Smith:
| `target_fields` | `fast_executemany=False` | `fast_executemany=True`,
before | `fast_executemany=True`, after |
|---|---|---|---|
| `["last_name", "first_name", "note"]` | `('John', 'Smith', 'x')` |
`('Smith', 'John', 'x')` | `('John', 'Smith', 'x')` |
| `["first_name", "last_name"]` | `('John', 'Smith', 'n/a')` |
`InternalError` | `('John', 'Smith', 'n/a')` |
With this change, bulk ingest is only used when the batch holds every table
column in table order, which includes the default `target_fields=None`. Other
column lists are bound to the generated `INSERT` statement, as with
`fast_executemany=False`.
Drivers that match ingested columns by name, such as SQLite's, also bind for
these column lists now, so they lose the bulk ingest speed-up there, which is
the same behaviour as before 0.2.0. Reordering the batch into table order could
keep bulk ingest for full reorderings in a follow-up.
Tests:
- `test_insert_rows_target_fields_order_preserved` asserted that a reordered
subset was ingested. It now asserts that the batch is bound, and still checks
the batch order.
- A new parametrized test covers all columns in table order (ingested), all
columns reordered (bound) and a subset (bound). The bound cases fail without
this change.
- `test_insert_rows_sqlite_round_trip` inserts a subset, so it now binds for
both values of `fast_executemany`; its docstring is updated to say so.
- A new `test_insert_rows_sqlite_bulk_ingest` checks that rows holding every
column in table order still go through the real SQLite driver's bulk ingest.
The comment in the connection docs example is updated too: the example
inserts a subset of the table's columns, so it no longer uses bulk ingest.
The bug shipped in `apache-airflow-providers-apache-arrow` 0.2.0.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Code Puppy
Generated-by: Code Puppy following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
