maheshk114 commented on a change in pull request #2071:
URL: https://github.com/apache/hive/pull/2071#discussion_r615495349



##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/HiveWritableComparator.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveWritableComparator extends WritableComparator {
+    private WritableComparator comparator = null;
+    protected transient boolean nullSafe;
+    transient NullOrdering nullOrdering;
+    protected transient int not_null = 2;
+
+    HiveWritableComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        this.nullSafe = nullSafe;
+        this.nullOrdering = nullOrdering;
+    }
+
+    protected int checkNull(Object key1, Object key2) {

Review comment:
       This method is just for checking the null. This is  declared in base 
class to avoid code duplication.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveUnionComparator.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import 
org.apache.hadoop.hive.serde2.objectinspector.StandardUnionObjectInspector.StandardUnion;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveUnionComparator extends HiveWritableComparator {
+    WritableComparator comparator = null;

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveUnionComparator.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import 
org.apache.hadoop.hive.serde2.objectinspector.StandardUnionObjectInspector.StandardUnion;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveUnionComparator extends HiveWritableComparator {

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveMapComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.LinkedHashMap;
+
+public class HiveMapComparator extends HiveWritableComparator {
+    WritableComparator comparatorValue = null;
+    WritableComparator comparatorKey = null;

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveUnionComparator.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import 
org.apache.hadoop.hive.serde2.objectinspector.StandardUnionObjectInspector.StandardUnion;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveUnionComparator extends HiveWritableComparator {
+    WritableComparator comparator = null;
+
+    HiveUnionComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+
+        StandardUnion u1 = (StandardUnion) key1;
+        StandardUnion u2 = (StandardUnion) key2;
+
+        if (u1.getTag() != u2.getTag()) {
+            // If tag is not same, the keys may be of different data types. So 
can not be compared.
+            return u1.getTag() > u2.getTag() ? 1 : -1;

Review comment:
       The union value is based on two things tags and value. If tag is same 
then only value is checked. This case is tested with table having same value 
for diff tags.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveMapComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.LinkedHashMap;
+
+public class HiveMapComparator extends HiveWritableComparator {
+    WritableComparator comparatorValue = null;
+    WritableComparator comparatorKey = null;
+
+    HiveMapComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+
+        LinkedHashMap map1 = (LinkedHashMap) key1;
+        LinkedHashMap map2 = (LinkedHashMap) key2;

Review comment:
       done

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/HiveWritableComparator.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveWritableComparator extends WritableComparator {
+    private WritableComparator comparator = null;
+    protected transient boolean nullSafe;
+    transient NullOrdering nullOrdering;
+    protected transient int not_null = 2;
+
+    HiveWritableComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        this.nullSafe = nullSafe;
+        this.nullOrdering = nullOrdering;
+    }
+
+    protected int checkNull(Object key1, Object key2) {
+        if (key1 == null && key2 == null) {
+            if (nullSafe) {
+                return 0;
+            } else {
+                return -1;
+            }
+        } else if (key1 == null) {
+            return nullOrdering == null ? -1 : 
nullOrdering.getNullValueOption().getCmpReturnValue();
+        } else if (key2 == null) {
+            return nullOrdering == null ? 1 : 
-nullOrdering.getNullValueOption().getCmpReturnValue();
+        } else {
+            return not_null;
+        }
+    }
+
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+
+        if (comparator == null) {
+            comparator = WritableComparator.get(((WritableComparable) 
key1).getClass());
+        }
+        return comparator.compare((WritableComparable)key1, 
(WritableComparable)key2);
+    }
+}

Review comment:
       This class is to handle the primitive type that is the base 
WritableComparable type. So i am not sure if it will be of any help to make a 
abstract class and then derive a class from that.

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/HiveWritableComparator.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveWritableComparator extends WritableComparator {
+    private WritableComparator comparator = null;
+    protected transient boolean nullSafe;
+    transient NullOrdering nullOrdering;
+    protected transient int not_null = 2;
+
+    HiveWritableComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        this.nullSafe = nullSafe;
+        this.nullOrdering = nullOrdering;
+    }
+
+    protected int checkNull(Object key1, Object key2) {
+        if (key1 == null && key2 == null) {
+            if (nullSafe) {
+                return 0;
+            } else {
+                return -1;
+            }
+        } else if (key1 == null) {
+            return nullOrdering == null ? -1 : 
nullOrdering.getNullValueOption().getCmpReturnValue();
+        } else if (key2 == null) {
+            return nullOrdering == null ? 1 : 
-nullOrdering.getNullValueOption().getCmpReturnValue();
+        } else {
+            return not_null;

Review comment:
       We don't know which type to compare and if we need to do some other 
processing like checking the tag etc. So i think its better to keep it this way.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveListComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+
+public class HiveListComparator extends HiveWritableComparator {
+    // For List, all elements will have same type, so only one comparator is 
sufficient.
+    WritableComparator comparator = null;
+
+    HiveListComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+        ArrayList a1 = (ArrayList) key1;
+        ArrayList a2 = (ArrayList) key2;

Review comment:
       Removed List type as at run time its difficult to judge if its a List or 
Struct type.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveListComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+
+public class HiveListComparator extends HiveWritableComparator {
+    // For List, all elements will have same type, so only one comparator is 
sufficient.
+    WritableComparator comparator = null;

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveListComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+
+public class HiveListComparator extends HiveWritableComparator {
+    // For List, all elements will have same type, so only one comparator is 
sufficient.
+    WritableComparator comparator = null;
+
+    HiveListComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+        ArrayList a1 = (ArrayList) key1;
+        ArrayList a2 = (ArrayList) key2;
+        if (a1.size() != a2.size()) {
+            return a1.size() > a2.size() ? 1 : -1;
+        }
+        if (a1.size() == 0) {
+            return 0;
+        }
+
+        if (comparator == null) {
+            // For List, all elements should be of same type.
+            comparator = WritableComparatorFactory.get(a1.get(0), nullSafe, 
nullOrdering);

Review comment:
       Removed List type as at run time its difficult to judge if its a List or 
Struct type.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveStructComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+
+class HiveStructComparator extends HiveWritableComparator {
+    WritableComparator[] comparator = null;

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveMapComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.LinkedHashMap;
+
+public class HiveMapComparator extends HiveWritableComparator {
+    WritableComparator comparatorValue = null;
+    WritableComparator comparatorKey = null;
+
+    HiveMapComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+
+        LinkedHashMap map1 = (LinkedHashMap) key1;
+        LinkedHashMap map2 = (LinkedHashMap) key2;
+        if (map1.entrySet().size() != map2.entrySet().size()) {
+            return map1.entrySet().size() > map2.entrySet().size() ? 1 : -1;
+        }
+        if (map1.entrySet().size() == 0) {
+            return 0;
+        }
+
+        if (comparatorKey == null) {
+            comparatorKey =
+                    
WritableComparatorFactory.get(map1.keySet().iterator().next(), nullSafe, 
nullOrdering);
+            comparatorValue =
+                    
WritableComparatorFactory.get(map1.values().iterator().next(), nullSafe, 
nullOrdering);
+        }
+
+        result = comparatorKey.compare(map1.keySet().iterator().next(),
+                map2.keySet().iterator().next());
+        if (result != 0) {
+            return result;
+        }
+        return comparatorValue.compare(map1.values().iterator().next(), 
map2.values().iterator().next());

Review comment:
       Thanks for pointing this out. Modified the code and added a test case.

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/HiveWritableComparator.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveWritableComparator extends WritableComparator {
+    private WritableComparator comparator = null;
+    protected transient boolean nullSafe;
+    transient NullOrdering nullOrdering;
+    protected transient int not_null = 2;

Review comment:
       nullSafe and nullOrdering are made protected final as its used from sub 
classes.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveStructComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+
+class HiveStructComparator extends HiveWritableComparator {

Review comment:
       done

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveMapComparator.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.LinkedHashMap;
+
+public class HiveMapComparator extends HiveWritableComparator {
+    WritableComparator comparatorValue = null;
+    WritableComparator comparatorKey = null;
+
+    HiveMapComparator(boolean nullSafe, NullOrdering nullOrdering) {
+        super(nullSafe, nullOrdering);
+    }
+
+    @Override
+    public int compare(Object key1, Object key2) {
+        int result = checkNull(key1, key2);
+        if (result != not_null) {
+            return result;
+        }
+
+        LinkedHashMap map1 = (LinkedHashMap) key1;
+        LinkedHashMap map2 = (LinkedHashMap) key2;
+        if (map1.entrySet().size() != map2.entrySet().size()) {
+            return map1.entrySet().size() > map2.entrySet().size() ? 1 : -1;
+        }

Review comment:
       done

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/WritableComparatorFactory.java
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import 
org.apache.hadoop.hive.serde2.objectinspector.StandardUnionObjectInspector.StandardUnion;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+
+public final class WritableComparatorFactory {
+    public static WritableComparator get(TypeInfo typeInfo, boolean nullSafe, 
NullOrdering nullOrdering) {
+        switch (typeInfo.getCategory()) {
+            case PRIMITIVE:
+                return new HiveWritableComparator(nullSafe, nullOrdering);
+            case LIST:
+                return new HiveListComparator(nullSafe, nullOrdering);
+            case MAP:
+                return new HiveMapComparator(nullSafe, nullOrdering);
+            case STRUCT:
+                return new HiveStructComparator(nullSafe, nullOrdering);
+            case UNION:
+                return new HiveUnionComparator(nullSafe, nullOrdering);
+            default:
+                throw new IllegalStateException("Unexpected value: " + 
typeInfo.getCategory());
+        }
+    }
+
+    public static WritableComparator get(Object key, boolean nullSafe, 
NullOrdering nullOrdering) {

Review comment:
       Yes i think its fine. For the case where one or both the keys are null, 
the handling is done in HiveWritableComparator.

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/WritableComparatorFactory.java
##########
@@ -0,0 +1,58 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import 
org.apache.hadoop.hive.serde2.objectinspector.StandardUnionObjectInspector.StandardUnion;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+
+public final class WritableComparatorFactory {
+    public static WritableComparator get(TypeInfo typeInfo, boolean nullSafe, 
NullOrdering nullOrdering) {
+        switch (typeInfo.getCategory()) {
+            case PRIMITIVE:
+                return new HiveWritableComparator(nullSafe, nullOrdering);
+            case LIST:
+                return new HiveListComparator(nullSafe, nullOrdering);
+            case MAP:
+                return new HiveMapComparator(nullSafe, nullOrdering);
+            case STRUCT:
+                return new HiveStructComparator(nullSafe, nullOrdering);
+            case UNION:
+                return new HiveUnionComparator(nullSafe, nullOrdering);
+            default:
+                throw new IllegalStateException("Unexpected value: " + 
typeInfo.getCategory());
+        }
+    }
+
+    public static WritableComparator get(Object key, boolean nullSafe, 
NullOrdering nullOrdering) {
+        if (key instanceof ArrayList) {
+            // For array type struct is used as we do not know if all elements 
of array are of same type.
+            return new HiveStructComparator(nullSafe, nullOrdering);
+        } else if (key instanceof LinkedHashMap) {

Review comment:
       done

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/HiveWritableComparator.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.ql.exec;
+
+import org.apache.hadoop.hive.ql.util.NullOrdering;
+import org.apache.hadoop.io.WritableComparable;
+import org.apache.hadoop.io.WritableComparator;
+
+public class HiveWritableComparator extends WritableComparator {

Review comment:
       This is to keep the comparator class as WritableComparator during 
declaration. 




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to