fix: compute fixes for extension types #171
Conversation
| uidStorage, _ = scalar.MakeScalarParam(exampleUUID[:], | ||
| &arrow.FixedSizeBinaryType{ByteWidth: 16}) | ||
| uid = scalar.NewExtensionScalar(uidStorage, extensions.NewUUIDType()) |
There was a problem hiding this comment.
uuid is used as the name of the uuid package, so I'd end up with a conflict / issue if I use the name for the variable here. I can rename it as something else though if we prefer.
There was a problem hiding this comment.
Ah! uuidStorage and uuidScalar may be better. (I feel that uid is "user id"...)
| @@ -613,7 +632,7 @@ func executeScalarBatch(ctx context.Context, input compute.ExecBatch, exp expr.E | |||
| result.Release() | |||
There was a problem hiding this comment.
Do we need result = nil here? (Can we return Release()-ed result?)
There was a problem hiding this comment.
hmm, probably safer to make it nil you're right. just in case.
I can separate the change in the compute side from the change on the parquet side. While the changes on the compute side are fixing 2 different issues, they are intertwined and would end up having duplicate code between them if i separate them into different PRs. I'll split the PR into those two separate ones tomorrow. Essentially it's related to just reading UUID fields from parquet files and utilizing UUID extension types in the compute package to perform comparisons, along with a small error propagation issue. |
While working on github.com/apache/iceberg-go to enable reading data, I came across a few issues when dealing with UUID / extension types and some other small compute things. This fixes those issues and adds tests to account for them.