Skip to content

feat(athena): Part 3 — accept partitioned_by, table_properties and force_batch as model configs - #16368

Open
aoelvp94 wants to merge 3 commits into
dbt-labs:mainfrom
aoelvp94:athena/part-3-model-config-keys
Open

aoelvp94 wants to merge 3 commits into
dbt-labs:mainfrom
aoelvp94:athena/part-3-model-config-keys

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

WarehouseSpecificNodeConfig has no Athena keys. On a real project dbt parse reports every dbt-athena model config as UnusedConfigKey (partitioned_by x27, table_properties, force_batch) and every +table_properties block in dbt_project.yml as a SerializationError ("Custom keys must go under +meta"). 35 errors on a 750-model project, all this one class.

Solution

Adds every Athena model config the vendored macros read through config.get(...), following the existing per-warehouse pattern: struct field in common.rs; the _eq binding, && chain and named tuple in same_warehouse_config; the +key twin in ProjectModelConfig with its is_some(); the two conversions in model_config.rs; and the five exhaustive struct literals (source_config.rs, unit_test_config.rs, seed_config.rs, snapshot_config.rs, data_test_config.rs).

26 keys in total. partitioned_by, table_properties and force_batch are the three dbt parse flagged; the other 23 are bucket_count, bucketed_by, delete_condition, external_location, field_delimiter, ha, insert_condition, lf_grants, lf_inherited_tags, lf_tags_config, merge_update_columns_default_rule, merge_update_columns_rules, native_drop, partitions_limit, s3_data_dir, s3_data_naming, s3_tmp_table_dir, seed_by_insert, seed_s3_upload_args, temp_schema, update_condition, versions_to_keep, write_compression. Each is a hard error (UnusedConfigKey) when a model sets it, not a warning. format, unique_tmp_table_suffix and table_type are already declared for other adapters with the types Athena needs.

Note for reviewers: the compiler enforces the five struct literals but not the _eq bindings and the && chain. A key added to WarehouseSpecificNodeConfig and missed there compiles and silently makes state:modified blind to that config, so the test added here sets each of the 26 keys in turn and asserts same_warehouse_config notices.

Verification

  • cargo fmt --check, cargo clippy -p dbt-schemas --all-targets --all-features and cargo nextest run -p dbt-schemas (673 passed) clean on the pinned toolchain, including the new athena_model_configs_are_compared guard test.
  • dbt parse on the 750-model project: 35 errors to 0.

Feedback wanted: whether declaring each key individually is the shape you want here, or whether an adapter-owned passthrough would suit a config surface this wide.

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.
  • This PR has no interface changes (config schema gains keys that were previously rejected).

aoelvp94 and others added 2 commits September 18, 2026 12:47
… as model configs

`dbt parse` on a real 757-model Athena project (clean main) finished in
2.1 s with 35 errors, all of one class: these three dbt-athena model
configs are unknown to Fusion's config schema. 27 × `partitioned_by` and
one each of `table_properties`/`force_batch` as UnusedConfigKey, plus six
SerializationErrors for `+table_properties.*` blocks in dbt_project.yml
("Custom keys must go under +meta"). Nothing else failed — no Jinja,
macro, package or override errors across the whole project.

Extends the existing `// Athena — XXX: incomplete` section of
WarehouseSpecificNodeConfig, which held only `table_type`. Types follow
in-file precedent: `partitioned_by` like `dist` (StringOrArrayOfStrings),
`table_properties` like ClickHouse `settings` (BTreeMap<String, YmlValue>),
`force_batch` like `as_columnstore` (bool_or_string_bool).

A new warehouse config key touches many sites, and only some are
compiler-enforced:
  common.rs        struct field (enforced); `<k>_eq` let, `&& <k>_eq`, and
                   the named diff tuple in same_warehouse_config (NOT
                   enforced — a missed one compiles and silently blinds
                   state:modified to that key)
  model_config.rs  `+<k>`-renamed twin in ProjectModelConfig — the
                   dbt_project.yml path does not reuse the struct; the
                   is_some() predicate; two conversions (not enforced)
  seed/snapshot/source/data_test/unit_test_config.rs
                   exhaustive `WarehouseSpecificNodeConfig { .. }` literals
                   (enforced — E0063 caught all five). Set to None there
                   with a comment, following the in-file precedent for
                   `description: None, // Only for BigQuery models`: these
                   are model configs, and the parse flagged only models
                   and the models: tree of dbt_project.yml.
All sites are done for each of the three keys.

Deliberately scoped to the keys the parse flagged. dbt-athena's macros
read ~26 more (bucketed_by, bucket_count, external_location, s3_data_dir,
s3_data_naming, s3_tmp_table_dir, lf_tags_config, lf_grants, ...); each
is the same multi-site change and belongs in a follow-up, not folded in
here where a silent omission would be hard to review.

Co-Authored-By: Claude Opus 5 <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 with the stack: model configs the dbt-athena macros read are hard errors when set on a model (UnusedConfigKey), because only partitioned_by, table_properties and force_batch are declared. Hit in practice: ha and s3_data_naming. The full set of Athena-specific keys the macros config.get(...): bucket_count, bucketed_by, delete_condition, external_location, field_delimiter, format, ha, insert_condition, lf_grants, lf_inherited_tags, lf_tags_config, merge_update_columns_default_rule, merge_update_columns_rules, native_drop, partitions_limit, s3_data_dir, s3_data_naming, s3_tmp_table_dir, seed_by_insert, seed_s3_upload_args, temp_schema, unique_tmp_table_suffix, update_condition, versions_to_keep, write_compression (plus partitioned_by, table_properties, force_batch, table_type, which already pass).

The three keys this branch already declared were the ones `dbt parse`
flagged on one project. The macros read many more through
`config.get(...)`, and each undeclared key is an `UnusedConfigKey` hard
error when a model sets it; `ha` and `s3_data_naming` were hit in practice.

Adds the remaining 23: bucket_count, bucketed_by, delete_condition,
external_location, field_delimiter, ha, insert_condition, lf_grants,
lf_inherited_tags, lf_tags_config, merge_update_columns_default_rule,
merge_update_columns_rules, native_drop, partitions_limit, s3_data_dir,
s3_data_naming, s3_tmp_table_dir, seed_by_insert, seed_s3_upload_args,
temp_schema, update_condition, versions_to_keep, write_compression.
`format` and `unique_tmp_table_suffix` are already declared for other
adapters with the types Athena needs, as is `table_type`.

Also adds a guard test. The compiler enforces the exhaustive struct
literals in the sibling config modules, but not the `_eq` bindings and the
`&&` chain in `same_warehouse_config`: a key added to
`WarehouseSpecificNodeConfig` and missed there compiles and silently makes
`state:modified` blind to that config. The test sets each of the 26 Athena
keys in turn and asserts the comparison notices.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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