From b42d4605eefce78c0fa774f2426d6f6431f33427 Mon Sep 17 00:00:00 2001 From: Robin Kooyman Date: Mon, 7 Sep 2026 15:24:40 +0200 Subject: [PATCH 1/4] fix: let a real table take the default table name The startup stand-in for the default table blocked the name. CREATE TABLE and CREATE MATERIALIZED VIEW answered "already exists". The stand-in now yields to the first real table, and startup skips it when a loaded table holds the name. The stand-in also ignored sql.default_table and always registered "default", so BEACON_DEFAULT_TABLE=observations left that table missing. --- CHANGELOG.md | 12 ++ .../src/schema_persistence/default_table.rs | 81 ++++++++ .../beacon-core/src/schema_persistence/mod.rs | 10 +- .../src/schema_persistence/provider.rs | 57 ++++-- .../beacon-core/src/statement_plan/actions.rs | 7 +- .../src/statement_plan/materialized_view.rs | 10 +- beacon-db/beacon-core/tests/default_table.rs | 177 ++++++++++++++++++ beacon-db/beacon-core/tests/runtime_config.rs | 66 ++++--- docs/docs/2.0.0-rc5/server/configuration.md | 10 + 9 files changed, 392 insertions(+), 38 deletions(-) create mode 100644 beacon-db/beacon-core/src/schema_persistence/default_table.rs create mode 100644 beacon-db/beacon-core/tests/default_table.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c3e8b06..79f9d7a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -293,6 +293,18 @@ tag. Releases before 2.0.0 are recorded in the ### Fixed +- **The default table is a name you can claim.** At startup Beacon registers an empty stand-in + table, so a JSON query without a `from` field reports no missing table. That stand-in blocked + the name. `CREATE TABLE "default" (id BIGINT)` answered `Table 'default' already exists`, and a + `CREATE MATERIALIZED VIEW` on the same name answered the same way, while `CREATE EXTERNAL TABLE` + replaced the stand-in without a word. The stand-in now gives up the name to the first real + table, whichever `CREATE` statement makes it. No `DROP TABLE` is necessary first. Only the + stand-in yields: a real table under that name still refuses a second `CREATE TABLE` or + `CREATE MATERIALIZED VIEW`. It also keeps the name across a restart, because startup registers + the stand-in only for a name no loaded table holds. The stand-in also takes the configured name. `BEACON_DEFAULT_TABLE=observations` created a table called `default` and + left `observations` missing, so a `from`-less query failed on a fresh server. It now creates + `observations`. See + [Configuration](docs/docs/2.0.0-rc5/server/configuration.md#the-default-table). - **A long query no longer makes the API unreachable.** The HTTP API, Flight SQL and every query shared one Tokio runtime of `BEACON_WORKER_THREADS` threads. A scan holds a thread until a partition yields, and one query starts as many partitions as the machine has cores, so a long diff --git a/beacon-db/beacon-core/src/schema_persistence/default_table.rs b/beacon-db/beacon-core/src/schema_persistence/default_table.rs new file mode 100644 index 00000000..fcef8cd2 --- /dev/null +++ b/beacon-db/beacon-core/src/schema_persistence/default_table.rs @@ -0,0 +1,81 @@ +//! The stand-in for the configured default table. +//! +//! `sql.default_table` names the table a JSON query without a `from` resolves +//! against. Beacon registers an empty stand-in under that name so such a query +//! plans on a fresh database, and drops the stand-in as soon as a real table +//! takes the name. +//! +//! The stand-in is a distinct type, not a bare +//! [`EmptyTable`](datafusion::datasource::empty::EmptyTable), so the `CREATE` +//! paths recognize it. A `CREATE` statement replaces the stand-in instead of +//! reporting that the table exists. + +use std::{any::Any, sync::Arc}; + +use arrow::datatypes::{Schema, SchemaRef}; +use datafusion::{ + catalog::{Session, TableProvider}, + datasource::{empty::EmptyTable, TableType}, + error::Result as DataFusionResult, + logical_expr::Expr, + physical_plan::ExecutionPlan, + prelude::SessionContext, + sql::TableReference, +}; + +/// An empty, column-less table that holds the configured default-table name +/// until a real table takes it. +#[derive(Debug)] +pub struct DefaultTablePlaceholder { + inner: EmptyTable, +} + +impl DefaultTablePlaceholder { + pub fn new() -> Self { + Self { + inner: EmptyTable::new(Arc::new(Schema::empty())), + } + } +} + +impl Default for DefaultTablePlaceholder { + fn default() -> Self { + Self::new() + } +} + +#[async_trait::async_trait] +impl TableProvider for DefaultTablePlaceholder { + fn as_any(&self) -> &dyn Any { + self + } + + fn schema(&self) -> SchemaRef { + self.inner.schema() + } + + fn table_type(&self) -> TableType { + self.inner.table_type() + } + + async fn scan( + &self, + state: &dyn Session, + projection: Option<&Vec>, + filters: &[Expr], + limit: Option, + ) -> DataFusionResult> { + self.inner.scan(state, projection, filters, limit).await + } +} + +/// True when `name` holds nothing but the default-table stand-in. +/// +/// The `CREATE` paths call this to tell an occupied name from a name that only +/// carries the stand-in: the second one is free to take. +pub async fn holds_placeholder(session_ctx: &SessionContext, name: TableReference) -> bool { + match session_ctx.table_provider(name).await { + Ok(provider) => provider.as_any().is::(), + Err(_) => false, + } +} diff --git a/beacon-db/beacon-core/src/schema_persistence/mod.rs b/beacon-db/beacon-core/src/schema_persistence/mod.rs index 5298e1be..47e4f7a3 100644 --- a/beacon-db/beacon-core/src/schema_persistence/mod.rs +++ b/beacon-db/beacon-core/src/schema_persistence/mod.rs @@ -3,6 +3,8 @@ //! Beacon's catalog is the source of truth for which tables exist. This module owns: //! - [`PersistentSchemaProvider`] — the `beacon.public` schema provider that persists //! a table's definition on registration and removes it on deregistration; +//! - [`DefaultTablePlaceholder`] — the empty stand-in that holds the configured +//! default-table name until a real table takes it; //! - [`SchemaPersistenceService`] — the durable `db:///table.json` read/write path; //! - [`init_tables`] — startup recovery that rebuilds every provider from those files; //! - the private `loading`/`ordering` helpers `init_tables` drives. @@ -12,11 +14,13 @@ use std::{collections::HashMap, sync::Arc}; use beacon_datafusion_ext::table_ext::TableDefinition; use datafusion::{execution::object_store::ObjectStoreUrl, prelude::SessionContext}; +pub mod default_table; mod loading; mod ordering; pub mod provider; pub mod service; +pub use default_table::DefaultTablePlaceholder; pub use provider::PersistentSchemaProvider; pub use service::{definition_from_provider, SchemaPersistenceService}; @@ -27,7 +31,8 @@ pub use service::{definition_from_provider, SchemaPersistenceService}; /// definitions, then views in dependency order), builds each provider against /// the live session — so a view's defining query resolves the tables already /// registered ahead of it — and inserts it into `schema` without re-persisting. -/// Finishes by ensuring the empty `default` table exists. +/// Finishes by registering the empty stand-in for the configured default table, +/// but only when no loaded table already holds that name. /// /// `tables_store_url` is the store the persisted `/table.json` definitions /// are read from (the caller supplies it — the runtime uses its tables store). @@ -70,6 +75,7 @@ pub async fn init_tables( } } - schema.ensure_default_table(); + let default_table = crate::settings::SqlSettings::from_session(session_ctx).default_table; + schema.ensure_default_table(&default_table); Ok(()) } diff --git a/beacon-db/beacon-core/src/schema_persistence/provider.rs b/beacon-db/beacon-core/src/schema_persistence/provider.rs index bb73b3e0..993cee50 100644 --- a/beacon-db/beacon-core/src/schema_persistence/provider.rs +++ b/beacon-db/beacon-core/src/schema_persistence/provider.rs @@ -12,10 +12,8 @@ use std::{ sync::{Arc, Weak}, }; -use arrow::datatypes::Schema; use datafusion::{ catalog::{MemorySchemaProvider, SchemaProvider, TableProvider}, - datasource::empty::EmptyTable, error::DataFusionError, execution::object_store::ObjectStoreUrl, prelude::SessionContext, @@ -23,6 +21,7 @@ use datafusion::{ use beacon_datafusion_ext::table_ext::INTERNAL_TABLE_PREFIX; +use super::default_table::DefaultTablePlaceholder; use super::service::SchemaPersistenceService; /// Schema provider for `beacon.public` that persists table definitions on @@ -127,16 +126,18 @@ impl PersistentSchemaProvider { self.inner.register_table(name, table) } - /// Register the in-memory `default` table backed by an empty provider. + /// Register the in-memory stand-in for `name`, the configured default table. /// - /// The default table is not persisted; it is recreated on every startup so - /// queries against the configured default table always resolve. - pub fn ensure_default_table(&self) { - if self.inner.table_exist("default") { + /// A no-op when `name` already holds a table, so a table the user created + /// under that name keeps the name. The stand-in itself is not persisted; it + /// is recreated on every startup, so a query against the default table + /// always plans. + pub fn ensure_default_table(&self, name: &str) { + if self.inner.table_exist(name) { return; } - let provider: Arc = Arc::new(EmptyTable::new(Arc::new(Schema::empty()))); - let _ = self.inner.register_table("default".to_string(), provider); + let provider: Arc = Arc::new(DefaultTablePlaceholder::new()); + let _ = self.inner.register_table(name.to_string(), provider); } } @@ -312,8 +313,8 @@ mod tests { let (provider, _ctx, _store) = fixture(); assert!(!provider.table_exist("default")); - provider.ensure_default_table(); - provider.ensure_default_table(); + provider.ensure_default_table("default"); + provider.ensure_default_table("default"); assert!(provider.table_exist("default")); assert_eq!( @@ -326,4 +327,38 @@ mod tests { "the default table should be registered exactly once" ); } + + #[tokio::test(flavor = "multi_thread")] + async fn ensure_default_table_uses_the_configured_name() { + let (provider, _ctx, _store) = fixture(); + + provider.ensure_default_table("observations"); + + assert!(provider.table_exist("observations")); + assert!( + !provider.table_exist("default"), + "the stand-in takes the configured name, not the literal 'default'" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn ensure_default_table_keeps_an_existing_table() { + let (provider, ctx, _store) = fixture(); + provider + .register_table("default".to_string(), view(&ctx, "SELECT 1 AS x").await) + .expect("registration should succeed"); + + provider.ensure_default_table("default"); + + let table = provider + .table("default") + .await + .expect("lookup should succeed") + .expect("table should be present"); + assert_eq!( + table.schema().field(0).name(), + "x", + "the user's table keeps the name; the stand-in must not replace it" + ); + } } diff --git a/beacon-db/beacon-core/src/statement_plan/actions.rs b/beacon-db/beacon-core/src/statement_plan/actions.rs index dbba42db..7c131f95 100644 --- a/beacon-db/beacon-core/src/statement_plan/actions.rs +++ b/beacon-db/beacon-core/src/statement_plan/actions.rs @@ -515,7 +515,12 @@ pub(crate) async fn create_table( ) -> anyhow::Result> { let table_name = name.table().to_string(); - if session.table_exist(name.clone())? { + // The default-table stand-in holds its name only until a real table takes it, + // so it never blocks a create. + let occupied = session.table_exist(name.clone())? + && !crate::schema_persistence::default_table::holds_placeholder(session, name.clone()) + .await; + if occupied { if if_not_exists { return Ok(None); } diff --git a/beacon-db/beacon-core/src/statement_plan/materialized_view.rs b/beacon-db/beacon-core/src/statement_plan/materialized_view.rs index 332c6dea..61d37bdf 100644 --- a/beacon-db/beacon-core/src/statement_plan/materialized_view.rs +++ b/beacon-db/beacon-core/src/statement_plan/materialized_view.rs @@ -43,7 +43,15 @@ pub(crate) async fn create_materialized_view( ) -> anyhow::Result<()> { let table_ref = crate::table_name::table_reference(name); - if session_ctx.table_exist(table_ref.clone())? { + // The default-table stand-in holds its name only until a real table takes it, + // so it never blocks a create. + let occupied = session_ctx.table_exist(table_ref.clone())? + && !crate::schema_persistence::default_table::holds_placeholder( + session_ctx, + table_ref.clone(), + ) + .await; + if occupied { return Err(anyhow::anyhow!("Materialized view '{name}' already exists")); } diff --git a/beacon-db/beacon-core/tests/default_table.rs b/beacon-db/beacon-core/tests/default_table.rs new file mode 100644 index 00000000..3ba3b0d3 --- /dev/null +++ b/beacon-db/beacon-core/tests/default_table.rs @@ -0,0 +1,177 @@ +//! The default table (`sql.default_table`) is a name, not a fixed table. +//! +//! Beacon registers an empty stand-in under that name so a `from`-less JSON query +//! plans on a fresh database. These tests prove the stand-in yields: any `CREATE` +//! statement takes the name, the resulting table persists, and a real table under +//! that name is never replaced. + +mod common; + +use beacon_core::query::Query; +use beacon_core::settings::SqlSettings; +use beacon_core::AuthIdentity; +use futures::TryStreamExt; + +/// A JSON (non-SQL) query with no `from`, which the compiler resolves against the +/// runtime's configured `sql.default_table`. +fn json_query_without_from() -> Query { + serde_json::from_str(r#"{"select": [{"column": "id"}]}"#).expect("a valid JSON query body") +} + +/// Rows a `from`-less JSON query returns, so a test can prove which table the +/// default-table name resolves to. +async fn rows_without_from(rt: &common::TestRuntime) -> usize { + let batches = rt + .runtime + .run_query(json_query_without_from(), AuthIdentity::system()) + .await + .expect("a from-less query should plan against the default table") + .into_record_stream() + .expect("the result should be a record stream") + .try_collect::>() + .await + .expect("the stream should run"); + common::total_rows(&batches) +} + +/// `CREATE TABLE` takes the default-table name without a `DROP` first: the +/// stand-in is a placeholder, not a table the user has to clear out of the way. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn create_table_takes_the_default_table_name() { + let rt = common::runtime("default-create-table").await; + + rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; + rt.sql(r#"INSERT INTO "default" VALUES (1), (2)"#).await; + + let rows = rt.sql(r#"SELECT count(*) FROM "default""#).await; + assert_eq!(common::scalar_i64(&rows), 2); + assert_eq!( + rows_without_from(&rt).await, + 2, + "a from-less JSON query should read the table the user created" + ); +} + +/// A real table under the default-table name still blocks a second `CREATE TABLE`. +/// Only the stand-in yields. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_real_default_table_blocks_a_second_create() { + let rt = common::runtime("default-create-twice").await; + rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; + + let error = rt + .try_sql(r#"CREATE TABLE "default" (id BIGINT)"#) + .await + .expect_err("a real table must not be overwritten"); + + assert!( + error.to_string().contains("already exists"), + "unexpected error: {error}" + ); +} + +/// `CREATE VIEW` takes the default-table name too. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_view_takes_the_default_table_name() { + let rt = common::runtime("default-view").await; + + rt.sql(r#"CREATE VIEW "default" AS SELECT 1 AS id"#).await; + + assert_eq!(rows_without_from(&rt).await, 1); +} + +/// `DROP` then `CREATE EXTERNAL TABLE` leaves a table called `default`, and the +/// startup stand-in does not take the name back on the next start. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn an_external_default_table_survives_a_restart() { + let rt = common::restartable_runtime("default-external", |b| b).await; + common::write_file(&rt.datasets_dir().join("obs/a.csv"), "id,v\n1,2\n3,4\n"); + + rt.sql(r#"DROP TABLE "default""#).await; + rt.sql(r#"CREATE EXTERNAL TABLE "default" STORED AS CSV LOCATION 'obs/'"#) + .await; + assert_eq!( + common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), + 2 + ); + + let rt = rt.restart().await; + + assert_eq!( + common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), + 2, + "the stand-in must not replace the user's table after a restart" + ); + assert_eq!(rows_without_from(&rt).await, 2); +} + +/// The whole cycle an operator runs: start, `DROP` the stand-in, `CREATE TABLE` +/// under the same name, restart. The managed table and its rows come back, and +/// startup registers no stand-in over them. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_managed_default_table_survives_a_restart() { + let rt = common::restartable_runtime("default-managed", |b| b).await; + + rt.sql(r#"DROP TABLE "default""#).await; + rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; + rt.sql(r#"INSERT INTO "default" VALUES (1), (2), (3)"#).await; + + let rt = rt.restart().await; + + assert_eq!( + common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), + 3, + "the managed table and its rows should survive the restart" + ); + assert_eq!(rows_without_from(&rt).await, 3); + // The stand-in is column-less; the surviving table reports its own column. + assert_eq!( + common::column_strings(&rt.sql(r#"SHOW COLUMNS FROM "default""#).await, 3), + vec!["id"], + "the resolved table should be the user's, not a fresh stand-in" + ); +} + +/// `CREATE MATERIALIZED VIEW` takes the default-table name too. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_materialized_view_takes_the_default_table_name() { + let rt = common::runtime("default-materialized-view").await; + + rt.sql(r#"CREATE MATERIALIZED VIEW "default" AS SELECT 1 AS id"#) + .await; + + assert_eq!(rows_without_from(&rt).await, 1); +} + +/// The stand-in uses the configured name. A deployment that sets +/// `sql.default_table` to `observations` gets `observations`, not `default`. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn the_stand_in_uses_the_configured_name() { + let rt = common::runtime_with("default-configured", |builder| { + builder.with_sql_settings(SqlSettings { + default_table: "observations".to_string(), + ..Default::default() + }) + }) + .await; + + let names = common::column_strings(&rt.sql("SHOW TABLES").await, 2); + assert!( + names.iter().any(|name| name == "observations"), + "the stand-in should hold the configured name: {names:?}" + ); + assert!( + !names.iter().any(|name| name == "default"), + "no table should hold the literal name 'default': {names:?}" + ); + assert_eq!( + common::total_rows(&rt.sql("SELECT * FROM observations").await), + 0, + "the stand-in should be queryable under the configured name" + ); + + // The stand-in yields to a real table under the configured name too. + rt.sql("CREATE TABLE observations (id BIGINT)").await; + rt.sql("INSERT INTO observations VALUES (7)").await; + assert_eq!(rows_without_from(&rt).await, 1); +} diff --git a/beacon-db/beacon-core/tests/runtime_config.rs b/beacon-db/beacon-core/tests/runtime_config.rs index dbbe0fea..6fe8f737 100644 --- a/beacon-db/beacon-core/tests/runtime_config.rs +++ b/beacon-db/beacon-core/tests/runtime_config.rs @@ -8,6 +8,7 @@ use beacon_core::settings::SqlSettings; use beacon_datafusion_ext::listing_factory::RootStore; use common::TestRuntime; use datafusion::execution::object_store::ObjectStoreUrl; +use futures::TryStreamExt; /// Builds a runtime whose `sql.default_table` is `default_table`, on its own temp /// root with its own (in-memory) tables store. Config is passed explicitly to the @@ -30,20 +31,36 @@ fn json_query_without_from() -> Query { serde_json::from_str(r#"{"select": [{"column": "id"}]}"#).expect("a valid JSON query body") } -/// Reads back the default table a runtime resolves a `from`-less JSON query against. +/// Reads back how many rows a runtime's `from`-less JSON query returns. /// -/// `Runtime` exposes no config getter — the setting is observed through behavior: -/// the table is never created, so planning fails with an error naming the exact -/// table the runtime resolved to. -async fn resolved_default_table_error(rt: &TestRuntime) -> String { - match rt +/// `Runtime` exposes no config getter, so the setting is observed through +/// behavior: each runtime holds a distinct number of rows under its own +/// configured default-table name, and the row count identifies the table the +/// runtime resolved to. +async fn rows_from_default_table(rt: &TestRuntime) -> usize { + let batches = rt .runtime .run_query(json_query_without_from(), beacon_core::AuthIdentity::system()) .await - { - Ok(_) => panic!("expected the (never-created) default table to be missing"), - Err(error) => error.to_string(), - } + .expect("a from-less query should resolve the configured default table") + .into_record_stream() + .expect("the result should be a record stream") + .try_collect::>() + .await + .expect("the stream should run"); + common::total_rows(&batches) +} + +/// Fills a runtime's configured default table with `row_count` rows of `id`. +/// The startup stand-in yields the name, so no `DROP` is needed first. +async fn fill_default_table(rt: &TestRuntime, table: &str, row_count: usize) { + rt.sql(&format!("CREATE TABLE {table} (id BIGINT)")).await; + let values = (1..=row_count) + .map(|row| format!("({row})")) + .collect::>() + .join(", "); + rt.sql(&format!("INSERT INTO {table} VALUES {values}")) + .await; } /// Two runtimes built from different configs in the same process each reflect @@ -54,23 +71,26 @@ async fn two_runtimes_honor_their_own_config() { let rt_alpha = runtime_with_default_table("alpha_table", "alpha").await; let rt_bravo = runtime_with_default_table("bravo_table", "bravo").await; - let alpha = resolved_default_table_error(&rt_alpha).await; - assert!( - alpha.contains("alpha_table") && !alpha.contains("bravo_table"), - "alpha runtime should resolve its own default table: {alpha}" - ); + // Distinct row counts, so a `from`-less query names the table it read. + fill_default_table(&rt_alpha, "alpha_table", 1).await; + fill_default_table(&rt_bravo, "bravo_table", 2).await; - let bravo = resolved_default_table_error(&rt_bravo).await; - assert!( - bravo.contains("bravo_table") && !bravo.contains("alpha_table"), - "bravo runtime should resolve its own default table: {bravo}" + assert_eq!( + rows_from_default_table(&rt_alpha).await, + 1, + "alpha runtime should resolve its own default table" + ); + assert_eq!( + rows_from_default_table(&rt_bravo).await, + 2, + "bravo runtime should resolve its own default table" ); // The first runtime is unaffected by the second's construction. - let alpha_again = resolved_default_table_error(&rt_alpha).await; - assert!( - alpha_again.contains("alpha_table") && !alpha_again.contains("bravo_table"), - "alpha runtime should still resolve its own default table: {alpha_again}" + assert_eq!( + rows_from_default_table(&rt_alpha).await, + 1, + "alpha runtime should still resolve its own default table" ); } diff --git a/docs/docs/2.0.0-rc5/server/configuration.md b/docs/docs/2.0.0-rc5/server/configuration.md index eca7c555..087bc933 100644 --- a/docs/docs/2.0.0-rc5/server/configuration.md +++ b/docs/docs/2.0.0-rc5/server/configuration.md @@ -82,6 +82,16 @@ rejects that `CREATE`. Beacon never writes plaintext. | `BEACON_STATS_CACHE_CAPACITY` | `10000` | Maximum number of per-file statistics entries cached for query pruning. Read once at startup. | | `BEACON_TYPE_WIDENING_ON_CONFLICT` | `fail` | What a schema merge does with a column that two files type in two families, such as a number and a string. `fail` refuses the collection and names the column, both types and both files. `keep_first` keeps the type of the first file, casts every other file to it, and reads a value that type cannot hold as null. A numeric pair widens either way. An unknown value logs a warning and reads as `fail`. See [a column has two types](/docs/2.0.0-rc5/troubleshooting#a-column-has-two-types-across-the-files). | +### The default table + +At startup Beacon puts an empty stand-in table under the `BEACON_DEFAULT_TABLE` name. +The stand-in keeps a JSON query without a `from` field from a missing-table error. + +The stand-in gives up the name to the first real table. Use `CREATE TABLE`, +`CREATE EXTERNAL TABLE`, `CREATE VIEW` or `CREATE MATERIALIZED VIEW` on that +name. No `DROP TABLE` is necessary first. Your table then holds the name, and it +survives a restart. + ### SQL result-stream coalescing A query can produce small record batches. Beacon merges them into larger batches From 6d479248cbd7b926988ce2e864cb2381541e45c5 Mon Sep 17 00:00:00 2001 From: Robin Kooyman Date: Mon, 7 Sep 2026 17:15:53 +0200 Subject: [PATCH 2/4] test: cover the default table name over HTTP The admin external-table endpoint builds the CREATE statement itself, so the stand-in has to yield there too. --- .../tests/admin_endpoints_http.rs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/beacon-server/beacon-server/tests/admin_endpoints_http.rs b/beacon-server/beacon-server/tests/admin_endpoints_http.rs index 07dd5c11..9b9fd385 100644 --- a/beacon-server/beacon-server/tests/admin_endpoints_http.rs +++ b/beacon-server/beacon-server/tests/admin_endpoints_http.rs @@ -225,6 +225,56 @@ async fn create_external_table_from_fields() { assert_eq!(count_rows(&router, &admin, "ext_obs").await, 3); } +/// The configured default table is a name an external table can claim. The +/// startup stand-in yields it, so no `DROP TABLE` has to come first. +#[tokio::test(flavor = "multi_thread")] +async fn create_external_table_takes_the_default_table_name() { + let (router, _lake, cfg) = app(config(false)).await; + let admin = admin(&cfg); + let default_table = cfg.sql.default_table.clone(); + place_dataset(&cfg, "obs/a.csv", "v\n1\n2\n3\n"); + + // The stand-in holds the name, and it is column-less. + assert_eq!(count_rows(&router, &admin, r#""default""#).await, 0); + + let created = send( + &router, + json_req( + "POST", + "/api/admin/external-tables", + json!({ "name": default_table, "location": "obs/", "file_type": "CSV" }), + Some(&admin), + ), + ) + .await; + + assert_eq!( + created.status, + StatusCode::OK, + "the stand-in should not block the create, got: {}", + String::from_utf8_lossy(&created.body) + ); + assert_eq!(count_rows(&router, &admin, r#""default""#).await, 3); + + // The endpoint that reports the default table now answers with the real one. + let schema = send( + &router, + req( + "GET", + "/api/default-table-schema", + Some(&admin), + Body::empty(), + ), + ) + .await; + assert_eq!(schema.status, StatusCode::OK); + let body = String::from_utf8_lossy(&schema.body).to_string(); + assert!( + body.contains("\"v\""), + "the default-table schema should report the external table's column, got: {body}" + ); +} + #[tokio::test(flavor = "multi_thread")] async fn create_external_table_rejects_an_unknown_format() { let (router, _lake, cfg) = app(config(false)).await; From 6481d1bcec0c51ce80b40b25bdf2b966c5bd67d5 Mon Sep 17 00:00:00 2001 From: Robin Kooyman Date: Mon, 7 Sep 2026 17:32:25 +0200 Subject: [PATCH 3/4] fix: keep the create-on-existing-name error, fix the name The stand-in is an ordinary table again: a CREATE on its name fails, and you drop it first. Only the startup rule changes, so the stand-in takes the configured sql.default_table instead of the literal "default". The error names the stand-in, because a table nobody made is a confusing thing to collide with. --- CHANGELOG.md | 22 ++- .../src/schema_persistence/default_table.rs | 28 +++- .../beacon-core/src/statement_plan/actions.rs | 14 +- .../src/statement_plan/materialized_view.rs | 11 +- beacon-db/beacon-core/tests/default_table.rs | 152 ++++++++++-------- beacon-db/beacon-core/tests/runtime_config.rs | 3 +- .../tests/admin_endpoints_http.rs | 28 +++- docs/docs/2.0.0-rc5/server/configuration.md | 22 ++- 8 files changed, 169 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29bc93c3..6ec50fe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -317,18 +317,16 @@ tag. Releases before 2.0.0 are recorded in the ### Fixed -- **The default table is a name you can claim.** At startup Beacon registers an empty stand-in - table, so a JSON query without a `from` field reports no missing table. That stand-in blocked - the name. `CREATE TABLE "default" (id BIGINT)` answered `Table 'default' already exists`, and a - `CREATE MATERIALIZED VIEW` on the same name answered the same way, while `CREATE EXTERNAL TABLE` - replaced the stand-in without a word. The stand-in now gives up the name to the first real - table, whichever `CREATE` statement makes it. No `DROP TABLE` is necessary first. Only the - stand-in yields: a real table under that name still refuses a second `CREATE TABLE` or - `CREATE MATERIALIZED VIEW`. It also keeps the name across a restart, because startup registers - the stand-in only for a name no loaded table holds. The stand-in also takes the configured name. `BEACON_DEFAULT_TABLE=observations` created a table called `default` and - left `observations` missing, so a `from`-less query failed on a fresh server. It now creates - `observations`. See - [Configuration](docs/docs/2.0.0-rc5/server/configuration.md#the-default-table). +- **The default table takes the name you configured.** At startup Beacon registers an empty + stand-in table, so a JSON query without a `from` field reports no missing table. The stand-in + ignored `BEACON_DEFAULT_TABLE` and always took the literal name `default`. A server started with + `BEACON_DEFAULT_TABLE=observations` therefore held a table called `default`, left `observations` + missing, and answered a `from`-less query with a missing-table error. Startup now registers the + stand-in under the configured name. The rest of the rule is unchanged and now under test: Beacon + fills that name only when the name is free, so your own table under it survives a restart, and a + `CREATE` on a name a table holds still fails. That last error now names the stand-in and tells + you to run `DROP TABLE` first, because a table nobody made is a confusing thing to collide with. + See [Configuration](docs/docs/2.0.0-rc5/server/configuration.md#the-default-table). - **A long query no longer makes the API unreachable.** The HTTP API, Flight SQL and every query shared one Tokio runtime of `BEACON_WORKER_THREADS` threads. A scan holds a thread until a partition yields, and one query starts as many partitions as the machine has cores, so a long diff --git a/beacon-db/beacon-core/src/schema_persistence/default_table.rs b/beacon-db/beacon-core/src/schema_persistence/default_table.rs index fcef8cd2..78d9824c 100644 --- a/beacon-db/beacon-core/src/schema_persistence/default_table.rs +++ b/beacon-db/beacon-core/src/schema_persistence/default_table.rs @@ -5,10 +5,12 @@ //! plans on a fresh database, and drops the stand-in as soon as a real table //! takes the name. //! +//! A `CREATE` statement on that name fails, as it does on any other table that +//! exists. Run `DROP TABLE` first to take the name. +//! //! The stand-in is a distinct type, not a bare //! [`EmptyTable`](datafusion::datasource::empty::EmptyTable), so the `CREATE` -//! paths recognize it. A `CREATE` statement replaces the stand-in instead of -//! reporting that the table exists. +//! paths recognize it and say that in the error. use std::{any::Any, sync::Arc}; @@ -70,12 +72,28 @@ impl TableProvider for DefaultTablePlaceholder { } /// True when `name` holds nothing but the default-table stand-in. -/// -/// The `CREATE` paths call this to tell an occupied name from a name that only -/// carries the stand-in: the second one is free to take. pub async fn holds_placeholder(session_ctx: &SessionContext, name: TableReference) -> bool { match session_ctx.table_provider(name).await { Ok(provider) => provider.as_any().is::(), Err(_) => false, } } + +/// The reason a `CREATE` statement refuses `name`. +/// +/// A name the stand-in holds looks free to the user, because no one made that +/// table. The message therefore names the stand-in and says how to free the name. +pub async fn already_exists_error( + session_ctx: &SessionContext, + name: TableReference, + subject: &str, +) -> anyhow::Error { + let table = name.table().to_string(); + if holds_placeholder(session_ctx, name).await { + return anyhow::anyhow!( + "{subject} '{table}' already exists. Beacon creates it at startup as the default \ + table. Run `DROP TABLE \"{table}\"` first to take the name." + ); + } + anyhow::anyhow!("{subject} '{table}' already exists") +} diff --git a/beacon-db/beacon-core/src/statement_plan/actions.rs b/beacon-db/beacon-core/src/statement_plan/actions.rs index 7c131f95..73d49d43 100644 --- a/beacon-db/beacon-core/src/statement_plan/actions.rs +++ b/beacon-db/beacon-core/src/statement_plan/actions.rs @@ -515,16 +515,16 @@ pub(crate) async fn create_table( ) -> anyhow::Result> { let table_name = name.table().to_string(); - // The default-table stand-in holds its name only until a real table takes it, - // so it never blocks a create. - let occupied = session.table_exist(name.clone())? - && !crate::schema_persistence::default_table::holds_placeholder(session, name.clone()) - .await; - if occupied { + if session.table_exist(name.clone())? { if if_not_exists { return Ok(None); } - return Err(anyhow::anyhow!("Table '{table_name}' already exists")); + return Err(crate::schema_persistence::default_table::already_exists_error( + session, + name.clone(), + "Table", + ) + .await); } let arrow_schema = child.schema(); diff --git a/beacon-db/beacon-core/src/statement_plan/materialized_view.rs b/beacon-db/beacon-core/src/statement_plan/materialized_view.rs index 61d37bdf..8cd132b1 100644 --- a/beacon-db/beacon-core/src/statement_plan/materialized_view.rs +++ b/beacon-db/beacon-core/src/statement_plan/materialized_view.rs @@ -43,16 +43,13 @@ pub(crate) async fn create_materialized_view( ) -> anyhow::Result<()> { let table_ref = crate::table_name::table_reference(name); - // The default-table stand-in holds its name only until a real table takes it, - // so it never blocks a create. - let occupied = session_ctx.table_exist(table_ref.clone())? - && !crate::schema_persistence::default_table::holds_placeholder( + if session_ctx.table_exist(table_ref.clone())? { + return Err(crate::schema_persistence::default_table::already_exists_error( session_ctx, table_ref.clone(), + "Materialized view", ) - .await; - if occupied { - return Err(anyhow::anyhow!("Materialized view '{name}' already exists")); + .await); } // Execute the defining query and persist its result as a single Parquet file diff --git a/beacon-db/beacon-core/tests/default_table.rs b/beacon-db/beacon-core/tests/default_table.rs index 3ba3b0d3..a56228b3 100644 --- a/beacon-db/beacon-core/tests/default_table.rs +++ b/beacon-db/beacon-core/tests/default_table.rs @@ -1,9 +1,9 @@ -//! The default table (`sql.default_table`) is a name, not a fixed table. +//! The default table (`sql.default_table`) is a name Beacon fills only when it is free. //! -//! Beacon registers an empty stand-in under that name so a `from`-less JSON query -//! plans on a fresh database. These tests prove the stand-in yields: any `CREATE` -//! statement takes the name, the resulting table persists, and a real table under -//! that name is never replaced. +//! At startup Beacon registers an empty stand-in under that name, so a `from`-less +//! JSON query plans on a fresh database. The stand-in is an ordinary table: a +//! `CREATE` on that name fails, and you run `DROP TABLE` first to take it. Once a +//! real table holds the name, Beacon leaves it alone, restart included. mod common; @@ -34,54 +34,56 @@ async fn rows_without_from(rt: &common::TestRuntime) -> usize { common::total_rows(&batches) } -/// `CREATE TABLE` takes the default-table name without a `DROP` first: the -/// stand-in is a placeholder, not a table the user has to clear out of the way. +/// The stand-in is an ordinary table: `CREATE TABLE` on its name fails. The error +/// names the stand-in, because no user made that table. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn create_table_takes_the_default_table_name() { - let rt = common::runtime("default-create-table").await; - - rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; - rt.sql(r#"INSERT INTO "default" VALUES (1), (2)"#).await; - - let rows = rt.sql(r#"SELECT count(*) FROM "default""#).await; - assert_eq!(common::scalar_i64(&rows), 2); - assert_eq!( - rows_without_from(&rt).await, - 2, - "a from-less JSON query should read the table the user created" - ); -} - -/// A real table under the default-table name still blocks a second `CREATE TABLE`. -/// Only the stand-in yields. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_real_default_table_blocks_a_second_create() { - let rt = common::runtime("default-create-twice").await; - rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; +async fn create_table_fails_while_the_stand_in_holds_the_name() { + let rt = common::runtime("default-create-blocked").await; let error = rt .try_sql(r#"CREATE TABLE "default" (id BIGINT)"#) .await - .expect_err("a real table must not be overwritten"); + .expect_err("the name is taken, so the create should fail"); + let message = error.to_string(); assert!( - error.to_string().contains("already exists"), - "unexpected error: {error}" + message.contains("already exists"), + "unexpected error: {message}" + ); + assert!( + message.contains("DROP TABLE"), + "the error should say how to free the name: {message}" ); } -/// `CREATE VIEW` takes the default-table name too. +/// The cycle an operator runs: `DROP` the stand-in, `CREATE TABLE` under the same +/// name, restart. The table and its rows come back, and Beacon adds no stand-in +/// over them. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_view_takes_the_default_table_name() { - let rt = common::runtime("default-view").await; +async fn a_managed_default_table_survives_a_restart() { + let rt = common::restartable_runtime("default-managed", |b| b).await; - rt.sql(r#"CREATE VIEW "default" AS SELECT 1 AS id"#).await; + rt.sql(r#"DROP TABLE "default""#).await; + rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; + rt.sql(r#"INSERT INTO "default" VALUES (1), (2), (3)"#).await; - assert_eq!(rows_without_from(&rt).await, 1); + let rt = rt.restart().await; + + assert_eq!( + common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), + 3, + "the managed table and its rows should survive the restart" + ); + assert_eq!(rows_without_from(&rt).await, 3); + // The stand-in is column-less; the surviving table reports its own column. + assert_eq!( + common::column_strings(&rt.sql(r#"SHOW COLUMNS FROM "default""#).await, 3), + vec!["id"], + "the resolved table should be the user's, not a fresh stand-in" + ); } -/// `DROP` then `CREATE EXTERNAL TABLE` leaves a table called `default`, and the -/// startup stand-in does not take the name back on the next start. +/// The same cycle with `CREATE EXTERNAL TABLE`. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn an_external_default_table_survives_a_restart() { let rt = common::restartable_runtime("default-external", |b| b).await; @@ -100,47 +102,62 @@ async fn an_external_default_table_survives_a_restart() { assert_eq!( common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), 2, - "the stand-in must not replace the user's table after a restart" + "Beacon must not put a stand-in over the user's table after a restart" ); assert_eq!(rows_without_from(&rt).await, 2); } -/// The whole cycle an operator runs: start, `DROP` the stand-in, `CREATE TABLE` -/// under the same name, restart. The managed table and its rows come back, and -/// startup registers no stand-in over them. +/// `CREATE MATERIALIZED VIEW` refuses the name for the same reason, and its error +/// names the stand-in too. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_managed_default_table_survives_a_restart() { - let rt = common::restartable_runtime("default-managed", |b| b).await; - - rt.sql(r#"DROP TABLE "default""#).await; - rt.sql(r#"CREATE TABLE "default" (id BIGINT)"#).await; - rt.sql(r#"INSERT INTO "default" VALUES (1), (2), (3)"#).await; +async fn a_materialized_view_fails_while_the_stand_in_holds_the_name() { + let rt = common::runtime("default-materialized-view").await; - let rt = rt.restart().await; + let error = rt + .try_sql(r#"CREATE MATERIALIZED VIEW "default" AS SELECT 1 AS id"#) + .await + .expect_err("the name is taken, so the create should fail"); - assert_eq!( - common::scalar_i64(&rt.sql(r#"SELECT count(*) FROM "default""#).await), - 3, - "the managed table and its rows should survive the restart" - ); - assert_eq!(rows_without_from(&rt).await, 3); - // The stand-in is column-less; the surviving table reports its own column. - assert_eq!( - common::column_strings(&rt.sql(r#"SHOW COLUMNS FROM "default""#).await, 3), - vec!["id"], - "the resolved table should be the user's, not a fresh stand-in" + let message = error.to_string(); + assert!( + message.contains("already exists") && message.contains("DROP TABLE"), + "unexpected error: {message}" ); + + // After the drop the name is free. + rt.sql(r#"DROP TABLE "default""#).await; + rt.sql(r#"CREATE MATERIALIZED VIEW "default" AS SELECT 1 AS id"#) + .await; + assert_eq!(rows_without_from(&rt).await, 1); } -/// `CREATE MATERIALIZED VIEW` takes the default-table name too. +/// A dropped default table comes back as an empty stand-in on the next start, +/// because the name is free again. This is what keeps a `from`-less query planning. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_materialized_view_takes_the_default_table_name() { - let rt = common::runtime("default-materialized-view").await; +async fn a_free_name_gets_a_stand_in_on_the_next_start() { + let rt = common::restartable_runtime("default-dropped", |b| b).await; - rt.sql(r#"CREATE MATERIALIZED VIEW "default" AS SELECT 1 AS id"#) - .await; + rt.sql(r#"DROP TABLE "default""#).await; + assert!( + !common::column_strings(&rt.sql("SHOW TABLES").await, 2) + .iter() + .any(|name| name == "default"), + "the drop should hold for this run" + ); - assert_eq!(rows_without_from(&rt).await, 1); + let rt = rt.restart().await; + + assert!( + common::column_strings(&rt.sql("SHOW TABLES").await, 2) + .iter() + .any(|name| name == "default"), + "a free default-table name gets a stand-in again" + ); + assert_eq!( + common::total_rows(&rt.sql(r#"SELECT * FROM "default""#).await), + 0, + "the fresh stand-in is empty" + ); } /// The stand-in uses the configured name. A deployment that sets @@ -170,7 +187,8 @@ async fn the_stand_in_uses_the_configured_name() { "the stand-in should be queryable under the configured name" ); - // The stand-in yields to a real table under the configured name too. + // The configured name behaves like any other: drop, then create. + rt.sql("DROP TABLE observations").await; rt.sql("CREATE TABLE observations (id BIGINT)").await; rt.sql("INSERT INTO observations VALUES (7)").await; assert_eq!(rows_without_from(&rt).await, 1); diff --git a/beacon-db/beacon-core/tests/runtime_config.rs b/beacon-db/beacon-core/tests/runtime_config.rs index 6fe8f737..83d63200 100644 --- a/beacon-db/beacon-core/tests/runtime_config.rs +++ b/beacon-db/beacon-core/tests/runtime_config.rs @@ -52,8 +52,9 @@ async fn rows_from_default_table(rt: &TestRuntime) -> usize { } /// Fills a runtime's configured default table with `row_count` rows of `id`. -/// The startup stand-in yields the name, so no `DROP` is needed first. +/// The startup stand-in holds the name, so drop it before the create. async fn fill_default_table(rt: &TestRuntime, table: &str, row_count: usize) { + rt.sql(&format!("DROP TABLE {table}")).await; rt.sql(&format!("CREATE TABLE {table} (id BIGINT)")).await; let values = (1..=row_count) .map(|row| format!("({row})")) diff --git a/beacon-server/beacon-server/tests/admin_endpoints_http.rs b/beacon-server/beacon-server/tests/admin_endpoints_http.rs index 9b9fd385..85c4d1c8 100644 --- a/beacon-server/beacon-server/tests/admin_endpoints_http.rs +++ b/beacon-server/beacon-server/tests/admin_endpoints_http.rs @@ -225,17 +225,34 @@ async fn create_external_table_from_fields() { assert_eq!(count_rows(&router, &admin, "ext_obs").await, 3); } -/// The configured default table is a name an external table can claim. The -/// startup stand-in yields it, so no `DROP TABLE` has to come first. +/// The operator cycle over HTTP: drop the startup stand-in, then create an +/// external table under the default-table name. The endpoint that reports the +/// default table then answers with the real one. #[tokio::test(flavor = "multi_thread")] -async fn create_external_table_takes_the_default_table_name() { +async fn an_external_table_takes_the_default_table_name_after_a_drop() { let (router, _lake, cfg) = app(config(false)).await; let admin = admin(&cfg); let default_table = cfg.sql.default_table.clone(); place_dataset(&cfg, "obs/a.csv", "v\n1\n2\n3\n"); - // The stand-in holds the name, and it is column-less. + // The stand-in holds the name, and it is empty. assert_eq!(count_rows(&router, &admin, r#""default""#).await, 0); + let dropped = send( + &router, + json_req( + "POST", + "/api/query", + json!({ "sql": r#"DROP TABLE "default""# }), + Some(&admin), + ), + ) + .await; + assert_eq!( + dropped.status, + StatusCode::OK, + "the drop should succeed, got: {}", + String::from_utf8_lossy(&dropped.body) + ); let created = send( &router, @@ -251,12 +268,11 @@ async fn create_external_table_takes_the_default_table_name() { assert_eq!( created.status, StatusCode::OK, - "the stand-in should not block the create, got: {}", + "the freed name should accept the external table, got: {}", String::from_utf8_lossy(&created.body) ); assert_eq!(count_rows(&router, &admin, r#""default""#).await, 3); - // The endpoint that reports the default table now answers with the real one. let schema = send( &router, req( diff --git a/docs/docs/2.0.0-rc5/server/configuration.md b/docs/docs/2.0.0-rc5/server/configuration.md index 26c554e5..848eccc8 100644 --- a/docs/docs/2.0.0-rc5/server/configuration.md +++ b/docs/docs/2.0.0-rc5/server/configuration.md @@ -85,13 +85,23 @@ rejects that `CREATE`. Beacon never writes plaintext. ### The default table -At startup Beacon puts an empty stand-in table under the `BEACON_DEFAULT_TABLE` name. -The stand-in keeps a JSON query without a `from` field from a missing-table error. +Beacon fills the `BEACON_DEFAULT_TABLE` name only when the name is free. At startup +it puts an empty stand-in table there. The stand-in keeps a JSON query without a +`from` field from a missing-table error. -The stand-in gives up the name to the first real table. Use `CREATE TABLE`, -`CREATE EXTERNAL TABLE`, `CREATE VIEW` or `CREATE MATERIALIZED VIEW` on that -name. No `DROP TABLE` is necessary first. Your table then holds the name, and it -survives a restart. +To put your own table under that name, drop the stand-in first: + +```sql +DROP TABLE "default"; +CREATE EXTERNAL TABLE "default" STORED AS PARQUET LOCATION 'obs/'; +``` + +Beacon then leaves the name alone. Your table holds it after a restart, because +startup adds a stand-in only for a free name. Drop your table and Beacon puts a +stand-in back on the next start. + +A `CREATE` on a name that a table holds fails, the stand-in included. The error +names the stand-in and tells you to drop it. ### SQL result-stream coalescing From 7bf8f963a789f73a223d53e746c67ad8081b03b7 Mon Sep 17 00:00:00 2001 From: Robin Kooyman Date: Mon, 7 Sep 2026 17:47:21 +0200 Subject: [PATCH 4/4] fix: refuse a taken name in CREATE EXTERNAL TABLE and CREATE VIEW Both registered over whatever held the name, so a typo repointed a table or swapped a view with no warning, while CREATE TABLE refused the same name. The admin API documented the erroring behaviour it never had. IF NOT EXISTS and OR REPLACE now do the work the SQL reference already describes. REFRESH, ALTER TABLE and the crawler register directly and keep replacing on purpose. --- CHANGELOG.md | 15 +++ .../beacon-core/src/statement_plan/actions.rs | 31 ++++- .../src/statement_plan/physical.rs | 8 +- .../src/statement_plan/query_planner.rs | 1 + beacon-db/beacon-core/tests/default_table.rs | 37 ++++++ .../beacon-core/tests/external_tables.rs | 113 ++++++++++++++++++ docs/docs/2.0.0-rc5/server/configuration.md | 9 +- 7 files changed, 210 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ec50fe3..814ebe25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -327,6 +327,21 @@ tag. Releases before 2.0.0 are recorded in the `CREATE` on a name a table holds still fails. That last error now names the stand-in and tells you to run `DROP TABLE` first, because a table nobody made is a confusing thing to collide with. See [Configuration](docs/docs/2.0.0-rc5/server/configuration.md#the-default-table). +- **`CREATE EXTERNAL TABLE` and `CREATE VIEW` no longer discard the table under the name.** Both + registered straight over whatever held the name. `CREATE EXTERNAL TABLE obs STORED AS CSV + LOCATION 'other/'` therefore repointed an existing `obs` with no warning, and a second + `CREATE VIEW v` swapped the view a report depended on. Only `CREATE TABLE` and + `CREATE MATERIALIZED VIEW` checked first, so the same typo either failed or destroyed a table + depending on which statement carried it. The admin API said as much and did not do it: the + `if_not_exists` field of `POST /api/admin/external-tables` is documented as skipping "instead of + erroring", and nothing ever errored. Both statements now refuse a name that a table or a view + holds, as `CREATE TABLE` does. The two modifiers the SQL reference already documented now do the + work: `CREATE EXTERNAL TABLE IF NOT EXISTS` keeps the existing table and reports success, which + is what that field always promised, and `CREATE OR REPLACE EXTERNAL TABLE` overwrites it. + `CREATE OR REPLACE VIEW` swaps a view. Neither guard reaches the paths that replace a provider on + purpose: a materialized-view `REFRESH`, an `ALTER TABLE`, and a crawler that re-registers a table + it owns all register directly and are unchanged. A script that relied on a bare re-`CREATE` to + repoint a table needs `OR REPLACE` added, or a `DROP TABLE` in front of it. - **A long query no longer makes the API unreachable.** The HTTP API, Flight SQL and every query shared one Tokio runtime of `BEACON_WORKER_THREADS` threads. A scan holds a thread until a partition yields, and one query starts as many partitions as the machine has cores, so a long diff --git a/beacon-db/beacon-core/src/statement_plan/actions.rs b/beacon-db/beacon-core/src/statement_plan/actions.rs index 73d49d43..41e4d53d 100644 --- a/beacon-db/beacon-core/src/statement_plan/actions.rs +++ b/beacon-db/beacon-core/src/statement_plan/actions.rs @@ -218,10 +218,26 @@ pub(crate) async fn show_secrets( /// Register a `CREATE EXTERNAL TABLE` via the listing-table factory and persist /// it to the catalog. +/// +/// Errors if a table with `cmd.name` already exists. `IF NOT EXISTS` keeps the +/// existing table instead, and `OR REPLACE` overwrites it. The guard covers every +/// `STORED AS` variant below, so no variant replaces a table the user did not drop. pub(crate) async fn create_external_table( session: &Arc, cmd: &CreateExternalTable, ) -> anyhow::Result<()> { + if !cmd.or_replace && session.table_exist(cmd.name.clone())? { + if cmd.if_not_exists { + return Ok(()); + } + return Err(crate::schema_persistence::default_table::already_exists_error( + session, + cmd.name.clone(), + "Table", + ) + .await); + } + // `STORED AS REMOTE` registers a federated table pointing at another Beacon // instance, rather than a listing table over the datasets store. if cmd.file_type.eq_ignore_ascii_case("REMOTE") { @@ -491,12 +507,25 @@ async fn create_sql_db_table( } /// Register a `CREATE VIEW` as a `ViewTable` (re-plans its query on each scan). -pub(crate) fn create_view( +/// +/// Errors if a table or view with `name` already exists. `CREATE OR REPLACE VIEW` +/// (`or_replace`) swaps it instead. +pub(crate) async fn create_view( session: &Arc, name: &TableReference, input: &LogicalPlan, definition: &Option, + or_replace: bool, ) -> anyhow::Result<()> { + if !or_replace && session.table_exist(name.clone())? { + return Err(crate::schema_persistence::default_table::already_exists_error( + session, + name.clone(), + "View", + ) + .await); + } + let table = ViewTable::new(input.clone(), definition.clone()); session.register_table(name.clone(), Arc::new(table))?; Ok(()) diff --git a/beacon-db/beacon-core/src/statement_plan/physical.rs b/beacon-db/beacon-core/src/statement_plan/physical.rs index 71b0c8f5..c4aad4b7 100644 --- a/beacon-db/beacon-core/src/statement_plan/physical.rs +++ b/beacon-db/beacon-core/src/statement_plan/physical.rs @@ -650,6 +650,7 @@ pub(crate) struct CreateViewExec { name: TableReference, input: LogicalPlan, definition: Option, + or_replace: bool, session: SessionCell, cache: Arc, } @@ -659,12 +660,14 @@ impl CreateViewExec { name: TableReference, input: LogicalPlan, definition: Option, + or_replace: bool, session: SessionCell, ) -> Self { Self { name, input, definition, + or_replace, session, cache: Arc::new(side_effect_properties()), } @@ -679,8 +682,11 @@ side_effect_exec!(CreateViewExec, "CreateViewExec", |exec: &CreateViewExec| { let name = exec.name.clone(); let input = exec.input.clone(); let definition = exec.definition.clone(); + let or_replace = exec.or_replace; Ok(side_effect_stream(async move { - actions::create_view(&session, &name, &input, &definition).map_err(to_df_err) + actions::create_view(&session, &name, &input, &definition, or_replace) + .await + .map_err(to_df_err) })) }); diff --git a/beacon-db/beacon-core/src/statement_plan/query_planner.rs b/beacon-db/beacon-core/src/statement_plan/query_planner.rs index 2474cede..aef57c7a 100644 --- a/beacon-db/beacon-core/src/statement_plan/query_planner.rs +++ b/beacon-db/beacon-core/src/statement_plan/query_planner.rs @@ -74,6 +74,7 @@ impl QueryPlanner for BeaconQueryPlanner { view.name.clone(), view.input.as_ref().clone(), view.definition.clone(), + view.or_replace, session, ))) } diff --git a/beacon-db/beacon-core/tests/default_table.rs b/beacon-db/beacon-core/tests/default_table.rs index a56228b3..a13786ae 100644 --- a/beacon-db/beacon-core/tests/default_table.rs +++ b/beacon-db/beacon-core/tests/default_table.rs @@ -56,6 +56,43 @@ async fn create_table_fails_while_the_stand_in_holds_the_name() { ); } +/// Every `CREATE` statement refuses the name the stand-in holds, and each one +/// says how to free it. No statement replaces the stand-in quietly. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn every_create_statement_refuses_the_stand_in_name() { + let rt = common::runtime("default-create-blocked-all").await; + common::write_file(&rt.datasets_dir().join("obs/a.csv"), "id\n1\n"); + + for statement in [ + r#"CREATE TABLE "default" (id BIGINT)"#, + r#"CREATE EXTERNAL TABLE "default" STORED AS CSV LOCATION 'obs/'"#, + r#"CREATE VIEW "default" AS SELECT 1 AS id"#, + r#"CREATE MATERIALIZED VIEW "default" AS SELECT 1 AS id"#, + ] { + let message = rt + .try_sql(statement) + .await + .err() + .unwrap_or_else(|| panic!("should fail while the stand-in holds the name: {statement}")) + .to_string(); + + assert!( + message.contains("already exists"), + "`{statement}` gave an unexpected error: {message}" + ); + assert!( + message.contains("DROP TABLE"), + "`{statement}` should say how to free the name: {message}" + ); + } + + // The stand-in is still there, and still empty. + assert_eq!( + common::total_rows(&rt.sql(r#"SELECT * FROM "default""#).await), + 0 + ); +} + /// The cycle an operator runs: `DROP` the stand-in, `CREATE TABLE` under the same /// name, restart. The table and its rows come back, and Beacon adds no stand-in /// over them. diff --git a/beacon-db/beacon-core/tests/external_tables.rs b/beacon-db/beacon-core/tests/external_tables.rs index 6cccfd43..fe2974d7 100644 --- a/beacon-db/beacon-core/tests/external_tables.rs +++ b/beacon-db/beacon-core/tests/external_tables.rs @@ -92,3 +92,116 @@ async fn dropped_external_table_is_gone_and_the_name_is_reusable() { "the re-created table should read the new location" ); } + +/// A name a table holds is not free. `CREATE EXTERNAL TABLE` refuses it rather +/// than replacing the table under it, so no statement discards a table silently. +#[tokio::test(flavor = "multi_thread")] +async fn create_external_table_refuses_a_name_a_table_holds() { + let rt = runtime("ext-collision").await; + write_file(&rt.datasets_dir().join("d/a.csv"), "v\n1\n"); + write_file(&rt.datasets_dir().join("d2/b.csv"), "v\n1\n2\n"); + rt.sql("CREATE EXTERNAL TABLE taken STORED AS CSV LOCATION 'd/'") + .await; + + let error = rt + .try_sql("CREATE EXTERNAL TABLE taken STORED AS CSV LOCATION 'd2/'") + .await + .expect_err("the second create should fail"); + + assert!( + error.to_string().contains("already exists"), + "unexpected error: {error}" + ); + assert_eq!( + scalar_i64(&rt.sql("SELECT count(*) FROM taken").await), + 1, + "the first table should survive the refused create" + ); +} + +/// `IF NOT EXISTS` keeps the table that holds the name and reports success. +#[tokio::test(flavor = "multi_thread")] +async fn create_external_table_if_not_exists_keeps_the_existing_table() { + let rt = runtime("ext-if-not-exists").await; + write_file(&rt.datasets_dir().join("d/a.csv"), "v\n1\n"); + write_file(&rt.datasets_dir().join("d2/b.csv"), "v\n1\n2\n"); + rt.sql("CREATE EXTERNAL TABLE keep STORED AS CSV LOCATION 'd/'") + .await; + + rt.sql("CREATE EXTERNAL TABLE IF NOT EXISTS keep STORED AS CSV LOCATION 'd2/'") + .await; + + assert_eq!( + scalar_i64(&rt.sql("SELECT count(*) FROM keep").await), + 1, + "the existing table should be untouched" + ); +} + +/// A view follows the same rule, and `OR REPLACE` is how you swap one. +#[tokio::test(flavor = "multi_thread")] +async fn create_view_refuses_a_taken_name_unless_it_replaces() { + let rt = runtime("view-collision").await; + rt.sql("CREATE VIEW v AS SELECT 1 AS a").await; + + let error = rt + .try_sql("CREATE VIEW v AS SELECT 2 AS a") + .await + .expect_err("the second create should fail"); + assert!( + error.to_string().contains("already exists"), + "unexpected error: {error}" + ); + assert_eq!(scalar_i64(&rt.sql("SELECT a FROM v").await), 1); + + rt.sql("CREATE OR REPLACE VIEW v AS SELECT 2 AS a").await; + assert_eq!( + scalar_i64(&rt.sql("SELECT a FROM v").await), + 2, + "OR REPLACE should swap the view" + ); +} + +/// A view cannot take a table's name either, and the reverse holds too. +#[tokio::test(flavor = "multi_thread")] +async fn a_view_and_a_table_do_not_share_a_name() { + let rt = runtime("view-table-collision").await; + write_file(&rt.datasets_dir().join("d/a.csv"), "v\n1\n"); + rt.sql("CREATE EXTERNAL TABLE both STORED AS CSV LOCATION 'd/'") + .await; + + assert!( + rt.try_sql("CREATE VIEW both AS SELECT 1 AS a") + .await + .is_err(), + "a view should not take a table's name" + ); + + rt.sql("CREATE VIEW only_view AS SELECT 1 AS a").await; + assert!( + rt.try_sql("CREATE EXTERNAL TABLE only_view STORED AS CSV LOCATION 'd/'") + .await + .is_err(), + "an external table should not take a view's name" + ); +} + +/// `OR REPLACE` is how you repoint a table without a `DROP`, as the SQL reference +/// documents. +#[tokio::test(flavor = "multi_thread")] +async fn create_or_replace_external_table_repoints_the_name() { + let rt = runtime("ext-or-replace").await; + write_file(&rt.datasets_dir().join("d/a.csv"), "v\n1\n"); + write_file(&rt.datasets_dir().join("d2/b.csv"), "v\n1\n2\n"); + rt.sql("CREATE EXTERNAL TABLE swap STORED AS CSV LOCATION 'd/'") + .await; + + rt.sql("CREATE OR REPLACE EXTERNAL TABLE swap STORED AS CSV LOCATION 'd2/'") + .await; + + assert_eq!( + scalar_i64(&rt.sql("SELECT count(*) FROM swap").await), + 2, + "OR REPLACE should repoint the table at the new location" + ); +} diff --git a/docs/docs/2.0.0-rc5/server/configuration.md b/docs/docs/2.0.0-rc5/server/configuration.md index 848eccc8..5faf18f3 100644 --- a/docs/docs/2.0.0-rc5/server/configuration.md +++ b/docs/docs/2.0.0-rc5/server/configuration.md @@ -100,8 +100,13 @@ Beacon then leaves the name alone. Your table holds it after a restart, because startup adds a stand-in only for a free name. Drop your table and Beacon puts a stand-in back on the next start. -A `CREATE` on a name that a table holds fails, the stand-in included. The error -names the stand-in and tells you to drop it. +A `CREATE` on a name that a table holds fails, the stand-in included. This covers +`CREATE TABLE`, `CREATE EXTERNAL TABLE`, `CREATE VIEW` and +`CREATE MATERIALIZED VIEW`. The error names the stand-in and tells you to drop it. + +Three forms take a name that a table holds. `CREATE EXTERNAL TABLE IF NOT EXISTS` +keeps the table and reports success. `CREATE OR REPLACE EXTERNAL TABLE` and +`CREATE OR REPLACE VIEW` overwrite it. ### SQL result-stream coalescing