You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tv42 (via GitHub)" <gi...@apache.org> on 2023/09/08 17:47:38 UTC
[GitHub] [arrow-datafusion] tv42 opened a new issue, #7511: Expose Constraints internals
tv42 opened a new issue, #7511:
URL: https://github.com/apache/arrow-datafusion/issues/7511
### Describe the bug
DataFusion v29.0.0 added commit 5f031465447e60d482d5236e2795f7975bbfe45d, which contains this change:
```diff
diff --git a/datafusion/expr/src/logical_plan/ddl.rs b/datafusion/expr/src/logical_plan/ddl.rs
index e005f1147..dc247da36 100644
--- a/datafusion/expr/src/logical_plan/ddl.rs
+++ b/datafusion/expr/src/logical_plan/ddl.rs
@@ -222,8 +217,8 @@ impl Hash for CreateExternalTable {
pub struct CreateMemoryTable {
/// The table name
pub name: OwnedTableReference,
- /// The ordered list of columns in the primary key, or an empty vector if none
- pub primary_key: Vec<Column>,
+ /// The list of constraints in the schema, such as primary key, unique, etc.
+ pub constraints: Constraints,
/// The logical plan
pub input: Arc<LogicalPlan>,
/// Option to not error if table already exists
```
So, previously, `primary_key` was just a `Vec`, and `Column` was public. Now it's `Constraints`, and `Constraints` has only private fields and no accessor methods: https://docs.rs/datafusion/latest/datafusion/common/struct.Constraints.html
Previously, I could take a logical plan and introspect it. Now I can't.
Can we make `Constraint` public, and either make the inner of `Constraints` public, or add an accessor method?
The change should be pretty simple, but I'm trying to understand the thinking behind the current API.
Cc @mustafasrepo @metesynnada @ozankabak
### To Reproduce
```rust
let statement = session_state.sql_to_statement(sql, sql_dialect)?;
let logical_plan = session_state.statement_to_plan(statement).await?;
match logical_plan {
datafusion::logical_expr::LogicalPlan::Ddl(
datafusion::logical_expr::DdlStatement::CreateMemoryTable(create_table),
) => {
for key in &create_table.primary_key {
println!("key {} ref {:?}", key.name, key.relation);
}
_ => (),
}
```
### Expected behavior
Be able to introspect the LogicalPlan.
### Additional context
_No response_
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #7511: Expose Constraints internals
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7511:
URL: https://github.com/apache/arrow-datafusion/issues/7511#issuecomment-1725262422
I think this would be a great first issue:
1. Add `Constraints::iter()` and `Contraints::into_inner() --> Vec<Constraint>`
2. Write some tests
--
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
[GitHub] [arrow-datafusion] tv42 commented on issue #7511: Expose Constraints internals
Posted by "tv42 (via GitHub)" <gi...@apache.org>.
tv42 commented on issue #7511:
URL: https://github.com/apache/arrow-datafusion/issues/7511#issuecomment-1725770795
I have a simple patch publishes `Constraint`, implements `Deref` to `&[Constraint]` -- that seems like the right thing, and gives iteration for free. I need to read the contributing guide, but the code is trivial.
--
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
[GitHub] [arrow-datafusion] alamb commented on issue #7511: Expose Constraints internals
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7511:
URL: https://github.com/apache/arrow-datafusion/issues/7511#issuecomment-1726628242
Thanks @tv42
--
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
[GitHub] [arrow-datafusion] ozankabak commented on issue #7511: Expose Constraints internals
Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on issue #7511:
URL: https://github.com/apache/arrow-datafusion/issues/7511#issuecomment-1712134163
> Can we make Constraint public, and either make the inner of Constraints public, or add an accessor method (maybe an iterator)?
This sounds reasonable to me.
--
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
[GitHub] [arrow-datafusion] alamb closed issue #7511: Expose Constraints internals
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7511: Expose Constraints internals
URL: https://github.com/apache/arrow-datafusion/issues/7511
--
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