Hi JB, Thanks for the clear write-up. I went through the PR, mostly the scan path (DirectoryScanner, DirectoryCatalogHandler.scanDirectory) since that is where I have been spending time with commits. +1 on the overall direction. Some notes, the first one is the one I'd fix before 1.9.0.
1/ Cleanup on CommitStateUnknownException In DirectoryScanner.scan, any RuntimeException from overwrite.commit() deletes the new Parquet file. If the commit fails with CommitStateUnknownException the overwrite may actually have landed, and then the current snapshot points at a file we just deleted, so every read of the inventory fails. Iceberg's own SnapshotProducer skips cleanup for that exception for this reason. Suggest catching CommitStateUnknownException first and rethrowing it without deleting, and a unit test with a table ops that throws it after committing. 2/ Memory and request time The scan collects every record in a List before writing, inside the HTTP request. A prefix with a few million objects means a few million Records on the heap, and a request that can easily outlive a proxy or client timeout. Two cheap options: stream the listing into a rolling writer (one file per N rows) instead of a list, and add a configurable max object count that fails the scan with a clear 4xx when exceeded. Longer term this is a good fit for an async task, but that can wait. 3/ Base location without a trailing slash The listing uses the base location as the raw prefix. With base_location = s3://bucket/images, the ListObjectsV2 prefix is "images", which also matches images-old/ and images2/. And I think the vended credentials will refuse it: the list policy condition is s3:prefix "images/*", so on real S3 the call may get AccessDenied. Normalizing the base location to end with "/" at creation (or before listing) would fix both. Worth a quick check since the manual S3 test may have used a trailing slash. 4/ Inventory table inside the scanned prefix The inventory table is created with the default location, under the namespace location. If the base location covers the namespace location (for example a directory over the whole namespace prefix), each scan records the inventory's own data and metadata files, and the inventory grows with every scan. Either reject that overlap at creation or exclude the inventory table location from the listing. 5/ Filters include/exclude are Java regexes compiled at scan time. Two small things: compile them in createDirectory so a bad pattern is a 400 at creation instead of a failed scan later, and since any principal who can create tables in the namespace can create a directory, a pattern with catastrophic backtracking runs on the server against every object URI. Globs (or a length limit plus a match timeout) would avoid that. Happy either way, but it is worth a line in the docs. On your questions: 1. API shape: looks good to me. On <directory>__inventory: since the directory and its inventory are separate entities, I'd return the inventory table identifier in LoadDirectoryResponse so clients don't have to rebuild the convention, and it leaves room to change the naming later. 2. Privileges: fine to reuse the table privileges for the beta. One gap: the inventory is an ordinary table, so anyone with TABLE_WRITE_DATA on it can rewrite it directly, and the next scan silently overwrites their changes (the overwrite uses alwaysTrue with no conflict validation). That is fine as "last scan wins", but it should be written down, together with the fact that an SPI implementation that adds checksums incrementally will race with the built-in full scan. 3. Server-side scan: agree on keeping it simple with the SPI. Points 1 and 2 above are what I'd want for the simple version. 4. Feature flags: I'd keep ENABLE_DIRECTORIES true and set ENABLE_DIRECTORY_SCAN to false by default for 1.9.0, mainly because of the memory/timeout point. Users who opt in know they are running a synchronous scan. 5. Dependencies: no concern, same Parquet version. Smaller things for the PR itself: dropDirectory calls dropTable(tableId, false), so the inventory's data files stay in storage after a drop; probably fine but worth documenting. And createDirectory writes the entity before the table, so a crash in between leaves a directory with no inventory; scanDirectory then fails on loadTable with a NoSuchTableException, which a clearer message would help with. Thanks again for pushing this, happy to help with tests on the scan path. Regards, Ankur On Sun, Oct 18, 2026 05:21 AM, Jean-Baptiste Onofré <[email protected]> wrote: > Hi everyone, > > I know that has been discussed for a while, with some periods of pause, but > I want to provide a clear view about the Polaris Directories proposal, and > the status. > To be short: I updated/rebased the PR (#4613), addressing your comments and > implementing your suggestions. > The PR is now ready to review: > > https://github.com/apache/polaris/pull/4613 > > Imho, it would be great to have a beta in Polaris 1.9.0. > > In order to give a clear view for the community, I'm taking a step back > here :) > (I will create a proposal issue with the following content) > > Motivation > -------------- > More and more lakehouse workloads (especially AI) need to work with data > that is not a table, such as images, videos, PDF documents, etc. > These objects often live in the same buckets as Iceberg tables, but they > are invisible to the catalog, so users have to fall back to listing the > object store directly, with no governance and no SQL access. > > Directories make these objects discoverable alongside Iceberg tables in a > Polaris catalog. > > How it works > ------------------ > A directory is a new table-like entity in a namespace. It points to a base > location (prefix) on an object store (S3, GCS, Azure, file) and tracks the > objects it contains in an Iceberg "inventory" table, one row per object, > with the following schema: > > file_uri, content_type, size, checksum_algorithm, checksum, > last_modified, metadata (map<string, string>) > > When a directory is created, Polaris stores its configuration (name, > base_location, include/exclude filters, scan schedule) and creates the > associated inventory table, named <directory>__inventory (a directory and > table can't share a name in a namespace). > > Because the inventory is a regular Iceberg table, any engine that reads > Iceberg can query it, for instance: > > SELECT file_uri, size FROM cat.ns.`images__inventory` > WHERE content_type = 'image/jpeg' ORDER BY last_modified DESC; > > The PR adds: > - New REST endpoints (Polaris specific, next to generic tables and > policies): > GET .../polaris/v1/{prefix}/namespaces/{ns}/directories > POST .../polaris/v1/{prefix}/namespaces/{ns}/directories > GET .../polaris/v1/{prefix}/namespaces/{ns}/directories/{directory} > DELETE .../polaris/v1/{prefix}/namespaces/{ns}/directories/{directory} > POST > .../polaris/v1/{prefix}/namespaces/{ns}/directories/{directory}/scan > The OpenAPI spec is in spec/polaris-catalog-apis/directories-api.yaml > - Location validation: base_location is validated like a table location > (catalog allowed locations, namespace location unless > allow.unstructured.table.location is set), at creation and before each > scan. > - A smple, synchronous scan endpoint: it lists the objects under > base_locatiion (with credentials scoped to that location), applies the > filters, and replaces the inventory table content in a single overwrite > commit. A failed scan leaves the table unchanged. It populates file_uri, > content_type (guessed from the file name), size and last_modified. > - A DirectoryScanService SPI, so the scan is pluggable: the build-in > implementation is a @DefaultBean that an extension can replace (e.g. to > compute checksums, read object metadata or update the table incrementally). > - Two feature flags: ENABLE_DIRECTORIES and ENABLE_DIRECTORY_SCAN. > - Documentation: site/content/in-dev/unreleased/directories.md > > Polaris does not schedule scans: scan-schedule is informational, and > periodic scans are left to the caller or to an external service (which can > read the directory configuration through the REST API and write the > inventory table itself). > > Feedback welcome on > ------------------------------ > 1. API shape: endpoint paths, request/response payloads, and the > <directory>__inventory naminig convention. > 2. Directories currently reuse the table privileges (LIST_TABLES, > CREATE_TABLE_DIRECT, LOAD_TABLE, DROP_TABLE_WITHOUT_PURGE, UPDATE_TABLE for > the scan). I don't want to introduce dedicated directory privileges, but > I'm happy to revisit it. > 3. Server-side scan: that's a simple on-demand scan. If users want > something richer we can provide new scan services thanks to the SPI. I > would like to keep it simple and self-provided. > 4. The two feature flags are true by default. I'm happy to change to false > by default if it helps to land Directories in Polaris 1.9.0. > 5. The simple scan service adds iceberg-data, iceberg-parquet and > parquet-hadoop dependencies to the Polaris runtime service (same Parquet > version already on the classpath). > > Follow-ups > --------------- > - Credential vending for the objects themselves via the "indirection URL" > (giving engines access to the files referenced in the inventory) is not > addressed in the PR. I propose to tackle credential vending as a follow-up. > - The scan endpoint doesn't emit Polaris events yet. > - The PR includes unit tests and an end-to-end tests on a file-based > catalog. I did "manual tests" on S3. I haven't tested it on GCS or Azure > yet. > > Thanks to those who already did first rounds of review on the PR. Any > feedback, here or on the PR, is welcome. > > As said earlier, I would love to include Directories as a beta feature in > Polaris 1.9.0. > > Thanks! > > Regards > JB >
