Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support user defined display for UDF #10376

Closed
jayzhan211 opened this issue May 4, 2024 · 2 comments · Fixed by #10417
Closed

Support user defined display for UDF #10376

jayzhan211 opened this issue May 4, 2024 · 2 comments · Fixed by #10417
Labels
enhancement New feature or request

Comments

@jayzhan211
Copy link
Contributor

jayzhan211 commented May 4, 2024

Is your feature request related to a problem or challenge?

The feature request is based on the need that I would like get_field(expr, key) to displayed as expr[key].

two reason

  1. Mimic the pattern as Expr::GetIndexedField for Remove Expr::GetIndexedField and GetFieldAccess and always use function get_field for indexing #10374
  2. expr[key] looks nicer!

Expr::GetIndexedField(GetIndexedField { field, expr }) => match field {
GetFieldAccess::NamedStructField { name } => {
write!(f, "({expr})[{name}]")
}
GetFieldAccess::ListIndex { key } => write!(f, "({expr})[{key}]"),
GetFieldAccess::ListRange {
start,
stop,
stride,
} => {
write!(f, "({expr})[{start}:{stop}:{stride}]")
}
},

Given the current function to build the name has only a fixed pattern, I think enable user-defined display is a good idea

fn create_function_name(fun: &str, distinct: bool, args: &[Expr]) -> Result<String> {
let names: Vec<String> = args.iter().map(create_name).collect::<Result<_>>()?;
let distinct_str = match distinct {
true => "DISTINCT ",
false => "",
};
Ok(format!("{}({}{})", fun, distinct_str, names.join(",")))
}

Describe the solution you'd like

No response

Adding display_name to ScalarUDFImpl

Describe alternatives you've considered

No response

Additional context

No response

@jayzhan211 jayzhan211 added the enhancement New feature or request label May 4, 2024
@yyy1000
Copy link
Contributor

yyy1000 commented May 5, 2024

Looks like it doesn't need a lot of changes.
Since I worked on another issue related to display_name, I want to take this. :)

@yyy1000
Copy link
Contributor

yyy1000 commented May 5, 2024

I'm working on it and I found after #10325 is merged, it will avoid a conflict. Given it's close to be merged, I think we can wait a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants