Hi Yufei, Re: the use case: ALLOW_EXTERNAL_METADATA_FILE_LOCATION is a config setting that controls where table files are permitted to be written. Tables exist within a catalog. The FeatureConfiguration class supports per-catalog overrides. I think it is pretty natural to allow this config to be set differently in different catalogs to give more control and flexibility to catalog administrators
Cheers, Dmitri. On Wed, Jul 8, 2026 at 6:44 PM Yufei Gu <[email protected]> wrote: > Hi Dmitri, > > Making ALLOW_EXTERNAL_METADATA_FILE_LOCATION overridable at the catalog > level doesn't seem a bad idea. However, it is a behavior change. And AFAIK, > there is no use case requesting it. My understanding is that the main > motivation for PR 4966 was to resolve the inconsistency between > ALLOW_EXTERNAL_METADATA_FILE_LOCATION and ALLOW_EXTERNAL_TABLE_LOCATION. If > we deprecate ALLOW_EXTERNAL_TABLE_LOCATION and keep it as a backward > compatible alias, that inconsistency goes away. > > In that case, do we still need the change in PR 4966? > > Yufei > > > On Wed, Jul 8, 2026 at 10:51 AM Yufei Gu <[email protected]> wrote: > > > Here is a PR to fix the doc mismatch, > > https://github.com/apache/polaris/pull/5010 > > > > Yufei > > > > > > On Wed, Jul 8, 2026 at 12:50 AM Jean-Baptiste Onofré <[email protected]> > > wrote: > > > >> Hi > >> > >> It looks good to me. > >> > >> Thanks, > >> Regards > >> JB > >> > >> On Wed, Jul 8, 2026 at 1:45 AM Yufei Gu <[email protected]> wrote: > >> > >> > Hi folks, > >> > > >> > While reviewing the PR https://github.com/apache/polaris/pull/4966, > I'm > >> > thinking maybe we could simplify the location-related flags a bit. > >> > > >> > Today we have both ALLOW_EXTERNAL_TABLE_LOCATION and > >> > ALLOW_EXTERNAL_METADATA_FILE_LOCATION. Their descriptions are similar > >> but > >> > serve different purposes. From reading the code, > >> > ALLOW_EXTERNAL_TABLE_LOCATION mostly acts like a metadata-location > >> escape > >> > hatch: it relaxes the check that metadata files must live under the > >> table > >> > location. That overlaps heavily with > >> ALLOW_EXTERNAL_METADATA_FILE_LOCATION, > >> > and the name is easy to confuse with > ALLOW_UNSTRUCTURED_TABLE_LOCATION. > >> > > >> > I think the clearer model is: > >> > > >> > - ALLOW_UNSTRUCTURED_TABLE_LOCATION: table/view locations may be > >> outside > >> > the structured namespace layout. > >> > - ALLOW_EXTERNAL_METADATA_FILE_LOCATION: metadata files may be > >> outside > >> > the table location. > >> > > >> > Proposal: deprecate ALLOW_EXTERNAL_TABLE_LOCATION, keep it as a > >> > backward-compatible alias for the metadata-location behavior, and > update > >> > docs/tests accordingly. > >> > > >> > There’s also one related doc mismatch: > >> > DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED says it requires > >> > ALLOW_EXTERNAL_TABLE_LOCATION, but the implementation checks > >> > ALLOW_UNSTRUCTURED_TABLE_LOCATION. [1] > >> > > >> > Thoughts? > >> > > >> > 1. > >> > > >> > > >> > https://github.com/apache/polaris/blob/1ef1d722521e460381d9cc8e8461619727df4c18/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java#L1148 > >> > > >> > > >> > Yufei > >> > > >> > > >
