Skip to content

Conversation

@felipecrv
Copy link
Contributor

No description provided.

…d fields (apache#47)

Co-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
@felipecrv felipecrv requested a review from zeroshade as a code owner August 6, 2025 00:20
@felipecrv felipecrv requested review from Copilot and removed request for zeroshade August 6, 2025 00:20
@github-actions github-actions bot added this to the ADBC Libraries 20 milestone Aug 6, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in parsing repeated records with nested fields in the BigQuery ADBC driver. The fix removes special case handling for repeated records and instead applies a unified approach where any repeated field becomes a list type.

  • Removes complex conditional logic for handling repeated record fields with single vs multiple nested fields
  • Simplifies the code by applying list wrapping to all repeated fields uniformly after type construction
  • Adds comprehensive test coverage for various combinations of repeated and non-repeated record fields

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
go/adbc/driver/bigquery/connection.go Refactors buildField function to fix repeated record parsing by removing special case logic and applying unified list wrapping
go/adbc/driver/bigquery/connection_test.go Adds new test file with comprehensive test cases for buildField function covering various field type scenarios
Comments suppressed due to low confidence (1)

go/adbc/driver/bigquery/connection.go:810

  • [nitpick] The variable name 'nestedFieldSchema' is inconsistent with the original code style. Consider using 'nestedSchema' to maintain consistency with the existing codebase pattern.
		for i, nestedFieldSchema := range schema.Schema {

@felipecrv felipecrv requested a review from zeroshade August 6, 2025 00:20
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks!

@lidavidm lidavidm merged commit 637fc28 into apache:main Aug 7, 2025
45 checks passed
@felipecrv felipecrv deleted the bq-array-record branch August 8, 2025 17:55
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.

3 participants