TAJO-1681 Fix TajoDump invalid null check for database name

[charsyam]

isAcceptableDumpingDatabase return true even if databaseName is null
so it cause exception in dumpDatabase

[blrunner]

Hi @charsyam

Could you rebase the patch against the master branch?

[charsyam]

@blrunner I fixed bug and rebased it :) Thank you.

[jihoonson]

It would be nice if the dump result is verified. Please refer to other tests in this class.

[jihoonson]

@charsyam, sorry for late review.
This patch looks good, but test seems to be improved.
Please consider my comment.

[charsyam]

@jihoonson Thanks for your review :)
But this patch just check null point exception of null parameters.
and other 2 testcases check dump result is verified.

so I think this test is more intuitive.
and It can't get the result because test database name is null :)

What do you think?

[jihoonson]

I got your point. As you said, the dump result doesn't have to be verified since this ticket is related to fix wrong null checking on database name.
If so, how about adding an error message when the given database name is wrong? I mean, isAcceptableDumpingDatabase() returns false, it would be better to print a message to notify users that something is going wrong. In addition, we can verify error handling with that message.

[charsyam]

@jihoonson Thanks your review :), Yes, I think It is good idea.
but I still have a question about this behavior :)

Because isAcceptableDumpingDatabase's return value doesn't mean error.
it jsut check database name is not CatalogConstants.INFORMATION_SCHEMA_DB_NAME

What do you think?
but I found indent is wrong T.T I should fix it first :)

[jihoonson]

Oh, right. It was my misunderstanding.
I still have a doubt whether the unit test verifies correctly.
How about removing the test? I think it's enough because this change is straightforward.

[charsyam]

@jihoonson Ok, I removed testcase and fix indent Thank you for your review :)

[jihoonson]

+1. Thanks!