Skip to content

Spec: Remove misleading statement about source-ids#12948

Merged
RussellSpitzer merged 1 commit intoapache:mainfrom
wypoon:spec_source_ids_cleanup
May 1, 2025
Merged

Spec: Remove misleading statement about source-ids#12948
RussellSpitzer merged 1 commit intoapache:mainfrom
wypoon:spec_source_ids_cleanup

Conversation

@wypoon
Copy link
Copy Markdown
Contributor

@wypoon wypoon commented May 1, 2025

In #12644, the spec was clarified that for a partition field, source-id should always be written if has a single-argument transform, and source-ids should be written instead if it has a multi-argument transform.

There is a statement left over that conflicts with the above. In this PR, we remove that statement.

@github-actions github-actions Bot added the Specification Issues that may introduce spec changes. label May 1, 2025
@wypoon
Copy link
Copy Markdown
Contributor Author

wypoon commented May 1, 2025

@Fokko you missed this.
cc @rdblue

@wypoon
Copy link
Copy Markdown
Contributor Author

wypoon commented May 1, 2025

cc @RussellSpitzer @jbonofre

Copy link
Copy Markdown
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Good catch @wypoon

@jbonofre
Copy link
Copy Markdown
Member

jbonofre commented May 1, 2025

That's correct: source-id and source-ids are exclusive.
Source-Id for single arg transforms. Source-ids for multi args transforms.

@RussellSpitzer
Copy link
Copy Markdown
Member

Merging this because we already voted on this in general and this is essentially just a typo we forgot to remove.

@RussellSpitzer RussellSpitzer merged commit 61e8ace into apache:main May 1, 2025
2 checks passed
@RussellSpitzer
Copy link
Copy Markdown
Member

Thanks @wypoon for the fix! Thanks @jbonofre and @Fokko for reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Specification Issues that may introduce spec changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants