Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

Feature request for DROP TABLE #41

Open
tomz opened this issue Oct 18, 2015 · 8 comments
Open

Feature request for DROP TABLE #41

tomz opened this issue Oct 18, 2015 · 8 comments

Comments

@tomz
Copy link

tomz commented Oct 18, 2015

Hi DROP TABLE does not currently work as documented, any plan to get it working? Looks like some code changed in Python is needed to get this working?

Thanks,
Tom

@frsyuki
Copy link
Member

frsyuki commented Oct 19, 2015

In fact, code is in pgpool-2 code (PRESTOGRES_DEST prestogres_send_to_where(Node *node) function in src/context/pool_query_context.c). Not in Python code.
prestogres_send_to_where detects whether a statements include system catalog or not. If a statement includes system catalog, Prestogres should run it on PostgreSQL without rewriting the statement. Usually, prestogres_send_to_where uses pool_has_system_catalog function to detect about it.
Struct Node has a NodeTag field which represents type of a AST node. One of the types is T_DropStmt. As _outDropStmt function in src/parser/outfuncs.c shows, T_DropStmt is used for DROP TABLE, DROP SCHEMA, DROP INDEX, etc.
The problem is that pool_has_system_catalog function doesn't consider T_DropStmt.

@frsyuki
Copy link
Member

frsyuki commented Oct 19, 2015

So, to support DROP TABLE,

  • modify pool_has_system_catalog to consider T_DropStmt.
  • modify prestogres_send_to_where so that it calls pool_has_system_catalog when type of Node is T_DropStmt.

are necessary.
Or, another idea is to send all DROP TABLE statements to Presto by assuming that no statements drop tables in system catalog.

@tomz
Copy link
Author

tomz commented Oct 19, 2015

Ok great, thanks for the info Sadayuki

@tomz
Copy link
Author

tomz commented Nov 5, 2015

Hi Sadayuki,

I tried to add T_DropStmt check in prestogres_send_to_where (not sure how/where to change pool_has_system_catalog), and got the following error, and looks like ALTER TABLE does not work either:

hive=> \d
List of relations
Schema | Name | Type | Owner
---------+-----------+-------+------------
default | orders | table | prestogres
default | sample_07 | table | prestogres
default | sample_08 | table | prestogres
(3 rows)

hive=> drop table orders;
WARNING: there is no transaction in progress
ERROR: PrestoQueryException: Query 20151105_002358_00019_vpcx5 failed: Access Denied: Cannot drop table default.orders
hive=> ALTER TABLE orders ADD COLUMN zip varchar;
ERROR: must be owner of relation orders

@tomz
Copy link
Author

tomz commented Nov 6, 2015

I was able to get DROP TABLE working after setting hive.allow-drop-table=true for the Presto Hive connector. But the deleted Hive table still shows in Prestogres/postgres until next session, will need to figure out how to change pool_has_system_catalog to make this fully working.

@frsyuki
Copy link
Member

frsyuki commented Nov 6, 2015

I see. System catalog (including list of tables) is generated when a client runs the first query, and cached until the connection is closed.
Generating system catalog is triggered at https://github.com/treasure-data/prestogres/blob/master/src/context/pool_query_context.c#L2391-L2426 and implemented at https://github.com/treasure-data/prestogres/blob/master/prestogres/pgsql/prestogres.py#L248.
Setting NULL to prestogres_system_catalog_relcache after DROP query might solve the problem.

@tomz
Copy link
Author

tomz commented Nov 9, 2015

Hi Sadayuki,
Thanks will give that a try. And I am also wondering how to turn on debug log. I see DEBUG, DEBUG1, and DEBUG2 defined in the code, not sure how to turn those on.

Thanks,
Tom

@tomz
Copy link
Author

tomz commented Nov 10, 2015

Hi Sadayuki, thanks, setting NULL to prestogres_system_catalog_relcache worked, I submitted a pull request for it, not sure if I did it in the best place, let me know if you have any suggestions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants