Fix multiple properties error handling

Currently we throw a GENERIC_INTERNAL_ERROR when we have
multiple properties mentioned in the SQL statement. This commit
fixes this issue to throw a SYNTAX_ERROR instead.

presto-main/src/main/java/com/facebook/presto/sql/NodeUtils.java

@@ -13,17 +13,21 @@
  */
 package com.facebook.presto.sql;
 
+import com.facebook.presto.spi.PrestoException;
 import com.facebook.presto.sql.tree.Expression;
 import com.facebook.presto.sql.tree.OrderBy;
 import com.facebook.presto.sql.tree.Property;
 import com.facebook.presto.sql.tree.SortItem;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.facebook.presto.spi.StandardErrorCode.SYNTAX_ERROR;
+import static java.lang.String.format;
 
 public class NodeUtils
 {
@@ -36,8 +40,16 @@ public static List<SortItem> getSortItemsFromOrderBy(Optional<OrderBy> orderBy)
 
     public static Map<String, Expression> mapFromProperties(List<Property> properties)
     {
-        return properties.stream().collect(toImmutableMap(
-                property -> property.getName().getValue(),
-                Property::getValue));
+        Map<String, Expression> outputMap = new HashMap<>();
+
+        for (Property property : properties) {
+            String key = property.getName().getValue();
+            if (outputMap.containsKey(key)) {
+                throw new PrestoException(SYNTAX_ERROR, format("Duplicate property found: %s=%s and %s=%s", key, outputMap.get(key), key, property.getValue()));
+            }
+            outputMap.put(key, property.getValue());
+        }
+
+        return ImmutableMap.copyOf(outputMap);
     }
 }

presto-main/src/test/java/com/facebook/presto/sql/TestNodeUtils.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed 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 com.facebook.presto.sql;
+
+import com.facebook.presto.spi.PrestoException;
+import com.facebook.presto.sql.tree.ArrayConstructor;
+import com.facebook.presto.sql.tree.Expression;
+import com.facebook.presto.sql.tree.Identifier;
+import com.facebook.presto.sql.tree.Property;
+import com.facebook.presto.sql.tree.StringLiteral;
+import com.google.common.collect.ImmutableList;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import static com.facebook.presto.spi.StandardErrorCode.SYNTAX_ERROR;
+import static java.lang.String.format;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+public class TestNodeUtils
+{
+    @Test
+    public void testMapFromProperties()
+    {
+        List<Property> propertyList = new ArrayList<>();
+        propertyList.add(new Property(new Identifier("a"), new StringLiteral("apple")));
+        propertyList.add(new Property(new Identifier("b"), new StringLiteral("ball")));
+        Map<String, Expression> stringExpressionMap = NodeUtils.mapFromProperties(propertyList);
+        assertEquals(2, stringExpressionMap.size());
+        assertEquals(new StringLiteral("apple"), stringExpressionMap.get("a"));
+        assertEquals(new StringLiteral("ball"), stringExpressionMap.get("b"));
+    }
+
+    @Test
+    public void testDuplicateKeyFromProperties()
+    {
+        List<Property> propertyList = new ArrayList<>();
+        propertyList.add(new Property(new Identifier("partitioned_by"), new ArrayConstructor(ImmutableList.of(new StringLiteral("ds")))));
+        propertyList.add(new Property(new Identifier("partitioned_by"), new ArrayConstructor(ImmutableList.of(new StringLiteral("source")))));
+
+        try {
+            NodeUtils.mapFromProperties(propertyList);
+            fail("PrestoException not thrown for duplicate properties");
+        }
+        catch (Exception e) {
+            assertTrue(e instanceof PrestoException, format("Expected PrestoException but found %s", e));
+            PrestoException prestoException = (PrestoException) e;
+            assertEquals(prestoException.getErrorCode(), SYNTAX_ERROR.toErrorCode());
+            assertEquals(prestoException.getMessage(), "Duplicate property found: partitioned_by=ARRAY['ds'] and partitioned_by=ARRAY['source']");
+        }
+    }
+}