feat(gooddata-pipelines): support composite key references on parent datasets by benkeanna · Pull Request #1608 · gooddata/gooddata-python-sdk

benkeanna · 2026-05-12T14:09:13Z

Summary

https://gooddata.atlassian.net/browse/MCMIC-2430

Adds a new ParentDatasetReference type and a new optional parent_dataset_references: list[ParentDatasetReference] field on CustomDatasetDefinition, allowing callers to express composite-key joins to the parent dataset (e.g. 2-column foreign key) — a shape the underlying gooddata_sdk already supports via list[CatalogDeclarativeReferenceSource] but the wrapper currently restricts to a single column.

Driven by a real need on the MIC BCA tooling: BCA datasets reference parent dim datasets that can have composite primary keys, and the existing wrapper API couldn't express that.

Backward compatibility

Legacy fields parent_dataset_reference_attribute_id, dataset_reference_source_column, dataset_reference_source_column_data_type remain accepted but are now optional and marked deprecated=True (Pydantic emits a DeprecationWarning on access).
A check_reference_form_exclusive validator rejects mixing the legacy fields with the new parent_dataset_references — chosen over silent precedence to avoid surprising callers later when one path is ignored.
The processor uses parent_dataset_references when set, otherwise falls back to the legacy form. Existing single-column callers see no behavior change.

Test plan

pytest tests/ — 189 passing (composite-only branch).
ruff check src/ tests/ — clean.
Validate behavior end-to-end against a real workspace with a composite-key parent dim.

🤖 Generated with Claude Code

codecov · 2026-05-12T14:12:59Z

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.99%. Comparing base (efc6b2a) to head (ff2c33d).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
...ooddata_pipelines/ldm_extension/input_processor.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1608      +/-   ##
==========================================
+ Coverage   78.83%   78.99%   +0.15%     
==========================================
  Files         230      231       +1     
  Lines       15486    15603     +117     
==========================================
+ Hits        12208    12325     +117     
  Misses       3278     3278

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

janmatzek · 2026-05-12T14:26:52Z

