Ensure symbols are not reused across plan nodes

This fixes an aliasing issue that causes queries to return incorrect results.
It happens because symbol allocator can produce non-globally unique names when
columns in the table share the same prefix and have the form xxx_nnn. E.g.,

CREATE TABLE t AS SELECT * FROM (VALUES (1,2)) t (foo_1, foo_2_4);
SELECT foo_1, foo_2_4 from t;

Incorrect plan:

 - Output[foo_1, foo_2_4] => [foo:bigint, foo:bigint]
         foo_1 := foo
         foo_2_4 := foo
     - Exchange[GATHER] => foo:bigint
         - TableScan[t, original constraint=true] => [foo:bigint]
                 foo := <column foo_1>

Expected plan:

 - Output[foo_1, foo_2_4] => [foo:bigint, foo_2:bigint]
         foo_1 := foo
         foo_2_4 := foo_2
     - Exchange[GATHER] => foo:bigint, foo_2:bigint
         - TableScan[t, original constraint=true] => [foo:bigint, foo_2:bigint]
                 foo := <column foo_1>
                 foo_2 := <column foo_2_4>

presto-main/src/main/java/com/facebook/presto/sql/planner/SymbolAllocator.java

@@ -66,11 +66,12 @@ public Symbol newSymbol(String nameHint, Type type, String suffix)
  unique = unique + "$" + suffix;
  }
 
- if (symbols.containsKey(new Symbol(unique))) {
- unique += "_" + nextId();
+ String attempt = unique;
+ while (symbols.containsKey(new Symbol(attempt))) {
+ attempt = unique + "_" + nextId();
  }
 
- Symbol symbol = new Symbol(unique);
+ Symbol symbol = new Symbol(attempt);
  symbols.put(symbol, type);
  return symbol;
  }

presto-main/src/test/java/com/facebook/presto/sql/planner/TestSymbolAllocator.java

@@ -0,0 +1,27 @@
+package com.facebook.presto.sql.planner;
+
+import com.facebook.presto.spi.type.BigintType;
+import com.google.common.collect.ImmutableSet;
+import org.testng.annotations.Test;
+
+import java.util.Set;
+
+import static org.testng.Assert.assertEquals;
+
+public class TestSymbolAllocator
+{
+ @Test
+ public void testUnique()
+ throws Exception
+ {
+ SymbolAllocator allocator = new SymbolAllocator();
+ Set<Symbol> symbols = ImmutableSet.<Symbol>builder()
+ .add(allocator.newSymbol("foo_1_0", BigintType.BIGINT))
+ .add(allocator.newSymbol("foo", BigintType.BIGINT))
+ .add(allocator.newSymbol("foo", BigintType.BIGINT))
+ .add(allocator.newSymbol("foo", BigintType.BIGINT))
+ .build();
+
+ assertEquals(symbols.size(), 4);
+ }
+}

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java

@@ -319,6 +319,15 @@ public void testTableSamplePoissonizedRescaled()
  assertTrue(all.getMaterializedRows().containsAll(sample.getMaterializedRows()));
  }
 
+ @Test
+ public void testSymbolAliasing()
+ throws Exception
+ {
+ assertQueryTrue("CREATE TABLE test_symbol_aliasing AS SELECT 1 foo_1, 2 foo_2_4");
+ assertQuery("SELECT foo_1, foo_2_4 FROM test_symbol_aliasing", "SELECT 1, 2");
+ assertQueryTrue("DROP TABLE test_symbol_aliasing");
+ }
+
  private static void assertContains(MaterializedResult actual, MaterializedResult expected)
  {
  for (MaterializedRow row : expected.getMaterializedRows()) {