Skip to content

Phoenix 331- Phoenix-Hive initial commit - #74

Closed
nmaillard wants to merge 3 commits into
apache:masterfrom
nmaillard:PHOENIX-331
Closed

Phoenix 331- Phoenix-Hive initial commit #74
nmaillard wants to merge 3 commits into
apache:masterfrom
nmaillard:PHOENIX-331

Conversation

@nmaillard

Copy link
Copy Markdown

No description provided.

Nicolas Maillard added 2 commits April 23, 2015 02:59
added Phoenix Hive first phase of code
First UTs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please remove these tags. This should just be a javadoc explaining the high level functionality of the class. Look at some other class in Phoenix if you want to get an idea...

@mravi

Copy link
Copy Markdown
Contributor

Thanks a lot for the patch @nmaillard . Couple of additional minor fixes

  1. Couple of classes have your name as the author. Please remove it.
  2. Certain classes have lot of commented code .

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Who does the datatype conversion from Hive to Phoenix?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The type conversion is all done in HiveTypeUtil, HiveType2PdataType does Hive to phoenix.
In PhoenixHiveDBWritable the write class leverage that information to create the prepared statement. T extends DBWritable and is a PhoenixHiveDBWritable in a this instance.
Some complex types like Array are still missing, this will be follow up work

Refactor based on initial feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this code repeated, as it's in the base MR integration already?

@JamesRTaylor

Copy link
Copy Markdown
Contributor

This is looking good, @nmaillard. Still some copy/paste code that we should aim to cleanup. Also, take care that no exceptions are swallowed. Wrap checked exceptions in a SQLException if it's declared in the method and otherwise wrap in a RuntimeException and throw it. Also, make sure to remove or convert all System.out.println calls to logging instead.

@JamesRTaylor

Copy link
Copy Markdown
Contributor

@mravi - would you mind giving this a review?

@stoty

stoty commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Already merged.
(And since has been factored out to the phoenix-connectors repo)

@stotystoty closed this Aug 1, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@nmaillard@mravi@JamesRTaylor@ddraj@stoty