Spark: Add register Truncate UDF#3708
Conversation
| import org.junit.Test; | ||
| import org.junit.runners.Parameterized; | ||
|
|
||
| public class TestPartitionedWritesAsSelect extends SparkCatalogTestBase { |
There was a problem hiding this comment.
I think you can use SparkTestBaseWithCatalog instead. No need to run this with multiple catalogs since this is testing direct registration with Spark.
There was a problem hiding this comment.
Thank you for pointing out, I will understand the latest code
| AssertHelpers.assertThrows("should registered temporary function" + | ||
| "nor a permanent function registered in the database 'default'", | ||
| AnalysisException.class, | ||
| "Undefined function: 'iceberg_bucket8'", |
There was a problem hiding this comment.
I don't think it is necessary to test the case where you reference a function that has not been registered.
There was a problem hiding this comment.
Thanks for your review. This code is really redundant
| } | ||
|
|
||
| @Test | ||
| public void testInsertAsSelectWithTruncateFailure() { |
There was a problem hiding this comment.
Why test these failure cases? I think we test these elsewhere and it doesn't make sense to fail if something gets better later. I'd remove this and the test above.
rdblue
left a comment
rdblue
left a comment
There was a problem hiding this comment.
Overall, I think this is fine to have if anyone wants to call the truncate functions from Spark. I'd fix a couple of tests before merging though.
|
@xiaotianzhang01 do you want to take a look at test failures and update this? It would be good to get it into the next release! |
Timzhang01
force-pushed
the
registerTruncate
branch
from
c20fed6 to
42d52bf
Compare
|
@rdblue Thank you for your comment. Sorry, the reply is a bit late, I have updated the pr and successfully ran ut locally |
|
@rdblue Do you have time to take a look? |
|
Thanks, @xiaotianzhang01! |
2 participants
When writing to the partition table, we need to register the truncate transform function in Spark to specify it during sort.
issue: #3707
cc/ @rdblue @aokolnychyi