[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2020-10-28 Thread GitBox


cloud-fan commented on a change in pull request #24246:
URL: https://github.com/apache/spark/pull/24246#discussion_r513932838



##
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
##
@@ -0,0 +1,131 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   * 
+   * If the catalog supports views, this must return identifiers for only 
tables and not views.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   * 
+   * If the catalog supports views and contains a view for the identifier and 
not a table, this
+   * must throw {@link NoSuchTableException}.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist or is a view
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Invalidate cached table metadata for an {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data. If the table 
does not exist or is
+   * not cached, do nothing. Calling this method should not query remote 
services.
+   *
+   * @param ident a table identifier
+   */
+  default void invalidateTable(Identifier ident) {

Review comment:
   Yes, we should add this kind of missing actions into v2 commands as 
well. I think DROP TABLE has the same issue.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2020-10-21 Thread GitBox


cloud-fan commented on a change in pull request #24246:
URL: https://github.com/apache/spark/pull/24246#discussion_r509354725



##
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
##
@@ -0,0 +1,131 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   * 
+   * If the catalog supports views, this must return identifiers for only 
tables and not views.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   * 
+   * If the catalog supports views and contains a view for the identifier and 
not a table, this
+   * must throw {@link NoSuchTableException}.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist or is a view
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Invalidate cached table metadata for an {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data. If the table 
does not exist or is
+   * not cached, do nothing. Calling this method should not query remote 
services.
+   *
+   * @param ident a table identifier
+   */
+  default void invalidateTable(Identifier ident) {

Review comment:
   table cache is managed by Spark, so the API here can not affect table 
cache inside Spark.





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:
us...@infra.apache.org



-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-12-04 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r354100501
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,137 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ * 
+ * TableCatalog implementations may be case sensitive or case insensitive. 
Spark will pass
+ * {@link Identifier table identifiers} without modification. Field names 
passed to
+ * {@link #alterTable(Identifier, TableChange...)} will be normalized to match 
the case used in the
+ * table schema when updating, renaming, or dropping existing columns when 
catalyst analysis is case
+ * insensitive.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   * 
+   * If the catalog supports views, this must return identifiers for only 
tables and not views.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   * 
+   * If the catalog supports views and contains a view for the identifier and 
not a table, this
+   * must throw {@link NoSuchTableException}.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist or is a view
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Invalidate cached table metadata for an {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data. If the table 
does not exist or is
+   * not cached, do nothing. Calling this method should not query remote 
services.
+   *
+   * @param ident a table identifier
+   */
+  default void invalidateTable(Identifier ident) {
+  }
+
+  /**
+   * Test whether a table exists using an {@link Identifier identifier} from 
the catalog.
+   * 
+   * If the catalog supports views and contains a view for the identifier and 
not a table, this
+   * must return false.
+   *
+   * @param ident a table identifier
+   * @return true if the table exists, false otherwise
+   */
+  default boolean tableExists(Identifier ident) {
+try {
+  return loadTable(ident) != null;
+} catch (NoSuchTableException e) {
+  return false;
+}
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @param partitions transforms to use for partitioning data in the table
+   * @param properties a string map of table properties
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table or view already exists for 
the identifier
+   * @throws UnsupportedOperationException If a requested partition transform 
is not supported
+   * @throws NoSuchNamespaceException If the identifier namespace does not 
exist (optional)
+   */
+  Table createTable(
+  Identifier ident,
+  StructType schema,
+  Transform[] partitions,
+  Map properties) throws TableAlreadyExistsException, 
NoSuchNamespaceException;
+
+  /**
+   * Apply a set of {@link TableChange changes} to a table in the catalog.
+   * 
+   * Implementations may reject the requested changes. If any change is 
rejected, none of the
+   * changes should be applied to the table.
+   * 
+   * If the catalog supports views and contains a vi

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-07 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281753933
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,137 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ * 
+ * TableCatalog implementations may be case sensitive or case insensitive. 
Spark will pass
 
 Review comment:
   It may worth to add a method to report this information. We can think about 
it later.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-06 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281441873
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
 
 Review comment:
   i see, makes sense.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-06 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281441822
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn(["a", "b", "c"], "x") should produce column 
a.b.x.
+   * 
+   * If the field does not exist, the change will result in an {@link 
IllegalArgumentException}.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the re

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-06 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281253639
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,131 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   * 
+   * If the catalog supports views, this must return identifiers for only 
tables and not views.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   * 
+   * If the catalog supports views and contains a view for the identifier and 
not a table, this
+   * must throw {@link NoSuchTableException}.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist or is a view
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Invalidate cached table metadata for an {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data. If the table 
does not exist or is
+   * not cached, do nothing. Calling this method should not query remote 
services.
+   *
+   * @param ident a table identifier
+   */
+  default void invalidateTable(Identifier ident) {
 
 Review comment:
   what's the use case of this method? I can only think of one use case: when 
end users run `REFRESH TABLE`. Is there any more use cases?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-06 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281249305
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn(["a", "b", "c"], "x") should produce column 
a.b.x.
+   * 
+   * If the field does not exist, the change will result in an {@link 
IllegalArgumentException}.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the re

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-06 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281248583
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
 
 Review comment:
   since we decided to drop the overloads at the first version, shall we also 
remove the overloads for `addColumn`? Then it's consistent with the API design 
of `createTable` and we can change them together in the future if we agree to.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-05-05 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r281051805
 
 

 ##
 File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalog/v2/TableCatalogSuite.scala
 ##
 @@ -0,0 +1,639 @@
+/*
+ * 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.spark.sql.catalog.v2
+
+import java.util
+import java.util.Collections
+
+import scala.collection.JavaConverters._
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.analysis.{NoSuchTableException, 
TableAlreadyExistsException}
+import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types.{DoubleType, IntegerType, LongType, 
StringType, StructField, StructType, TimestampType}
+import org.apache.spark.sql.util.CaseInsensitiveStringMap
+
+class TableCatalogSuite extends SparkFunSuite {
 
 Review comment:
   BTW I think it's too complicated to let the `TableCatalog` respect the 
Spark's case sensitive config, maybe we should pick one specific behavior and 
ask `TableCatalog` to follow it. Since Spark is case insensitive by default, 
how about we add a javadoc and say that `TableCatalog` name lookup is case 
insensitive?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-23 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277973531
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,153 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Refresh table metadata for {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data and reload it. 
If the table is not
+   * already loaded, load it and update caches with the new version.
+   *
+   * @param ident a table identifier
+   * @return the table's current metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  default Table refreshTable(Identifier ident) throws NoSuchTableException {
+return loadTable(ident);
+  }
+
+  /**
+   * Test whether a table exists using an {@link Identifier identifier} from 
the catalog.
+   *
+   * @param ident a table identifier
+   * @return true if the table exists, false otherwise
+   */
+  default boolean tableExists(Identifier ident) {
+try {
+  return loadTable(ident) != null;
+} catch (NoSuchTableException e) {
+  return false;
+}
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table already exists for the 
identifier
+   * @throws NoSuchNamespaceException If the identifier namespace does not 
exist (optional)
+   */
+  default Table createTable(
 
 Review comment:
   The drawback I can think of is that we can't guarantee the default values if 
the implementation overrides these overload methods.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277349669
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,153 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Refresh table metadata for {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data and reload it. 
If the table is not
+   * already loaded, load it and update caches with the new version.
+   *
+   * @param ident a table identifier
+   * @return the table's current metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  default Table refreshTable(Identifier ident) throws NoSuchTableException {
+return loadTable(ident);
+  }
+
+  /**
+   * Test whether a table exists using an {@link Identifier identifier} from 
the catalog.
+   *
+   * @param ident a table identifier
+   * @return true if the table exists, false otherwise
+   */
+  default boolean tableExists(Identifier ident) {
+try {
+  return loadTable(ident) != null;
+} catch (NoSuchTableException e) {
+  return false;
+}
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table already exists for the 
identifier
+   * @throws NoSuchNamespaceException If the identifier namespace does not 
exist (optional)
+   */
+  default Table createTable(
 
 Review comment:
   This is implemented by users and called by Spark, which makes me think that 
overload doesn't make the API better.
   
   There are 3 `createTable`, and users can override all of them, which is not 
expected. I think we should make API as strict as possible, and only have one 
`createTable`.
   
   On the other hand, it's called by Spark so it doesn't hurt to write down the 
default values at the caller side.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277347568
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
 
 Review comment:
   >  We don't want to require callers to think about what missing values 
should be
   
   The caller is Spark, that's why I think it doesn't matter.
   
   The drawback of putting default value here is: we need to add several 
overloads. There are 3 `addColumn` methods here. We may need more overloads if 
we decide to add column properties.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277345464
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   * 
+   * If the field does not exist, the change will result in an {@link 
IllegalArgumentException}.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277324745
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   * 
+   * If the field does not exist, the change will result in an {@link 
IllegalArgumentException}.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277325409
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   * 
+   * If the field does not exist, the change will result in an {@link 
IllegalArgumentException}.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277326394
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalog/v2/CatalogV2Implicits.scala
 ##
 @@ -0,0 +1,98 @@
+/*
+ * 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.spark.sql.catalog.v2
+
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalog.v2.expressions.{BucketTransform, 
IdentityTransform, LogicalExpressions, Transform}
+import org.apache.spark.sql.catalyst.catalog.BucketSpec
+import org.apache.spark.sql.types.StructType
+
+/**
+ * Conversion helpers for working with v2 [[CatalogPlugin]].
+ */
+object CatalogV2Implicits {
+  implicit class PartitionTypeHelper(partitionType: StructType) {
+def asTransforms: Array[Transform] = 
partitionType.names.map(LogicalExpressions.identity)
+  }
+
+  implicit class BucketSpecHelper(spec: BucketSpec) {
+def asTransform: BucketTransform = {
+  if (spec.sortColumnNames.nonEmpty) {
+throw new AnalysisException(
+  s"Cannot convert bucketing with sort columns to a transform: $spec")
+  }
+
+  LogicalExpressions.bucket(spec.numBuckets, spec.bucketColumnNames: _*)
+}
+  }
+
+  implicit class TransformHelper(transforms: Seq[Transform]) {
+def asPartitionColumns: Seq[String] = {
+  val (idTransforms, nonIdTransforms) = 
transforms.partition(_.isInstanceOf[IdentityTransform])
+
+  if (nonIdTransforms.nonEmpty) {
+throw new AnalysisException("Transforms cannot be converted to 
partition columns: " +
+nonIdTransforms.map(_.describe).mkString(", "))
+  }
+
+  idTransforms.map(_.asInstanceOf[IdentityTransform]).map(_.reference).map 
{ ref =>
+val parts = ref.fieldNames
+if (parts.size > 1) {
+  throw new AnalysisException(s"Cannot partition by nested column: 
$ref")
+} else {
+  parts(0)
+}
+  }
+}
+  }
+
+  implicit class CatalogHelper(plugin: CatalogPlugin) {
+def asTableCatalog: TableCatalog = plugin match {
+  case tableCatalog: TableCatalog =>
+tableCatalog
+  case _ =>
+throw new AnalysisException(s"Cannot use catalog ${plugin.name}: not a 
TableCatalog")
+}
+  }
+
+  implicit class NamespaceHelper(namespace: Array[String]) {
+def quoted: String = namespace.map(quote).mkString(".")
+  }
+
+  implicit class IdentifierHelper(ident: Identifier) {
+def quoted: String = {
+  if (ident.namespace.nonEmpty) {
+ident.namespace.map(quote).mkString(".") + "." + quote(ident.name)
+  } else {
+quote(ident.name)
+  }
+}
+  }
+
+  implicit class MultipartIdentifierHelper(namespace: Seq[String]) {
 
 Review comment:
   what's the difference between this and `NamespaceHelper`?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277322263
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,363 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   * 
+   * If the property already exists, it will be replaced with the new value.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   * 
+   * If the property does not exist, the change will succeed.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column.
+   * 
+   * If the field already exists, the change will result in an {@link 
IllegalArgumentException}.
+   * If the new field is nested and its parent does not exist or is not a 
struct, the change will
+   * result in an {@link IllegalArgumentException}.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
 
 Review comment:
   Do we need these overloads? This is called from Spark and it seems more 
reasonable to let Spark decides the default values.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277332041
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,150 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Refresh table metadata for {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data and reload it. 
If the table is not
+   * already loaded, load it and update caches with the new version.
+   *
+   * @param ident a table identifier
+   * @return the table's current metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  default Table refreshTable(Identifier ident) throws NoSuchTableException {
 
 Review comment:
   This API design looks a bit weird, I think a more common approach is: 
`refreshTable` is just an action:
   ```
   Table loadTable(Identifier ident)
   default void refreshTable(Identifier ident) {}
   ```
   
   When Spark want to get a fresh table, just call `refreshTable` first, then 
call `loadTable`. Is there any specific reason to not go with this design?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277324369
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   */
+  static TableChange renameColumn(String[] fieldNames, String newName) {
+return new RenameColumn(fieldNames, newName);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType) {
+return new UpdateColumn(fieldNames, newDataType, true);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType, 
boolean isNullable) {
+return new UpdateC

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-22 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r277329968
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,150 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchNamespaceException;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.sources.v2.Table;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   * @throws NoSuchNamespaceException If the namespace does not exist 
(optional).
+   */
+  Identifier[] listTables(String[] namespace) throws NoSuchNamespaceException;
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Refresh table metadata for {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data and reload it. 
If the table is not
+   * already loaded, load it and update caches with the new version.
+   *
+   * @param ident a table identifier
+   * @return the table's current metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  default Table refreshTable(Identifier ident) throws NoSuchTableException {
+return loadTable(ident);
+  }
+
+  /**
+   * Test whether a table exists using an {@link Identifier identifier} from 
the catalog.
+   *
+   * @param ident a table identifier
+   * @return true if the table exists, false otherwise
+   */
+  default boolean tableExists(Identifier ident) {
+try {
+  return loadTable(ident) != null;
+} catch (NoSuchTableException e) {
+  return false;
+}
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table already exists for the 
identifier
+   */
+  default Table createTable(
+  Identifier ident,
+  StructType schema) throws TableAlreadyExistsException {
+return createTable(ident, schema, new Transform[0], 
Collections.emptyMap());
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @param properties a string map of table properties
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table already exists for the 
identifier
 
 Review comment:
   What if the namespace of the table does not exist?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-18 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276873864
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,143 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   */
+  Identifier[] listTables(String[] namespace);
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
 
 Review comment:
   will we support the Hive LOAD TABLE in the future?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-18 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276873533
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   */
+  static TableChange renameColumn(String[] fieldNames, String newName) {
+return new RenameColumn(fieldNames, newName);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType) {
+return new UpdateColumn(fieldNames, newDataType, true);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType, 
boolean isNullable) {
+return new UpdateC

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276182194
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,143 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   */
+  Identifier[] listTables(String[] namespace);
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
 
 Review comment:
   Also, the word "load" implies that, the table will be loaded to somewhere. 
Will we cache the table metadata somewhere inside Spark?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276185183
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   */
+  static TableChange renameColumn(String[] fieldNames, String newName) {
+return new RenameColumn(fieldNames, newName);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType) {
+return new UpdateColumn(fieldNames, newDataType, true);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType, 
boolean isNullable) {
 
 Review comment:
   I

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276183709
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
 
 Review comment:
   it must be a top-level column?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276185577
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
+   *
+   * @param fieldNames the current field names
+   * @param newName the new name
+   * @return a TableChange for the rename
+   */
+  static TableChange renameColumn(String[] fieldNames, String newName) {
+return new RenameColumn(fieldNames, newName);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType) {
+return new UpdateColumn(fieldNames, newDataType, true);
+  }
+
+  /**
+   * Create a TableChange for updating the type of a field.
+   * 
+   * The field names are used to find the field to update.
+   *
+   * @param fieldNames field names of the column to update
+   * @param newDataType the new data type
+   * @return a TableChange for the update
+   */
+  static TableChange updateColumn(String[] fieldNames, DataType newDataType, 
boolean isNullable) {
+return new UpdateC

[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276186070
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
 
 Review comment:
   do we need APIs to manage namespaces?


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276180056
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCapability.java
 ##
 @@ -47,23 +47,23 @@
* 
* Truncating a table removes all existing rows.
* 
-   * See {@link org.apache.spark.sql.sources.v2.writer.SupportsTruncate}.
 
 Review comment:
   +1 to move everything to catalyst


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276183586
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
 
 Review comment:
   let's mention the behavior if the column already exists.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276183876
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
+   *
+   * @param property the property name
+   * @return a TableChange for the addition
+   */
+  static TableChange removeProperty(String property) {
+return new RemoveProperty(property);
+  }
+
+  /**
+   * Create a TableChange for adding an optional column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType) {
+return new AddColumn(fieldNames, dataType, true, null);
+  }
+
+  /**
+   * Create a TableChange for adding a column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(String[] fieldNames, DataType dataType, boolean 
isNullable) {
+return new AddColumn(fieldNames, dataType, isNullable, null);
+  }
+
+  /**
+   * Create a TableChange for adding a top-level column to a table.
+   *
+   * @param fieldNames field names of the new column
+   * @param dataType the new column's data type
+   * @param isNullable whether the new column can contain null
+   * @param comment the new field's comment string
+   * @return a TableChange for the addition
+   */
+  static TableChange addColumn(
+  String[] fieldNames,
+  DataType dataType,
+  boolean isNullable,
+  String comment) {
+return new AddColumn(fieldNames, dataType, isNullable, comment);
+  }
+
+  /**
+   * Create a TableChange for renaming a field.
+   * 
+   * The name is used to find the field to rename. The new name will replace 
the leaf field name.
+   * For example, renameColumn("a.b.c", "x") should produce column a.b.x.
 
 Review comment:
   ditto, let's mention the behavior if the column doesn't exist


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276182523
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,143 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   */
+  Identifier[] listTables(String[] namespace);
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
+
+  /**
+   * Refresh table metadata for {@link Identifier identifier}.
+   * 
+   * If the table is already loaded or cached, drop cached data and reload it. 
If the table is not
+   * already loaded, load it and update caches with the new version.
+   *
+   * @param ident a table identifier
+   * @return the table's current metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  default Table refreshTable(Identifier ident) throws NoSuchTableException {
+return loadTable(ident);
+  }
+
+  /**
+   * Test whether a table exists using an {@link Identifier identifier} from 
the catalog.
+   *
+   * @param ident a table identifier
+   * @return true if the table exists, false otherwise
+   */
+  default boolean tableExists(Identifier ident) {
+try {
+  return loadTable(ident) != null;
+} catch (NoSuchTableException e) {
+  return false;
+}
+  }
+
+  /**
+   * Create a table in the catalog.
+   *
+   * @param ident a table identifier
+   * @param schema the schema of the new table, as a struct type
+   * @return metadata for the new table
+   * @throws TableAlreadyExistsException If a table already exists for the 
identifier
+   */
+  default Table createTable(Identifier ident,
+StructType schema) throws 
TableAlreadyExistsException {
 
 Review comment:
   nit: Spark code style requires
   ```
   def func(
   para1: T,
   para2: U,
   ...)
   ```
   
   The same applies to java code as well.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276183339
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
 
 Review comment:
   nit: let's also mention that, if the property already exists, we will 
replace it with the new value.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276183455
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableChange.java
 ##
 @@ -0,0 +1,293 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.types.DataType;
+
+/**
+ * TableChange subclasses represent requested changes to a table. These are 
passed to
+ * {@link TableCatalog#alterTable}. For example,
+ * 
+ *   import TableChange._
+ *   val catalog = Catalogs.load(name)
+ *   catalog.asTableCatalog.alterTable(ident,
+ *   addColumn("x", IntegerType),
+ *   renameColumn("a", "b"),
+ *   deleteColumn("c")
+ * )
+ * 
+ */
+public interface TableChange {
+
+  /**
+   * Create a TableChange for setting a table property.
+   *
+   * @param property the property name
+   * @param value the new property value
+   * @return a TableChange for the addition
+   */
+  static TableChange setProperty(String property, String value) {
+return new SetProperty(property, value);
+  }
+
+  /**
+   * Create a TableChange for removing a table property.
 
 Review comment:
   nit: let's also mention that, if the property does not exist, we will do 
nothing.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] [spark] cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add TableCatalog API

2019-04-17 Thread GitBox
cloud-fan commented on a change in pull request #24246: [SPARK-24252][SQL] Add 
TableCatalog API
URL: https://github.com/apache/spark/pull/24246#discussion_r276181341
 
 

 ##
 File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/catalog/v2/TableCatalog.java
 ##
 @@ -0,0 +1,143 @@
+/*
+ * 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.spark.sql.catalog.v2;
+
+import org.apache.spark.sql.catalog.v2.expressions.Transform;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException;
+import org.apache.spark.sql.types.StructType;
+
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Catalog methods for working with Tables.
+ */
+public interface TableCatalog extends CatalogPlugin {
+  /**
+   * List the tables in a namespace from the catalog.
+   *
+   * @param namespace a multi-part namespace
+   * @return an array of Identifiers for tables
+   */
+  Identifier[] listTables(String[] namespace);
+
+  /**
+   * Load table metadata by {@link Identifier identifier} from the catalog.
+   *
+   * @param ident a table identifier
+   * @return the table's metadata
+   * @throws NoSuchTableException If the table doesn't exist.
+   */
+  Table loadTable(Identifier ident) throws NoSuchTableException;
 
 Review comment:
   In `ExternalCatalog`, we have `getTable` and `loadTable`. It's a little 
weird to see `loadTable` here which is actually the same as 
`ExternalCatalog.getTable`.


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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org