Skip to content

Commit f52f4be

Browse files
rampage644rampage644
authored
Remove READ ONLY flag (#70)
* Remove READ ONLY flag * Revert env changes
1 parent efc8588 commit f52f4be

8 files changed

Lines changed: 0 additions & 254 deletions

File tree

‎crates/embucket-lambda/src/config.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ pub struct EnvConfig {
1616
pub embucket_version: String,
1717
pub metastore_config: Option<PathBuf>,
1818
pub jwt_secret: Option<String>,
19-
pub read_only: bool,
2019
pub max_concurrent_table_fetches: usize,
2120
}
2221

@@ -37,7 +36,6 @@ impl EnvConfig {
3736
embucket_version: env_or_default("EMBUCKET_VERSION", "0.1.0"),
3837
metastore_config: env::var("METASTORE_CONFIG").ok().map(PathBuf::from),
3938
jwt_secret: env::var("JWT_SECRET").ok(),
40-
read_only: parse_env("READ_ONLY").unwrap_or(true),
4139
max_concurrent_table_fetches: parse_env("MAX_CONCURRENT_TABLE_FETCHES").unwrap_or(5),
4240
}
4341
}
@@ -53,7 +51,6 @@ impl EnvConfig {
5351
mem_pool_size_mb: self.mem_pool_size_mb,
5452
mem_enable_track_consumers_pool: self.mem_enable_track_consumers_pool,
5553
disk_pool_size_mb: self.disk_pool_size_mb,
56-
read_only: self.read_only,
5754
max_concurrent_table_fetches: self.max_concurrent_table_fetches,
5855
}
5956
}
@@ -63,12 +60,6 @@ fn env_or_default(name: &str, default: &str) -> String {
6360
env::var(name).unwrap_or_else(|_| default.to_string())
6461
}
6562

66-
fn env_bool(name: &str) -> bool {
67-
env::var(name)
68-
.map(|value| matches!(value.to_lowercase().as_str(), "1" | "true" | "yes" | "on"))
69-
.unwrap_or(false)
70-
}
71-
7263
fn parse_mem_pool_type() -> Option<MemPoolType> {
7364
env::var("MEM_POOL_TYPE")
7465
.ok()

‎crates/embucket-lambda/src/main.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ async fn main() -> Result<(), LambdaError> {
4040
mem_pool_type = ?env_config.mem_pool_type,
4141
mem_pool_size_mb = ?env_config.mem_pool_size_mb,
4242
disk_pool_size_mb = ?env_config.disk_pool_size_mb,
43-
read_only = env_config.read_only,
4443
metastore_config = env_config.metastore_config.as_ref().map(|p| p.display().to_string()),
4544
"Loaded Lambda configuration"
4645
);

‎crates/embucketd/src/cli.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,6 @@ pub struct CliOpts {
153153
)]
154154
jwt_secret: Option<String>,
155155

156-
#[arg(
157-
long,
158-
env = "READ_ONLY",
159-
default_value = "true",
160-
help = "If the service should only accept read only commands (selects)"
161-
)]
162-
pub read_only: bool,
163-
164156
#[arg(
165157
long,
166158
env = "MAX_CONCURRENT_TABLE_FETCHES",

‎crates/embucketd/src/main.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ async fn async_main(
130130
mem_pool_size_mb: opts.mem_pool_size_mb,
131131
mem_enable_track_consumers_pool: opts.mem_enable_track_consumers_pool,
132132
disk_pool_size_mb: opts.disk_pool_size_mb,
133-
read_only: opts.read_only,
134133
max_concurrent_table_fetches: opts.max_concurrent_table_fetches,
135134
};
136135
let host = opts.host.clone().unwrap();

‎crates/executor/src/error.rs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -632,13 +632,6 @@ pub enum Error {
632632
#[snafu(implicit)]
633633
location: Location,
634634
},
635-
636-
#[snafu(display("Statement not supported in read_only mode: {statement}"))]
637-
NotSupportedStatementInReadOnlyMode {
638-
statement: String,
639-
#[snafu(implicit)]
640-
location: Location,
641-
},
642635
}
643636

