Skip to content

Tests for foreign keys with different encodings#2914

Merged
zachmu merged 12 commits into
mainfrom
zachmu/foreign-key-encoding
Jul 10, 2026
Merged

Tests for foreign keys with different encodings#2914
zachmu merged 12 commits into
mainfrom
zachmu/foreign-key-encoding

Conversation

@zachmu

@zachmu zachmu commented Jul 10, 2026

Copy link
Copy Markdown
Member

Relies on dolthub/dolt#11285

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
Main PR
covering_index_scan_postgres 1903.36/s 1916.08/s +0.6%
groupby_scan_postgres 126.78/s 131.79/s +3.9%
index_join_postgres 666.75/s 667.24/s 0.0%
index_join_scan_postgres 850.49/s 856.62/s +0.7%
index_scan_postgres 27.32/s 27.90/s +2.1%
oltp_delete_insert_postgres 838.90/s 827.25/s -1.4%
oltp_insert 716.14/s 715.62/s -0.1%
oltp_point_select 3080.39/s 3070.85/s -0.4%
oltp_read_only 3052.48/s 3063.01/s +0.3%
oltp_read_write 2304.23/s 2332.95/s +1.2%
oltp_update_index 734.49/s 760.06/s +3.4%
oltp_update_non_index 786.68/s 804.64/s +2.2%
oltp_write_only 1778.61/s 1789.48/s +0.6%
select_random_points 1941.83/s 1965.87/s +1.2%
select_random_ranges 1599.88/s 1608.59/s +0.5%
table_scan_postgres 26.10/s 26.42/s +1.2%
types_delete_insert_postgres 781.55/s 793.13/s +1.4%
types_table_scan_postgres 11.46/s 11.15/s -2.8%

@itoqa

itoqa Bot commented Jul 10, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 1388b8b: 10 test cases ran, 8 passed ✅, 2 additional findings ⚠️.

Summary

This run exercised cross-version compatibility behavior end to end, including branch/merge flows, foreign-key enforcement and violation tracking, suite discovery/coverage continuity, and release/version handling under both normal and adversarial inputs. Overall product behavior remains stable for the changed paths, with compatibility workflows continuing to behave as expected.

Safe to merge — there are no regressions, new failures, or previously flagged PR-attributable failures in this run, so the change appears low risk. The remaining failures are pre-existing compatibility-runner hardening gaps outside this PR and are important follow-up work, but not a merge blocker for this change.

Tests run by Ito

View full run

Result Severity Type Description
Backward The backward compatibility workflow preserved foreign key enforcement after extending an old-repo dataset with the new binary: valid child inserts succeeded and the invalid 'grape' insert was rejected.
Backward The dangling-child merge scenario behaved as expected: the unforced merge was rejected, then a forced merge completed and recorded exactly one child constraint violation for id=20, ref=banana across both varchar and text variants.
Backward The backward workflow compatibility suite passed for all six FK scenarios, confirming that repositories created with the legacy binary behaved consistently when continued with the current binary across varchar/text pairings, merge behavior, and violation tracking.
Merge The bidirectional compatibility workflow passed across both binary handoffs, including the final merge assertion that row pk=20 from old_branch is present after HEAD merges old_branch.
Runner The backward phase runs both bats ./test_files/bats/ and bats ./test_files/bats/backwards/ for each backward version, and all discovered tests pass.
Runner Directory-based suite discovery preserved all previously executed compatibility coverage and added backward-only workflow coverage, with no silent drops detected across backward, forward, bidirectional, and sanity phases.
Version The runner correctly fails with a non-zero exit when an invalid release tag is injected, so missing artifacts do not produce a false compatibility pass.
Version The compatibility runner completed forward and bidirectional coverage with v0.56.5 and finished with 126 passing BATS checks and exit code 0.
⚠️ High severity Version The runner treated a requested v0.56.5 artifact as valid even when the extracted binary reported v0.56.7, so expected per-version compatibility identity checks were not enforced.
⚠️ Medium severity Version The runner does not report when compatibility matrix phases execute zero iterations, and it still returns success while always running HEAD-vs-HEAD sanity tests that produce normal 'ok' output.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟠 Release artifact identity is not verified
  • Severity: High High severity
  • Description: The runner treated a requested v0.56.5 artifact as valid even when the extracted binary reported v0.56.7, so expected per-version compatibility identity checks were not enforced.
  • Impact: Users can receive and run a mismatched release artifact for a requested version without any warning, leading to silently incorrect version behavior and trust in invalid results.
  • Steps to Reproduce:
    1. Run compatibility for a listed tag (for example v0.56.5) and observe that download_release fetches and extracts the tarball before returning a binary path.
    2. Provide a structurally valid tarball under the requested tag path whose extracted binary reports a different version than the requested tag.
    3. Execute the extracted binary and confirm the runner proceeds without failing on checksum, signature, or version-identity mismatch.
  • Stub / mock content: A controlled local artifact-mismatch setup was applied by supplying a valid tarball for the requested tag whose extracted binary reported a different version, in order to verify runner validation behavior.
  • Code Analysis: In runner.sh, download_release (lines 59-75) only performs curl -L -o and tar -zxf, then returns the bin path with no verification step. The PR diff for runner.sh changes suite-selection logic later in the file and does not modify this download/validation path, so the bug is code-backed but outside changed lines for this PR. In compatibility.bats line 39, the version assertion accepts any x.y.z string, which also does not enforce that the binary version matches the requested tag.
