Skip to content

PARQUET-1771: Support configurable for DirectByteBufferAllocator from Hadoop Configuration#749

Closed
LiShuMing wants to merge 2 commits intoapache:masterfrom
LiShuMing:PARQUET-1771
Closed

PARQUET-1771: Support configurable for DirectByteBufferAllocator from Hadoop Configuration#749
LiShuMing wants to merge 2 commits intoapache:masterfrom
LiShuMing:PARQUET-1771

Conversation

@LiShuMing
Copy link

Jira

Tests

  • Old unit tests.

Commits

  • PARQUET-1771: Support configurable for DirectByteBufferAllocator from Hadoop Configuration

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

We already have:

    public Builder withAllocator(ByteBufferAllocator allocator) {
      this.allocator = allocator;
      return this;
    }

which provides similar functionality. Or am I missing something.

return this;
}

public Builder useDirectByteBufferAllocator(boolean val) {
Copy link
Contributor

@Fokko Fokko Jan 17, 2020

Choose a reason for hiding this comment

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

Suggested change
public Builder useDirectByteBufferAllocator(boolean val) {
public Builder useDirectByteBufferAllocator(boolean useDirectByteBuffer) {

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for replies.

In Spark, ParquetFileReader passes options from Hadoop Configuration (using ParquetReadOptions.Builder) rather than ParquetReadOptions.

In ParquetReadOptions.Builder constructor, now Allocator param cannot be passed to ParquetReadOptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, could you still rename val to useDirectByteBuffer?

Copy link
Author

Choose a reason for hiding this comment

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

Hi~ I have fixed this and rebased the newest codes.

Can you review this again?

@vdiravka
Copy link
Member

I wonder, if it solves https://issues.apache.org/jira/browse/PARQUET-1006 too. Could you check it please?

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.

3 participants