644637
impl Error {

‎crates/executor/src/query.rs‎

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -266,69 +266,6 @@ impl UserQuery {
266266
// 3. Single place to construct Logical plan from this AST
267267
// 4. Single place to rewrite-optimize-adjust logical plan
268268
// etc
269-
if self.session.config.read_only {
270-
match statement {
271-
DFStatement::Statement(s) => match *s {
272-
Statement::Query(subquery) => {
273-
return self.execute_query_statement(subquery).await;
274-
}
275-
Statement::Use(entity) => {
276-
return self.execute_use_statement(entity).await;
277-
}
278-
Statement::ShowDatabases { .. }
279-
| Statement::ShowSchemas { .. }
280-
| Statement::ShowTables { .. }
281-
| Statement::ShowColumns { .. }
282-
| Statement::ShowViews { .. }
283-
| Statement::ShowFunctions { .. }
284-
| Statement::ShowObjects { .. }
285-
| Statement::ShowVariables { .. }
286-
| Statement::ShowVariable { .. } => return Box::pin(self.show_query(*s)).await,
287-
other => {
288-
return ex_error::NotSupportedStatementInReadOnlyModeSnafu {
289-
statement: other.to_string(),
290-
}
291-
.fail();
292-
}
293-
},
294-
DFStatement::Explain(explain) => match *explain.statement {
295-
DFStatement::Statement(s) => match *s {
296-
Statement::Query(..)
297-
| Statement::Use(..)
298-
| Statement::ShowDatabases { .. }
299-
| Statement::ShowSchemas { .. }
300-
| Statement::ShowTables { .. }
301-
| Statement::ShowColumns { .. }
302-
| Statement::ShowViews { .. }
303-
| Statement::ShowFunctions { .. }
304-
| Statement::ShowObjects { .. }
305-
| Statement::ShowVariables { .. }
306-
| Statement::ShowVariable { .. } => {
307-
return self.execute_sql(&self.query).await;
308-
}
309-
other => {
310-
return ex_error::NotSupportedStatementInReadOnlyModeSnafu {
311-
statement: other.to_string(),
312-
}
313-
.fail();
314-
}
315-
},
316-
other => {
317-
return ex_error::NotSupportedStatementInReadOnlyModeSnafu {
318-
statement: other.to_string(),
319-
}
320-
.fail();
321-
}
322-
},
323-
other => {
324-
return ex_error::NotSupportedStatementInReadOnlyModeSnafu {
325-
statement: other.to_string(),
326-
}
327-
.fail();
328-
}
329-
}
330-
}
331-
332269
if let DFStatement::Statement(s) = statement {
333270
match *s {
334271
Statement::AlterSession {

‎crates/executor/src/tests/service.rs‎

Lines changed: 0 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use crate::models::{QueryContext, QueryResult};
22
use crate::service::{CoreExecutionService, ExecutionService};
33
use crate::utils::Config;
44
use catalog_metastore::InMemoryMetastore;
5-
use catalog_metastore::metastore_config::MetastoreBootstrapConfig;
65
use catalog_metastore::models::table::TableIdent as MetastoreTableIdent;
76
use catalog_metastore::{
87
Database as MetastoreDatabase, Schema as MetastoreSchema, SchemaIdent as MetastoreSchemaIdent,
@@ -435,159 +434,3 @@ async fn test_query_timeout() {
435434
"Expected query execution exceeded timeout error but got {res:?}"
436435
);
437436
}
438-
439-
#[tokio::test]
440-
#[allow(clippy::expect_used)]
441-
async fn test_execute_read_only_mode() {
442-
//setup
443-
let metastore = Arc::new(InMemoryMetastore::new());
444-
let metastore_config = MetastoreBootstrapConfig::bootstrap();
445-
metastore_config
446-
.apply(metastore.clone())
447-
.await
448-
.expect("Failed to apply config");
449-
450-
let execution_svc = CoreExecutionService::new(metastore.clone(), Arc::new(Config::default()))
451-
.await
452-
.expect("Failed to create execution service");
453-
454-
execution_svc
455-
.create_session("test_session_id")
456-
.await
457-
.expect("Failed to create session");
458-
459-
execution_svc
460-
.query(
461-
"test_session_id",
462-
"CREATE OR REPLACE TABLE fetch_test(c1 INT)",
463-
QueryContext::default(),
464-
)
465-
.await
466-
.expect("Failed to execute query");
467-
468-
execution_svc
469-
.query(
470-
"test_session_id",
471-
"INSERT INTO fetch_test VALUES (1),(2),(3),(4)",
472-
QueryContext::default(),
473-
)
474-
.await
475-
.expect("Failed to execute query");
476-
477-
drop(execution_svc);
478-
479-
//read only mode test
480-
let execution_svc =
481-
CoreExecutionService::new(metastore, Arc::new(Config::default().with_read_only(true)))
482-
.await
483-
.expect("Failed to create execution service");
484-
485-
execution_svc
486-
.create_session("test_session_id")
487-
.await
488-
.expect("Failed to create session");
489-
490-
//should fail
491-
execution_svc
492-
.query(
493-
"test_session_id",
494-
"CREATE OR REPLACE TABLE fetch_test(c1 INT)",
495-
QueryContext::default(),
496-
)
497-
.await
498-
.expect_err("Read only mode failed");
499-
500-
//should fail
501-
execution_svc
502-
.query(
503-
"test_session_id",
504-
"INSERT INTO fetch_test VALUES (1),(2),(3),(4)",
505-
QueryContext::default(),
506-
)
507-
.await
508-
.expect_err("Read only mode failed");
509-
510-
execution_svc
511-
.query("test_session_id", "SELECT 1", QueryContext::default())
512-
.await
513-
.expect("Failed to execute query in read only mode");
514-
515-
execution_svc
516-
.query(
517-
"test_session_id",
518-
"EXPLAIN SELECT 1",
519-
QueryContext::default(),
520-
)
521-
.await
522-
.expect("Failed to execute query in read only mode");
523-
524-
execution_svc
525-
.query(
526-
"test_session_id",
527-
"WITH limited_data AS (
528-
SELECT c1 FROM fetch_test ORDER BY c1 FETCH FIRST 3 ROWS
529-
)
530-
SELECT * FROM limited_data ORDER BY c1;",
531-
QueryContext::default(),
532-
)
533-
.await
534-
.expect("Failed to execute query in read only mode");
535-
536-
execution_svc
537-
.query(
538-
"test_session_id",
539-
"EXPLAIN WITH limited_data AS (
540-
SELECT c1 FROM fetch_test ORDER BY c1 FETCH FIRST 3 ROWS
541-
)
542-
SELECT * FROM limited_data ORDER BY c1;",
543-
QueryContext::default(),
544-
)
545-
.await
546-
.expect("Failed to execute query in read only mode");
547-
548-
execution_svc
549-
.query(
550-
"test_session_id",
551-
"SELECT 1 UNION ALL SELECT c1 FROM fetch_test;",
552-
QueryContext::default(),
553-
)
554-
.await
555-
.expect("Failed to execute query in read only mode");
556-
557-
execution_svc
558-
.query(
559-
"test_session_id",
560-
"EXPLAIN SELECT 1 UNION ALL SELECT c1 FROM fetch_test;",
561-
QueryContext::default(),
562-
)
563-
.await
564-
.expect("Failed to execute query in read only mode");
565-
566-
execution_svc
567-
.query(
568-
"test_session_id",
569-
"USE SCHEMA public;",
570-
QueryContext::default(),
571-
)
572-
.await
573-
.expect("Failed to execute query in read only mode");
574-
575-
execution_svc
576-
.query("test_session_id", "SHOW TABLES;", QueryContext::default())
577-
.await
578-
.expect("Failed to execute query in read only mode");
579-
580-
execution_svc
581-
.query("test_session_id", "SHOW SCHEMAS;", QueryContext::default())
582-
.await
583-
.expect("Failed to execute query in read only mode");
584-
585-
execution_svc
586-
.query(
587-
"test_session_id",
588-
"SHOW DATABASES;",
589-
QueryContext::default(),
590-
)
591-
.await
592-
.expect("Failed to execute query in read only mode");
593-
}

‎crates/executor/src/utils.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ pub struct Config {
3939
pub mem_pool_size_mb: Option<usize>,
4040
pub mem_enable_track_consumers_pool: Option<bool>,
4141
pub disk_pool_size_mb: Option<usize>,
42-
pub read_only: bool,
4342
pub max_concurrent_table_fetches: usize,
4443
}
4544

@@ -62,7 +61,6 @@ impl Default for Config {
6261
mem_pool_size_mb: None,
6362
mem_enable_track_consumers_pool: None,
6463
disk_pool_size_mb: None,
65-
read_only: false,
6664
max_concurrent_table_fetches: 5,
6765
}
6866
}
@@ -80,12 +78,6 @@ impl Config {
8078
self.query_timeout_secs = timeout_secs;
8179
self
8280
}
83-
84-
#[must_use]
85-
pub const fn with_read_only(mut self, read_only: bool) -> Self {
86-
self.read_only = read_only;
87-
self
88-
}
8981
}
9082

9183
#[derive(Copy, Clone, PartialEq, Eq, EnumString, Debug, Display, Default)]

0 commit comments

Comments
 (0)