Skip to content

feat(athena): Part 2 — fill the parse-path todo!() arms and the config alias - #16367

Open
aoelvp94 wants to merge 4 commits into
dbt-labs:mainfrom
aoelvp94:athena/part-2-core-wiring
Open

aoelvp94 wants to merge 4 commits into
dbt-labs:mainfrom
aoelvp94:athena/part-2-core-wiring

Conversation

@aoelvp94

@aoelvp94 aoelvp94 commented Sep 18, 2026 •

Copy link
Copy Markdown

Part of #16252. Related: #13822. Independent PR; the sequence is at the bottom.

Problem

With an athena profile parsing (#16274), dbt parse panics at the first todo!("Athena") it reaches, relation/factory.rs. Five of the nine todo!("Athena") arms are on the parse path; the other four are metadata, execution and dbt init, handled in later parts.

Solution

Fills the five parse-path arms. Each is the smallest thing that is correct for Athena, with the reason in a comment at the site:

  • relation/factory.rs: Athena joins the generic RelationStatic arm with the other plain-SQL adapters. Same three lines as feat(athena): AthenaDbConfig profile schema #16274; whichever merges second drops them on rebase.
  • adapter_impl.rs: the schema-listing column is schema_name, since Athena's information_schema is Trino's.
  • column/column_builder.rs (two arms): build_athena, structurally identical to build_exasol, delegating every type decision to sql_types, which already carries ATHENA_KEYS. Deliberately not routed through build_postgres_like, whose Timestamp -> "datetime" rendering that path documents as a legacy quirk.
  • relations/base.rs: microbatch event-time boundaries as TIMESTAMP '...' literals. The generic arm emits a bare varchar, which Trino rejects (Cannot apply operator: timestamp >= varchar); Trino's literal takes no T and no zone offset.
  • seed_io.rs: column-name inference is Lowercase, because the Glue catalog stores column names lowercase whatever the DDL says.

adapter.convert_type also returns dbt-athena's names for Athena: convert_text_type gives string and convert_number_type gives integer for whole numbers, with no width distinction. The macros compare against those names, so returning the DDL spellings varchar and bigint made an insert_overwrite model partitioned on a text or bigint column fail on its second run in get_partition_batches. Mapped at the convert_type boundary rather than in format_arrow_type_as_sql, which renders unit-test fixtures and seed DDL and must keep the Trino spellings and the integer widths.

Plus config_aliases: dbt-athena 1.11.0 declares _ALIASES = {"catalog": "database"} (connections_legacy.py), which canonicalises node config keys. Athena leaves the "no aliases" group and gets its own arm.

The two remaining adapter todo!("Athena") arms (MetadataAdapter construction, get_relation) are annotated, not filled; Part 5 fills them.

Verification

  • cargo fmt --check, cargo clippy on dbt-adapter, dbt-adapter-core, dbt-df-providers, dbt-schemas clean on the pinned toolchain.
  • With Parts 1, 3 and 4 applied on top, dbt parse on a 750-model project completes with 0 errors in 1.8 s; dbt ls exits 0. Without Part 4 it stops at the missing dbt-athena package, as feat(athena): AthenaDbConfig profile schema #16274 describes.

Feedback wanted: build_athena mirrors build_exasol rather than build_postgres_like; confirm that is the intended direction for Trino-family adapters.

Sequence (independent PRs, each compiles alone against main)

Checklist

  • I have read the contributing guide and understand what's expected of me.
  • I have run this code in development, and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required or relevant for this PR. (unit tests for the column builder and the timestamp literal are a pending follow-up on this branch)
  • This PR has no interface changes.

aoelvp94 and others added 3 commits September 18, 2026 12:47
…on ones

Resolves five of the nine todo!("Athena") sites — the ones parse can reach
— and leaves the two execution-layer sites in place with comments saying
why.

Filled:
- relation/factory.rs: Athena joins the generic RelationStatic arm with
  the other plain SQL adapters. Same change dbt#16274 makes.
- adapter_impl.rs: schema-listing column is `schema_name`, since Athena's
  information_schema is Trino's.
- column_builder.rs (two sites): a `build_athena` structurally identical
  to `build_exasol`, delegating every adapter-specific decision to
  `sql_types`, which already carries ATHENA_KEYS. Deliberately NOT routed
  through build_postgres_like, whose Timestamp -> "datetime" rendering
  is a legacy quirk that path documents as broken.
- relations/base.rs: microbatch event-time boundaries as explicit
  TIMESTAMP literals. The generic arm emits a bare varchar, which Trino
  rejects ("Cannot apply operator: timestamp >= varchar"); Trino's
  literal syntax also takes no 'T' and no zone offset.
- seed_io.rs: column-name inference is Lowercase, because the Glue
  catalog stores column names lowercase whatever the DDL says.

Left as todo!(), now annotated:
- metadata/get_relation.rs: needs an athena_get_relation querying
  information_schema over a live connection.
- adapter_impl.rs MetadataAdapter construction: needs an
  AthenaMetadataAdapter type against a live connection.

Both are rung-3 work that parse never reaches. Guessing them would be
worse than leaving an honest panic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dbt-athena 1.11.0 declares `_ALIASES = {"catalog": "database"}`
(connections_legacy.py), which canonicalises node config keys. Athena
leaves the "no aliases" group and gets its own arm.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@fpiped fpiped left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested merged with the rest of the stack against a real workgroup: debug, compile --write-catalog, docs generate work, and with Part 6 the materializations do too.

One divergence that bites: convert_type returns varchar and bigint for Athena where AthenaAdapter.convert_text_type / convert_number_type return string and integer. delete_overlapping_partitions and get_partition_batches only handle integer / string / date / timestamp, so an insert_overwrite model partitioned by a text or bigint column fails with Need to add support for column type varchar on its second run. The seed macros already map string to varchar themselves, so returning dbt-athena's names is safe there.

Landing note: #16374 touches the same Athena arms in adapter_impl.rs and get_relation.rs.

`adapter.convert_type` backs dbt-athena's `convert_text_type` and
`convert_number_type`, which return `string` and `integer` with no width
distinction. Fusion returned the DDL spellings `varchar` and `bigint`.

The macros compare against dbt-athena's names: `get_partition_batches` and
`delete_overlapping_partitions` handle only `integer`, `string`, `date`
and `timestamp`, so an `insert_overwrite` model partitioned on a text or
bigint column failed on its second run with "Need to add support for column
type varchar".

Mapped at the `convert_type` boundary rather than in
`format_arrow_type_as_sql`, which renders DDL for unit-test fixtures and
seeds and must keep the Trino spellings and the integer widths (`integer`
is 32-bit there, so a collapsed name overflows bigint values). The seed
macros reach the DDL spellings through `ddl_data_type`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants