Skip to content

refactor: use std::from_chars instead of stoi/stoll/stod#556

Open
HuaHuaY wants to merge 1 commit intoapache:mainfrom
HuaHuaY:use_from_chars_instead_of_stoi
Open

refactor: use std::from_chars instead of stoi/stoll/stod#556
HuaHuaY wants to merge 1 commit intoapache:mainfrom
HuaHuaY:use_from_chars_instead_of_stoi

Conversation

@HuaHuaY
Copy link
Contributor

@HuaHuaY HuaHuaY commented Feb 7, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 7, 2026 11:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors numeric parsing to prefer std::from_chars-based parsing and consolidates call sites onto a single StringUtils helper, reducing reliance on exception-throwing std::stoi/stoll/stod in parsing paths.

Changes:

  • Replace StringUtils::ParseInt with a new templated StringUtils::ParseNumber and update call sites across the codebase.
  • Switch various parsing sites (JSON/type parsing, transform parsing, metrics parsing, schema utilities) to use ParseNumber instead of stoi/stod/direct from_chars.
  • Minor tweak to error helper overload in result.h (take std::string by value and move).

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/iceberg/util/string_util.h Introduces ParseNumber to centralize numeric parsing via std::from_chars.
src/iceberg/util/decimal.cc Uses ParseNumber<uint64_t> in decimal digit-group parsing.
src/iceberg/util/config.h Replaces stoll/stod with ParseNumber for default config conversions.
src/iceberg/update/update_properties.cc Switches format-version parsing to ParseNumber<int32_t>.
src/iceberg/update/snapshot_update.cc Switches summary integer parsing to ParseNumber<int64_t>.
src/iceberg/transform.cc Switches regex param parsing from stoi to ParseNumber<int32_t>.
src/iceberg/snapshot.cc Switches snapshot property parsing to ParseNumber<int64_t>.
src/iceberg/result.h Adjusts error factory overload to move std::string messages.
src/iceberg/parquet/parquet_writer.cc Switches codec level parsing to ParseNumber<int32_t>.
src/iceberg/parquet/parquet_schema_util.cc Switches field-id parsing to ParseNumber<int32_t>; adjusts includes.
src/iceberg/metrics_config.cc Switches truncate length parsing to ParseNumber<int32_t>.
src/iceberg/json_serde.cc Switches fixed/decimal param parsing from stoi to ParseNumber<int32_t>.
src/iceberg/avro/avro_writer.cc Switches codec level parsing to ParseNumber<int32_t>.
src/iceberg/avro/avro_schema_util.cc Switches attribute id parsing to ParseNumber<int32_t>; cleans up includes/formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@HuaHuaY HuaHuaY force-pushed the use_from_chars_instead_of_stoi branch 2 times, most recently from 663af14 to 8c2b509 Compare February 7, 2026 11:56
@HuaHuaY HuaHuaY force-pushed the use_from_chars_instead_of_stoi branch from 8c2b509 to d29ea9b Compare February 7, 2026 12:00
Copy link
Collaborator

@zhjwpku zhjwpku left a comment

Choose a reason for hiding this comment

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

std::from_chars can parse floating point number, so +1 for changing ParseInt to ParseNumber.

Thanks for working on this.

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.

2 participants