You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ByteBaker (via GitHub)" <gi...@apache.org> on 2026/07/12 18:54:17 UTC
[PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
ByteBaker opened a new pull request, #10327:
URL: https://github.com/apache/arrow-rs/pull/10327
# Which issue does this PR close?
- Closes #1002.
# Rationale for this change
`parquet_derive` is a proc-macro crate, but it currently has a normal dependency on `parquet`. This causes `parquet` and its dependency graph to be compiled for the proc-macro host even though the macro only needs to emit references to Parquet APIs.
As discussed in the issue, derive crates such as `serde_derive` avoid depending normally on the runtime crate. The generated code is instead resolved against the consumer's direct dependency.
# What changes are included in this PR?
- Replaces compile-time use of `parquet::basic::Type` with a private physical-type classifier.
- Keeps generated `::parquet::...` paths unchanged.
- Removes `parquet` as a normal dependency of `parquet_derive`.
- Retains `parquet` as a dev-dependency for doctests.
- Removes the now-unnecessary `extern crate parquet`.
# Are these changes tested?
Yes. Ran:
- `cargo test -p parquet_derive` passed.
- `cargo test -p parquet_derive_test` passed.
- `cargo clippy -p parquet_derive --all-features -- -D warnings`.
- `cargo fmt --all -- --check`.
- `cargo tree -p parquet_derive --edges normal`, confirming its normal dependencies are now limited to `proc-macro2`, `quote`, and `syn`.
# Are there any user-facing changes?
None.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#issuecomment-4999924065
Happy to contribute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585339650
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arms come from `self.ty.physical_type()`, but the said method never really returned `INT96` or constructed it inside the implementation.
Which is why I did not add an `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585483958
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
makes sense to remove if it was already unreachable, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585339650
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arm comes from `self.ty.physical_type()`, but the said method never originally returned `INT96` or constructed it inside the implementation.
Which is why I did not add an `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey merged PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#issuecomment-4952362375
@alamb please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3584599827
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
int96 disappears?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#issuecomment-4999884975
thanks @ByteBaker
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585339650
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arms come from `self.ty.physical_type()`, but the said method never really returned `INT96` or constructed it inside the implementation.
Which is why I did not add a `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ske6data (via GitHub)" <gi...@apache.org>.
ske6data commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585336511
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arms come from `self.ty.physical_type()`, but the said method never really returned `INT96` or constructed it inside the implementation.
Which is why I did not add a `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ByteBaker (via GitHub)" <gi...@apache.org>.
ByteBaker commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585339650
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arms come from `self.ty.physical_type()`, but the said method never originally returned `INT96` or constructed it inside the implementation.
Which is why I did not add an `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: remove parquet dependency from parquet_derive [arrow-rs]
Posted by "ske6data (via GitHub)" <gi...@apache.org>.
ske6data commented on code in PR #10327:
URL: https://github.com/apache/arrow-rs/pull/10327#discussion_r3585336511
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -299,28 +312,25 @@ impl Field {
// becomes a column named `type` in the parquet schema
let field_name = self.ident.unraw().to_string();
let physical_type = match self.ty.physical_type() {
- parquet::basic::Type::BOOLEAN => quote! {
+ PhysicalType::Boolean => quote! {
::parquet::basic::Type::BOOLEAN
},
- parquet::basic::Type::INT32 => quote! {
+ PhysicalType::Int32 => quote! {
::parquet::basic::Type::INT32
},
- parquet::basic::Type::INT64 => quote! {
+ PhysicalType::Int64 => quote! {
::parquet::basic::Type::INT64
},
- parquet::basic::Type::INT96 => quote! {
- ::parquet::basic::Type::INT96
Review Comment:
Sorry for not explaining this.
The match arms come from `self.ty.physical_type()`, but the said method never really returned `INT96` or constructed it inside the implementation.
Which is why I did not add a `Int96` variant in the new (private to `parquet_derive`) `PhysicalType`.
```rust
enum PhysicalType {
Boolean,
Int32,
Int64,
Float,
Double,
ByteArray,
FixedLenByteArray,
}
```
At the same time, I also noticed that INT96 has been marked deprecated inside `parquet/src/basic.rs`, making it unlikely that anyone in the future would construct this type.
```rust
enum Type {
INT96 = 3; // deprecated, only used by legacy implementations.
}
```
Which convinced me that this can be safely removed. All it does is remove an unreachable branch.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org