mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-05 00:09:35 +00:00
Hengfei Yang
ca75da8b78
Fixes #11342. ## Problem Upgrading fails on any deployment using a non-`public` postgres schema: ``` [POSTGRES] Fresh install: creating partitioned table file_list db init failed: SqlxError# relation "file_list" already exists ``` The DDL and DML we emit are **unqualified** and resolve through `search_path`, but the catalog / `information_schema` lookups that decide *what* to emit were pinned to a literal `public`. The two disagree: `get_table_relkind` reports "table missing" for a table that plainly exists, we take the fresh-install branch, and `CREATE TABLE` collides with the table already sitting in the user's schema. **The panic is not the worst case.** When a stale same-named table exists in `public` (old deployment, another tenant), the probe returns `'r'` and we take the *regular-to-partitioned migration* branch against the wrong schema's table — renaming its indexes. That matches the index-name conflicts the reporter described. The reporter found 2 sites; there were 14 in total. ## Approach Replace the literal `'public'` with `current_schema()` inline in each catalog query. It evaluates on the very connection that runs the query — the first existing entry of that connection's `search_path`, i.e. exactly where its unqualified DDL/DML lands — so a probe can never disagree with the writes again. No Rust-side state. Two details: - `pg_class` lookups join `pg_namespace` on `nspname = current_schema()` instead of casting to `regnamespace`: the cast parses its input as an identifier and fails on schema names with special characters (e.g. `My-Schema.X`). - `verify_schema_consistency()` at startup checks the read-write, read-only and DDL pools resolve to the same schema. They come from three separately configured DSNs, and `ZO_META_DDL_DSN` is documented as a different **role**, which can carry its own `search_path` via `ALTER ROLE ... SET search_path` — e.g. `cache_indices` reads on `CLIENT` while `create_index` writes on `CLIENT_DDL`, so a divergence would silently relocate the same bug class. Fail loudly instead. ## Sites fixed | File | Functions | |---|---| | `file_list/postgres.rs` | `get_table_relkind`, `check_partition_exists`, `drop_empty_partitions`, `get_varchar_column_width`, `apply_autovacuum_tuning`, `align_identity_sequence` | | `db/postgres.rs` | `cache_indices` (had **no** schema filter at all), `add_column`, `drop_column`, meta `start_dt` probe | | `migration/adapter/postgres.rs` | `list_tables`, `get_columns`, `get_foreign_keys`, `sync_sequences` | | 2 sea-orm migrations | postgres branch had no schema filter, while the mysql branch correctly used `DATABASE()` | The migration-adapter ones matter independently: under a non-public schema `list_tables()` returned **zero** rows, so the cross-database migration tool reported success having migrated nothing. ## Verification Against a real PostgreSQL 18.4 instance with the app configured with `search_path=o2`: - **Fresh install** into schema `o2`: boots clean, all 121 tables land in `o2`. - **Forced upgrade path** (`db_schema_version` lowered) **with a stale regular `public.file_list` planted**: logs `Table file_list already partitioned, ensuring partitions`, upgrade completes, and the stale `public` table is untouched. On main this exact setup either panics (`relation "file_list" already exists`) or migrates the wrong schema's table. - Every rewritten query shape additionally verified in psql under schema `o2test` and under a schema named `My-Schema.X` (stale-table isolation, partition listing, varchar width, identity-sequence alignment, index cache, adapter `list_tables`). `cargo check --bin openobserve`, `cargo fmt --check`, `cargo clippy -p infra --all-targets` all clean; 108 infra postgres unit tests pass. ## Not covered here 1. **Recovery for already-broken databases.** This prevents the failure but does not unstick installs that are mid-migration with renamed indexes. Worth a follow-up (idempotent cleanup, or a documented recovery procedure). 2. **CI coverage.** There is no job exercising a non-`public` `search_path`, which is why these 14 sites accumulated across different changes. A postgres job with `search_path=o2test` would stop this recurring.