Skip to content
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

ARROW-15979: [C++][Doc] Expose more functions of parquet::WriterProperties in doc #12673

Closed
wants to merge 5 commits into from

Conversation

XinyuZeng
Copy link
Contributor

No description provided.

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW

Opening JIRAs ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@XinyuZeng XinyuZeng changed the title [C++][Doc]Expose more functions of parquet::WriterProperties in doc ARROW-15979: C++][Doc]Expose more functions of parquet::WriterProperties in doc Mar 19, 2022
@XinyuZeng XinyuZeng changed the title ARROW-15979: C++][Doc]Expose more functions of parquet::WriterProperties in doc ARROW-15979: [C++][Doc]Expose more functions of parquet::WriterProperties in doc Mar 19, 2022
@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

Builder* write_batch_size(int64_t write_batch_size) {
write_batch_size_ = write_batch_size;
return this;
}

/**
* Specify the max row group length.
* Default 64M.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this should be changed to 1M, as discussed in user group a month ago with @westonpace

cpp/src/parquet/properties.h Outdated Show resolved Hide resolved
cpp/src/parquet/properties.h Show resolved Hide resolved
cpp/src/parquet/properties.h Show resolved Hide resolved
@XinyuZeng XinyuZeng requested a review from lidavidm March 21, 2022 02:35
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks! Just two small tweaks.

cpp/src/parquet/properties.h Outdated Show resolved Hide resolved
cpp/src/parquet/properties.h Outdated Show resolved Hide resolved
@XinyuZeng
Copy link
Contributor Author

XinyuZeng commented Mar 21, 2022

Thanks! Just two small tweaks.

Just fixed them. A side question: I did not expose created_by to the doc, since I think it should not even be a public function exposed to users? It simply logs parquet-cpp-arrow version @ARROW_VERSION@ when writing the meta data. Does it mean to let someone modify it to be, e.g "created by Shawn"?

@lidavidm
Copy link
Member

Ah, I missed that. Yes, it just sets some metadata about what implementation produced the Parquet file. We can document it, but probably people won't want to/shouldn't change it.

@XinyuZeng
Copy link
Contributor Author

Guess it is able to merge? The default row group size can be fixed in another PR. created_by does not need to expose in doc.

@lidavidm lidavidm changed the title ARROW-15979: [C++][Doc]Expose more functions of parquet::WriterProperties in doc ARROW-15979: [C++][Doc] Expose more functions of parquet::WriterProperties in doc Mar 24, 2022
@lidavidm lidavidm closed this in 7711182 Mar 24, 2022
@ursabot
Copy link

ursabot commented Mar 24, 2022

Benchmark runs are scheduled for baseline = 864b54d and contender = 7711182. 7711182 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.34% ⬆️0.08%] test-mac-arm
[Failed ⬇️0.36% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.13% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants