You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/11/21 07:42:31 UTC
[GitHub] [drill] Leon-WTF opened a new pull request #2384: Add mongodb Metastore
Leon-WTF opened a new pull request #2384:
URL: https://github.com/apache/drill/pull/2384
# [DRILL-8015](https://issues.apache.org/jira/browse/DRILL-8015): Add MongoDB Metastore implementation
## Description
Add a new metastore implementation backened by mongoDB
## Documentation
TODO
## Testing
Test by ut
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-975571642
> @Leon-WTF Thanks for the contribution, this pull request is great! After completing the unit test and docs, please let me know, we can start a review.
I have finished the ut, they are all passed. You can start the review, I will update the doc while replying the comments.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-989444420
> @Leon-WTF @luocooong, w.r.t to the release of 1.20 and having read this PR's comments I just want to check if we're deadlocked here with author waiting for reviewers and reviewers waiting for author?
@dzamo We can progress now, I will try my best to solve the comments by this weekend, is that ok for you to delay two days before freezing the master branch?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-975432320
@Leon-WTF Thanks for the contribution, this pull request is great!
After completing the unit test and docs, please let me know, we can start a review.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r765752883
##########
File path: metastore/metastore-api/src/test/java/org/apache/drill/metastore/TestData.java
##########
@@ -55,7 +55,7 @@ public static TableMetadataUnit basicTableMetadataUnit() {
.lastModifiedTime(System.currentTimeMillis())
.partitionKeys(Collections.singletonMap("dir0", "2018"))
.additionalMetadata("additional test metadata")
- .metadataIdentifier("part_int=3/part_varchar=g/0_0_0.parquet")
+ .metadataIdentifier(MetadataInfo.GENERAL_INFO_KEY)
Review comment:
yes, the original value is not match with the real case.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r764881739
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.ConnectionString;
+import com.mongodb.MongoClientSettings;
+import com.mongodb.MongoCredential;
+import com.mongodb.ServerAddress;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private static final Logger logger = LoggerFactory.getLogger(MongoMetastore.class);
+
+ private final ConnectionString clientURI;
+ private final MongoClient client;
+
+ public MongoMetastore(DrillConfig config) {
+ this.clientURI =
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION));
+ this.client = getClient();
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ public MongoClient getClient() {
Review comment:
`private` the keyword if this function is not called publicly.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.ConnectionString;
+import com.mongodb.MongoClientSettings;
+import com.mongodb.MongoCredential;
+import com.mongodb.ServerAddress;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private static final Logger logger = LoggerFactory.getLogger(MongoMetastore.class);
+
+ private final ConnectionString clientURI;
+ private final MongoClient client;
+
+ public MongoMetastore(DrillConfig config) {
+ this.clientURI =
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION));
+ this.client = getClient();
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ public MongoClient getClient() {
+ List<String> hosts = clientURI.getHosts();
+ List<ServerAddress> addresses = Lists.newArrayList();
+ for (String host : hosts) {
+ addresses.add(new ServerAddress(host));
+ }
+ return getClient(addresses);
+ }
+
+ public MongoClient getClient(List<ServerAddress> addresses) {
+ // Take the first replica from the replicated servers
+ ServerAddress serverAddress = addresses.get(0);
+ MongoCredential credential = clientURI.getCredential();
+ String userName = credential == null ? null : credential.getUserName();
+ logger.info("Created connection to {} by {}.", serverAddress, userName);
+ MongoClientSettings.Builder settings = MongoClientSettings.builder()
+ .applyToClusterSettings(builder -> builder.hosts(addresses));
+ if (credential != null) {
+ settings.credential(credential);
+ }
+ return MongoClients.create(settings.build());
+ }
+
+ @Override
+ public void close() {
Review comment:
Need to close the mongo client(s).
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/transform/OperationTransformer.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.transform;
+
+import org.apache.drill.metastore.expressions.FilterExpression;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.operate.Delete;
+import org.apache.drill.metastore.mongo.operate.MongoOperation;
+import org.apache.drill.metastore.mongo.operate.Overwrite;
+
+import java.util.List;
+
+/**
+ * Base class to transforms given input into {@link MongoOperation} implementations.
+ *
+ * @param <T> Metastore component unit type
+ */
+public abstract class OperationTransformer<T> {
+
+ protected final MongoMetastoreContext<T> context;
+
+ protected OperationTransformer(MongoMetastoreContext<T> context) {
+ this.context = context;
+ }
+
+ public Delete toDelete() {
Review comment:
Rename ***toDeleteAll()*** ?
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/transform/OutputDataTransformer.java
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.transform;
+
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.bson.Document;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Base class to convert list of {@link Document}
+ * into Metastore component units for the given list of column names.
+ *
+ * @param <T> Metastore component unit type
+ */
+public abstract class OutputDataTransformer<T> {
+ private final Map<String, MethodHandle> unitSetters;
+ private final List<String> columns = new ArrayList<>();
+ private final List<Document> documents = new ArrayList<>();
+
+ protected OutputDataTransformer(Map<String, MethodHandle> unitSetters) {
+ this.unitSetters = unitSetters;
+ }
+
+ public OutputDataTransformer<T> columns(List<String> columns) {
+ this.columns.addAll(columns);
+ return this;
+ }
+
+ public OutputDataTransformer<T> documents(List<Document> documents) {
+ this.documents.addAll(documents);
+ return this;
+ }
+
+ /**
+ * Converts given list of {@link Document} into Metastore component units.
+ * Specific for each Metastore component.
+ *
+ * @return list of Metastore component units
+ */
+ public abstract List<T> execute();
+
+ /**
+ * For each given record prepares specific methods handler and its value
+ * to be set into Metastore specific component unit.
+ * Ignores absent setters for columns and null values.
+ *
+ * @return list of methods handlers and values to set
+ */
+ protected List<Map<MethodHandle, Object>> valuesToSet() {
+ List<Map<MethodHandle, Object>> results = Lists.newLinkedList();
+ for (Document d : documents) {
Review comment:
Would be better to define as ***doc***.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.ConnectionString;
+import com.mongodb.MongoClientSettings;
+import com.mongodb.MongoCredential;
+import com.mongodb.ServerAddress;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private static final Logger logger = LoggerFactory.getLogger(MongoMetastore.class);
+
+ private final ConnectionString clientURI;
+ private final MongoClient client;
+
+ public MongoMetastore(DrillConfig config) {
+ this.clientURI =
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION));
+ this.client = getClient();
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ public MongoClient getClient() {
+ List<String> hosts = clientURI.getHosts();
+ List<ServerAddress> addresses = Lists.newArrayList();
+ for (String host : hosts) {
+ addresses.add(new ServerAddress(host));
+ }
+ return getClient(addresses);
+ }
+
+ public MongoClient getClient(List<ServerAddress> addresses) {
Review comment:
It's getting too complicated...
We can use the `ConnectionString` quickly to create mongo client(s).
```java
MongoClient client = MongoClients.create(new ConnectionString("mongodb://[username:password@]host1[:port1][,host2[:port2],...[,hostN[:portN]]][/[database.collection][?options]]"));
```
Also, can use the `Builder` to create the mongo client(s).
```java
List<ServerAddress> servers = new ArrayList<>();
servers.add(new ServerAddress("host:port"));
// ...
MongoClientSettings.Builder settingsBuilder = MongoClientSettings.builder();
settingsBuilder.applyToClusterSettings(builder -> builder.hosts(servers));
MongoClient client = MongoClients.create(settingsBuilder.build());
```
See also [Class ConnectionString](http://mongodb.github.io/mongo-java-driver/4.4/apidocs/mongodb-driver-core/com/mongodb/ConnectionString.html)
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/config/MongoConfigConstants.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.config;
+
+import org.apache.drill.metastore.config.MetastoreConfigConstants;
+
+/**
+ * Drill Mongo Metastore configuration which is defined
+ * in {@link MetastoreConfigConstants#MODULE_RESOURCE_FILE_NAME} file.
+ */
+public interface MongoConfigConstants {
+
+ /**
+ * Drill Mongo Metastore configuration properties namespace.
+ */
+ String BASE = MetastoreConfigConstants.BASE + "mongo.";
+
+ /**
+ * Mongo Metastore data source url property. Required.
+ */
+ String CONNECTION = BASE + "connection";
+
+ /**
+ * Mongo Metastore database.
+ */
+ String DATABASE = "meta";
Review comment:
Will users be allowed to define these values in the `drill-metastore-module.conf`?
1. Get the storage db / table name from the configuration.
2. If not present, using this default values.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/exception/MongoMetastoreException.java
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.exception;
+
+import org.apache.drill.metastore.exceptions.MetastoreException;
+
+/**
+ * Specific Mongo Drill Metastore runtime exception to indicate exceptions thrown
+ * during Mongo Drill Metastore code execution.
+ */
+public class MongoMetastoreException extends MetastoreException {
Review comment:
The serializable class MongoMetastoreException does not declare a static final serialVersionUID field of type long.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/components/tables/TablesOperationTransformer.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.operate.Overwrite;
+import org.apache.drill.metastore.mongo.transform.OperationTransformer;
+import org.bson.Document;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Metastore Tables component operations transformer that provides mechanism
+ * to convert {@link TableMetadataUnit} data to Metastore overwrite / delete operations.
+ */
+public class TablesOperationTransformer extends OperationTransformer<TableMetadataUnit> {
+
+ public TablesOperationTransformer(MongoMetastoreContext<TableMetadataUnit> context) {
+ super(context);
+ }
+
+ /**
+ * Groups given list of {@link TableMetadataUnit}, convert them to list of overwrite operations
+ *
+ * @param units Metastore component units
+ * @return list of overwrite operations
+ */
+ @Override
+ public List<Overwrite> toOverwrite(List<TableMetadataUnit> units) {
+ return context.transformer().inputData()
+ .units(units)
+ .execute().stream().map(
+ document -> new Overwrite(document, (Document)document.get("_id")))
Review comment:
We can define the ***_id*** (ObjectId type) in the `MongoConfigConstants` class.
##########
File path: metastore/metastore-api/src/test/java/org/apache/drill/metastore/TestData.java
##########
@@ -55,7 +55,7 @@ public static TableMetadataUnit basicTableMetadataUnit() {
.lastModifiedTime(System.currentTimeMillis())
.partitionKeys(Collections.singletonMap("dir0", "2018"))
.additionalMetadata("additional test metadata")
- .metadataIdentifier("part_int=3/part_varchar=g/0_0_0.parquet")
+ .metadataIdentifier(MetadataInfo.GENERAL_INFO_KEY)
Review comment:
Is the change only for debugging?
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/operate/MongoModify.java
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.operate;
+
+import com.mongodb.client.ClientSession;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.transform.OperationTransformer;
+import org.apache.drill.metastore.operate.AbstractModify;
+import org.apache.drill.metastore.operate.MetadataTypeValidator;
+import org.apache.drill.metastore.operate.Modify;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Implementation of {@link Modify} interface based on {@link AbstractModify} parent class.
+ * Modifies information in Mongo collection based on given overwrite or delete operations.
+ * Executes given operations in one transaction.
+ *
+ * @param <T> Metastore component unit type
+ */
+public class MongoModify<T> extends AbstractModify<T> {
+
+ private final OperationTransformer<T> transformer;
+ private final MongoMetastoreContext<T> context;
+ private final List<MongoOperation> operations = new ArrayList<>();
+
+ public MongoModify(MetadataTypeValidator metadataTypeValidator, MongoMetastoreContext<T> context) {
+ super(metadataTypeValidator);
+ this.context = context;
+ this.transformer = context.transformer().operation();
+ }
+
+ @Override
+ public void execute() {
+ if (operations.isEmpty()) {
+ return;
+ }
+ executeOperations(operations);
+ }
+
+ @Override
+ public void purge() {
+ executeOperations(Collections.singletonList(transformer.toDelete()));
+ }
+
+ @Override
+ protected void addOverwrite(List<T> units) {
+ operations.addAll(transformer.toOverwrite(units));
+ }
+
+ @Override
+ protected void addDelete(org.apache.drill.metastore.operate.Delete delete) {
+ operations.add(transformer.toDelete(delete));
+ }
+
+ private void executeOperations(List<MongoOperation> operations) {
+ ClientSession clientSession = context.client().startSession();
Review comment:
Thank you for adding the `Transaction` feature.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/transform/FilterTransformer.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.transform;
+
+import com.mongodb.client.model.Filters;
+import org.apache.drill.metastore.MetastoreColumn;
+import org.apache.drill.metastore.expressions.FilterExpression;
+import org.apache.drill.metastore.metadata.MetadataType;
+import org.bson.BsonDocument;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Transforms given input into Mongo {@link Document} which is used as filter
+ * to retrieve, overwrite or delete Metastore component data.
+ */
+public class FilterTransformer {
+
+ private static final FilterExpression.Visitor<Bson> FILTER_VISITOR =
+ FilterExpressionVisitor.get();
+
+ public Bson transform(FilterExpression filter) {
+ return filter == null ? new Document() : filter.accept(FILTER_VISITOR);
Review comment:
```java
filter.accept(FilterExpressionVisitor.get())
```
##########
File path: metastore/mongo-metastore/src/test/java/org/apache/drill/metastore/mongo/MongoBaseTest.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.drill.categories.MetastoreTest;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.components.tables.AbstractBasicTablesRequestsTest;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.experimental.categories.Category;
+import org.testcontainers.containers.MongoDBContainer;
+
+@Category(MetastoreTest.class)
+public class MongoBaseTest extends AbstractBasicTablesRequestsTest {
+ private static MongoDBContainer container;
+
+ @BeforeClass
+ public static void init() {
+ container = new MongoDBContainer("mongo:4.4.5");
Review comment:
Add the mini Replica Set.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/operate/MongoModify.java
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.operate;
+
+import com.mongodb.client.ClientSession;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.transform.OperationTransformer;
+import org.apache.drill.metastore.operate.AbstractModify;
+import org.apache.drill.metastore.operate.MetadataTypeValidator;
+import org.apache.drill.metastore.operate.Modify;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Implementation of {@link Modify} interface based on {@link AbstractModify} parent class.
+ * Modifies information in Mongo collection based on given overwrite or delete operations.
+ * Executes given operations in one transaction.
+ *
+ * @param <T> Metastore component unit type
+ */
+public class MongoModify<T> extends AbstractModify<T> {
+
+ private final OperationTransformer<T> transformer;
+ private final MongoMetastoreContext<T> context;
+ private final List<MongoOperation> operations = new ArrayList<>();
+
+ public MongoModify(MetadataTypeValidator metadataTypeValidator, MongoMetastoreContext<T> context) {
+ super(metadataTypeValidator);
+ this.context = context;
+ this.transformer = context.transformer().operation();
+ }
+
+ @Override
+ public void execute() {
+ if (operations.isEmpty()) {
+ return;
+ }
+ executeOperations(operations);
+ }
+
+ @Override
+ public void purge() {
+ executeOperations(Collections.singletonList(transformer.toDelete()));
+ }
+
+ @Override
+ protected void addOverwrite(List<T> units) {
+ operations.addAll(transformer.toOverwrite(units));
+ }
+
+ @Override
+ protected void addDelete(org.apache.drill.metastore.operate.Delete delete) {
+ operations.add(transformer.toDelete(delete));
+ }
+
+ private void executeOperations(List<MongoOperation> operations) {
+ ClientSession clientSession = context.client().startSession();
+ clientSession.withTransaction(() -> {
+ operations.forEach(o -> o.execute(context.table()));
+ return String.format("executed %s options", operations.size());
Review comment:
Maybe is that : ***executed %s operations***.
##########
File path: metastore/mongo-metastore/src/test/java/org/apache/drill/metastore/mongo/components/tables/TestTablesOperationTransformer.java
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import com.mongodb.client.model.Filters;
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.MetastoreColumn;
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+import org.apache.drill.metastore.expressions.FilterExpression;
+import org.apache.drill.metastore.metadata.MetadataType;
+import org.apache.drill.metastore.mongo.MongoMetastore;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.operate.Delete;
+import org.apache.drill.metastore.mongo.operate.Overwrite;
+import org.apache.drill.metastore.mongo.transform.InputDataTransformer;
+import org.apache.drill.metastore.mongo.transform.OperationTransformer;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class TestTablesOperationTransformer {
+
+ private static OperationTransformer<TableMetadataUnit> transformer;
+ private static MongoMetastore metastore;
+
+ @BeforeClass
+ public static void init() {
+ DrillConfig drillConfig = new DrillConfig(DrillConfig.create().withValue(MongoConfigConstants.CONNECTION,
+ ConfigValueFactory.fromAnyRef("mongodb://localhost:27017/?connectTimeoutMS=60000&maxPoolSize=1000&safe=true")));
+ metastore = new MongoMetastore(drillConfig);
+ transformer = new TablesOperationTransformer(((MongoTables)metastore.tables()).context());
+ }
+
+ @Test
+ public void testToOverwriteOperation() {
+ TableMetadataUnit unit = TableMetadataUnit.builder()
+ .storagePlugin("dfs").workspace("tmp").tableName("nation")
+ .metadataType(MetadataType.TABLE.name()).metadataIdentifier("s1").build();
+ List<Overwrite> operations = transformer.toOverwrite(Collections.singletonList(unit));
+ InputDataTransformer<TableMetadataUnit> inputDataTransformer =
+ ((MongoTables)metastore.tables()).transformer().inputData();
+ Document expected = new Document();
+ expected.append("storagePlugin", "dfs");
+ expected.append("workspace", "tmp");
+ expected.append("tableName", "nation");
+ expected.append("metadataType", MetadataType.TABLE.name());
+ expected.append("metadataIdentifier", "s1");
+
+ assertEquals(inputDataTransformer.createId(expected), operations.get(0).filter());
+ assertEquals(expected, operations.get(0).data().get("_id"));
+ }
+
+ @Test
+ public void testToOverwriteOperations() {
+ List<TableMetadataUnit> units = Arrays.asList(
+ TableMetadataUnit.builder().storagePlugin("dfs").workspace("tmp")
+ .tableName("nation").metadataType(MetadataType.ROW_GROUP.name())
+ .metadataIdentifier("s1/nation2.parquet/0").build(),
+ TableMetadataUnit.builder().storagePlugin("dfs").workspace("tmp")
+ .tableName("nation").metadataType(MetadataType.ROW_GROUP.name())
+ .metadataIdentifier("s1/nation.parquet/0").build(),
+ TableMetadataUnit.builder().storagePlugin("dfs").workspace("tmp")
+ .tableName("nation").metadataType(MetadataType.FILE.name())
+ .metadataIdentifier("s1/nation2.parquet").build(),
+ TableMetadataUnit.builder().storagePlugin("dfs").workspace("tmp")
+ .tableName("nation").metadataType(MetadataType.FILE.name())
+ .metadataIdentifier("s1/nation.parquet").build(),
+ TableMetadataUnit.builder().storagePlugin("dfs").workspace("tmp")
+ .tableName("region").metadataType(MetadataType.SEGMENT.name())
+ .metadataIdentifier("s1").build(),
+ TableMetadataUnit.builder().storagePlugin("s3").workspace("tmp")
+ .tableName("region").metadataType(MetadataType.TABLE.name())
+ .metadataIdentifier("GENERAL_INFO").build());
+
+ List<Overwrite> operations = transformer.toOverwrite(units);
+ assertEquals(6, operations.size());
+ }
+
+ @Test
+ public void testToDeleteOperation() {
+ Bson expected = Filters.and(
+ Filters.eq(MetastoreColumn.STORAGE_PLUGIN.columnName(), "dfs"),
+ Filters.eq(MetastoreColumn.WORKSPACE.columnName(), "tmp"));
+ FilterExpression filter = FilterExpression.and(
+ FilterExpression.equal(MetastoreColumn.STORAGE_PLUGIN, "dfs"),
+ FilterExpression.equal(MetastoreColumn.WORKSPACE, "tmp"));
+
+ org.apache.drill.metastore.operate.Delete delete = org.apache.drill.metastore.operate.Delete.builder()
+ .metadataType(MetadataType.ALL)
+ .filter(filter)
+ .build();
+
+ Delete operation = transformer.toDelete(delete);
Review comment:
Is it possible to test ***toDelete()*** and ***toDeleteAll()*** ?
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/operate/MongoRead.java
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.operate;
+
+import org.apache.drill.metastore.MetastoreColumn;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.transform.FilterTransformer;
+import org.apache.drill.metastore.operate.AbstractRead;
+import org.apache.drill.metastore.operate.MetadataTypeValidator;
+import org.apache.drill.metastore.operate.Read;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Implementation of {@link Read} interface based on {@link AbstractRead} parent class.
+ * Reads information from Mongo collection based on given filter expression.
+ * Supports reading information for specific columns.
+ *
+ * @param <T> Metastore component unit type
+ */
+public class MongoRead<T> extends AbstractRead<T> {
+
+ private final MongoMetastoreContext<T> context;
+
+ public MongoRead(MetadataTypeValidator metadataTypeValidator, MongoMetastoreContext<T> context) {
+ super(metadataTypeValidator);
+ this.context = context;
+ }
+
+ @Override
+ protected List<T> internalExecute() {
+ FilterTransformer filterTransformer = context.transformer().filter();
+ Bson rowFilter = filterTransformer.combine(
+ filterTransformer.transform(metadataTypes), filterTransformer.transform(filter));
+ List<Document> documents = Lists.newLinkedList();
+ context.table().find(rowFilter).forEach(documents::add);
+ return context.transformer().outputData()
Review comment:
Could you please check that the results included the table metadata type?
* BaseTableMetadata
* SegmentMetadata
* FileMetadata
* RowGroupMetadata
* PartitionMetadata
##########
File path: metastore/mongo-metastore/pom.xml
##########
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>metastore-parent</artifactId>
+ <groupId>org.apache.drill.metastore</groupId>
+ <version>1.20.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>drill-mongo-metastore</artifactId>
+ <name>Drill : Metastore : Mongo</name>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.drill</groupId>
+ <artifactId>drill-common</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.drill.metastore</groupId>
+ <artifactId>drill-metastore-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.mongodb</groupId>
+ <artifactId>mongodb-driver-sync</artifactId>
+ <version>4.3.1</version>
Review comment:
Keep consistent with mongo-storage module.
```xml
<version>4.3.3</version>
```
The best is that, define the version in the ```drill-root/pom.xml```, then reference this in the sub modules.
```xml
<version>${mongo.version}</version>
```
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/transform/OperationTransformer.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.transform;
+
+import org.apache.drill.metastore.expressions.FilterExpression;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.operate.Delete;
+import org.apache.drill.metastore.mongo.operate.MongoOperation;
+import org.apache.drill.metastore.mongo.operate.Overwrite;
+
+import java.util.List;
+
+/**
+ * Base class to transforms given input into {@link MongoOperation} implementations.
+ *
+ * @param <T> Metastore component unit type
+ */
+public abstract class OperationTransformer<T> {
+
+ protected final MongoMetastoreContext<T> context;
+
+ protected OperationTransformer(MongoMetastoreContext<T> context) {
+ this.context = context;
+ }
+
+ public Delete toDelete() {
+ return new Delete(context.transformer().filter().transform((FilterExpression) null));
+ }
+
+ public Delete toDelete(org.apache.drill.metastore.operate.Delete delete) {
+ // metadata types are ignored during delete since they are not part of the partition key
+ FilterTransformer filterTransformer = context.transformer().filter();
Review comment:
Keep consistent with usage style.
```java
context.transformer().filter().transform(); // at line 42.
// or
FilterTransformer filterTransformer = context.transformer().filter();
filterTransformer.transform();
```
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/operate/Delete.java
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.operate;
+
+import com.mongodb.client.MongoCollection;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+
+/**
+ * Mongo delete operation: deletes documents based on given row filter.
+ */
+public class Delete implements MongoOperation {
Review comment:
Can be defined as `MongoDelete`, save on these :
```java
(org.apache.drill.metastore.operate.Delete delete) { }
```
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r768755833
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/components/tables/TablesOutputDataTransformer.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.apache.drill.metastore.mongo.transform.OutputDataTransformer;
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Metastore Tables component output data transformer that transforms
+ * {@link org.bson.Document} into {@link TableMetadataUnit}.
+ */
+public class TablesOutputDataTransformer extends OutputDataTransformer<TableMetadataUnit> {
+
+ public TablesOutputDataTransformer(Map<String, MethodHandle> unitSetters) {
+ super(unitSetters);
+ }
+
+ @Override
+ public List<TableMetadataUnit> execute() {
+ List<TableMetadataUnit> results = new ArrayList<>();
+ for (Map<MethodHandle, Object> valueToSet : valuesToSet()) {
+ TableMetadataUnit.Builder builder = TableMetadataUnit.builder();
Review comment:
I think we need a new builder for each TableMetadataUnit
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-994918665
@Leon-WTF Thanks for the update. Please replace the log level of test class to `debug`, then squash all the commits.
I'll test the new metastore with `parquet` and `json` format in the cluster tomorrow.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r765752883
##########
File path: metastore/metastore-api/src/test/java/org/apache/drill/metastore/TestData.java
##########
@@ -55,7 +55,7 @@ public static TableMetadataUnit basicTableMetadataUnit() {
.lastModifiedTime(System.currentTimeMillis())
.partitionKeys(Collections.singletonMap("dir0", "2018"))
.additionalMetadata("additional test metadata")
- .metadataIdentifier("part_int=3/part_varchar=g/0_0_0.parquet")
+ .metadataIdentifier(MetadataInfo.GENERAL_INFO_KEY)
Review comment:
yes, the original value is not match the real case.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-989585110
> @dzamo We can make progress now, I will try my best to solve the comments by this weekend, is that ok for you to delay two days before freezing the master branch?
@Leon-WTF Great! We voted in the mailing list to delay the freeze until next week Friday due to multiple PRs that are near completion. We're grateful for your efforts.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-989463385
I'm not that familiar with the Drill metastore, but I do think this is a valuable contribution. I don't want to hold this up, so it looks good!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-988922680
@Leon-WTF @luocooong, w.r.t to the release of 1.20 and having read this PR's comments I just want to check if we're deadlocked here with author waiting for reviewers and reviewers waiting for author?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong merged pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
luocooong merged pull request #2384:
URL: https://github.com/apache/drill/pull/2384
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-983645245
Dear PR author and reviewers.
This is a generic message to say that we would like to merge this PR in time for the 1.20 release. Currently we're targeting a master branch freeze date of 2021-12-10 (10 Dec). Please strive to complete development and review by this time, or indicate that the PR will need more time (and how much).
Thank you.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r767245546
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/operate/MongoRead.java
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.operate;
+
+import org.apache.drill.metastore.MetastoreColumn;
+import org.apache.drill.metastore.mongo.MongoMetastoreContext;
+import org.apache.drill.metastore.mongo.transform.FilterTransformer;
+import org.apache.drill.metastore.operate.AbstractRead;
+import org.apache.drill.metastore.operate.MetadataTypeValidator;
+import org.apache.drill.metastore.operate.Read;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Implementation of {@link Read} interface based on {@link AbstractRead} parent class.
+ * Reads information from Mongo collection based on given filter expression.
+ * Supports reading information for specific columns.
+ *
+ * @param <T> Metastore component unit type
+ */
+public class MongoRead<T> extends AbstractRead<T> {
+
+ private final MongoMetastoreContext<T> context;
+
+ public MongoRead(MetadataTypeValidator metadataTypeValidator, MongoMetastoreContext<T> context) {
+ super(metadataTypeValidator);
+ this.context = context;
+ }
+
+ @Override
+ protected List<T> internalExecute() {
+ FilterTransformer filterTransformer = context.transformer().filter();
+ Bson rowFilter = filterTransformer.combine(
+ filterTransformer.transform(metadataTypes), filterTransformer.transform(filter));
+ List<Document> documents = Lists.newLinkedList();
+ context.table().find(rowFilter).forEach(documents::add);
+ return context.transformer().outputData()
Review comment:
All tests in AbstractBasicTablesRequestsTest will be executed, they will read meta data with different metadata types.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r768746854
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.ConnectionString;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.bson.Document;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private final MongoClient client;
+ private final String database;
+ private final String tableCollection;
+
+ public MongoMetastore(DrillConfig config) {
+ this.client = MongoClients.create(
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION)));
+ if(config.hasPath(MongoConfigConstants.DATABASE)) {
+ this.database = config.getString(MongoConfigConstants.DATABASE);
+ } else {
+ this.database = MongoConfigConstants.DEFAULT_DATABASE;
+ }
+ if(config.hasPath(MongoConfigConstants.TABLE_COLLECTION)) {
+ this.tableCollection = config.getString(MongoConfigConstants.TABLE_COLLECTION);
+ } else {
+ this.tableCollection = MongoConfigConstants.DEFAULT_TABLE_COLLECTION;
+ }
+ if (config.hasPath(MongoConfigConstants.IS_SHARDED) &&
+ config.getBoolean(MongoConfigConstants.IS_SHARDED)) {
+ initShardedCollection();
+ }
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(
+ client.getDatabase(database).getCollection(tableCollection), client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ private void initShardedCollection() {
+ FindIterable<Document> dbs = this.client.getDatabase("config")
+ .getCollection("databases")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database));
+ if (!dbs.cursor().hasNext()) {
+ shardDatabase();
+ shardCollection();
+ } else {
+ Document doc = dbs.cursor().next();
+ if (!doc.getBoolean("partitioned")) {
+ shardDatabase();
+ }
+ FindIterable<Document> collections = this.client.getDatabase("config")
+ .getCollection("collections")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database+"."+tableCollection)
+ .append("distributionMode", "sharded"));
+ if (!collections.cursor().hasNext()) {
+ shardCollection();
+ } else {
+ Document shardKey = (Document) dbs.cursor().next().get("key");
+ if (shardKey.size() != 1 || !shardKey.containsKey(MongoConfigConstants.ID)
+ || !shardKey.containsValue("hashed")) {
+ throw new MongoMetastoreException(
+ String.format("%s.%s(with shard key %s) is not hash sharded and " +
+ "only hash sharded by %s", database, tableCollection,
+ shardKey.toString(), MongoConfigConstants.ID));
+ }
+ }
+ }
+ }
+
+ private void shardDatabase() {
Review comment:
Yes, you are right, I will choose the first one and document the requirements of database and collections.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF edited a comment on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF edited a comment on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-975571642
> @Leon-WTF Thanks for the contribution, this pull request is great! After completing the unit test and docs, please let me know, we can start a review.
@luocooong I have finished the ut, they are all passed. You can start the review, I will update the doc while replying the comments.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r765752883
##########
File path: metastore/metastore-api/src/test/java/org/apache/drill/metastore/TestData.java
##########
@@ -55,7 +55,7 @@ public static TableMetadataUnit basicTableMetadataUnit() {
.lastModifiedTime(System.currentTimeMillis())
.partitionKeys(Collections.singletonMap("dir0", "2018"))
.additionalMetadata("additional test metadata")
- .metadataIdentifier("part_int=3/part_varchar=g/0_0_0.parquet")
+ .metadataIdentifier(MetadataInfo.GENERAL_INFO_KEY)
Review comment:
no, it's for the ut. The original value is not match with the real case.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2384:
URL: https://github.com/apache/drill/pull/2384#discussion_r768390098
##########
File path: pom.xml
##########
@@ -140,6 +140,7 @@
<iceberg.version>0.12.1</iceberg.version>
<univocity-parsers.version>2.8.3</univocity-parsers.version>
<junit.args/>
+ <mongo.version>4.3.3</mongo.version>
Review comment:
Recommended to keep `<junit.args/>` at the end.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.ConnectionString;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.bson.Document;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private final MongoClient client;
+ private final String database;
+ private final String tableCollection;
+
+ public MongoMetastore(DrillConfig config) {
+ this.client = MongoClients.create(
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION)));
+ if(config.hasPath(MongoConfigConstants.DATABASE)) {
Review comment:
Keep a space between `if` and `()`. the same below.
Please check all code blocks (and other classes).
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.ConnectionString;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.bson.Document;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private final MongoClient client;
+ private final String database;
+ private final String tableCollection;
+
+ public MongoMetastore(DrillConfig config) {
+ this.client = MongoClients.create(
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION)));
+ if(config.hasPath(MongoConfigConstants.DATABASE)) {
+ this.database = config.getString(MongoConfigConstants.DATABASE);
+ } else {
+ this.database = MongoConfigConstants.DEFAULT_DATABASE;
+ }
+ if(config.hasPath(MongoConfigConstants.TABLE_COLLECTION)) {
+ this.tableCollection = config.getString(MongoConfigConstants.TABLE_COLLECTION);
+ } else {
+ this.tableCollection = MongoConfigConstants.DEFAULT_TABLE_COLLECTION;
+ }
+ if (config.hasPath(MongoConfigConstants.IS_SHARDED) &&
+ config.getBoolean(MongoConfigConstants.IS_SHARDED)) {
+ initShardedCollection();
+ }
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(
+ client.getDatabase(database).getCollection(tableCollection), client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ private void initShardedCollection() {
+ FindIterable<Document> dbs = this.client.getDatabase("config")
+ .getCollection("databases")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database));
+ if (!dbs.cursor().hasNext()) {
+ shardDatabase();
+ shardCollection();
+ } else {
+ Document doc = dbs.cursor().next();
+ if (!doc.getBoolean("partitioned")) {
+ shardDatabase();
+ }
+ FindIterable<Document> collections = this.client.getDatabase("config")
+ .getCollection("collections")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database+"."+tableCollection)
Review comment:
Keep a space between `+` and variable. the same below.
Please check all code blocks (and other classes).
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.ConnectionString;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.bson.Document;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private final MongoClient client;
+ private final String database;
+ private final String tableCollection;
+
+ public MongoMetastore(DrillConfig config) {
+ this.client = MongoClients.create(
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION)));
+ if(config.hasPath(MongoConfigConstants.DATABASE)) {
+ this.database = config.getString(MongoConfigConstants.DATABASE);
+ } else {
+ this.database = MongoConfigConstants.DEFAULT_DATABASE;
+ }
+ if(config.hasPath(MongoConfigConstants.TABLE_COLLECTION)) {
+ this.tableCollection = config.getString(MongoConfigConstants.TABLE_COLLECTION);
+ } else {
+ this.tableCollection = MongoConfigConstants.DEFAULT_TABLE_COLLECTION;
+ }
+ if (config.hasPath(MongoConfigConstants.IS_SHARDED) &&
+ config.getBoolean(MongoConfigConstants.IS_SHARDED)) {
+ initShardedCollection();
+ }
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(
+ client.getDatabase(database).getCollection(tableCollection), client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ private void initShardedCollection() {
+ FindIterable<Document> dbs = this.client.getDatabase("config")
+ .getCollection("databases")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database));
+ if (!dbs.cursor().hasNext()) {
+ shardDatabase();
+ shardCollection();
+ } else {
+ Document doc = dbs.cursor().next();
+ if (!doc.getBoolean("partitioned")) {
+ shardDatabase();
+ }
+ FindIterable<Document> collections = this.client.getDatabase("config")
+ .getCollection("collections")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database+"."+tableCollection)
+ .append("distributionMode", "sharded"));
+ if (!collections.cursor().hasNext()) {
+ shardCollection();
+ } else {
+ Document shardKey = (Document) dbs.cursor().next().get("key");
+ if (shardKey.size() != 1 || !shardKey.containsKey(MongoConfigConstants.ID)
+ || !shardKey.containsValue("hashed")) {
+ throw new MongoMetastoreException(
+ String.format("%s.%s(with shard key %s) is not hash sharded and " +
+ "only hash sharded by %s", database, tableCollection,
+ shardKey.toString(), MongoConfigConstants.ID));
+ }
+ }
+ }
+ }
+
+ private void shardDatabase() {
Review comment:
In general, we use strict role permissions in production environment, mongo DBA can create db with `dbOwner` role for you (custom database) at most. If the app need access to `admin` and `config` (built-in databases), it must need more permissions... In fact, there are two options :
1. Remove these and return the operation to the DBA/USER (use these codes only in unit tests).
2. Document this requirement in the docs, please see also [Built-In Roles](https://docs.mongodb.com/v4.2/reference/built-in-roles/).
##########
File path: distribution/src/main/resources/drill-metastore-override-example.conf
##########
@@ -21,8 +21,16 @@
drill.metastore: {
# For Drill Iceberg Metastore use: org.apache.drill.metastore.iceberg.IcebergMetastore
+ # For Drill Mongo Metastore use: org.apache.drill.metastore.mongo.MongoMetastore
implementation.class: "org.apache.drill.metastore.rdbms.RdbmsMetastore",
+ mongo: {
+ # connection: "mongodb://localhost:27017/",
+ # is_sharded: false,
+ # database: meta,
Review comment:
```suggestion
# database: "meta",
```
The same below.
##########
File path: metastore/mongo-metastore/src/test/java/org/apache/drill/metastore/mongo/components/tables/TestTablesOperationTransformer.java
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import com.mongodb.client.model.Filters;
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.MetastoreColumn;
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+import org.apache.drill.metastore.expressions.FilterExpression;
+import org.apache.drill.metastore.metadata.MetadataType;
+import org.apache.drill.metastore.mongo.MongoMetastore;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.operate.MongoDelete;
+import org.apache.drill.metastore.mongo.operate.Overwrite;
+import org.apache.drill.metastore.mongo.transform.InputDataTransformer;
+import org.apache.drill.metastore.mongo.transform.OperationTransformer;
+import org.apache.drill.metastore.operate.Delete;
+import org.bson.Document;
+import org.bson.conversions.Bson;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class TestTablesOperationTransformer {
+
+ private static OperationTransformer<TableMetadataUnit> transformer;
+ private static MongoMetastore metastore;
+
+ @BeforeClass
+ public static void init() {
+ DrillConfig drillConfig = new DrillConfig(DrillConfig.create().withValue(MongoConfigConstants.CONNECTION,
+ ConfigValueFactory.fromAnyRef("mongodb://localhost:27017/?connectTimeoutMS=60000&maxPoolSize=1000&safe=true")));
+ metastore = new MongoMetastore(drillConfig);
+ transformer = new TablesOperationTransformer(((MongoTables)metastore.tables()).context());
+ }
+
+ @Test
+ public void testToOverwriteOperation() {
+ TableMetadataUnit unit = TableMetadataUnit.builder()
+ .storagePlugin("dfs").workspace("tmp").tableName("nation")
+ .metadataType(MetadataType.TABLE.name()).metadataIdentifier("s1").build();
+ List<Overwrite> operations = transformer.toOverwrite(Collections.singletonList(unit));
+ InputDataTransformer<TableMetadataUnit> inputDataTransformer =
+ ((MongoTables)metastore.tables()).transformer().inputData();
Review comment:
Keep a space between `()` and variable the same below.
Please check all code blocks (and other classes).
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/components/tables/TablesOutputDataTransformer.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.apache.drill.metastore.mongo.transform.OutputDataTransformer;
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Metastore Tables component output data transformer that transforms
+ * {@link org.bson.Document} into {@link TableMetadataUnit}.
+ */
+public class TablesOutputDataTransformer extends OutputDataTransformer<TableMetadataUnit> {
+
+ public TablesOutputDataTransformer(Map<String, MethodHandle> unitSetters) {
+ super(unitSetters);
+ }
+
+ @Override
+ public List<TableMetadataUnit> execute() {
+ List<TableMetadataUnit> results = new ArrayList<>();
+ for (Map<MethodHandle, Object> valueToSet : valuesToSet()) {
+ TableMetadataUnit.Builder builder = TableMetadataUnit.builder();
Review comment:
Is it possible to move this line outside the loop?
##########
File path: metastore/mongo-metastore/src/main/resources/drill-metastore-module.conf
##########
@@ -0,0 +1,21 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+drill.metastore.mongo: {
+ connection: "mongodb://localhost:27017/?connectTimeoutMS=60000&maxPoolSize=1000&safe=true"
Review comment:
In order to be "out of the box", just keep `"mongodb://localhost:27017"`.
##########
File path: metastore/mongo-metastore/src/main/java/org/apache/drill/metastore/mongo/MongoMetastore.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.ConnectionString;
+import com.mongodb.client.FindIterable;
+import com.mongodb.client.MongoClient;
+import com.mongodb.client.MongoClients;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.Metastore;
+import org.apache.drill.metastore.components.tables.Tables;
+import org.apache.drill.metastore.components.views.Views;
+import org.apache.drill.metastore.mongo.components.tables.MongoTables;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.metastore.mongo.exception.MongoMetastoreException;
+import org.bson.Document;
+
+/**
+ * Mongo Drill Metastore implementation.
+ */
+public class MongoMetastore implements Metastore {
+
+ private final MongoClient client;
+ private final String database;
+ private final String tableCollection;
+
+ public MongoMetastore(DrillConfig config) {
+ this.client = MongoClients.create(
+ new ConnectionString(config.getString(MongoConfigConstants.CONNECTION)));
+ if(config.hasPath(MongoConfigConstants.DATABASE)) {
+ this.database = config.getString(MongoConfigConstants.DATABASE);
+ } else {
+ this.database = MongoConfigConstants.DEFAULT_DATABASE;
+ }
+ if(config.hasPath(MongoConfigConstants.TABLE_COLLECTION)) {
+ this.tableCollection = config.getString(MongoConfigConstants.TABLE_COLLECTION);
+ } else {
+ this.tableCollection = MongoConfigConstants.DEFAULT_TABLE_COLLECTION;
+ }
+ if (config.hasPath(MongoConfigConstants.IS_SHARDED) &&
+ config.getBoolean(MongoConfigConstants.IS_SHARDED)) {
+ initShardedCollection();
+ }
+ }
+
+ @Override
+ public Tables tables() {
+ return new MongoTables(
+ client.getDatabase(database).getCollection(tableCollection), client);
+ }
+
+ @Override
+ public Views views() {
+ throw new UnsupportedOperationException("Views metadata support is not implemented");
+ }
+
+ private void initShardedCollection() {
+ FindIterable<Document> dbs = this.client.getDatabase("config")
+ .getCollection("databases")
+ .find(new Document()
+ .append(MongoConfigConstants.ID, database));
Review comment:
New line is not recommended.
##########
File path: metastore/mongo-metastore/src/test/java/org/apache/drill/metastore/mongo/MongoBaseTest.java
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo;
+
+import com.typesafe.config.Config;
+import com.typesafe.config.ConfigValueFactory;
+import org.apache.drill.categories.MetastoreTest;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.metastore.components.tables.AbstractBasicTablesRequestsTest;
+import org.apache.drill.metastore.mongo.config.MongoConfigConstants;
+import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testcontainers.containers.Container;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.MongoDBContainer;
+import org.testcontainers.containers.Network;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+@Category(MetastoreTest.class)
+public class MongoBaseTest extends AbstractBasicTablesRequestsTest {
+ private static final Logger logger = LoggerFactory.getLogger(MongoBaseTest.class);
+
+ private static final String MONGO_IMAGE_NAME = "mongo:4.4.5";
Review comment:
The `mongo-storage` use the 4.4.10.
##########
File path: metastore/mongo-metastore/src/test/java/org/apache/drill/metastore/mongo/components/tables/TestMongoBasicTablesRequests.java
##########
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.metastore.mongo.components.tables;
+
+import org.apache.drill.metastore.components.tables.MetastoreTableInfo;
+import org.apache.drill.metastore.components.tables.TableMetadataUnit;
+import org.apache.drill.metastore.metadata.MetadataType;
+import org.apache.drill.metastore.mongo.MongoBaseTest;
+import org.apache.drill.metastore.operate.Delete;
+import org.apache.drill.metastore.operate.Metadata;
+import org.junit.Test;
+
+import java.util.List;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestMongoBasicTablesRequests extends MongoBaseTest {
+
+ @Test
+ public void testMetastoreTableInfoExistingTable() {
+ MetastoreTableInfo metastoreTableInfo = basicRequests.metastoreTableInfo(nationTableInfo);
+ assertTrue(metastoreTableInfo.isExists());
+ assertEquals(nationTableInfo, metastoreTableInfo.tableInfo());
+ assertEquals(nationTable.lastModifiedTime(), metastoreTableInfo.lastModifiedTime());
+ assertEquals(Metadata.UNDEFINED, metastoreTableInfo.metastoreVersion());
+ }
+
+ @Test
+ public void testDelete() {
+ MetastoreTableInfo metastoreTableInfo = basicRequests.metastoreTableInfo(nationTableInfo);
+ assertTrue(metastoreTableInfo.isExists());
+ tables.modify()
+ .delete(Delete.builder()
+ .metadataType(MetadataType.TABLE)
+ .filter(nationTableInfo.toFilter())
+ .build())
+ .execute();
+ metastoreTableInfo = basicRequests.metastoreTableInfo(nationTableInfo);
+ assertFalse(metastoreTableInfo.isExists());
+
+ List<TableMetadataUnit> res =
+ tables.read().metadataType(MetadataType.ALL).execute();
+ assertFalse(res.isEmpty());
+ tables.modify().purge();
+ res =
+ tables.read().metadataType(MetadataType.ALL).execute();
Review comment:
New line is not recommended.
##########
File path: distribution/src/main/resources/drill-metastore-override-example.conf
##########
@@ -21,8 +21,16 @@
drill.metastore: {
# For Drill Iceberg Metastore use: org.apache.drill.metastore.iceberg.IcebergMetastore
+ # For Drill Mongo Metastore use: org.apache.drill.metastore.mongo.MongoMetastore
implementation.class: "org.apache.drill.metastore.rdbms.RdbmsMetastore",
+ mongo: {
Review comment:
Recommended to put the configuration snippet behind the `Iceberg`.
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaUtil.java
##########
@@ -214,7 +214,13 @@ public static TupleMetadata fromBatchSchema(BatchSchema batchSchema) {
currentNames.add(columnMetadata.name());
result.addAll(getColumnPaths(columnMetadata.tupleSchema(), currentNames));
} else {
- result.add(Collections.singletonList(columnMetadata.name()));
+ if (parentNames != null) {
Review comment:
Thanks for your fix!
And then, how do we reproduce this bug? is it possible to add an unit test for this case?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Leon-WTF edited a comment on pull request #2384: DRILL-8015: Add MongoDB Metastore implementation
Posted by GitBox <gi...@apache.org>.
Leon-WTF edited a comment on pull request #2384:
URL: https://github.com/apache/drill/pull/2384#issuecomment-989444420
> @Leon-WTF @luocooong, w.r.t to the release of 1.20 and having read this PR's comments I just want to check if we're deadlocked here with author waiting for reviewers and reviewers waiting for author?
@dzamo We can make progress now, I will try my best to solve the comments by this weekend, is that ok for you to delay two days before freezing the master branch?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org