moomindani commented on PR #16574: URL: https://github.com/apache/iceberg/pull/16574#issuecomment-5213071139
@gaborkaszab I've reworked the PR to address both of your concerns. Rather than argue for the original shape, I went looking for how other projects solve the same problem, since the dev@ thread [1] hasn't drawn input — that seems to be the norm for the list rather than a signal about this proposal, as eight of the twelve [DISCUSS] threads started last month have no replies. Three projects that filter tables for exactly this reason converge on the same design: | | Coarse level | Fine level | Value format | |---|---|---|---| | **Debezium** | `database.include.list` | `table.include.list` / `.exclude.list` | comma-separated regexes, matched against the fully-qualified name | | **DataHub** | `schema_pattern` | `table_pattern` (allow/deny) | list of regexes; deny wins over allow | | **OpenMetadata** | `schemaFilterPattern` | `tableFilterPattern` | includes/excludes lists | Two things stood out, and both are now in the PR. **1. Both levels, not one instead of the other.** All three offer a coarse level *and* a per-table level. That matches my own operational experience: selecting whole databases is the common case, but production configuration eventually needs to exclude a specific table inside an otherwise interesting database, and a namespace-only filter cannot express that. So the PR keeps table-name filtering and adds `metrics-reporter.namespace.include` / `.exclude` alongside it. The two levels apply independently, so you can select `prod` and `analytics` while still dropping `bench_*` tables anywhere: ``` metrics-reporter.namespace.include=prod,analytics metrics-reporter.table-name.exclude=.*\.bench_.* ``` This directly addresses your point about name-based prediction being fragile. A namespace is part of a table's identity rather than a naming convention, so a table added to an included namespace later is picked up automatically, and a table outside it cannot match by accident. Namespace filtering is the safer default; the table-name level remains for what a namespace cannot express. **2. A list of patterns, not a single one.** All three accept a list. The PR now does too, so covering several databases no longer means hand-writing an alternation. On the surprise-capture risk specifically, it's worth stating something that was already true but undocumented: patterns are matched against the **entire** name via `Matcher.matches()`, never a substring. Debezium documents the same choice as an "anchored regular expression" — "the specified expression is matched against the entire name string of the table; it does not match substrings". Concretely, `prod\..*` matches `prod.db.table` but not `production.db.table` or `prod_sandbox.db.table`. An unanchored implementation would silently capture all three, which is the failure mode you were pointing at. It's now documented and pinned by a test. Deriving the namespace does not touch `ScanReport`/`CommitReport`, deliberately — those are defined in the REST OpenAPI spec, so changing them is a much larger conversation than this PR. Instead the catalog passes its own name when it loads the reporter, and the namespace is derived from the reported table name by stripping that prefix. Knowing the catalog name is what makes this unambiguous: `CatalogUtil#fullTableName` switches its separator for URI-like catalog names and gives no way to tell a dotted catalog name from the namespace that follows. Tests cover both shapes plus multi-level and empty namespaces. **On your second concern (static lifecycle).** I haven't changed this, and I still think the honest framing is the one from my earlier comment: adding config reload would mean inventing a mechanism Iceberg doesn't have, which shouldn't be settled as a side effect of this feature. What did change is that namespace filtering reduces how often you'd need to touch the config at all — adding a table to an included namespace requires no config change, and therefore no restart. That doesn't eliminate the limitation you identified, but it makes it bite less often in the case that motivated this. Two commits, split for review: one for the pattern list, one for the namespace filter. All 39 CI checks are green. Happy to keep iterating on the shape. [1] https://lists.apache.org/thread/dmf60fs64swkcy8cpdgkxn2nldx836xo -- 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]
