added column properties

updated result files for new system.metadata.column_properties table changes requested by review
prestodb · Jul 24, 2018 · 0772921 · 0772921
1 parent ad909cd
commit 0772921
Show file tree

Hide file tree

Showing 30 changed files with 335 additions and 71 deletions.
diff --git a/presto-docs/src/main/sphinx/sql/alter-table.rst b/presto-docs/src/main/sphinx/sql/alter-table.rst
@@ -8,7 +8,7 @@ Synopsis
 .. code-block:: none
 
     ALTER TABLE name RENAME TO new_name
-    ALTER TABLE name ADD COLUMN column_name data_type [ COMMENT comment ]
+    ALTER TABLE name ADD COLUMN column_name data_type [ COMMENT comment ] [ WITH ( property_name = expression [, ...] ) ]
     ALTER TABLE name DROP COLUMN column_name
     ALTER TABLE name RENAME COLUMN column_name TO new_column_name
 

diff --git a/presto-docs/src/main/sphinx/sql/create-table.rst b/presto-docs/src/main/sphinx/sql/create-table.rst
@@ -9,7 +9,7 @@ Synopsis
 
     CREATE TABLE [ IF NOT EXISTS ]
     table_name (
-      { column_name data_type [ COMMENT comment ]
+      { column_name data_type [ COMMENT comment ] [ WITH ( property_name = expression [, ...] ) ]
       | LIKE existing_table_name [ { INCLUDING | EXCLUDING } PROPERTIES ] }
       [, ...]
     )
@@ -27,11 +27,15 @@ The optional ``IF NOT EXISTS`` clause causes the error to be
 suppressed if the table already exists.
 
 The optional ``WITH`` clause can be used to set properties
-on the newly created table.  To list all available table
+on the newly created table or on single columns.  To list all available table
 properties, run the following query::
 
     SELECT * FROM system.metadata.table_properties
 
+To list all available column properties, run the following query::
+
+    SELECT * FROM system.metadata.column_properties
+
 The ``LIKE`` clause can be used to include all the column definitions from
 an existing table in the new table. Multiple ``LIKE`` clauses may be
 specified, which allows copying the columns from multiple tables.
@@ -42,6 +46,7 @@ name as one of the copied properties, the value from the ``WITH`` clause
 will be used. The default behavior is ``EXCLUDING PROPERTIES``. The
 ``INCLUDING PROPERTIES`` option maybe specified for at most one table.
 
+
 Examples
 --------
 

diff --git a/presto-main/src/main/java/com/facebook/presto/connector/ConnectorManager.java b/presto-main/src/main/java/com/facebook/presto/connector/ConnectorManager.java
@@ -265,6 +265,7 @@ private synchronized void addConnectorInternal(MaterializedConnector connector)
                 .ifPresent(accessControl -> accessControlManager.addCatalogAccessControl(connectorId, accessControl));
 
         metadataManager.getTablePropertyManager().addProperties(connectorId, connector.getTableProperties());
+        metadataManager.getColumnPropertyManager().addProperties(connectorId, connector.getColumnProperties());
         metadataManager.getSchemaPropertyManager().addProperties(connectorId, connector.getSchemaProperties());
         metadataManager.getSessionPropertyManager().addConnectorSessionProperties(connectorId, connector.getSessionProperties());
     }
@@ -334,6 +335,7 @@ private static class MaterializedConnector
         private final List<PropertyMetadata<?>> sessionProperties;
         private final List<PropertyMetadata<?>> tableProperties;
         private final List<PropertyMetadata<?>> schemaProperties;
+        private final List<PropertyMetadata<?>> columnProperties;
 
         public MaterializedConnector(ConnectorId connectorId, Connector connector)
         {
@@ -418,6 +420,10 @@ public MaterializedConnector(ConnectorId connectorId, Connector connector)
             List<PropertyMetadata<?>> schemaProperties = connector.getSchemaProperties();
             requireNonNull(schemaProperties, "Connector %s returned a null schema properties set");
             this.schemaProperties = ImmutableList.copyOf(schemaProperties);
+
+            List<PropertyMetadata<?>> columnProperties = connector.getColumnProperties();
+            requireNonNull(columnProperties, "Connector %s returned a null column properties set");
+            this.columnProperties = ImmutableList.copyOf(columnProperties);
         }
 
         public ConnectorId getConnectorId()
