Skip to content

feat: push --exclude down to driver query level#828

Draft
patkujawa-wf wants to merge 2 commits into
k1LoW:mainfrom
patkujawa-wf:skip-partitions
Draft

feat: push --exclude down to driver query level#828
patkujawa-wf wants to merge 2 commits into
k1LoW:mainfrom
patkujawa-wf:skip-partitions

Conversation

@patkujawa-wf

@patkujawa-wf patkujawa-wf commented Apr 20, 2026

Copy link
Copy Markdown

Summary

Pushes --exclude patterns down to the driver's table-listing query, so excluded tables are never scanned in the first place. For postgres, that skips the 5–6 detail queries (columns, indexes, constraints, triggers, FKs) per excluded table.

Replaces the original --skip-partitions proposal per review feedback — partitioned-schema users now write --exclude 'partition_prefix_*' and get the same perf win without a postgres-specific flag.

Changes

  • New optional drivers.TableFilterer interface (SetTableExcludes([]string)); drivers opt in.
  • datasource.Analyze now accepts ...AnalyzeOption; WithTableExcludes(...) plumbs patterns through.
  • config.PushDownableTableExcludes() returns excludes only when no Include/IncludeLabels are set, so distance traversal can't silently drop tables. Post-fetch schema.Filter remains authoritative.
  • Postgres driver implements TableFilterer: converts glob * → SQL LIKE '%' (escaping _, %, \), matches both bare and schema-qualified relnames, drops FK relations whose parent was filtered.
  • --skip-partitions flag and cmdutil/skippartitions.go removed.
  • Unit tests for globToLike and queryForTables (no DB required).

Test plan

  • go build ./..., go vet ./...
  • go test ./schema/... ./config/... ./cmdutil/... ./drivers/postgres/...
  • Manual e2e: partitioned schema with --exclude 'events_p*' — confirm child partitions skipped
  • Without --exclude: behavior unchanged

@patkujawa-wf patkujawa-wf force-pushed the skip-partitions branch 2 times, most recently from 4b42aaf to 987c0cd Compare April 20, 2026 16:07
@patkujawa-wf patkujawa-wf marked this pull request as ready for review April 22, 2026 01:10

@k1LoW k1LoW left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

I can see the value in excluding partition tables at the query level for performance. However, --skip-partitions is a PostgreSQL-specific flag, and I'm hesitant to add driver-specific options to the CLI surface.

I think a more general approach would be to improve the existing table scanning and exclude filtering so that exclusions can be applied earlier (e.g., at the query level) rather than after the full schema is fetched. That would benefit all drivers and work with the existing configuration, not just for partitions.

Also, this PR depends on #827 (--verbose), which I don't plan to merge at this time.

- Add --skip-partitions flag to skip PostgreSQL table partitions
- Skip relations referencing excluded partition tables
- Reduces unnecessary scanning with partitioned tables
@patkujawa-wf patkujawa-wf marked this pull request as draft April 27, 2026 15:37
Per review feedback on k1LoW#828: drop the postgres-specific --skip-partitions
flag and instead push the existing --exclude patterns down to the
table-listing query so all drivers can benefit.

- Add drivers.TableFilterer optional interface (SetTableExcludes)
- Plumb excludes via datasource.WithTableExcludes(...) AnalyzeOption
- Add config.PushDownableTableExcludes() that returns excludes only
  when no Include/IncludeLabels are set (so distance traversal can't
  re-pull excluded tables, and post-fetch schema.Filter remains
  authoritative)
- Implement TableFilterer on the postgres driver: convert glob '*' to
  SQL LIKE '%' (escaping literal % and _), match both bare and
  schema-qualified relnames, drop relations whose parent was filtered
- Remove --skip-partitions flag and cmdutil/skippartitions.go;
  partitioned-schema users should now write
  --exclude 'partition_prefix_*' instead
@patkujawa-wf patkujawa-wf changed the title feat: add --skip-partitions flag for postgres feat: push --exclude down to driver query level Apr 27, 2026
@patkujawa-wf

Copy link
Copy Markdown
Author

@k1LoW is this more along the lines of what you are thinking?

@k1LoW k1LoW added enhancement New feature or request tagpr:minor labels May 8, 2026
@k1LoW

k1LoW commented May 8, 2026

Copy link
Copy Markdown
Owner

Thanks for reworking this. The direction looks great to me. Pushing --exclude down via an opt-in TableFilterer interface keeps the change driver-agnostic and leaves schema.Filter as the source of truth, which I think is exactly the right shape.

I'll come back with a detailed review later.

@patkujawa-wf

Copy link
Copy Markdown
Author

Thanks @k1LoW ! Sorry, I keep missing notifications on this PR

@k1LoW

k1LoW commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Thanks for the rework — the direction (opt-in TableFilterer, schema.Filter as source of truth) looks great. Since the PR touches several layers, let me start with three pieces of feedback and follow up with the rest after we align on these.

1. Distance > 0 should also disable push-down

PushDownableTableExcludes bails out when Include / includeLabels are set, but not when Distance > 0. In schema/filter.go, the distance traversal walks from every non-excluded table and can pull excluded tables back into the result set (the excludes2 loop drops anything that ended up in includes2). So today, --exclude foo + distance: 2 can legitimately keep foo in the schema if it's reachable from a non-excluded table.

With push-down, foo is filtered at the SQL stage and is no longer available for the traversal to re-include, which is a silent behavior change. I'd suggest extending the guard:

if len(c.Include) > 0 || len(c.includeLabels) > 0 || c.Distance > 0 {
    return nil
}

2. diff and lint also call datasource.Analyze

cmd/cmd.go and cmd/doc.go are updated, but cmd/diff.go (two call sites) and cmd/lint.go still call datasource.Analyze(c.DSN) without the new option. They share the same c.DSN / c.Exclude, so leaving them out means the perf win and the new filtering behavior diverge across commands. Unless this was intentional, it would be good to plumb WithTableExcludes(c.PushDownableTableExcludes()) through those too so the behavior is consistent.

3. TableFilterer API shape

The mutating setter on an interface (SetTableExcludes(patterns []string)) is a bit unusual for Go and requires callers to inject state after the driver is constructed. The rest of the driver layer already has an Option type (drivers.Option func(Driver) error), so I think it would fit more naturally to expose this as an option, e.g.:

// drivers/drivers.go
func WithExcludes(patterns []string) Option { ... }

and have datasource.Analyze translate WithTableExcludes(...) into a driver option that only filterer-capable drivers honor. Same opt-in semantics, but the configuration flows in at construction time and the interface stays read-only. Not a blocker — just want to surface it before more drivers start implementing it.


I'll follow up with the remaining notes (relation drop condition, schema-qualified LIKE redundancy, --exclude '*' edge case, docs) once these three are settled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request tagpr:minor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants