search

apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
6.37k stars 1.2k forks source link

Add support for Utf8View to string_to_array and array_to_string #13403

Closed Omega359 closed 4 days ago

Omega359 commented 1 week ago

Which issue does this PR close?

Closes #13383

Rationale for this change

Completing support for Utf8View in all functions.

What changes are included in this PR?

Code tests

Are these changes tested?

Yes

Are there any user-facing changes?

No

Omega359 commented 1 week ago

Do not merge - this PR does not properly handle largeutf8 and I want to try to refactor the code to reduce the # of lines of code

alamb commented 1 week ago

Converting to draft while @Omega359 makes the changes described in https://github.com/apache/datafusion/pull/13403#issuecomment-2483441618

Omega359 commented 5 days ago

Ready for review again.

Omega359 commented 4 days ago

I do wonder if we really need this level of specialization, but I also think since this is clearly better than what is on main we can/should merge it in

That is a very good point @alamb and one that I don't have an answer to. Is there any guidelines anywhere as to what should be covered and what shouldn't? From my reading of functions its all over the place with no consistency at all.

alamb commented 4 days ago

That is a very good point @alamb and one that I don't have an answer to. Is there any guidelines anywhere as to what should be covered and what shouldn't? From my reading of functions its all over the place with no consistency at all.

I agree -- there is no guideline that I know of. Any chance you would be willing to propose one?

alamb commented 4 days ago

Let's go with this and iterate on improvements going forward

Omega359 commented 3 days ago

I agree -- there is no guideline that I know of. Any chance you would be willing to propose one?

I'll have to think about it tbh @alamb. Part of that is to know the cost to go from utf8 -> utf8View / utf8view -> utf8. I remember seeing a comment somewhere but my search-fu isn't advanced enough to find it again