Evidence Package
🟡 Runner reports success when compatibility matrix executes zero iterations
  • Severity: Medium Medium severity
  • Description: The runner does not report when compatibility matrix phases execute zero iterations, and it still returns success while always running HEAD-vs-HEAD sanity tests that produce normal 'ok' output.
  • Impact: Compatibility regressions can reach releases undetected, causing users on older or newer supported versions to encounter upgrade or interoperability failures.
  • Steps to Reproduce:
    1. In integration-tests/compatibility/test_files, set backward_compatible_versions.txt and forward_compatible_versions.txt to comments-only content so each file is non-empty but has zero real version entries after filtering.
    2. Run runner.sh with DOLTGRES_SKIP_BACKWARD=1 DOLTGRES_SKIP_FORWARD=1 DOLTGRES_SKIP_BIDIR=1 and observe exit 0 with only HEAD setup and sanity-check output, without any explicit zero-iteration declaration.
    3. Run again with DOLTGRES_SKIP_BACKWARD=1 and DOLTGRES_SKIP_BIDIR=1 while leaving DOLTGRES_SKIP_FORWARD unset; observe the forward banner prints but the forward loop still executes zero iterations and the script exits 0.
    4. Compare both runs with a real populated matrix run; output remains indistinguishable enough to mask missing compatibility coverage.
  • Stub / mock content: The run intentionally used skip flags and comments-only compatibility version files to exercise matrix-gating boundaries; no network or service mocks were introduced.
  • Code Analysis: The list helpers filter out comment lines (grep -v '^ *#'), but the phase gates only check -s (file size > 0). A comments-only list passes the gate, prints the phase banner, and then executes zero loop iterations. If skip flags are set, phases are skipped silently with no explanation. Because the HEAD-vs-HEAD sanity checks always run afterward, the command still emits passing BATS output and exits 0, masking missing matrix coverage. A targeted fix is to compute/filter effective version entries first, then print per-phase iteration counts and explicit zero-iteration/skip messages (and optionally fail when total matrix iterations are zero).
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

@github-actions

Copy link
Copy Markdown
Contributor
Main PR
Total 42090 42090
Successful 18275 18275
Failures 23815 23815
Partial Successes1 5335 5335
Main PR
Successful 43.4189% 43.4189%
Failures 56.5811% 56.5811%

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

@Hydrocharged Hydrocharged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall LGTM, but seems like including auth.db was an accident? That seems like something that should be in our .gitignore

@zachmu

zachmu commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

Overall LGTM, but seems like including auth.db was an accident? That seems like something that should be in our .gitignore

Good catch, will revert

@zachmu zachmu enabled auto-merge July 10, 2026 16:16
@zachmu zachmu disabled auto-merge July 10, 2026 16:53
@zachmu zachmu merged commit 2e49243 into main Jul 10, 2026
24 of 27 checks passed
@zachmu zachmu deleted the zachmu/foreign-key-encoding branch July 10, 2026 16:53
@itoqa

itoqa Bot commented Jul 10, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report1388b8b6f20d27: 7 test cases ran, 5 passing ✅, 2 additional findings ⚠️.

Diff Summary

This run covered core compatibility and release-readiness behavior across version handoffs, dependency verification and recovery, and repository hygiene rules, including both normal success paths and resilience-oriented edge conditions. Overall health looks stable for the changes under review, with checks showing expected behavior in the main upgrade and validation flows while also surfacing pre-existing robustness gaps in the broader workflow.

Safe to merge — the failures reported here are non-attributable additional findings rather than regressions or still-failing PR-linked issues. They are medium-severity, flag-for-later hardening items in existing compatibility workflow behavior, not merge blockers for this PR.

Tests run by Ito

View full run

