PHOENIX-5619 CREATE TABLE AS SELECT for Phoenix table doesn't work co…

[brfrn169]

…rrectly in Hive

[chrajeshbabu]

@brfrn169 can you please add a test case for this?

[brfrn169]

@chrajeshbabu Thank you for reviewing it!

I'm trying to add a test case for this to the integration test, but I could't run the integration test correctly. I ran the following command in phoenix-hive directory, but it looks like it didn't work correctly:
mvn clean package failsafe:integration-test

Could you please suggest how to run the integration test correctly or appropriate documentation?

[joshelser]

I ran the following command in phoenix-hive directory, but it looks like it didn't work correctly:
mvn clean package failsafe:integration-test

a mvn [clean] verify should work for you, @brfrn169

[joshelser]

Some minor requests to just explain the code-change a little more. A test case would also be great.

Sorry for the delay in getting here, Toshi.

[joshelser]

Could you add a comment as to why this change is needed for future readers? I'm guessing that Configuration is coming from the Hive framework, but the Properties is what the user actually provided to us?

[brfrn169]

@joshelser Thank you for reviewing it! I'll modify the PR for your review.

a mvn [clean] verify should work for you

Actually, I already tried this but any integration tests didn't run. So I tried mvn failsafe:integration-test, and it looked like the integration tests ran but they failed. The output here: mvn.out.zip

The integration tests are broken?

[joshelser]

I knew that the one IT has been broken since the migration into a new repo, but it is news to me if they're all broken.

[brfrn169]

I tried to add an integration test for this but I couldn't reproduce the issue by using HiveTestUtil for some reason.

So, let me explain the details of the issue here.

I added the code to populate from the table properties into config in PhoenixSerializer because these values are used in the initialization of PhoenixResultWritable if the table is transactional here:
https://github.com/brfrn169/phoenix-connectors/blob/PHOENIX-5619/phoenix-hive/src/main/java/org/apache/phoenix/hive/mapreduce/PhoenixResultWritable.java#L208-L209

And, in PhoenixUtil.getPrimaryKeyColumnList(), a connection object is created with empty properties:
https://github.com/brfrn169/phoenix-connectors/blob/PHOENIX-5619/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixUtil.java#L112

Because PhoenixConnectionUtil.getInputConnection() needs ZOOKEEPER_QUORUM, ZOOKEEPER_PORT and ZOOKEEPER_PARENT, we need the code I added in PhoenixSerializer:
https://github.com/brfrn169/phoenix-connectors/blob/PHOENIX-5619/phoenix-hive/src/main/java/org/apache/phoenix/hive/util/PhoenixConnectionUtil.java#L45-L65

[joshelser]

I remember the semantics of hive connectors depending largely on how they're called. That is, the mapreduce execution engine could hit a remarkably different code path than the tez engine. Maybe the same is the case with the test util?

If you are stuck and can't get a test showing the issue, I am OK committing this as it is.

[brfrn169]

Thank you @joshelser . I'm stuck and can't get a test showing the issue actually. Please commit this if you are okay with the patch. I tested it locally and it resolved the issue.