cgivre commented on PR #3080: URL: https://github.com/apache/drill/pull/3080#issuecomment-6060370059
Thanks for the review @pjfanning. Addressed in c5ac8ee: 1. **Bypass via table function:** `allowLinkedDatabases` is removed from `MSAccessFormatConfig` (the constructor is back to its original form). It is now a boot option, `drill.exec.storage.msaccess.allow_linked_databases` (default `false`), so only an administrator can enable it in `drill-override.conf`. The per-query section is gone from the README, and there's a test that passing `allowLinkedDatabases => true` in a table function doesn't open the file. 2. **Metadata listing:** the reader checks `getTableMetaData(name).isLinked()` before opening each table. Linked tables are listed by name with null dates/counts and an empty column list instead of failing the query. The date/count columns in the metadata schema are now nullable to support this. Added a test. 3. **Error message:** reading a linked table now raises a permission `UserException` that says linked tables are disabled and names the boot option. A table that doesn't exist now gets a proper "not found" error instead of an NPE. The rejecting `LinkResolver` stays as a backstop. 4. **Tests:** no more UNC paths; link targets live under `dirTestWatcher`. The new `TestMSAccessLinkedTables` starts a cluster with the boot option on, links to a real database containing a row, and asserts the row comes back. 5. **Minor:** `updated_date` now uses `getUpdatedDate()` (expected value in `testMetadataStarQuery` updated), import order fixed, and the README notes that linked tables are off by default from 1.23. -- 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]