Result State Severity Type Description
Passing Version The compatibility runner completed backward, forward, bidirectional, and HEAD sanity suites with the updated dolt/go and go-mysql-server pseudo-versions, finishing with 252 passing assertions, 0 failures, and exit code 0.
Passing Version The removed integration-tests/compatibility/auth.db fixture is absent and untracked, compatibility scripts do not reference it, and the compatibility BATS suite completes successfully without missing-file or auth bootstrap errors.
Passing Version The runner marked this case blocked due to a harness-level completion parsing failure, but the captured evidence shows go mod download and go mod verify both exited 0 with all modules verified, matching checksum and clean working-tree checks.
Passing Version After a forced mid-download proxy failure, rerunning dependency download with normal network access restored both pinned module versions and produced a successful verified build.
Passing Version The auth.db ignore rule behaved predictably across root and nested paths, and no local-versus-CI mismatch was found that would hide unexpected artifacts.
⏸️ Skipped Backward The backward compatibility workflow preserved foreign key enforcement after extending an old-repo dataset with the new binary: valid child inserts succeeded and the invalid 'grape' insert was rejected.
⏸️ Skipped Backward The dangling-child merge scenario behaved as expected: the unforced merge was rejected, then a forced merge completed and recorded exactly one child constraint violation for id=20, ref=banana across both varchar and text variants.
⏸️ Skipped Backward The backward workflow compatibility suite passed for all six FK scenarios, confirming that repositories created with the legacy binary behaved consistently when continued with the current binary across varchar/text pairings, merge behavior, and violation tracking.
⏸️ Skipped Merge The bidirectional compatibility workflow passed across both binary handoffs, including the final merge assertion that row pk=20 from old_branch is present after HEAD merges old_branch.
⏸️ Skipped Runner The backward phase runs both bats ./test_files/bats/ and bats ./test_files/bats/backwards/ for each backward version, and all discovered tests pass.
⏸️ Skipped Runner Directory-based suite discovery preserved all previously executed compatibility coverage and added backward-only workflow coverage, with no silent drops detected across backward, forward, bidirectional, and sanity phases.
⏸️ Skipped Version The runner correctly fails with a non-zero exit when an invalid release tag is injected, so missing artifacts do not produce a false compatibility pass.
⏸️ Skipped Version The compatibility runner completed forward and bidirectional coverage with v0.56.5 and finished with 126 passing BATS checks and exit code 0.
⚠️ Additional Finding Medium severity Version The runner does not publish an explicit signal that zero compatibility iterations executed, so a no-op matrix run is indistinguishable from a fully exercised successful matrix run.
⚠️ Additional Finding Medium severity Version download_release fetches and untars the artifact, then returns the bin path without checking checksum, signature, or extracted binary version against the requested tag.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Zero-iteration compatibility matrix run is reported as a normal success
  • Severity: Medium Medium severity
  • Description: The runner does not publish an explicit signal that zero compatibility iterations executed, so a no-op matrix run is indistinguishable from a fully exercised successful matrix run.
  • Impact: Compatibility validation can report a clean run even when zero version-pair checks actually executed, creating false confidence in release readiness. This can let unsupported version combinations reach users without being exercised.
  • Steps to Reproduce:
    1. Set DOLTGRES_SKIP_BACKWARD=1, DOLTGRES_SKIP_FORWARD=1, and DOLTGRES_SKIP_BIDIR=1, and run integration-tests/compatibility/runner.sh with compatibility version lists that produce no runnable entries.
    2. Observe that the script still runs HEAD repo setup and sanity checks, then exits 0.
    3. Verify the output contains no explicit skipped or zero-iteration declaration for backward, forward, or bidirectional compatibility phases.
  • Stub / mock content: The run intentionally used skip flags for all compatibility phases and reduced compatibility version inputs to no runnable entries to exercise zero-iteration behavior; no mocks or network interception were applied.
  • Code Analysis: In _main, each matrix phase is guarded by an if condition (backward lines 268-273, forward lines 280-285, bidirectional lines 288-293). When a skip flag is set or list processing yields no runnable entries, those blocks are bypassed and no status line is emitted. There is no else branch or iteration-count summary that records zero executed compatibility iterations. A targeted fix is to track per-phase iteration counts and print explicit skipped/zero-iteration messages before exit.
Evidence Package
🟡 Release download accepts mismatched artifact
  • Severity: Medium Medium severity
  • Description: download_release fetches and untars the artifact, then returns the bin path without checking checksum, signature, or extracted binary version against the requested tag.
  • Impact: Artifact downloads can accept a mismatched release binary, so compatibility checks may run against the wrong version and produce misleading release decisions. If an untrusted mirror or cache serves a substituted tarball, operators could execute unintended code during validation workflows.
  • Steps to Reproduce:
    1. Intercept the v0.56.5 release URL and return a structurally valid tarball containing a different doltgres binary version.
    2. Run download_release v0.56.5 from integration-tests/compatibility/runner.sh with the interception active.
    3. Observe that the function extracts and returns binaries/v0.56.5/.../bin, then verify the extracted binary reports v0.56.7 instead of v0.56.5.
  • Stub / mock content: A curl shim was intentionally used to substitute the requested release download with a different tarball so artifact-identity checks could be exercised in a non-production setup.
  • Code Analysis: In integration-tests/compatibility/runner.sh, download_release builds a GitHub release URL, runs curl -L -o to write the tarball, and immediately runs tar -zxf before returning the binary directory. There is no verification step between download and use, so a mismatched but well-formed artifact is accepted. A small practical fix is to enforce artifact identity before returning: verify a pinned checksum for the requested tag and/or run the extracted binary with --version and fail when it does not match the requested version.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants