Enhance Avro handling in Result package with decimal support and union extraction#98
Enhance Avro handling in Result package with decimal support and union extraction#98amol-patel wants to merge 2 commits intomainfrom
Conversation
…n extraction - Introduced `extractUnionValue` and `decodeDecimal` functions for better handling of Avro union types and decimal values. - Updated `FromAvro` method to utilize new functions for processing records. - Enhanced `Column` struct to include `Scale` and `AvroTypeName` for improved metadata extraction during JSON unmarshalling. - Refined `Type` unmarshalling to support complex types and improve error handling.
There was a problem hiding this comment.
Pull request overview
This PR improves Avro result decoding in the result package by unwrapping goavro union values more robustly and adding support for Avro logical decimal types, while enhancing schema/type unmarshalling to better handle unions and complex type objects.
Changes:
- Updated
FromAvroto unwrap union-encoded values viaextractUnionValueand to decode logical decimals viadecodeDecimal. - Added JSON unmarshalling logic to
column.Columnto capture Avro metadata needed for decoding (e.g., decimal scale, type name). - Refined
column.TypeJSON unmarshalling to handle union arrays and complex type objects more safely.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/result/result.go | Adds union unwrapping + decimal decoding during Avro row extraction. |
| pkg/result/column/type.go | Improves Type unmarshalling for unions and complex types. |
| pkg/result/column/column.go | Adds custom JSON unmarshalling to extract decimal/type metadata from Avro schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test PlanBugQuerying tables with When results are returned, decimal columns appear as base64-encoded byte arrays instead of numeric values. Root Cause
Changes
Automated Tests
go test ./pkg/result/... -v # 20/20 passingManual Tests
No changes to Snowflake, Trino, ClickHouse, or other plugin result handling |
hladush
left a comment
There was a problem hiding this comment.
Please add unit tests for your changes
Why do you need these fields if you fully ignore them?
Column.Type.UnmarshalJSON only handled simple strings ("string") and string arrays (["null", "string"]). Avro represents DECIMAL(38,2) as complex JSON objects ({"type": "fixed", "logicalType": "decimal", ...}) or nullable variants (["null", {"type": "fixed", ...}]), which caused the unmarshal crash. Manual Tests
Teststype_test.txt (14 tests) — Type unmarshaling for all Avro type formats, Column decimal metadata extraction, full schema test matching the production table |

extractUnionValueanddecodeDecimalfunctions for better handling of Avro union types and decimal values.FromAvromethod to utilize new functions for processing records.Columnstruct to includeScaleandAvroTypeNamefor improved metadata extraction during JSON unmarshalling.Typeunmarshalling to support complex types and improve error handling.