Skip to content

fix(local-dev): don't abort up on an already-applied sql/updates changeSet - #7076

Open
Yicong-Huang wants to merge 2 commits into
apache:mainfrom
Yicong-Huang:fix/local-dev-fresh-db-replay
Open

fix(local-dev): don't abort up on an already-applied sql/updates changeSet#7076
Yicong-Huang wants to merge 2 commits into
apache:mainfrom
Yicong-Huang:fix/local-dev-fresh-db-replay

Conversation

@Yicong-Huang

@Yicong-Huang Yicong-Huang commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

bin/local-dev.sh up against a fresh docker volume never reached the sbt build: it died on the last sql/updates changeSet.

Postgres applies sql/texera_ddl.sql itself — compose mounts sql/ into /docker-entrypoint-initdb.d — and that DDL is kept in sync with sql/updates/*, so the changeSets local-dev replays immediately afterwards re-create objects that are already there. 23–27 are incidentally idempotent and pass; 28.sql's dataset_owner_uid_name_key is not, and sql/texera_ddl.sql's UNIQUE (owner_uid, name) on dataset already created it under exactly that auto-generated name.

infra_ensure_db_schema picks seed-vs-replay by probing for the feedback table. On a fresh volume the entrypoint has just created it, so the replay path is taken — and the seed branch written for this very case (record every changeSet as applied without executing it) is unreachable, because the entrypoint always wins the race.

Before:  fresh volume -> entrypoint applies full DDL -> replay 23-28 -> 28 fails -> no build
After:   fresh volume -> entrypoint applies full DDL -> 28 recorded as applied -> build runs

The fix is in the replay loop rather than the probe: a psql failure whose every ERROR: line is already exists means the changeSet's effect is already in the schema, so record it and carry on. That covers the next sql/updates/N.sql that isn't accidentally idempotent too, instead of fixing only 28.sql.

_sql_errors_all_already_exist is deliberately narrow — a duplicate key is a data conflict rather than an applied schema change, and a failure with no ERROR: line at all is never assumed harmless — so an incomplete schema still stops the build instead of reaching jOOQ codegen with tables that aren't there.

While in the same lines: psql's stderr is kept instead of redirected to /dev/null. It holds the one line that explains the abort, and the old code discarded it and then told the operator to re-run the file by hand to find out why.

Any related issues, documentation, discussions?

Closes #7064

How was this PR tested?

Unit coverage for the detector in both directions, plus two structural guards on the wiring, in the existing infra-job suite:

$ bash bin/local-dev/tests/test_local_dev_sh.sh
...
  ✓ already-applied detector: relation already exists (the #7064 failure)
  ✓ already-applied detector: several already-exists errors, nothing else
  ✓ already-applied detector: already-exists around harmless NOTICE/ROLLBACK chatter
  ✓ already-applied detector: non-ASCII identifier already exists
  ✓ already-applied detector: syntax error
  ✓ already-applied detector: missing relation
  ✓ already-applied detector: one already-exists mixed with one real error
  ✓ already-applied detector: duplicate key is a data conflict, not an applied change
  ✓ already-applied detector: empty stderr
  ✓ already-applied detector: no ERROR line at all
  ✓ already-applied detector: missing stderr file
  ✓ already-applied detector: no argument
  ✓ infra_apply_sql_updates consults the already-applied detector
  ✓ infra_apply_sql_updates keeps psql stderr for diagnosis

64 passed, 0 failed

The pytest half of the same job reports 1 failed, 42 passed on this branch. That failure is test_is_dirty_after_seed_then_edit, which is unrelated to this change — it reproduces identically on a pristine main checkout, and this PR touches neither tui.py nor that test. It is #7075, fixed separately.

End-to-end on the real stack (Ubuntu 24.04.4, docker 29.1.3), reproducing the failure and then confirming the fix:

bin/local-dev.sh down
docker volume rm texera-local-dev_postgres_data
bin/local-dev.sh up

Before:

  →  postgres: applying sql/updates/28.sql (changeSet 28)
  ✗  postgres: sql/updates/28.sql failed -- inspect with: docker exec -i texera-postgres psql -U texera -d texera_db < sql/updates/28.sql

After:

  →  postgres: applying sql/updates/27.sql (changeSet 27)
  →  postgres: applying sql/updates/28.sql (changeSet 28)
  ○  postgres: sql/updates/28.sql already in schema (recording changeSet 28)
  ✓  postgres: 6 sql/update(s) applied
  ...
  ✓ 14 of 14 services healthy

The changeSet is recorded, so it is not retried on the next run:

$ docker exec texera-postgres psql -U texera -d texera_db -tAc \
    "SELECT id||':'||exectype FROM public.databasechangelog ORDER BY orderexecuted"
23:EXECUTED
24:EXECUTED
25:EXECUTED
26:EXECUTED
27:EXECUTED
28:EXECUTED

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

…angeSet

On a fresh docker volume postgres applies sql/texera_ddl.sql itself — compose
mounts sql/ into /docker-entrypoint-initdb.d — and that DDL is kept in sync
with sql/updates/*, so the changeSets local-dev replays immediately afterwards
re-create objects that are already there. 23-27 are incidentally idempotent
and pass; 28.sql's dataset_owner_uid_name_key is not, so `up` died before the
sbt build ever started. The `seed` path written for exactly this case was
unreachable, because the container entrypoint always wins the race.

Treat a psql failure whose every ERROR line is "already exists" as
already-applied: record the changeSet and carry on. The check is deliberately
narrow — a duplicate-key conflict is a data problem, and a failure with no
ERROR line at all is never assumed harmless — so an incomplete schema still
stops the build instead of reaching jOOQ codegen.

psql's stderr is also kept rather than sent to /dev/null, so a real failure
no longer requires re-running the file by hand to find out why.

Closes apache#7064
@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
⚠️ release/v1.2 Not labeled automatically — none of the files this PR modifies exist on this branch (bin/local-dev/main.sh, bin/local-dev/tests/test_local_dev_sh.sh). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.2 by hand if this fix should be backported here.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @aglinxinyuan
    You can notify them by mentioning @aglinxinyuan in a comment.

… drop CJK test fixture

Note in the header comment that the "already in the schema" equivalence relies
on texera_ddl.sql staying in sync with sql/updates/* (ON_ERROR_STOP halts at the
first error, so it proves a no-op only when every object the changeSet touches is
already present). Replace the Chinese identifier in the non-ASCII detector test
with a Latin-accented one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Fresh local-dev postgres volume fails on sql/updates/28.sql before the build starts

2 participants