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/12/08 15:41:37 UTC

[GitHub] [drill] luocooong commented on a change in pull request #2384: DRILL-8015: Add MongoDB Metastore implementation

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