+    parent_dataset_reference_attribute_id: str | None = Field(
+        default=None,
+        deprecated=(
+            "Use `parent_dataset_references` for richer (composite-key) joins. "


I'd leave out the "for richer ... joins" part in the deprecation messages.

janmatzek · 2026-05-12T14:32:33Z

+            "Composite-key reference to the parent dataset. When provided and "
+            "non-empty, supersedes the legacy single-column reference fields."


I'd go with something like "List of references to parent datasets." or something This phrasing assumes that join on multiple keys is the default use case (llm is going off the conversation context) but for the most users, joining on one field will be more than enough.

janmatzek · 2026-05-12T14:36:48Z

+        Forcing callers to pick one form prevents silent precedence surprises:
+        without this check, setting both would quietly use the new list and
+        ignore the legacy values, which is easy to miss when debugging.


I'd delete this - it sounds more like AI reasoning than a useful docstring (It tries to justify why the code is present, rather than explain what the code is doing)

janmatzek · 2026-05-12T14:39:45Z

+          one-element list. Missing legacy fields yield an empty list, which
+          will be rejected downstream by the GoodData API.


actually, would it not be better to fail fast instead of waiting for a an API call to fail?

janmatzek · 2026-05-12T14:40:54Z

+            # `parent_dataset_references` list takes precedence when set and
+            # non-empty; otherwise fall back to the legacy single-column fields.


this is just repeating the docstring of the function that is called. This only needs to exist in one place.

janmatzek · 2026-05-13T12:22:00Z

+| parent_dataset_reference_attribute_id | string \| None | **Deprecated** — single-column reference to the parent attribute. Use `parent_dataset_references` instead. |
+| dataset_reference_source_column | string \| None | **Deprecated** — single-column name on the custom dataset. Use `parent_dataset_references` instead. |
+| dataset_reference_source_column_data_type | [ColumnDataType](#columndatatype) \| None | **Deprecated** — column data type for the single-column reference. Use `parent_dataset_references` instead. |
+| parent_dataset_references | [ParentDatasetReference](#parentdatasetreference)[] \| None | Composite-key reference to the parent dataset (one entry per join column). When set, supersedes the three legacy single-column fields above. |


I'd consider using just

**Deprecated** — use `parent_dataset_references` instead.

janmatzek · 2026-05-13T12:23:06Z


 Either `dataset_source_table` or `dataset_source_sql` must be specified with a truthy value, but not both. An exception will be raised if both parameters are falsy or if both have truthy values.

+The parent-dataset reference can be expressed via either the three legacy fields (`parent_dataset_reference_attribute_id`, `dataset_reference_source_column`, `dataset_reference_source_column_data_type`) or the new `parent_dataset_references` list — never both. Mixing the two forms raises a `ValidationError`. New code should prefer `parent_dataset_references`; the legacy fields will be removed in a future release.


I'd just lead the user straight to the new field. No need to mention the legacy fields here

janmatzek · 2026-05-13T12:24:21Z

+| parent_dataset_reference_attribute_id | string \| None | **Deprecated** — single-column reference to the parent attribute. Use `parent_dataset_references` instead. |
+| dataset_reference_source_column | string \| None | **Deprecated** — single-column name on the custom dataset. Use `parent_dataset_references` instead. |
+| dataset_reference_source_column_data_type | [ColumnDataType](#columndatatype) \| None | **Deprecated** — column data type for the single-column reference. Use `parent_dataset_references` instead. |
+| parent_dataset_references | [ParentDatasetReference](#parentdatasetreference)[] \| None | Composite-key reference to the parent dataset (one entry per join column). When set, supersedes the three legacy single-column fields above. |


I think the reference to the composite key is "too much information" here.

janmatzek · 2026-05-13T12:34:34Z

+        assert definition.parent_dataset_reference_attribute_id is not None
+        assert definition.dataset_reference_source_column is not None
+        assert definition.dataset_reference_source_column_data_type is not None


Maybe it would be better to explicitly throw an exception when any of these is None? I know it should be unreachable because of the validators, but still.

didn't notice the comments on docs are still unresolved, sorry

…datasets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

janmatzek requested changes May 12, 2026

View reviewed changes

benkeanna force-pushed the aben/composite-refs branch 3 times, most recently from 9017191 to a1f565a Compare May 13, 2026 06:54

benkeanna marked this pull request as ready for review May 13, 2026 11:42

benkeanna requested review from hkad98, jaceksan, lupko and pcerny as code owners May 13, 2026 11:42

janmatzek reviewed May 13, 2026

View reviewed changes

benkeanna force-pushed the aben/composite-refs branch from a1f565a to 65f1088 Compare May 13, 2026 12:34

janmatzek previously approved these changes May 13, 2026

View reviewed changes

feat(gooddata-pipelines): support composite key references on parent …

ff2c33d

…datasets Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

benkeanna force-pushed the aben/composite-refs branch from 65f1088 to ff2c33d Compare May 13, 2026 12:58

janmatzek approved these changes May 13, 2026

View reviewed changes

benkeanna merged commit 0c299a0 into master May 13, 2026
14 checks passed

benkeanna deleted the aben/composite-refs branch May 13, 2026 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gooddata-pipelines): support composite key references on parent datasets#1608

feat(gooddata-pipelines): support composite key references on parent datasets#1608
benkeanna merged 1 commit into
masterfrom
aben/composite-refs

benkeanna commented May 12, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 12, 2026 •

edited

Loading

Uh oh!

janmatzek May 12, 2026

Uh oh!

janmatzek May 12, 2026

Uh oh!

janmatzek May 12, 2026

Uh oh!

janmatzek May 12, 2026

Uh oh!

janmatzek May 12, 2026

Uh oh!

janmatzek May 13, 2026

Uh oh!

janmatzek May 13, 2026

Uh oh!

janmatzek May 13, 2026

Uh oh!

janmatzek May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		"Composite-key reference to the parent dataset. When provided and "
		"non-empty, supersedes the legacy single-column reference fields."

		one-element list. Missing legacy fields yield an empty list, which
		will be rejected downstream by the GoodData API.

		# `parent_dataset_references` list takes precedence when set and
		# non-empty; otherwise fall back to the legacy single-column fields.


		Either `dataset_source_table` or `dataset_source_sql` must be specified with a truthy value, but not both. An exception will be raised if both parameters are falsy or if both have truthy values.

		The parent-dataset reference can be expressed via either the three legacy fields (`parent_dataset_reference_attribute_id`, `dataset_reference_source_column`, `dataset_reference_source_column_data_type`) or the new `parent_dataset_references` list — never both. Mixing the two forms raises a `ValidationError`. New code should prefer `parent_dataset_references`; the legacy fields will be removed in a future release.

Conversation

benkeanna commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Backward compatibility

Test plan

Uh oh!

codecov Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

benkeanna commented May 12, 2026 •

edited

Loading

codecov Bot commented May 12, 2026 •

edited

Loading