[
https://issues.apache.org/jira/browse/HIVE-26277?focusedWorklogId=783794&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-783794
]
ASF GitHub Bot logged work on HIVE-26277:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Jun/22 09:51
Start Date: 22/Jun/22 09:51
Worklog Time Spent: 10m
Work Description: zabetak commented on code in PR #3339:
URL: https://github.com/apache/hive/pull/3339#discussion_r903525231
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/columnstats/aggr/ColumnStatsAggregatorTest.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.hadoop.hive.metastore.columnstats.aggr;
+
+import org.apache.hadoop.hive.metastore.TableType;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData;
+import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;
+import org.apache.hadoop.hive.metastore.api.Date;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.columnstats.ColStatsBuilder;
+import org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static
org.apache.hadoop.hive.metastore.StatisticsTestUtils.createStatsWithInfo;
+
+@Category(MetastoreUnitTest.class)
+@RunWith(Parameterized.class)
+public class ColumnStatsAggregatorTest {
Review Comment:
I went over the test classes and I noticed that everywhere we are basically
testing ColumnStatsAggregator#aggregate with different inputs. Moreover tests
for the `LongColumnStatsAggregator` are extremely similar to
`StringColumnStatsAggregator` etc. From those observations I get the impression
that having one (or multiple) parameterized test might be more appropriate so
that's why I added this POC to discuss further.
I am not pushing into refactoring everything here unless we are convinced
that it improves readability and future maintenance. WDYT?
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/StatisticsTestUtils.java:
##########
@@ -0,0 +1,172 @@
+/*
+ * 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.hadoop.hive.metastore;
+
+import org.apache.hadoop.hive.common.ndv.fm.FMSketch;
+import org.apache.hadoop.hive.common.ndv.hll.HyperLogLog;
+import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
+import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData;
+import org.apache.hadoop.hive.metastore.api.ColumnStatisticsDesc;
+import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.api.Table;
+import
org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils.ColStatsObjWithSourceInfo;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import static org.apache.hadoop.hive.metastore.Warehouse.DEFAULT_CATALOG_NAME;
+
+public class StatisticsTestUtils {
+
+ private static final String HIVE_ENGINE = "hive";
+
+ private StatisticsTestUtils() {
+ throw new AssertionError("Suppress default constructor for non
instantiation");
+ }
+
+ /**
+ * Creates column statistics for a given table and partition.
+ * @param data the statistics data
+ * @param tbl the target table
+ * @param column the target column
+ * @param partName the target partition
+ * @return column statistics for a given table and partition.
+ */
+ public static ColumnStatistics createColStats(ColumnStatisticsData data,
Table tbl, FieldSchema column, String partName) {
+ ColumnStatisticsObj statObj = new ColumnStatisticsObj(column.getName(),
column.getType(), data);
+ ColumnStatistics colStats = new ColumnStatistics();
+ ColumnStatisticsDesc statsDesc = new ColumnStatisticsDesc(true,
tbl.getDbName(), tbl.getTableName());
+ statsDesc.setPartName(partName);
+ colStats.setStatsDesc(statsDesc);
+ colStats.setStatsObj(Collections.singletonList(statObj));
+ colStats.setEngine(HIVE_ENGINE);
+ return colStats;
+ }
+
+ public static ColStatsObjWithSourceInfo
createStatsWithInfo(ColumnStatisticsData data, Table tbl,
Review Comment:
I added this method here and refactored usages in
`LongColumnStatsAggregatorTest`. Do we need all the rest?
Issue Time Tracking
-------------------
Worklog Id: (was: 783794)
Time Spent: 3h 10m (was: 3h)
> NPEs and rounding issues in ColumnStatsAggregator classes
> ---------------------------------------------------------
>
> Key: HIVE-26277
> URL: https://issues.apache.org/jira/browse/HIVE-26277
> Project: Hive
> Issue Type: Bug
> Components: Standalone Metastore, Statistics, Tests
> Affects Versions: 4.0.0-alpha-2
> Reporter: Alessandro Solimando
> Assignee: Alessandro Solimando
> Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 10m
> Remaining Estimate: 0h
>
> Fix NPEs and rounding errors in _ColumnStatsAggregator_ classes, add
> unit-tests for all the involved classes.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
