[FEAT]: sql image_encode and image_resize #2764
Conversation
…d303/Daft into sql-image-functions-2
…d303/Daft into sql-image-functions-2
…d303/Daft into sql-image-functions-2
CodSpeed Performance ReportMerging #2764 will degrade performances by 51.78%Comparing Summary
Benchmarks breakdown
|
universalmind303
changed the title
| arg, | ||
| operator: FunctionArgOperator::Assignment, | ||
| } => match name.value.as_ref() { | ||
| "w" | "width" => { |
There was a problem hiding this comment.
Hmm how would be document this behavior? I'm guessing we're going to need a separate documentation page for SQL (or at least some kind of unified documentation across SQL and Python expressions) in which we should also document the argument naming behavior?
…image-functions-2
…image-functions-2
…image-functions-2
| } | ||
|
|
||
| #[typetag::serde] | ||
| impl ScalarUDF for ImageEncode { |
There was a problem hiding this comment.
Not relevant to this PR but why do we call this ScalarUDF when it is not user defined? Might be a good item to change when we clean up our expressions during clean up week.
There was a problem hiding this comment.
the idea was that they are "user defined", but these initial ones are managed by us. Eventually exposing an interface for third party udfs using the same underlying system.
| SQLExpr::Tuple(_) => unsupported_sql_err!("TUPLE"), | ||
| // Similar to rust and python conventions, tuples always have a fixed size, | ||
| // so we convert them to a fixed size list. | ||
| SQLExpr::Tuple(exprs) => { |
There was a problem hiding this comment.
How are tuples in SQL used? As in, do we have a supported SQL query/expression using tuples right now? Just curious
There was a problem hiding this comment.
yes, this here updates our sql support to include support for tuples.
you can use them anywhere exprs are accepted now.
fsl_from_sql = daft.sql_expr('(1, 2, 3)')this is the functional equivalent of
fsl_from_python = daft.lit(
(1, 2, 3)
).cast(daft.DataType.fixed_size_list(daft.DataType.int64(), 3))
|
Feel free to merge once you make all the requested changes btw |
depends on #2757