use default fpp for writing in Hive connector if not provided in table metadata#16589
Conversation
|
@Praveen2112 Hey, I was not sure who to tag so I tagged you as a reviewer because I noticed that you made changes to orc bloom filter previously. I hope you don't mind :) |
fa34f4f to
3b1b292
Compare
|
Hi ✋ PTAL whenever you have time. Thx :) |
Praveen2112
left a comment
There was a problem hiding this comment.
It looks good to me but can we add some test coverage here ? But even the table properties are default - doesn't hive specify them when creating a table ?
I will add it soon.
Yes, it does not specify when creating a table. when I do show create tables after creating in hive, it does not add When you look up
So one can wonder where does this
|
3b1b292 to
e565c47
Compare
|
Added test coverage when there isn't fpp value |
e53674e to
9907887
Compare
|
PTAL rebased and updated some minor changes :) |
Praveen2112
left a comment
There was a problem hiding this comment.
Thanks for fixing this issue.
plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergFileWriterFactory.java
Outdated
Show resolved
Hide resolved
9907887 to
c88edf0
Compare
607e0f5 to
3ef12a1
Compare
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
@Praveen2112 is this still good to go and we can merge? |
|
@Chaho12 Thanks for working on this. |
Description
I think that if table metadata does not have FPP value, default fpp value should be used for bloom filter for writing ORC files.
As of now, it returns error saying that
FPP for bloom filter is missing, which is ironic asAdditional context and related issues
Hive has default bloom filter of 0.05 (5%)
Trino doc saids that default value of fpp is 0.05
Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text: