Skip to content

Conversation

@macneale4
Copy link
Contributor

@macneale4 macneale4 commented Nov 3, 2025

User Contributed PR: #9946

Closes: #5861

codeaucafe and others added 14 commits October 9, 2025 16:54
Add ignore pattern filtering to dolt_diff_summary table function to
match dolt diff command behavior. Tables matching patterns in
dolt_ignore are now filtered out from diff summary results.

Changes include:
- Add getIgnorePatternsFromContext() to retrieve ignore patterns from
dolt_ignore
- Add shouldIgnoreDelta() to check if table deltas should be ignored
- Add shouldFilterSystemTable() to filter out system tables (dolt_*
prefix)
- Apply filtering to both general queries and specific table queries
- Only ignore added/dropped tables, not modified/renamed tables

Tests added:
- 5 integration tests in dolt_queries_diff.go
- 4 bats tests in ignore.bats covering basic patterns, wildcards,
dropped tables, and specific table queries

Refs: #5861
Fix test setup issues that were causing failures in integration tests:
- Correct ignore pattern from 'ignored_table' to 'ignored_table2'
- Add initial table creation before commit in three test cases
- Remove "nothing to commit" errors by establishing proper baseline

All dolt_diff_summary ignore functionality tests now pass.

Refs: #5861
Changed from filtering all dolt_* system tables to only filtering
the dolt_ignore table itself in dolt_diff_summary. This maintains
ignore pattern functionality while being more conservative about
which system tables are filtered, fixing bats test failures.

Refs: #5861
Update shouldFilterDoltIgnoreTable to use strings.EqualFold instead
of case-sensitive equality comparison when filtering dolt_ignore
tables from diff summary results.

In DoltgreSQL (PostgreSQL mode), table names may have different case
representations internally. The case-sensitive comparison (==) was
failing to properly filter out dolt_ignore tables, causing them to
appear in dolt_diff_summary results when they should be excluded.

Using strings.EqualFold ensures the filter works consistently
regardless of case variations, matching the behavior of other table
name comparisons in the codebase (e.g., findMatchingDelta).

This fixes DoltgreSQL integration test failures in
TestUserSpaceDoltTables/dolt_ignore test cases.

Refs: #5861
Update dolt_diff_summary to properly filter dolt_ignore table in
DoltgreSQL by handling schema-qualified table names (e.g.,
"public.dolt_ignore"). The previous implementation only checked
for exact matches against "dolt_ignore", which failed when table
names included schema prefixes.

Changes:
- Add isIgnoreTable helper function to check both simple and
  qualified table names
- Extract base table name from schema-qualified names using
  LastIndex for dot separator
- Maintain backward compatibility with unqualified names

This should fix the DoltgreSQL integration test failures where
dolt_ignore tables were incorrectly appearing in diff summaries.

Refs: #5861
Remove dolt_ignore auto-filter when user explicitly requests the
table by name. Previously, querying dolt_diff_summary with
dolt_ignore as the table parameter would incorrectly return empty
results even though the table was explicitly requested.

The dolt_ignore table is still auto-filtered from general listings
(when no table name is provided), but can now be queried directly
when specified by name, matching expected behavior where users can
query any table explicitly.

This fixes DoltgreSQL integration test failures where explicit
dolt_ignore queries were being blocked, something I missed in my
implementation originally.

Refs: #5861
Remove the shouldFilterDoltIgnoreTable function and its usage, which was
filtering out the dolt_ignore table itself from diff results in
dolt_diff_summary.

From my understanding of code, dolt_ignore table should appear in
diff summaries just like any other table. Only tables that match
patterns defined IN the dolt_ignore table should be filtered from
results, which is correctly handled by the shouldIgnoreDelta function.

This is an attempts to fix the DoltgreSQL integration test failure in
TestUserSpaceDoltTables/dolt_ignore which expects dolt_ignore table
changes to be visible in diff_summary results.

Refs: #5861
Update test expectations to correctly reflect that the dolt_ignore
table itself should appear in dolt_diff_summary results when
modified.

Fixed four tests in dolt_queries_diff.go:
- basic ignore functionality: expect dolt_ignore + filtered tables
- wildcard patterns: expect dolt_ignore + non-matching tables
- mixed patterns: expect dolt_ignore + explicitly included tables
- dropped tables: expect dolt_ignore as "modified" when updated

These tests were originally written with incorrect expectations that
dolt_ignore would be filtered from results. The DoltgreSQL
integration tests had the correct expectations all along.

Refs: #5861
Fix the "dropped tables" test to ensure dolt_ignore exists in HEAD
before modifying it, so it correctly appears as "modified" instead
of "added" in diff results.

The test now inserts a dummy row into dolt_ignore before the initial
commit, then deletes it and adds the real pattern after. This makes
dolt_ignore show diff_type "modified" rather than "added".

Refs: #5861
@coffeegoddd
Copy link
Contributor

@macneale4 DOLT

comparing_percentages
100.000000 to 100.000000
version result total
afb159b ok 5937471
version total_tests
afb159b 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@macneale4 DOLT

comparing_percentages
100.000000 to 100.000000
version result total
8ef3685 ok 5937471
version total_tests
8ef3685 5937471
correctness_percentage
100.0

@macneale4 macneale4 changed the title ci 9946 refactor: make dolt_diff_summary respect dolt_ignore patterns Nov 3, 2025
@macneale4 macneale4 marked this pull request as ready for review November 3, 2025 20:04
@macneale4 macneale4 requested a review from nicktobey November 3, 2025 20:05
@macneale4 macneale4 merged commit a74a4a6 into main Nov 3, 2025
23 checks passed
@macneale4 macneale4 deleted the macneale4/ci-9946 branch November 3, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dolt_diff_summary system table should hide tables ignored by dolt_ignore

5 participants