@@ -480,6 +486,11 @@ public List<PropertyMetadata<?>> getTableProperties()
             return tableProperties;
         }
 
+        public List<PropertyMetadata<?>> getColumnProperties()
+        {
+            return columnProperties;
+        }
+
         public List<PropertyMetadata<?>> getSchemaProperties()
         {
             return schemaProperties;

diff --git a/...-main/src/main/java/com/facebook/presto/connector/system/ColumnPropertiesSystemTable.java b/...-main/src/main/java/com/facebook/presto/connector/system/ColumnPropertiesSystemTable.java
@@ -0,0 +1,29 @@
+/*
+ * 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.connector.system;
+
+import com.facebook.presto.metadata.Metadata;
+import com.facebook.presto.transaction.TransactionManager;
+
+import javax.inject.Inject;
+
+public class ColumnPropertiesSystemTable
+        extends AbstractPropertiesSystemTable
+{
+    @Inject
+    public ColumnPropertiesSystemTable(TransactionManager transactionManager, Metadata metadata)
+    {
+        super("column_properties", transactionManager, () -> metadata.getColumnPropertyManager().getAllProperties());
+    }
+}
diff --git a/presto-main/src/main/java/com/facebook/presto/connector/system/SystemConnectorModule.java b/presto-main/src/main/java/com/facebook/presto/connector/system/SystemConnectorModule.java
@@ -54,6 +54,7 @@ public void configure(Binder binder)
         globalTableBinder.addBinding().to(CatalogSystemTable.class).in(Scopes.SINGLETON);
         globalTableBinder.addBinding().to(SchemaPropertiesSystemTable.class).in(Scopes.SINGLETON);
         globalTableBinder.addBinding().to(TablePropertiesSystemTable.class).in(Scopes.SINGLETON);
+        globalTableBinder.addBinding().to(ColumnPropertiesSystemTable.class).in(Scopes.SINGLETON);
         globalTableBinder.addBinding().to(TransactionsSystemTable.class).in(Scopes.SINGLETON);
 
         globalTableBinder.addBinding().to(AttributeJdbcTable.class).in(Scopes.SINGLETON);

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/AddColumnTask.java b/presto-main/src/main/java/com/facebook/presto/execution/AddColumnTask.java
@@ -14,12 +14,14 @@
 package com.facebook.presto.execution;
 
 import com.facebook.presto.Session;
+import com.facebook.presto.connector.ConnectorId;
 import com.facebook.presto.metadata.Metadata;
 import com.facebook.presto.metadata.QualifiedObjectName;
 import com.facebook.presto.metadata.TableHandle;
 import com.facebook.presto.security.AccessControl;
 import com.facebook.presto.spi.ColumnHandle;
 import com.facebook.presto.spi.ColumnMetadata;
+import com.facebook.presto.spi.PrestoException;
 import com.facebook.presto.spi.type.Type;
 import com.facebook.presto.sql.analyzer.SemanticException;
 import com.facebook.presto.sql.tree.AddColumn;
@@ -33,7 +35,9 @@
 import java.util.Optional;
 
 import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
+import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
 import static com.facebook.presto.spi.type.TypeSignature.parseTypeSignature;
+import static com.facebook.presto.sql.NodeUtils.mapFromProperties;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.COLUMN_ALREADY_EXISTS;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
 import static com.facebook.presto.sql.analyzer.SemanticErrorCode.TYPE_MISMATCH;
@@ -60,6 +64,9 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
             throw new SemanticException(MISSING_TABLE, statement, "Table '%s' does not exist", tableName);
         }
 
+        ConnectorId connectorId = metadata.getCatalogHandle(session, tableName.getCatalogName())
+                .orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + tableName.getCatalogName()));
+
         accessControl.checkCanAddColumns(session.getRequiredTransactionId(), session.getIdentity(), tableName);
 
         Map<String, ColumnHandle> columnHandles = metadata.getColumnHandles(session, tableHandle.get());
@@ -79,7 +86,16 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
             throw new SemanticException(COLUMN_ALREADY_EXISTS, statement, "Column '%s' already exists", element.getName());
         }
 
-        ColumnMetadata column = new ColumnMetadata(element.getName().getValue(), type, element.getComment().orElse(null), false);
+        Map<String, Expression> sqlProperties = mapFromProperties(element.getProperties());
+        Map<String, Object> columnProperties = metadata.getColumnPropertyManager().getProperties(
+                connectorId,
+                tableName.getCatalogName(),
+                sqlProperties,
+                session,
+                metadata,
+                parameters);
+
+        ColumnMetadata column = new ColumnMetadata(element.getName().getValue(), type, element.getComment().orElse(null), null, false, columnProperties);
 
         metadata.addColumn(session, tableHandle.get(), column);
 

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/CreateTableTask.java b/presto-main/src/main/java/com/facebook/presto/execution/CreateTableTask.java
@@ -95,6 +95,9 @@ public ListenableFuture<?> internalExecute(CreateTable statement, Metadata metad
             return immediateFuture(null);
         }
 
+        ConnectorId connectorId = metadata.getCatalogHandle(session, tableName.getCatalogName())
+                .orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + tableName.getCatalogName()));
+
         LinkedHashMap<String, ColumnMetadata> columns = new LinkedHashMap<>();
         Map<String, Object> inheritedProperties = ImmutableMap.of();
         boolean includingProperties = false;
@@ -115,7 +118,17 @@ public ListenableFuture<?> internalExecute(CreateTable statement, Metadata metad
                 if (columns.containsKey(name)) {
                     throw new SemanticException(DUPLICATE_COLUMN_NAME, column, "Column name '%s' specified more than once", column.getName());
                 }
-                columns.put(name, new ColumnMetadata(name, type, column.getComment().orElse(null), false));
+
+                Map<String, Expression> sqlProperties = mapFromProperties(column.getProperties());
+                Map<String, Object> columnProperties = metadata.getColumnPropertyManager().getProperties(
+                        connectorId,
+                        tableName.getCatalogName(),
+                        sqlProperties,
+                        session,
+                        metadata,
+                        parameters);
+
+                columns.put(name, new ColumnMetadata(name, type, column.getComment().orElse(null), null, false, columnProperties));
             }
             else if (element instanceof LikeClause) {
                 LikeClause likeClause = (LikeClause) element;
@@ -156,9 +169,6 @@ else if (element instanceof LikeClause) {
 
         accessControl.checkCanCreateTable(session.getRequiredTransactionId(), session.getIdentity(), tableName);
 
-        ConnectorId connectorId = metadata.getCatalogHandle(session, tableName.getCatalogName())
-                .orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + tableName.getCatalogName()));
-
         Map<String, Expression> sqlProperties = mapFromProperties(statement.getProperties());
         Map<String, Object> properties = metadata.getTablePropertyManager().getProperties(
                 connectorId,

diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/ColumnPropertyManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/ColumnPropertyManager.java
@@ -0,0 +1,25 @@
+/*
+ * 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.metadata;
+
+import static com.facebook.presto.spi.StandardErrorCode.INVALID_COLUMN_PROPERTY;
+
+public class ColumnPropertyManager
+        extends AbstractPropertyManager
+{
+    public ColumnPropertyManager()
+    {
+        super("column", INVALID_COLUMN_PROPERTY);
+    }
+}
diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java b/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java
@@ -289,4 +289,6 @@ public interface Metadata
     SchemaPropertyManager getSchemaPropertyManager();
 
     TablePropertyManager getTablePropertyManager();
+
+    ColumnPropertyManager getColumnPropertyManager();
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java
@@ -113,6 +113,7 @@ public class MetadataManager
     private final SessionPropertyManager sessionPropertyManager;
     private final SchemaPropertyManager schemaPropertyManager;
     private final TablePropertyManager tablePropertyManager;
+    private final ColumnPropertyManager columnPropertyManager;
     private final TransactionManager transactionManager;
 
     private final ConcurrentMap<String, Collection<ConnectorMetadata>> catalogsByQueryId = new ConcurrentHashMap<>();
@@ -123,6 +124,7 @@ public MetadataManager(FeaturesConfig featuresConfig,
             SessionPropertyManager sessionPropertyManager,
             SchemaPropertyManager schemaPropertyManager,
             TablePropertyManager tablePropertyManager,
+            ColumnPropertyManager columnPropertyManager,
             TransactionManager transactionManager)
     {
         this(featuresConfig,
@@ -132,6 +134,7 @@ public MetadataManager(FeaturesConfig featuresConfig,
                 sessionPropertyManager,
                 schemaPropertyManager,
                 tablePropertyManager,
+                columnPropertyManager,
                 transactionManager);
     }
 
@@ -143,6 +146,7 @@ public MetadataManager(FeaturesConfig featuresConfig,
             SessionPropertyManager sessionPropertyManager,
             SchemaPropertyManager schemaPropertyManager,
             TablePropertyManager tablePropertyManager,
+            ColumnPropertyManager columnPropertyManager,
             TransactionManager transactionManager)
     {
         functions = new FunctionRegistry(typeManager, blockEncodingSerde, featuresConfig);
@@ -153,6 +157,7 @@ public MetadataManager(FeaturesConfig featuresConfig,
         this.sessionPropertyManager = requireNonNull(sessionPropertyManager, "sessionPropertyManager is null");
         this.schemaPropertyManager = requireNonNull(schemaPropertyManager, "schemaPropertyManager is null");
         this.tablePropertyManager = requireNonNull(tablePropertyManager, "tablePropertyManager is null");
+        this.columnPropertyManager = requireNonNull(columnPropertyManager, "columnPropertyManager is null");
         this.transactionManager = requireNonNull(transactionManager, "transactionManager is null");
 
         verifyComparableOrderableContract();
@@ -183,6 +188,7 @@ public static MetadataManager createTestMetadataManager(CatalogManager catalogMa
                 new SessionPropertyManager(),
                 new SchemaPropertyManager(),
                 new TablePropertyManager(),
+                new ColumnPropertyManager(),
                 createTestTransactionManager(catalogManager));
     }
 
@@ -904,6 +910,12 @@ public TablePropertyManager getTablePropertyManager()
         return tablePropertyManager;
     }
 
+    @Override
+    public ColumnPropertyManager getColumnPropertyManager()
+    {
+        return columnPropertyManager;
+    }
+
     private ViewDefinition deserializeView(String data)
     {
         try {

diff --git a/presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java b/presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java
@@ -88,6 +88,7 @@
 import com.facebook.presto.memory.NodeMemoryConfig;
 import com.facebook.presto.memory.ReservedSystemMemoryConfig;
 import com.facebook.presto.metadata.CatalogManager;
+import com.facebook.presto.metadata.ColumnPropertyManager;
 import com.facebook.presto.metadata.DiscoveryNodeManager;
 import com.facebook.presto.metadata.ForNodeManager;
 import com.facebook.presto.metadata.HandleJsonModule;
@@ -274,6 +275,9 @@ protected void setup(Binder binder)
         // table properties
         binder.bind(TablePropertyManager.class).in(Scopes.SINGLETON);
 
+        // column properties
+        binder.bind(ColumnPropertyManager.class).in(Scopes.SINGLETON);
+
         // node manager
         discoveryBinder(binder).bindSelector("presto");
         binder.bind(DiscoveryNodeManager.class).in(Scopes.SINGLETON);