brianchandotcom/liferay-plugins — pull request #980 by samuelkong (2012-10-08 09:10:51+00:00)
Title: LPS-29577 Partial rollback of 5370c162eb1b465120ba350b87ce4f094d966ca4

I disagree with your changes.

> The test-sql is supposed to return 0 results if we are to run the import-sql.

> Your test for "select count(*) from Region where countryId = 9" will always return a result, which means the import-sql will never run.

"select count(*) from Region where countryId = 9" will return 0 if you've never deployed the hook. By default, the database doesn't contain any region info for Japan (9) so it must return 0. Only after import-sql runs will it return a result. (Note that when selecting from the Region table, I'm selecting by countryId and not be regionId). I've tested this and it will run.

> The second test-sql "select count(*) from Region where name = 'æ„›çŸ¥çœŒ'" will also return 0... but then when it runs the update, since the import never happened in the first clause, will also not update anything.

I don't think your current test case is as good. Consider the situation where someone has already added the region info for Japan, but the region info is in English. The test-sql will return 0 and you'll get duplicate index error when trying to insert the regions. It's probably better to just skip the imports (what I did in the first pull request) or update the name instead of inserting a new row (what I did in the second test-sql). The second test-sql will only return 0 in this scenario where the region info exists but is not in Japanese. If there's no region info at all, it should be caught by the first test-sql.