Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-1067: INSERT OVERWRITE INTO should not remove all partitions.#161

Closed
blrunner wants to merge 18 commits intoapache:masterfrom
blrunner:TAJO-1067
Closed

TAJO-1067: INSERT OVERWRITE INTO should not remove all partitions.#161
blrunner wants to merge 18 commits intoapache:masterfrom
blrunner:TAJO-1067

Conversation

@blrunner
Copy link
Copy Markdown
Contributor

I added new session variable for avoiding remove existing partition directories. Its name is COLUMN_PARITION_REMOVE_ALL_PARTITIONS. Its default value is false. Thus, if you run INSERT OVERWRITE statement with column partitioned table, tajo doesn't remove existing partition directories. Tajo just remove partition directories which is equals to staging directories.

And this patch has a lack that is related to TAJO-744. You left the comment for the lack at TAJO-1067.

@hyunsik
Copy link
Copy Markdown
Member

hyunsik commented Sep 26, 2014

I think that this feature should not depend on session variable. Session variable works implicitly. Users can miss to check if the session variable is enabled.

As a result, inadvertently, a user can lost his or her entire table by INSERT OVERWRITE clause.

I think that we should force users to manually use TRUNCATE statement instead of using session variable.

@blrunner
Copy link
Copy Markdown
Contributor Author

Hi @hyunsik

I agree with your opinion. I've just removed the variable.
If possible, could you check the patch again?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COLUMN_PARITION_REMOVE_ALL_PARTITIONS was already removed. So, you need to revise this comment line.

@blrunner
Copy link
Copy Markdown
Contributor Author

blrunner commented Oct 2, 2014

Hi @hyunsik

Thank you for your detailed review.
I agree with your opinions and I've just updated it.
Could you check it again?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'a' is duplicated.

@hyunsik
Copy link
Copy Markdown
Member

hyunsik commented Oct 5, 2014

Thank you for your work. The patch looks good to me. After I hear your through about my final comments, I'll finish this review.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants