Use a struct for ProjectionExpr

[adriangb]

#17395 (comment)

This makes code more legible and easier to follow.

[alamb]

Thanks @adriangb

I agree this is a nicer API. The only think I worry about is the disruption to existing users .

I left a suggestion that I think would reduce the API churn needed substantially. Let me know what you think

[alamb]

We might be able to make the API change easier to handle by using a From impl:

so like instead of

vec![
 (col("c1", schema.as_ref()).unwrap(), "c1".to_string()),
]

something like

vec![
 ProjectionExpr::from(col("c1", schema.as_ref()).unwrap(), "c1".to_string()),
]

This would mean that people upgrading could simply add ProjectionExpr::from

[alamb]

Going one step farther, I think we could make ProjectionExec generic, like this

impl ProjectionExec {
    /// Create a projection on an input
    pub fn try_new(
        expr: impl IntoIterator<Item=ProjectionExpr>,
        input: Arc<dyn ExecutionPlan>,
    ) -> Result<Self> {

That would let people use the same code mostly without change

Update: I tried it out and it seems to work pretty well. Check out

• Keep ProjectionExec mostly backwards compatible pydantic/datafusion#38

[adriangb]

Nice yeah I was thinking of doing that but thought it best to update our internal tests anyway.

The unfortunate part is that there's no way to mark an implementation as deprecated: ideally we'd allow this for a couple releases then remove it to simplify things, but that's not possible. I guess we keep it around forever.