diff --git a/CHANGELOG.md b/CHANGELOG.md index a4bc3bee..814ebe25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -317,6 +317,31 @@ tag. Releases before 2.0.0 are recorded in the ### Fixed +- **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). +- **`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/schema_persistence/default_table.rs b/beacon-db/beacon-core/src/schema_persistence/default_table.rs new file mode 100644 index 00000000..78d9824c --- /dev/null +++ b/beacon-db/beacon-core/src/schema_persistence/default_table.rs @@ -0,0 +1,99 @@ +//! 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. +//! +//! 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 and say that in the error. + +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. +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/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..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(()) @@ -519,7 +548,12 @@ pub(crate) async fn create_table( 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 332c6dea..8cd132b1 100644 --- a/beacon-db/beacon-core/src/statement_plan/materialized_view.rs +++ b/beacon-db/beacon-core/src/statement_plan/materialized_view.rs @@ -44,7 +44,12 @@ pub(crate) async fn create_materialized_view( let table_ref = crate::table_name::table_reference(name); if session_ctx.table_exist(table_ref.clone())? { - return Err(anyhow::anyhow!("Materialized view '{name}' already exists")); + return Err(crate::schema_persistence::default_table::already_exists_error( + session_ctx, + table_ref.clone(), + "Materialized view", + ) + .await); } // Execute the defining query and persist its result as a single Parquet file 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 new file mode 100644 index 00000000..a13786ae --- /dev/null +++ b/beacon-db/beacon-core/tests/default_table.rs @@ -0,0 +1,232 @@ +//! The default table (`sql.default_table`) is a name Beacon fills only when it is free. +//! +//! 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; + +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) +} + +/// 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_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("the name is taken, so the create should fail"); + + let message = error.to_string(); + assert!( + message.contains("already exists"), + "unexpected error: {message}" + ); + assert!( + message.contains("DROP TABLE"), + "the error should say how to free the name: {message}" + ); +} + +/// 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. +#[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" + ); +} + +/// 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; + 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, + "Beacon must not put a stand-in over the user's table after a restart" + ); + assert_eq!(rows_without_from(&rt).await, 2); +} + +/// `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_materialized_view_fails_while_the_stand_in_holds_the_name() { + let rt = common::runtime("default-materialized-view").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"); + + 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); +} + +/// 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_free_name_gets_a_stand_in_on_the_next_start() { + let rt = common::restartable_runtime("default-dropped", |b| b).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" + ); + + 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 +/// `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 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/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/beacon-db/beacon-core/tests/runtime_config.rs b/beacon-db/beacon-core/tests/runtime_config.rs index dbbe0fea..83d63200 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,37 @@ 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 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})")) + .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 +72,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/beacon-server/beacon-server/tests/admin_endpoints_http.rs b/beacon-server/beacon-server/tests/admin_endpoints_http.rs index 07dd5c11..85c4d1c8 100644 --- a/beacon-server/beacon-server/tests/admin_endpoints_http.rs +++ b/beacon-server/beacon-server/tests/admin_endpoints_http.rs @@ -225,6 +225,72 @@ async fn create_external_table_from_fields() { assert_eq!(count_rows(&router, &admin, "ext_obs").await, 3); } +/// 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 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 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, + 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 freed name should accept the external table, got: {}", + String::from_utf8_lossy(&created.body) + ); + assert_eq!(count_rows(&router, &admin, r#""default""#).await, 3); + + 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; diff --git a/docs/docs/2.0.0-rc5/server/configuration.md b/docs/docs/2.0.0-rc5/server/configuration.md index 858204c6..5faf18f3 100644 --- a/docs/docs/2.0.0-rc5/server/configuration.md +++ b/docs/docs/2.0.0-rc5/server/configuration.md @@ -83,6 +83,31 @@ rejects that `CREATE`. Beacon never writes plaintext. | `BEACON_TYPE_WIDENING_STRATEGY` | `default` | The rule a schema merge applies to a column that two files type in two ways. `default` widens inside one family: a wider integer, a finer timestamp, a longer string. It refuses a boolean beside a number and a number beside a string, and it reads every integer beside a `Float32` as `Float64`. `numpy` promotes as `numpy.result_type` does: a boolean joins the numbers, `Float16` joins the floats, a narrow integer beside a `Float32` stays a `Float32`, a number beside a string reads as text, and a date beside a timestamp is a timestamp. numpy resolves the set of types of a column at once, so the listing order does not change the result. `numpy` reads every schema in one pass, as `keep_first` does. An unknown value logs a warning and reads as `default`. See [a column has two types](/docs/2.0.0-rc5/troubleshooting#a-column-has-two-types-across-the-files). | | `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 timestamp. `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 pair the strategy widens, such as `Int32` beside `Float64`, 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 + +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. + +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. 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 A query can produce small record batches. Beacon merges them into larger batches