[GitHub] [hive] kgyrtkirk commented on a change in pull request #1271: HIVE-23851: MSCK REPAIR Command With Partition Filtering Fails While Dropping Partitions

[GitBox <gi...@apache.org>]

kgyrtkirk commented on a change in pull request #1271:
URL: https://github.com/apache/hive/pull/1271#discussion_r484949144



##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MsckPartitionExpressionProxy.java
##########
@@ -1,114 +0,0 @@
-package org.apache.hadoop.hive.metastore;
-/*
- * 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
- * <p>
- * http://www.apache.org/licenses/LICENSE-2.0
- * <p>
- * 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.
- */
-
-import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-import org.apache.hadoop.hive.metastore.api.FieldSchema;
-import org.apache.hadoop.hive.metastore.api.FileMetadataExprType;
-import org.apache.hadoop.hive.metastore.api.MetaException;
-import org.apache.hadoop.hive.metastore.utils.FileUtils;
-import org.apache.hadoop.hive.ql.io.sarg.SearchArgument;
-import org.apache.hadoop.util.StringUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-// This is added as part of moving MSCK code from ql to standalone-metastore. There is a metastore API to drop
-// partitions by name but we cannot use it because msck typically will contain partition value (year=2014). We almost
-// never drop partition by name (year). So we need to construct expression filters, the current
-// PartitionExpressionProxy implementations (PartitionExpressionForMetastore and HCatClientHMSImpl.ExpressionBuilder)
-// all depend on ql code to build ExprNodeDesc for the partition expressions. It also depends on kryo for serializing
-// the expression objects to byte[]. For MSCK drop partition, we don't need complex expression generator. For now,
-// all we do is split the partition spec (year=2014/month=24) into filter expression year='2014' and month='24' and
-// rely on metastore database to deal with type conversions. Ideally, PartitionExpressionProxy default implementation
-// should use SearchArgument (storage-api) to construct the filter expression and not depend on ql, but the usecase
-// for msck is pretty simple and this specific implementation should suffice.

Review comment:
       I don't know - have you explored using the SA approach ? seeing the ql class thru reflection looks a bit wierd.
   
   other way around could be to move Expr related classes out from ql (not sure if that's possible)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org