Skip to content

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Apr 1, 2025

No description provided.

@wgtmac wgtmac force-pushed the from_arrow_schema branch 2 times, most recently from 524fa68 to 0f4faef Compare April 1, 2025 08:41
@wgtmac
Copy link
Member Author

wgtmac commented Apr 1, 2025

@lidavidm @zhjwpku Please take a look when you have time. Thanks!

@wgtmac wgtmac force-pushed the from_arrow_schema branch from 0f4faef to d4c0068 Compare April 1, 2025 12:34
Copy link
Collaborator

@zhjwpku zhjwpku left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@wgtmac
Copy link
Member Author

wgtmac commented Apr 2, 2025

Thanks @lidavidm @zhjwpku!

@Fokko @Xuanwo Could you help review and merge this?


int32_t GetFieldId(const ArrowSchema& schema) {
if (schema.metadata == nullptr) {
return kUnknownFieldId;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now. Later on, we can fetch the field IDs from name mapping if they are not in the metadata. Or we can assign fresh IDs in case we want to create a new table from an Arrow schema 👍

@Fokko Fokko merged commit d05a9b2 into apache:main Apr 3, 2025
6 checks passed
@Fokko
Copy link
Contributor

Fokko commented Apr 3, 2025

Thanks @wgtmac for working on this, and thanks @lidavidm and @zhjwpku for the review 🙌

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants