-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bugs and add features for hive integration #1565
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
d919332
to
ef541f4
Compare
This comment was marked as resolved.
This comment was marked as resolved.
3c83f7a
to
9c12349
Compare
… process will overwrite the content of table instead of appending. Fix bug: spark will rename a empty file and it will trigle a exception because the objectID is invalid. Support char and varchar. Fix bug that getSplits will try to open a dir. Signed-off-by: vegetableysm <[email protected]>
Fix the bug of renaming table(Including partitioned table). Fixed a bug that compound queries don't work. Signed-off-by: vegetableysm <[email protected]>
… when the file is deleted. Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Refactor setValue and getValue. Refactor string, varchar, char and binary type. Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
Signed-off-by: vegetableysm <[email protected]>
9c12349
to
198b553
Compare
Signed-off-by: vegetableysm <[email protected]>
198b553
to
9465f4c
Compare
+ tableFilePath | ||
+ ", content: " | ||
+ new String(buffer, StandardCharsets.UTF_8)); | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use instanceof
to check the exception type, rather, use Multiple-exception catches instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
+ new String(buffer, StandardCharsets.UTF_8)); | ||
break; | ||
} | ||
throw e; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
case CHAR: | ||
case VARCHAR: | ||
return Types.MinorType.LARGEVARCHAR.getType(); | ||
return Types.MinorType.VARCHAR.getType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use large var char for string types to avoid exceeding the size limit of arrow buffers in VarCharArray.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
meta.addMember("buffer_", buffer.seal(client)); | ||
meta.addMember("null_bitmap_", BufferBuilder.empty(client)); | ||
meta.addMember("data_buffer_", dataBufferBuilder.seal(client)); | ||
meta.addMember("validity_buffer_", validityBufferBuilder.seal(client)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't rename these key names. They MUST be kept consistent with the builders/resolvers in existing C++/Python/Go/Rust SDKs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
.resolve( | ||
meta.getMemberMeta( | ||
"buffer_" + String.valueOf(i) + "_"))) | ||
.getBuffer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you refactor this part (e.g., use some temporary variables) to make it looks better?
public ObjectTransformer() {} | ||
|
||
public Object defaultTransform(Object object) { | ||
return object; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultTransform
-> transform
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
public int intTransform(Object object) { | ||
return (int) object; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- intTransform -> transformInt
- longTransform -> transformLong
- ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Once ready for review, remember removing the "WIP" in the RP's title. |
Signed-off-by: vegetableysm <[email protected]>
What do these changes do?
Fix bugs and add features based on test results
Related issue number
Fixes #issue number