jiangxt2 opened a new pull request, #13190:
URL: https://github.com/apache/gravitino/pull/13190

   ### What changes were proposed in this pull request?
   
   - Reject a table property whose key is exactly `lance.storage.` before 
prefix removal can produce an empty Lance storage-option key.
   - Add regression coverage for the exact-prefix case, normal options, 
arbitrary non-empty provider-specific keys, insertion ordering, and 
catalog-to-table precedence.
   - Keep the change limited to `LancePropertiesUtils.getLanceStorageOptions()` 
and its unit tests; reverse conversion and operation call sites are unchanged.
   
   ### Why are the changes needed?
   
   `lance.storage.` currently passes Gravitino's prefix-property validation and 
is converted into the empty Lance option key `""`. A direct Docker-based 
experiment with Lance 6.0.0 and MinIO confirmed that Lance can retain this 
malformed key, so the invalid input can cross the Gravitino-to-Lance boundary 
without an actionable error at the source.
   
   This PR intentionally fixes the extraction boundary only. On current `main`, 
ordinary create can pass the malformed option to dataset creation, declare can 
persist metadata while response-side resolution still accepts it, and register 
can persist the property and return success without resolving storage options. 
After this guard, ordinary create fails before dataset creation or metadata 
persistence; declare may persist metadata before response-side resolution 
returns an error, while register still bypasses that resolution. 
Pre-persistence validation for `toTableProperties()` and the relevant 
catalog/table input paths is tracked as a separate follow-up.
   
   Fix: #13189
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. An exact `lance.storage.` property key now raises an input-validation 
error instead of producing an empty Lance storage-option key. Existing 
non-empty options, including provider-specific keys, retain their current 
behavior; no public API or property precedence is changed. This guard is 
evaluated when storage options are resolved, not when a property is initially 
set. Existing malformed metadata may therefore cause describe and Lance alter 
operations to fail, while a non-external drop may delete metadata before 
storage cleanup fails, leaving an orphaned dataset.
   
   ### How was this patch tested?
   
   - `./gradlew :lance:lance-common:test --tests 
'org.apache.gravitino.lance.common.utils.TestLancePropertiesUtils' -PskipITs` — 
3 tests passed with no skips, failures, or errors.
   - `./gradlew :lance:lance-common:spotlessCheck` — passed.
   - `./gradlew rat` — passed.
   - `git diff --check` — passed.
   - A separate Docker-based Lance 6.0.0 probe with MinIO confirmed the 
malformed empty key is retained by Lance; this is supporting evidence, not an 
additional Gravitino CI gate.
   


-- 
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]

Reply via email to