Skip to content

Optimize configuration value casts (#3688)#3805

Merged
dimas-b merged 1 commit intoapache:mainfrom
AryanPatel226:optimize-config-value-casts
Feb 24, 2026
Merged

Optimize configuration value casts (#3688)#3805
dimas-b merged 1 commit intoapache:mainfrom
AryanPatel226:optimize-config-value-casts

Conversation

@AryanPatel226
Copy link
Contributor

Optimizes tryCast to avoid unnecessary string conversions when configuration values are already the correct type.

The method was converting every value to string and parsing it back, even when the value already matched the expected type (Boolean, Integer, Long, Double). This adds type checks before string conversion to eliminate unnecessary overhead.

Fixes #3688

Checklist

  • 🛡️ Don't disclose security issues! (contact [email protected])
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes Optimize configuration value casts #3688
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

return cast(value);
}

if (defaultValue instanceof Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would it make sense to turn this if-else if to switch expression, something like this:

    return switch (defaultValue()) {
          case Boolean ignored -> cast(Boolean.valueOf(String.valueOf(value)));
          case Integer ignored -> cast(Integer.valueOf(String.valueOf(value)));
          case Long ignored -> cast(Long.valueOf(String.valueOf(value)));
          case Double ignored -> cast(Double.valueOf(String.valueOf(value)));
          case Float ignored -> cast(Float.valueOf(String.valueOf(value)));
          case List<?> ignored -> cast(new ArrayList<>((List<?>) value));
          case null, default -> cast(value);
      };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nandorKollar
Good suggestion, but I think this compiles with --release 17, and switch pattern matching requires Java 21+. So we probably need to keep the if-else. But let me know if you have some suggestion.
Thank you!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay, that makes sense. Do we plan to compile with Java 21 in the near future?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @jbonofre I just noticed that we recently moved to Java 17 minimum version for the clients.

Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @AryanPatel226 ! Changes LGTM 👍

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Feb 20, 2026
@AryanPatel226 AryanPatel226 force-pushed the optimize-config-value-casts branch from 6e306f5 to a3986ed Compare February 24, 2026 06:39
@dimas-b dimas-b merged commit 8d40a33 into apache:main Feb 24, 2026
17 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Feb 24, 2026
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.

Optimize configuration value casts

5 participants