You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2019/04/10 08:07:28 UTC
[GitHub] [flink] hequn8128 commented on a change in pull request #8007: [FLINK-11474][table] Add ReadableCatalog, ReadableWritableCatalog, and other …
hequn8128 commented on a change in pull request #8007: [FLINK-11474][table] Add ReadableCatalog, ReadableWritableCatalog, and other …
URL: https://github.com/apache/flink/pull/8007#discussion_r273835736
##########
File path: flink-table/flink-table-common/src/main/java/org/apache/flink/table/catalog/ReadableWritableCatalog.java
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.flink.table.catalog;
+
+import org.apache.flink.table.catalog.exceptions.DatabaseAlreadyExistException;
+import org.apache.flink.table.catalog.exceptions.DatabaseNotExistException;
+import org.apache.flink.table.catalog.exceptions.TableAlreadyExistException;
+import org.apache.flink.table.catalog.exceptions.TableNotExistException;
+
+/**
+ * An interface responsible for manipulating catalog metadata.
+ */
+public interface ReadableWritableCatalog extends ReadableCatalog {
+
+ // ------ databases ------
+
+ /**
+ * Create a database.
+ *
+ * @param name Name of the database to be created
+ * @param database The database definition
+ * @param ignoreIfExists Flag to specify behavior when a database with the given name already exists:
+ * if set to false, throw a DatabaseAlreadyExistException,
+ * if set to true, do nothing.
+ * @throws DatabaseAlreadyExistException if the given database already exists and ignoreIfExists is false
+ */
+ void createDatabase(String name, CatalogDatabase database, boolean ignoreIfExists)
+ throws DatabaseAlreadyExistException;
+
+ /**
+ * Drop a database.
+ *
+ * @param name Name of the database to be dropped.
+ * @param ignoreIfNotExists Flag to specify behavior when the database does not exist:
+ * if set to false, throw an exception,
+ * if set to true, do nothing.
+ * @throws DatabaseNotExistException if the given database does not exist
+ */
+ void dropDatabase(String name, boolean ignoreIfNotExists) throws DatabaseNotExistException;
+
+ /**
+ * Modify an existing database.
+ *
+ * @param name Name of the database to be modified
+ * @param newDatabase The new database definition
+ * @param ignoreIfNotExists Flag to specify behavior when the given database does not exist:
+ * if set to false, throw an exception,
+ * if set to true, do nothing.
+ * @throws DatabaseNotExistException if the given database does not exist
+ */
+ void alterDatabase(String name, CatalogDatabase newDatabase, boolean ignoreIfNotExists)
+ throws DatabaseNotExistException;
+
+ // ------ tables and views ------
+
+ /**
+ * Drop a table or view.
+ *
+ * @param tablePath Path of the table or view to be dropped
+ * @param ignoreIfNotExists Flag to specify behavior when the table or view does not exist:
+ * if set to false, throw an exception,
+ * if set to true, do nothing.
+ * @throws TableNotExistException if the table or view does not exist
+ */
+ void dropTable(ObjectPath tablePath, boolean ignoreIfNotExists) throws TableNotExistException;
+
+ /**
+ * Rename an existing table or view.
+ *
+ * @param tablePath Path of the table or view to rename
+ * @param newTableName the new name of the table or view
+ * @param ignoreIfNotExists Flag to specify behavior when the table or view does not exist:
+ * if set to false, throw an exception,
+ * if set to true, do nothing.
+ * @throws TableNotExistException if the table does not exist
+ * @throws DatabaseNotExistException if the database in tablePath to doesn't exist
+ */
+ void renameTable(ObjectPath tablePath, String newTableName, boolean ignoreIfNotExists)
+ throws TableNotExistException, DatabaseNotExistException;
+
+ // ------ tables ------
+
+ /**
+ * Create a new table. Note that TableStats in the table is ignored for table creation.
+ *
+ * @param tablePath Path of the table to be created
+ * @param table The table definition
+ * @param ignoreIfExists Flag to specify behavior when a table already exists at the given path:
+ * if set to false, it throws a TableAlreadyExistException,
+ * if set to true, do nothing.
+ * @throws TableAlreadyExistException if table already exists and ignoreIfExists is false
+ * @throws DatabaseNotExistException if the database in tablePath doesn't exist
+ */
+ void createTable(ObjectPath tablePath, CatalogTable table, boolean ignoreIfExists)
Review comment:
Sorry to jump into the discussion. Maybe it would be better if we can tell the differences directly and clearly from the method name.
Currently, the method names are not trusted and we have to look at the signature to tell the differences. However, when we looking at the `List<String> listTables(String dbName)` method, it would again bring confusions. From the signature, a user will think both tables and views will be listed while actually only tables but not views are listed.
I agree that the implementations of createTable and creatView are expected to be different, but I think it's ok to use one api for both of them. Take inmemory catalog as an example, we even don't need the `private helper method`. And we can use the `a private helper method` to solve the problem if the implementations are different.
What do you guys think?
Best,
Hequn
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services