[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[ https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17071257#comment-17071257 ] ASF GitHub Bot commented on DRILL-7672: --- asfgit commented on pull request #2042: DRILL-7672: Make metadata type required when reading from / writing into Drill Metastore URL: https://github.com/apache/drill/pull/2042 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Make metadata type required when reading from / writing into Drill Metastore > > > Key: DRILL-7672 > URL: https://issues.apache.org/jira/browse/DRILL-7672 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Arina Ielchiieva >Assignee: Arina Ielchiieva >Priority: Major > Labels: ready-to-commit > Fix For: 1.18.0 > > > Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are > implemented). Each component metadata can have types. For examples, TABLES > metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP, > PARTITION. > During initial Metastore implementation when reading from / writing into > Metastore, metadata type was indicated in filter expressions. > For Iceberg Metastore where all data is stored in files this was not this > critical, basically when information is retrieved about the table, table > folder is queried. > For other Metastore implementations knowing metadata type can be more > critical. For example, RDBMS Metastore would store TABLES metadata in > different tables thus knowing which table to query would improve performance > rather than trying to query all tables. > Of course, we could traverse query filter and look for the hints which > metadata type is needed but it is much better to know required metadata type > beforehand without any extra logic. > Taking into account that Metastore metadata is queried only in Drill code, > developer knows beforehand what he needs to get / update / delete. > This Jira aims to make metadata type required when reading from / writing > into Drill Metastore. This change does not have any affect on the users, just > internal code refactoring. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[ https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17070792#comment-17070792 ] ASF GitHub Bot commented on DRILL-7672: --- arina-ielchiieva commented on issue #2042: DRILL-7672: Make metadata type required when reading from / writing into Drill Metastore URL: https://github.com/apache/drill/pull/2042#issuecomment-605866287 @vvysotskyi thanks, @paul-rogers is the PR ok to go? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Make metadata type required when reading from / writing into Drill Metastore > > > Key: DRILL-7672 > URL: https://issues.apache.org/jira/browse/DRILL-7672 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Arina Ielchiieva >Assignee: Arina Ielchiieva >Priority: Major > Labels: ready-to-commit > Fix For: 1.18.0 > > > Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are > implemented). Each component metadata can have types. For examples, TABLES > metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP, > PARTITION. > During initial Metastore implementation when reading from / writing into > Metastore, metadata type was indicated in filter expressions. > For Iceberg Metastore where all data is stored in files this was not this > critical, basically when information is retrieved about the table, table > folder is queried. > For other Metastore implementations knowing metadata type can be more > critical. For example, RDBMS Metastore would store TABLES metadata in > different tables thus knowing which table to query would improve performance > rather than trying to query all tables. > Of course, we could traverse query filter and look for the hints which > metadata type is needed but it is much better to know required metadata type > beforehand without any extra logic. > Taking into account that Metastore metadata is queried only in Drill code, > developer knows beforehand what he needs to get / update / delete. > This Jira aims to make metadata type required when reading from / writing > into Drill Metastore. This change does not have any affect on the users, just > internal code refactoring. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[ https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17069972#comment-17069972 ] ASF GitHub Bot commented on DRILL-7672: --- arina-ielchiieva commented on issue #2042: DRILL-7672: Make metadata type required when reading from / writing into Drill Metastore URL: https://github.com/apache/drill/pull/2042#issuecomment-605494807 @paul-rogers addressed code review comments. Had to do some refactoring to ensure we are always refer to `MetadataColumn` instead of its string representation, thus now PR contains slightly more files. @vvysotskyi you might want to take a look as well since I have touched some new areas. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Make metadata type required when reading from / writing into Drill Metastore > > > Key: DRILL-7672 > URL: https://issues.apache.org/jira/browse/DRILL-7672 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Arina Ielchiieva >Assignee: Arina Ielchiieva >Priority: Major > Labels: ready-to-commit > Fix For: 1.18.0 > > > Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are > implemented). Each component metadata can have types. For examples, TABLES > metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP, > PARTITION. > During initial Metastore implementation when reading from / writing into > Metastore, metadata type was indicated in filter expressions. > For Iceberg Metastore where all data is stored in files this was not this > critical, basically when information is retrieved about the table, table > folder is queried. > For other Metastore implementations knowing metadata type can be more > critical. For example, RDBMS Metastore would store TABLES metadata in > different tables thus knowing which table to query would improve performance > rather than trying to query all tables. > Of course, we could traverse query filter and look for the hints which > metadata type is needed but it is much better to know required metadata type > beforehand without any extra logic. > Taking into account that Metastore metadata is queried only in Drill code, > developer knows beforehand what he needs to get / update / delete. > This Jira aims to make metadata type required when reading from / writing > into Drill Metastore. This change does not have any affect on the users, just > internal code refactoring. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[
https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17069970#comment-17069970
]
ASF GitHub Bot commented on DRILL-7672:
---
arina-ielchiieva commented on pull request #2042: DRILL-7672: Make metadata
type required when reading from / writing into Drill Metastore
URL: https://github.com/apache/drill/pull/2042#discussion_r399689046
##
File path:
metastore/iceberg-metastore/src/test/java/org/apache/drill/metastore/iceberg/components/tables/TestIcebergTablesMetastore.java
##
@@ -297,6 +302,7 @@ public void testOverwrite() {
.workspace(tableInfo.workspace())
.tableName(tableInfo.name())
.metadataKey("dir0")
+ .metadataType(MetadataType.TABLE.name())
Review comment:
In `TableMetadataUnit` most of the data is stored as string to be suitable
for storage (for example, in the file), since this is unit test I had to create
`TableMetadataUnit` manually and thus indicate metadata type string value, in
the code we usually do not do this.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Make metadata type required when reading from / writing into Drill Metastore
>
>
> Key: DRILL-7672
> URL: https://issues.apache.org/jira/browse/DRILL-7672
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.17.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.18.0
>
>
> Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are
> implemented). Each component metadata can have types. For examples, TABLES
> metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP,
> PARTITION.
> During initial Metastore implementation when reading from / writing into
> Metastore, metadata type was indicated in filter expressions.
> For Iceberg Metastore where all data is stored in files this was not this
> critical, basically when information is retrieved about the table, table
> folder is queried.
> For other Metastore implementations knowing metadata type can be more
> critical. For example, RDBMS Metastore would store TABLES metadata in
> different tables thus knowing which table to query would improve performance
> rather than trying to query all tables.
> Of course, we could traverse query filter and look for the hints which
> metadata type is needed but it is much better to know required metadata type
> beforehand without any extra logic.
> Taking into account that Metastore metadata is queried only in Drill code,
> developer knows beforehand what he needs to get / update / delete.
> This Jira aims to make metadata type required when reading from / writing
> into Drill Metastore. This change does not have any affect on the users, just
> internal code refactoring.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[
https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17069969#comment-17069969
]
ASF GitHub Bot commented on DRILL-7672:
---
arina-ielchiieva commented on pull request #2042: DRILL-7672: Make metadata
type required when reading from / writing into Drill Metastore
URL: https://github.com/apache/drill/pull/2042#discussion_r399688777
##
File path:
metastore/iceberg-metastore/src/main/java/org/apache/drill/metastore/iceberg/components/tables/IcebergTables.java
##
@@ -42,16 +45,15 @@
*/
public class IcebergTables implements Tables,
MetastoreContext {
- public static final String STORAGE_PLUGIN = "storagePlugin";
- public static final String WORKSPACE = "workspace";
- public static final String TABLE_NAME = "tableName";
- public static final String METADATA_KEY = "metadataKey";
-
/**
* Metastore Tables component partition keys, order of partitioning will be
determined based
* on order in {@link List} holder.
*/
- private static final List PARTITION_KEYS =
Arrays.asList(STORAGE_PLUGIN, WORKSPACE, TABLE_NAME, METADATA_KEY);
+ private static final List PARTITION_KEYS = Arrays.asList(
+TableInfo.STORAGE_PLUGIN,
+TableInfo.WORKSPACE,
+TableInfo.TABLE_NAME,
+MetadataInfo.METADATA_KEY);
Review comment:
Actually this a good idea to pull all Metastore column names into one place.
I have created `MetastoreColumn` enum where all columns and their names are
stored. Also made sure that in the code we refer only to `MetastoreColumn` and
not to its string representation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Make metadata type required when reading from / writing into Drill Metastore
>
>
> Key: DRILL-7672
> URL: https://issues.apache.org/jira/browse/DRILL-7672
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.17.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.18.0
>
>
> Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are
> implemented). Each component metadata can have types. For examples, TABLES
> metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP,
> PARTITION.
> During initial Metastore implementation when reading from / writing into
> Metastore, metadata type was indicated in filter expressions.
> For Iceberg Metastore where all data is stored in files this was not this
> critical, basically when information is retrieved about the table, table
> folder is queried.
> For other Metastore implementations knowing metadata type can be more
> critical. For example, RDBMS Metastore would store TABLES metadata in
> different tables thus knowing which table to query would improve performance
> rather than trying to query all tables.
> Of course, we could traverse query filter and look for the hints which
> metadata type is needed but it is much better to know required metadata type
> beforehand without any extra logic.
> Taking into account that Metastore metadata is queried only in Drill code,
> developer knows beforehand what he needs to get / update / delete.
> This Jira aims to make metadata type required when reading from / writing
> into Drill Metastore. This change does not have any affect on the users, just
> internal code refactoring.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[
https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17069213#comment-17069213
]
ASF GitHub Bot commented on DRILL-7672:
---
paul-rogers commented on pull request #2042: DRILL-7672: Make metadata type
required when reading from / writing into Drill Metastore
URL: https://github.com/apache/drill/pull/2042#discussion_r399614183
##
File path:
metastore/iceberg-metastore/src/main/java/org/apache/drill/metastore/iceberg/components/tables/IcebergTables.java
##
@@ -42,16 +45,15 @@
*/
public class IcebergTables implements Tables,
MetastoreContext {
- public static final String STORAGE_PLUGIN = "storagePlugin";
- public static final String WORKSPACE = "workspace";
- public static final String TABLE_NAME = "tableName";
- public static final String METADATA_KEY = "metadataKey";
-
/**
* Metastore Tables component partition keys, order of partitioning will be
determined based
* on order in {@link List} holder.
*/
- private static final List PARTITION_KEYS =
Arrays.asList(STORAGE_PLUGIN, WORKSPACE, TABLE_NAME, METADATA_KEY);
+ private static final List PARTITION_KEYS = Arrays.asList(
+TableInfo.STORAGE_PLUGIN,
+TableInfo.WORKSPACE,
+TableInfo.TABLE_NAME,
+MetadataInfo.METADATA_KEY);
Review comment:
Thanks for cleaning up this code. If these are the four types, does it make
sense to define them in an enum, with the string keys as a property of the
enum? Maybe as part of `MetadataType`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Make metadata type required when reading from / writing into Drill Metastore
>
>
> Key: DRILL-7672
> URL: https://issues.apache.org/jira/browse/DRILL-7672
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.17.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.18.0
>
>
> Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are
> implemented). Each component metadata can have types. For examples, TABLES
> metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP,
> PARTITION.
> During initial Metastore implementation when reading from / writing into
> Metastore, metadata type was indicated in filter expressions.
> For Iceberg Metastore where all data is stored in files this was not this
> critical, basically when information is retrieved about the table, table
> folder is queried.
> For other Metastore implementations knowing metadata type can be more
> critical. For example, RDBMS Metastore would store TABLES metadata in
> different tables thus knowing which table to query would improve performance
> rather than trying to query all tables.
> Of course, we could traverse query filter and look for the hints which
> metadata type is needed but it is much better to know required metadata type
> beforehand without any extra logic.
> Taking into account that Metastore metadata is queried only in Drill code,
> developer knows beforehand what he needs to get / update / delete.
> This Jira aims to make metadata type required when reading from / writing
> into Drill Metastore. This change does not have any affect on the users, just
> internal code refactoring.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[
https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17069214#comment-17069214
]
ASF GitHub Bot commented on DRILL-7672:
---
paul-rogers commented on pull request #2042: DRILL-7672: Make metadata type
required when reading from / writing into Drill Metastore
URL: https://github.com/apache/drill/pull/2042#discussion_r399614392
##
File path:
metastore/iceberg-metastore/src/test/java/org/apache/drill/metastore/iceberg/components/tables/TestIcebergTablesMetastore.java
##
@@ -297,6 +302,7 @@ public void testOverwrite() {
.workspace(tableInfo.workspace())
.tableName(tableInfo.name())
.metadataKey("dir0")
+ .metadataType(MetadataType.TABLE.name())
Review comment:
Some of these use the `MetadataType.TABLE`, others
`MetadataType.TABLE.name()`. Do the tests make sure both work, or should they
be uniform?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Make metadata type required when reading from / writing into Drill Metastore
>
>
> Key: DRILL-7672
> URL: https://issues.apache.org/jira/browse/DRILL-7672
> Project: Apache Drill
> Issue Type: Improvement
>Affects Versions: 1.17.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>Priority: Major
> Labels: ready-to-commit
> Fix For: 1.18.0
>
>
> Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are
> implemented). Each component metadata can have types. For examples, TABLES
> metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP,
> PARTITION.
> During initial Metastore implementation when reading from / writing into
> Metastore, metadata type was indicated in filter expressions.
> For Iceberg Metastore where all data is stored in files this was not this
> critical, basically when information is retrieved about the table, table
> folder is queried.
> For other Metastore implementations knowing metadata type can be more
> critical. For example, RDBMS Metastore would store TABLES metadata in
> different tables thus knowing which table to query would improve performance
> rather than trying to query all tables.
> Of course, we could traverse query filter and look for the hints which
> metadata type is needed but it is much better to know required metadata type
> beforehand without any extra logic.
> Taking into account that Metastore metadata is queried only in Drill code,
> developer knows beforehand what he needs to get / update / delete.
> This Jira aims to make metadata type required when reading from / writing
> into Drill Metastore. This change does not have any affect on the users, just
> internal code refactoring.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (DRILL-7672) Make metadata type required when reading from / writing into Drill Metastore
[ https://issues.apache.org/jira/browse/DRILL-7672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17068808#comment-17068808 ] ASF GitHub Bot commented on DRILL-7672: --- arina-ielchiieva commented on pull request #2042: DRILL-7672: Make metadata type required when reading from / writing into Drill Metastore URL: https://github.com/apache/drill/pull/2042 # [DRILL-7672](https://issues.apache.org/jira/browse/DRILL-7672): Make metadata type required when reading from / writing into Drill Metastore ## Description 1. Upgraded Iceberg version and removed unneeded code for In / NotIn Expressions. 2. Updated Metastore Read / Write interfaces to support required metadata types: a. introduced abstract Read / Write classes with boilerplate code; b. added delete operation with filter and metadata type; c. added metadata type validator which checks supported metadata types for each component; d. made purge operation terminal; e. made necessary changes in REAME.md files. 3. Added / updated unit tests. ## Documentation NA ## Testing Added unit tests. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Make metadata type required when reading from / writing into Drill Metastore > > > Key: DRILL-7672 > URL: https://issues.apache.org/jira/browse/DRILL-7672 > Project: Apache Drill > Issue Type: Improvement >Affects Versions: 1.17.0 >Reporter: Arina Ielchiieva >Assignee: Arina Ielchiieva >Priority: Major > Fix For: 1.18.0 > > > Metastore consists of components: TABLES, VIEWS etc (so far only TABLES are > implemented). Each component metadata can have types. For examples, TABLES > metadata can be of the following types: TABLE, SEGMENT, FILE, ROW_GROUP, > PARTITION. > During initial Metastore implementation when reading from / writing into > Metastore, metadata type was indicated in filter expressions. > For Iceberg Metastore where all data is stored in files this was not this > critical, basically when information is retrieved about the table, table > folder is queried. > For other Metastore implementations knowing metadata type can be more > critical. For example, RDBMS Metastore would store TABLES metadata in > different tables thus knowing which table to query would improve performance > rather than trying to query all tables. > Of course, we could traverse query filter and look for the hints which > metadata type is needed but it is much better to know required metadata type > beforehand without any extra logic. > Taking into account that Metastore metadata is queried only in Drill code, > developer knows beforehand what he needs to get / update / delete. > This Jira aims to make metadata type required when reading from / writing > into Drill Metastore. This change does not have any affect on the users, just > internal code refactoring. > -- This message was sent by Atlassian Jira (v8.3.4#803005)
