dwsmith1983 opened a new pull request, #5653:
URL: https://github.com/apache/datafusion-comet/pull/5653
## Which issue does this PR close?
Part of the restructuring of #5365 requested in review: these are three
small core fixes that were previously bundled into that PR and are independent
of the Delta contrib work. The first two are prerequisites for it; the third is
standalone cleanup.
## Rationale for this change
1. `NativeConfig.extractObjectStoreOptions` iterates the Hadoop conf and
forwards raw entry values. `Configuration#get` expands `${...}` variable
references while `Configuration.Entry#getValue` does not, so any object store
option holding a reference (for example an endpoint composed from another
property) reached native as the unexpanded literal and diverged from what every
Hadoop-side consumer observes.
2. Comet appends synthetic constant metadata fields named
`_comet_metadata_<name>` to the scan schema. DataFusion substitutes partition
constants by name, so a user or partition column that happens to carry one of
those names would silently receive the constant metadata value instead of its
own data.
3. The `Java_org_apache_comet_parquet_Native_*` record batch reader entry
points (initRecordBatchReader, readNextRecordBatch, currentColumnBatch,
closeRecordBatchReader) and their BatchContext machinery have no JVM callers
anywhere in the tree. Dead code in a JNI surface is worth removing promptly
since nothing exercises or protects it.
## What changes are included in this PR?
- `extractObjectStoreOptions` resolves each value through
`Configuration#get`, falling back to the raw literal when `get` returns null
(deprecated key aliasing) or raises on a substitution cycle, so extraction
never aborts.
- A `uniqueConstantMetadataFields` helper uniquifies synthetic metadata
field names against the data and partition schema names with trailing
underscores; binding stays positional so renaming is always safe.
- Removal of the four dead JNI entry points and their supporting state (306
lines), plus one stale doc comment that referenced them.
## How are these changes tested?
- Two new tests in `NativeConfigSuite` cover `${...}` expansion and the
cycle fallback.
- New `CometNativeScanSuite` covers prefixing and collision uniquification
(written first and failing before the fix).
- `cargo test -p datafusion-comet --lib` (240 passed), clippy with `-D
warnings`, fmt, and the Spark 3.5 build with
`NativeConfigSuite`/`CometNativeScanSuite` all green. A repo-wide grep confirms
no callers of the removed JNI symbols.
--
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]
