andygrove commented on PR #49: URL: https://github.com/apache/datafusion-java/pull/49#issuecomment-4451466956
### Code review No issues found. Checked for bugs and CLAUDE.md compliance. Reviewed: - Proto evolution: new `repeated ConfigOption options = 7` is backward-compatible (no existing field changes, `repeated` correctly chosen over `map<string,string>` to preserve apply order) - JNI correctness: `getOptionNative` returns `null_mut()` for both the error-default and the `None`-value case, which is valid JNI for a `jstring` return type - Apply ordering: free-form options are applied after typed setters in `createSessionContextWithOptions` (typed knobs write to `config` first, then the `opts.options` loop writes to `config.options_mut()`, then `runtime_builder.build()`) — override semantics are correct - `setOption` dedup via `remove`-then-`put` correctly moves a re-issued key to the end of iteration order in `LinkedHashMap` - Overload resolution: `setOptions(LinkedHashMap)` vs `setOptions(Map)` — Java picks the more specific overload for a `LinkedHashMap`-typed argument, consistent with the documented intent - Prior PRs: no reviewer comments from PRs that touched these files apply to this change 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
