fix(destination-motherduck): fix for camelCase columns being NULL - #70438
Conversation
…olumn definition, fixing e.g. the issue that columns with camel-cased names would always be null if the source has case-sensitive columns names.
Added integration tests for this behavior as well, and make sure we skip ambiguous data such as {"a": 1, "A": 2}.
👋 Welcome to Airbyte!Thank you for your contribution from Donnype/airbyte! We're excited to have you in the Airbyte community. Helpful Resources
PR Slash CommandsAs needed or by request, Airbyte Maintainers can execute the following slash commands on your PR:
If you have any questions, feel free to ask in the PR comments or join our Slack community. Tips for Working with CI
|
Add changelog entry
|
/run-connector-tests
|
|
Donny Peeters (@Donnype) - This unfortunately got hit by a merge conflict when we merged+released the other. Do you mind updating and pinging me here when I can re-review? |
|
/ai-prove-fix
|
🧪 AI Prove Fix - Validation In ProgressSession URL: https://app.devin.ai/sessions/bc255d4b838642939ff31ac7162c4eb3 I'm validating PR #70438 (Destination MotherDuck: fix for camelCase columns being Current StatusInvestigating the fix and preparing test plan... 📋 Initial AssessmentPR Summary: This fix addresses a bug where columns with camelCase names (e.g., Fix Approach: Creates a mapping ( Blockers Identified:
This comment will be updated with the full test plan and results. Stand by... |
🧪 Fix Validation SummaryVerdict: NO DECISION - MORE DATA NEEDED
Evidence: PR has blockers that prevent live testing - merge conflict and version conflict need resolution. Next Steps:
📋 Connector & PR DetailsDetailsConnector: 🎯 Validation Thesis & Test PlanSuccess ThesisIf this fix works, columns with camelCase names (e.g., Failure ThesisIf this fix doesn't work, camelCase columns would still be Test Plan (Blocked)Primary Test Connections Identified:
Healthy Connection for Regression Testing:
Testing Scope Rationale: This is a functional code change that modifies how data fields are matched to schema columns. Batch testing on 2-3 additional connections would be needed to verify no regressions across different source types and configurations. ✅ Pre-flight ChecksChecks
🚫 Blockers Preventing Live Testing1. Merge ConflictThe PR has a merge conflict with master that needs to be resolved by the contributor. AJ noted this in comment. 2. Version ConflictThe PR bumps to version 3. Metadata Validation ErrorThe The Session URL: https://app.devin.ai/sessions/bc255d4b838642939ff31ac7162c4eb3 Once the blockers are resolved, I can publish a pre-release and run live connection tests to validate the fix. |
…f github.com:Donnype/airbyte into donnypy/fix-for-empty-columns-motherduck-destination
|
Aaron ("AJ") Steers (@aaronsteers) I fixed the conflicts, although I did not redirected the errors yet like you suggested, not sure where to put the error array yet. |
|
🙋 Escalating per Hands-Free AI Triage Project triage. Reason: Community PR has been stalled 21+ days. Merge conflict and version conflict (0.2.1 already published) remain unresolved since Jan 29 request. Escalated to #human-in-the-loop for a maintainer to either assist the contributor or take over the fix. Requested by Aaron ("AJ") Steers (@aaronsteers). |
|
Aaron ("AJ") Steers (@aaronsteers) Conflicts had been resolved earlier, can we move this forward and discuss redirecting the errors in a separate PR? |
NULLNULL
|
/run-connector-tests
|
eecb22a
into
airbytehq:master
What
More generally:
The
UnicodeAwareNormalizerchanges the column names in the target schema based on a set of rules:The current implementation of the destination code has a bug where the values in the columns that get a new name after normalization will be set to
NULLalways, because the normalized names were checked against the original column names.How
This fix creates a mapping between the old and new column names to use the right names (see
normalized_keys). It also checks if we "lose" columns in this normalization step, meaning the source data keys/columns were not unique after normalization, e.g.:{ "userid": 1, "userId": 2 }(I am not aware of any guarantee that this cannot happen within Airbyte, in which case we could remove the check.)
I updated the test to incorporate a camel case key and verified that invalid source data is skipped.
Review guide
User Impact
The columns should be ingested properly after this fix. This might require a full refresh.
Can this PR be safely reverted and rolled back?