Introduce DeltaLakeFileSystemFactory and VendedCredentialsProvider in Delta Lake
#26281
Conversation
chenjian2664
force-pushed
the
delta_filesystem
branch
from
ae9e745 to
4b6507d
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
...no-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/VendedCredentialsHandle.java
Outdated
Show resolved
Hide resolved
...no-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/VendedCredentialsHandle.java
Outdated
Show resolved
Hide resolved
…ider in Delta Lake
chenjian2664
force-pushed
the
delta_filesystem
branch
from
cc0c8cb to
1273970
Compare
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Show resolved
Hide resolved
| @@ -118,10 +118,11 @@ public AbstractDeltaLakePageSink( | |||
| List<DeltaLakeColumnHandle> inputColumns, | |||
| List<String> originalPartitionColumns, | |||
| PageIndexerFactory pageIndexerFactory, | |||
| TrinoFileSystemFactory fileSystemFactory, | |||
| DeltaLakeFileSystemFactory fileSystemFactory, | |||
There was a problem hiding this comment.
I'm wondering how you replaced TrinoFileSystemFactory with DeltaLakeFileSystemFactory. For instance, VacuumProcedure still uses TrinoFileSystemFactory. Is it intentional?
Also, what happens if the future developers introduce new classes with TrinoFileSystemFactory instead of DeltaLakeFileSystemFactory?
There was a problem hiding this comment.
how you replaced TrinoFileSystemFactory with DeltaLakeFileSystemFactory
honesty, I just look at every place that using TrinoFileSystemFactory and then replace it to DeltaLakeFileSystemFactory.
VacuumProcedure still uses TrinoFileSystemFactory. Is it intentional?
It's not intentional, I will update it to use DeltaLakeFileSystemFactory
what happens if the future developers introduce new classes TrinoFileSystemFactory instead of DeltaLakeFileSystemFactory
Good question, I'll investigate on it
…rovider` in Delta Lake
|
@ebyhr I just have a try on ecad7c6 for adapting the |
…entialsProvider` in Delta Lake" This reverts commit ecad7c6.
…rovider` in Delta Lake
…rovider` in Delta Lake
...no-delta-lake/src/main/java/io/trino/plugin/deltalake/DefaultDeltaLakeFileSystemFactory.java
Outdated
Show resolved
Hide resolved
...in/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/metastore/VendedCredentials.java
Outdated
Show resolved
Hide resolved
| @@ -24,4 +25,6 @@ public interface LocatedTableHandle | |||
| boolean managed(); | |||
|
|
|||
| String location(); | |||
|
|
|||
| VendedCredentialsHandle toCredentialsHandle(); | |||
There was a problem hiding this comment.
Do we still need the above managed() and location()? VendedCredentialsHandle contains those information in my understanding.
There was a problem hiding this comment.
we are not holding the instance of VendedCredentialsHandle in LocatedTableHandle sub classes, or
do you mean change the method to default? i,e boolean managed() to
default boolean managed()
{
return toCredentialsHandle().managed();
}
…rovider` in Delta Lake
Description
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: