This is an automated email from the ASF dual-hosted git repository. Yicong-Huang pushed a commit to branch fix/local-dev-fresh-db-replay in repository https://gitbox.apache.org/repos/asf/texera.git
commit 19364e77fa14c0aab1d78f188c45704a67d7bc87 Author: Yicong Huang <[email protected]> AuthorDate: Wed Jul 29 22:08:03 2026 +0000 fix(local-dev): don't abort `up` on an already-applied sql/updates changeSet 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 #7064 --- bin/local-dev/main.sh | 45 ++++++++++++++-- bin/local-dev/tests/test_local_dev_sh.sh | 91 ++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/bin/local-dev/main.sh b/bin/local-dev/main.sh index 8ee11a256b..4c5459c424 100755 --- a/bin/local-dev/main.sh +++ b/bin/local-dev/main.sh @@ -1398,6 +1398,33 @@ parse_changelog_changesets() { ' "$changelog" } +# True when every error psql reported is a complaint that the object it tried +# to create is already there — i.e. the changeSet's effect is already in the +# schema and replaying it was a no-op. +# +# Why this is needed: on a fresh volume postgres runs sql/texera_ddl.sql itself +# (compose mounts ../../sql to /docker-entrypoint-initdb.d), and that DDL is +# kept in sync with sql/updates/*, so the changeSets we replay right afterwards +# re-create objects that already exist. The ones incidentally written with +# IF NOT EXISTS pass; the rest aborted `up` before the build ever started +# (#7064) — e.g. 28.sql's dataset_owner_uid_name_key, which texera_ddl.sql +# already creates as the auto-generated name for UNIQUE (owner_uid, name). +# +# Deliberately narrow. Only `already exists` counts: +# • `duplicate key` is a data conflict, not an applied schema change +# • no ERROR line at all means we cannot see why psql failed, and a failure +# we can't explain is never assumed harmless +# Everything else — syntax errors, missing relations, permissions — keeps +# failing loudly, so a genuinely incomplete schema still stops the build +# instead of reaching jOOQ codegen. +_sql_errors_all_already_exist() { + local f="${1:-}" errs="" + [[ -f "$f" ]] || return 1 + errs=$(grep 'ERROR:' "$f" 2>/dev/null) || return 1 + [[ -n "$errs" ]] || return 1 + ! printf '%s\n' "$errs" | grep -qv 'already exists' +} + # Reconcile sql/updates/* with the live DB so jOOQ codegen (which reads the # database at sbt-compile time) sees the schema the checked-out code expects. # The repo's official runner is liquibase (sql/docker-compose.yml — manual, @@ -1465,12 +1492,24 @@ infra_apply_sql_updates() { return 1 fi tui_step "postgres: applying $path (changeSet $id)" + # Keep psql's stderr: it holds the one line that explains an abort, + # and _sql_errors_all_already_exist needs it to tell "this changeSet + # is already in the schema" apart from a real failure. + local psql_err="$LOG_DIR/psql-changeset-$id.err" if ! sed 's/^\\c.*$//' "$sql_file" \ | docker exec -i "$pg" psql -U texera -d texera_db \ - -v ON_ERROR_STOP=1 -f - >/dev/null 2>&1; then - tui_err "postgres: $path failed -- inspect with: docker exec -i texera-postgres psql -U texera -d texera_db < $path" - return 1 + -v ON_ERROR_STOP=1 -f - >/dev/null 2>"$psql_err"; then + if _sql_errors_all_already_exist "$psql_err"; then + # Fall through to the INSERT below so the changeSet is + # recorded as applied and later runs stop retrying it. + tui_skip "postgres: $path already in schema (recording changeSet $id)" + else + tui_err "postgres: $path failed -- inspect with: docker exec -i texera-postgres psql -U texera -d texera_db < $path" + sed 's/^/ /' "$psql_err" >&2 + return 1 + fi fi + rm -f "$psql_err" fi docker exec "$pg" psql -U texera -d texera_db -qc " INSERT INTO public.databasechangelog diff --git a/bin/local-dev/tests/test_local_dev_sh.sh b/bin/local-dev/tests/test_local_dev_sh.sh index c8d5e1d417..b4f4c66949 100755 --- a/bin/local-dev/tests/test_local_dev_sh.sh +++ b/bin/local-dev/tests/test_local_dev_sh.sh @@ -593,5 +593,96 @@ else _fail "changelog references missing files:$missing_files" fi +# 29) A changeSet the DB already satisfies must not abort `up`. On a fresh +# volume postgres' own /docker-entrypoint-initdb.d runs sql/texera_ddl.sql, +# which is kept in sync with sql/updates/*, so replaying those changeSets +# re-creates objects that are already there; the ones not written with +# IF NOT EXISTS used to kill `up` before the build ever started (#7064). +# `_sql_errors_all_already_exist` is the gate that tells that case apart +# from a real failure, so it's tested in both directions: it must say yes +# only for "already exists", and no for every other psql error, for a +# duplicate-key data conflict, and for input where it can't see any error +# at all (never assume harmless). +tolerate_fn=$(awk '/^_sql_errors_all_already_exist\(\)/{f=1} f{print} f&&/^}/{exit}' "$MAIN_SH") +if [[ -z "$tolerate_fn" ]]; then + _fail "_sql_errors_all_already_exist helper missing" +else + _tol_dir=$(mktemp -d 2>/dev/null || mktemp -d -t ldtol) + _tol_check() { # $1=label $2=expected rc $3=stderr contents + printf '%s' "$3" > "$_tol_dir/err" + local rc=0 + ( eval "$tolerate_fn"; _sql_errors_all_already_exist "$_tol_dir/err" ) || rc=$? + if (( rc == $2 )); then + _pass "already-applied detector: $1" + else + _fail "already-applied detector: $1" "expected rc=$2, got rc=$rc" + fi + } + # Positive: the real #7064 failure, and the other spellings postgres uses + # for an object that is already present. + _tol_check "relation already exists (the #7064 failure)" 0 \ + 'psql:<stdin>:44: ERROR: relation "dataset_owner_uid_name_key" already exists' + _tol_check "several already-exists errors, nothing else" 0 \ + 'psql:<stdin>:9: ERROR: column "x" of relation "dataset" already exists +psql:<stdin>:12: ERROR: constraint "y" for relation "dataset" already exists +psql:<stdin>:15: ERROR: type "z" already exists' + _tol_check "already-exists around harmless NOTICE/ROLLBACK chatter" 0 \ + 'NOTICE: Renamed 0 duplicate dataset name(s) +psql:<stdin>:44: ERROR: relation "dataset_owner_uid_name_key" already exists +ROLLBACK' + _tol_check "non-ASCII identifier already exists" 0 \ + 'psql:<stdin>:3: ERROR: relation "数据集_名称_key" already exists' + # Negative: anything we can't positively identify as already-applied has to + # keep failing loudly, or a genuinely broken schema reaches the sbt build. + _tol_check "syntax error" 1 \ + 'psql:<stdin>:7: ERROR: syntax error at or near "ALTERR"' + _tol_check "missing relation" 1 \ + 'psql:<stdin>:7: ERROR: relation "dataset" does not exist' + _tol_check "one already-exists mixed with one real error" 1 \ + 'psql:<stdin>:44: ERROR: relation "dataset_owner_uid_name_key" already exists +psql:<stdin>:51: ERROR: syntax error at or near "COMMITT"' + _tol_check "duplicate key is a data conflict, not an applied change" 1 \ + 'psql:<stdin>:44: ERROR: duplicate key value violates unique constraint "dataset_pkey"' + _tol_check "empty stderr" 1 '' + _tol_check "no ERROR line at all" 1 \ + 'NOTICE: table "dataset" does not exist, skipping' + # Edge: nothing to read. Guard against the helper "succeeding" on a path + # that was never written, which would swallow every failure. + rm -f "$_tol_dir/err" + rc=0 + ( eval "$tolerate_fn"; _sql_errors_all_already_exist "$_tol_dir/err" ) || rc=$? + if (( rc == 1 )); then + _pass "already-applied detector: missing stderr file" + else + _fail "already-applied detector: missing stderr file" "expected rc=1, got rc=$rc" + fi + rc=0 + ( eval "$tolerate_fn"; _sql_errors_all_already_exist ) || rc=$? + if (( rc == 1 )); then + _pass "already-applied detector: no argument" + else + _fail "already-applied detector: no argument" "expected rc=1, got rc=$rc" + fi + rm -rf "$_tol_dir" +fi + +# 30) Wiring for #29: the replay loop must consult the detector instead of +# aborting on the first psql failure, and must stop discarding psql's +# stderr — the old `2>&1` to /dev/null meant the one line that explains the +# abort ("relation ... already exists") never reached the operator, who was +# told to re-run the file by hand to find out why. +updates_body=$(awk '/^infra_apply_sql_updates\(\)/{f=1} f{print} f&&/^}/{exit}' "$MAIN_SH") +if [[ "$updates_body" == *"_sql_errors_all_already_exist"* ]]; then + _pass "infra_apply_sql_updates consults the already-applied detector" +else + _fail "infra_apply_sql_updates aborts without checking for already-applied changeSets" +fi +if [[ "$updates_body" == *"ON_ERROR_STOP"* ]] \ + && ! printf '%s' "$updates_body" | grep -qE '\-f -[[:space:]]*>/dev/null[[:space:]]*2>&1'; then + _pass "infra_apply_sql_updates keeps psql stderr for diagnosis" +else + _fail "infra_apply_sql_updates still throws psql stderr away" +fi + printf "\n%d passed, %d failed\n" "$PASS" "$FAIL" (( FAIL == 0 ))
