[ https://issues.apache.org/jira/browse/HIVE-18739?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16410633#comment-16410633 ]
Sergey Shelukhin edited comment on HIVE-18739 at 3/23/18 12:47 AM: ------------------------------------------------------------------- Code looks ok to me modulo some whitespace, todos, etc. However I'm -0 on the approach. First there are potential security implications with shared database. I think the table at least has to be session-temporary. I'm not sure how safe those are in the first place, but right now we create a globally visible table in a shared DB, potentially giving users with access to any database and export access to data from every other database (by grabbing it during export). It's protected only by the obscurity of the table name. I'm -1 on that. Overall it seems like using full compaction, so we could benefit every other reader of the table (and future export, see side note) would still be better, and also not multiply entities and have simpler code without compiling a query, having users set up a special DB, etc. No matter how compaction evolves it should still be possible to run full compaction by including all deltas; it will be a flag and minimal code difference from future improved compaction, during delta selection. But I'm not going to stop this patch as long as security issue is addressed :) If continuing with this approach, I wonder if we could, instead of relying on custom db, create a new type of privilege check, and allow users to create the export table in the source db even if they don't have permissions to create normal tables there? Or we can use session scoped temp table. Or materialized view? cc [~ashutoshc] [~thejas] to review As a side note, one optimization here is that if table is already fully compacted, we could just take the base and export it directly without using insert. was (Author: sershe): Code looks ok to me modulo some whitespace, todos, etc. However I'm -0 on the approach. First there are potential security implications with shared database. I think it the minimum, the table has to be session-temporary. I'm not sure how safe those are in the first place, but right now we create a globally visible table in a shared DB, potentially giving users with access to any database and export access to data from every other database (by grabbing it during export). It's protected only by the obscurity of the table name. I'm -1 on that. Overall it seems like using full compaction, so we could benefit every other reader of the table (and future export, see side note) would still be better, and also not multiply entities and have simpler code without compiling a query, having users set up a special DB, etc. No matter how compaction evolves it should still be possible to run full compaction by including all deltas; it will be a flag and minimal code difference from future improved compaction, during delta selection. But I'm not going to stop this patch as long as security issue is addressed :) If continuing with this approach, I wonder if we could, instead of relying on custom db, create a new type of privilege check, and allow users to create the export table in the source db even if they don't have permissions to create normal tables there? Or we can use session scoped temp table. Or materialized view? cc [~ashutoshc] [~thejas] to review As a side note, one optimization here is that if table is already fully compacted, we could just take the base and export it directly without using insert. > Add support for Export from Acid table > -------------------------------------- > > Key: HIVE-18739 > URL: https://issues.apache.org/jira/browse/HIVE-18739 > Project: Hive > Issue Type: Sub-task > Components: Transactions > Reporter: Eugene Koifman > Assignee: Eugene Koifman > Priority: Major > Attachments: HIVE-18739.01.patch, HIVE-18739.04.patch, > HIVE-18739.04.patch, HIVE-18739.06.patch, HIVE-18739.08.patch, > HIVE-18739.09.patch, HIVE-18739.10.patch, HIVE-18739.11.patch, > HIVE-18739.12.patch > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)