Fix dump/restore when chunk skipping is enabled

[erimatnor]

The chunk skipping feature uses a "special" entry in the catalog table chunk_column_stats to indicate that chunk skipping is enabled for that column. The special entry has the chunk_id column set to 0. However, at the same time the table schema defines a foreign key on the chunk_id column where the 0 entry violates that constraint. This prevents a dump/restore from working when such an entry is present.

To fix this issue, change the schema for chunk column stats to use NULL instead of 0 for "chunk_id" to indicate the special entry. Foreign keys work with such NULL entries so the foreign key can remain.

Add an update test to ensure that pg_dump/pg_restore works properly.

Fixes: #8130

Disable-check: commit-count

[codecov]

Codecov Report

Attention: Patch coverage is 84.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 81.98%. Comparing base (42a1c3a) to head (fd474b0).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/ts_catalog/chunk_column_stats.c	84.00%	1 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8211      +/-   ##
==========================================
- Coverage   82.37%   81.98%   -0.40%     
==========================================
  Files         256      256              
  Lines       48135    48129       -6     
  Branches    12141    12141              
==========================================
- Hits        39653    39457     -196     
- Misses       3615     3879     +264     
+ Partials     4867     4793      -74     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

@gayyappan, @svenklemm: please review this pull request.

Powered by pull-review

[mkindahl]

We should probably use NULL to represent this, since NULL means "no association with the referenced table", which is essentially what "invalid hypertable" means.

[erimatnor]

We should probably use NULL to represent this, since NULL means "no association with the referenced table", which is essentially what "invalid hypertable" means.

@mkindahl do you think that is a better solution than removing the FK constraint? Perhaps we should do this directly in this PR?

[mkindahl]

We should probably use NULL to represent this, since NULL means "no association with the referenced table", which is essentially what "invalid hypertable" means.

@mkindahl do you think that is a better solution than removing the FK constraint? Perhaps we should do this directly in this PR?

IMHO; this is a better alternative since this is this is what the semantics is used for.

[erimatnor]

@mkindahl @antekresic @fabriziomello I updated this PR to change the table schema to allow NULL values as some of you suggested. This is a bigger change than the quick fix I did first,, so I would appreciate if you could review one more time. I added some new update tests and it was a bit messy since not all tested versions support chunk skipping. Please check the update test changes to ensure I am doing sane things.

[mkindahl]

Patch looks fine, but it would be good if we could make sure that the downgrade is truly restoring the old state.

[mkindahl]

As we discussed, it was difficult to get it to work correctly through the downgrade with foreign keys intact, so approving the patch with the new warning since we expect downgrades to be rare.

We should probably consider removing foreign keys entirely in the catalog. After all, PostgreSQL does not use them, probably because it is hard to maintain them through catalog schema modifications.

[timescale-automation]

Automated backport to 2.20.x not done: cherry-pick failed.

Git status

HEAD detached from origin/2.20.x
You are currently cherry-picking commit 8a0a32934.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .unreleased/pr_8211
	modified:   scripts/test_update_from_version.sh
	modified:   scripts/test_updates.sh
	modified:   sql/pre_install/tables.sql
	modified:   src/ts_catalog/chunk_column_stats.c
	new file:   test/sql/updates/cleanup.chunk_skipping.sql
	modified:   test/sql/updates/cleanup.v7.sql
	new file:   test/sql/updates/cleanup.v8.sql
	new file:   test/sql/updates/cleanup.v9.sql
	new file:   test/sql/updates/post.chunk_skipping.sql
	new file:   test/sql/updates/post.v9.sql
	modified:   test/sql/updates/pre.cleanup.sql
	new file:   test/sql/updates/setup.chunk_skipping.sql
	new file:   test/sql/updates/setup.v9.sql
	modified:   tsl/test/expected/chunk_column_stats.out

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   sql/updates/latest-dev.sql
	both modified:   sql/updates/reverse-dev.sql

Job log