Merge pull request #514 from peterbae/SQLServerDataTableImprovement-497

Fix creating SQLServerDataTable being O(n^2) issue
microsoft · Oct 11, 2017 · 7a0fe66 · 7a0fe66
2 parents e7b68fd + adf10ea
commit 7a0fe66
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 24 deletions.
diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataTable.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataTable.java
@@ -13,17 +13,20 @@
 import java.time.OffsetDateTime;
 import java.time.OffsetTime;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.UUID;
 
 public final class SQLServerDataTable {
 
     int rowCount = 0;
     int columnCount = 0;
     Map<Integer, SQLServerDataColumn> columnMetadata = null;
+    Set<String> columnNames = null;
     Map<Integer, Object[]> rows = null;
 
     private String tvpName = null;
@@ -37,6 +40,7 @@ public final class SQLServerDataTable {
     // Name used in CREATE TYPE
     public SQLServerDataTable() throws SQLServerException {
         columnMetadata = new LinkedHashMap<>();
+        columnNames = new HashSet<>();
         rows = new HashMap<>();
     }
 
@@ -75,7 +79,7 @@ public synchronized Iterator<Entry<Integer, Object[]>> getIterator() {
     public synchronized void addColumnMetadata(String columnName,
             int sqlType) throws SQLServerException {
         // column names must be unique
-        Util.checkDuplicateColumnName(columnName, columnMetadata);
+        Util.checkDuplicateColumnName(columnName, columnNames);
         columnMetadata.put(columnCount++, new SQLServerDataColumn(columnName, sqlType));
     }
 
@@ -89,10 +93,11 @@ public synchronized void addColumnMetadata(String columnName,
      */
     public synchronized void addColumnMetadata(SQLServerDataColumn column) throws SQLServerException {
         // column names must be unique
-        Util.checkDuplicateColumnName(column.columnName, columnMetadata);
+        Util.checkDuplicateColumnName(column.columnName, columnNames);
         columnMetadata.put(columnCount++, column);
     }
 
+
     /**
      * Adds one row of data to the data table.
      * 

diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/TVP.java b/src/main/java/com/microsoft/sqlserver/jdbc/TVP.java
@@ -12,10 +12,12 @@
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.text.MessageFormat;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 enum TVPType {
     ResultSet,
@@ -48,6 +50,7 @@ class TVP {
     Iterator<Entry<Integer, Object[]>> sourceDataTableRowIterator = null;
     ISQLServerDataRecord sourceRecord = null;
     TVPType tvpType = null;
+    Set<String> columnNames = null;
 
     // MultiPartIdentifierState
     enum MPIState {
@@ -94,6 +97,7 @@ void initTVP(TVPType type,
             ISQLServerDataRecord tvpRecord) throws SQLServerException {
         initTVP(TVPType.ISQLServerDataRecord, tvpPartName);
         sourceRecord = tvpRecord;
+        columnNames = new HashSet<>();
         // Populate TVP metdata from ISQLServerDataRecord.
         populateMetadataFromDataRecord();
 
@@ -185,8 +189,9 @@ void populateMetadataFromDataRecord() throws SQLServerException {
             throw new SQLServerException(SQLServerException.getErrString("R_TVPEmptyMetadata"), null);
         }
         for (int i = 0; i < sourceRecord.getColumnCount(); i++) {
+            Util.checkDuplicateColumnName(sourceRecord.getColumnMetaData(i + 1).columnName, columnNames);
+
             // Make a copy here as we do not want to change user's metadata.
-            Util.checkDuplicateColumnName(sourceRecord.getColumnMetaData(i + 1).columnName, columnMetadata);
             SQLServerMetaData metaData = new SQLServerMetaData(sourceRecord.getColumnMetaData(i + 1));
             columnMetadata.put(i, metaData);
         }

diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/Util.java b/src/main/java/com/microsoft/sqlserver/jdbc/Util.java
@@ -19,6 +19,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
+import java.util.Set;
 import java.util.UUID;
 import java.util.logging.Level;
 import java.util.logging.LogManager;
@@ -557,28 +558,22 @@ static String escapeSQLId(String inID) {
         outID.append(']');
         return outID.toString();
     }
-
+
+    /**
+     * Checks if duplicate columns exists, in O(n) time.
+     * 
+     * @param columnName
+     *            the name of the column
+     * @throws SQLServerException
+     *             when a duplicate column exists
+     */
     static void checkDuplicateColumnName(String columnName,
-            Map<Integer, ?> columnMetadata) throws SQLServerException {
-        if (columnMetadata.get(0) instanceof SQLServerMetaData) {
-            for (Entry<Integer, ?> entry : columnMetadata.entrySet()) {
-                SQLServerMetaData value = (SQLServerMetaData) entry.getValue();
-                if (value.columnName.equals(columnName)) {
-                    MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_TVPDuplicateColumnName"));
-                    Object[] msgArgs = {columnName};
-                    throw new SQLServerException(null, form.format(msgArgs), null, 0, false);
-                }
-            }
-        }
-        else if (columnMetadata.get(0) instanceof SQLServerDataColumn) {
-            for (Entry<Integer, ?> entry : columnMetadata.entrySet()) {
-                SQLServerDataColumn value = (SQLServerDataColumn) entry.getValue();
-                if (value.columnName.equals(columnName)) {
-                    MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_TVPDuplicateColumnName"));
-                    Object[] msgArgs = {columnName};
-                    throw new SQLServerException(null, form.format(msgArgs), null, 0, false);
-                }
-            }
+            Set<String> columnNames) throws SQLServerException {
+        //columnList.add will return false if the same column name already exists
+        if (!columnNames.add(columnName)) {
+            MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_TVPDuplicateColumnName"));
+            Object[] msgArgs = {columnName};
+            throw new SQLServerException(null, form.format(msgArgs), null, 0, false);
         }
     }
 

diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/datatypes/TVPWithSqlVariantTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/datatypes/TVPWithSqlVariantTest.java
@@ -378,6 +378,23 @@ public void testIntStoredProcedure() throws SQLServerException {
             Cstatement.close();
         }
     }
+
+    /**
+     * Test for allowing duplicate columns
+     * 
+     * @throws SQLServerException
+     */
+    @Test
+    public void testDuplicateColumn() throws SQLServerException {
+        tvp = new SQLServerDataTable();
+        tvp.addColumnMetadata("c1", microsoft.sql.Types.SQL_VARIANT);
+        tvp.addColumnMetadata("c2", microsoft.sql.Types.SQL_VARIANT);
+        try {
+            tvp.addColumnMetadata("c2", microsoft.sql.Types.SQL_VARIANT);
+        } catch (SQLServerException e) {
+            assertEquals(e.getMessage(), "A column name c2 already belongs to this SQLServerDataTable.");
+        }
+    }
 
     private static String[] createNumericValues() {
         Boolean C1_BIT;