Skip to content

[IoTDB 226] Hive connector#425

Merged
qiaojialin merged 66 commits intoapache:masterfrom
JackieTien97:hive-connector
Oct 29, 2019
Merged

[IoTDB 226] Hive connector#425
qiaojialin merged 66 commits intoapache:masterfrom
JackieTien97:hive-connector

Conversation

@JackieTien97
Copy link
Contributor

No description provided.

Copy link
Contributor

@samperson1997 samperson1997 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your contributing and everything looks good to me.
By reading the User Guide documents you edited and seeing your demo based on real production environment, I believe this must be a fabulous and useful connector. Below I proposed some suggestions which do not affect functions and user interaction, so maybe you can choose to solve them during future work.
Really look forward to the connector tools for Hadoop 3.x and Hive 3.x!

* @return the index of blockLocation or -1 if no block could be found
*/
private int getBlockLocationIndex(BlockLocation[] blockLocations, long middle) {
private static int getBlockLocationIndex(BlockLocation[] blockLocations, long middle, Logger logger) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why passing Logger logger in the method instead of using the public logger? Same question with other places in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different callers have different loggers. In order to print right log, I need to pass the corresponding logger to the function.

* <code>Mapper</code> task.
*/
public class TSFInputSplit extends InputSplit implements Writable {
public class TSFInputSplit extends FileSplit implements Writable, org.apache.hadoop.mapred.InputSplit {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about import org.apache.hadoop.mapred.InputSplit; in the head of this file?

Suggested change
public class TSFInputSplit extends FileSplit implements Writable, org.apache.hadoop.mapred.InputSplit {
public class TSFInputSplit extends FileSplit implements Writable, InputSplit {

@@ -0,0 +1,51 @@
/**
Copy link
Contributor

@samperson1997 samperson1997 Oct 23, 2019

Choose a reason for hiding this comment

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

We are using block comments style for Apache License instead of Java-style comments due to this PR. Maybe you could change all the new files' Apache License in next PR, or we may face some problems when generating JavaDoc.

import java.util.Objects;

public class TsFileDeserializer {
private static final Logger LOG = LoggerFactory.getLogger(TsFileDeserializer.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

User logger instead of LOG to be consistent with other files.

continue;
}
if (columnType.getCategory() != ObjectInspector.Category.PRIMITIVE) {
throw new TsFileSerDeException("Unknown TypeInfo: " + columnType.getCategory());
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could add logger.error here so that users can know the exact error position. Same with other exceptions below.

Suggested change
throw new TsFileSerDeException("Unknown TypeInfo: " + columnType.getCategory());
logger.error("Unknown TypeInfo: {}", columnType.getCategory());

try {
writer.write(((HDFSTSRecord)writable).convertToTSRecord());
} catch (WriteProcessException e) {
throw new IOException(String.format("Write tsfile record error %s", e));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could add logger.error here so that users can know the exact error position. Same with other exceptions.

Suggested change
throw new IOException(String.format("Write tsfile record error %s", e));
logger.error("Write tsfile record error: {}", e);

* One example for reading TsFile with MapReduce.
* This MR Job is used to get the result of sum("device_1.sensor_3") in the tsfile.
* The source of tsfile can be generated by <code>TsFileHelper</code>.
* @author Yuan Tian
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to remove author here ; )

Copy link
Contributor

@LeiRui LeiRui left a comment

Choose a reason for hiding this comment

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

I followed the user guide and checked all the instructions mentioned. Everything works (on my hadoop 2.7.7 & hive 2.3.6).

@LeiRui
Copy link
Contributor

LeiRui commented Oct 26, 2019

The Travis CI build failed. Try merge the master into your pr to solve the problem.

@JackieTien97
Copy link
Contributor Author

The Travis CI build failed. Try merge the master into your pr to solve the problem.

image

It seems that there are some problems with the windows env.

@qiaojialin qiaojialin merged commit ea8e23d into apache:master Oct 29, 2019
Sign up for free to 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.

6 participants