Skip to content

Add ability to define composite dtypes, i.e. dtypes redefining meaning of another dtype - #103

Merged
robert3005 merged 3 commits into
developfrom
rk/composite-types
Mar 15, 2024
Merged

robert3005 merged 3 commits into
developfrom
rk/composite-types

Conversation

@robert3005

@robert3005 robert3005 commented Mar 14, 2024 •

Copy link
Copy Markdown
Contributor

I think we might want to add a pointer to the composite type to let you customize behaviour in some cases. Fix #100

@robert3005
robert3005 enabled auto-merge (squash) March 14, 2024 14:28
Comment thread vortex-array/src/composite_dtypes.rs Resolved
@robert3005
robert3005 requested a review from gatesn March 14, 2024 14:46

@gatesn gatesn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure what you mean by a pointer?

Comment thread vortex-array/src/arrow/convert.rs Resolved
Comment thread vortex-array/src/arrow/convert.rs Resolved
pub fn map(key_type: DType, value_type: DType) -> DType {
DType::Composite(
Arc::new(MAP_DTYPE.to_string()),
Box::new(DType::Struct(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This isn't what an Arrow map represents I don't think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is how it's logically made. The physical representation is different.

@robert3005

Copy link
Copy Markdown
Contributor Author

I mean in some cases you will want to prefrom operation based on dtype. Maybe every composite dtype needs an encoding.

@robert3005
robert3005 merged commit 4f831b7 into develop Mar 15, 2024
@robert3005
robert3005 deleted the rk/composite-types branch March 15, 2024 23:24
@robert3005

Copy link
Copy Markdown
Contributor Author

I have been thinking about the map case more and I think in our system we have to include the list explicitly? Overall it’s unclear how to use a composite type and minimally for any composite type there should be an associated encoding?

robert3005 pushed a commit that referenced this pull request Nov 11, 2026
## Summary

Fixes #11364.

Closing a Spark reader currently leaves its Java-owned native read
handles and file caches alive until Cleaner processing. Make that
release explicit and idempotent, while preserving the resources still
needed by derived scans, returned partitions and Arrow batches. Ports
the lifecycle fix from [QC
#103](quantcast#103).

## Changes

- Make `DataSource` and `Scan` implement `AutoCloseable`, retaining
Cleaner fallback. Keep native calls synchronized with explicit close;
fence scan options so their expressions remain alive across JNI.
- Release partitions prefetched by `hasNext()` but never returned.
- Let Spark's owning wrappers close at EOF. Reader and footer cleanup
attempts every close in ownership order and preserves the original
failure.
- Release native partition ownership when Arrow stream allocation or
import fails.
- Reject reads on closed handles with `IllegalStateException`. An open,
exhausted scan still returns `false` from `hasNext()` and throws
`NoSuchElementException` from `next()`.

---------

Signed-off-by: Christopher Wirt <[email protected]>
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.

Add Composite DType

2 participants