fix(query): add check window frame (#17316)

* fix(query): add check window frame * fix(query): add values checker * fix(query): add values checker * fix(query): add values checker
databendlabs · Jan 19, 2025 · f874851 · f874851
1 parent 07034df
commit f874851
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 8 deletions.
diff --git a/src/query/sql/src/planner/binder/bind_query/bind_value.rs b/src/query/sql/src/planner/binder/bind_query/bind_value.rs
@@ -402,6 +402,13 @@ pub fn bind_values(
     for (row_idx, row) in values.iter().enumerate() {
         for (column_idx, expr) in row.iter().enumerate() {
             let (scalar, data_type) = scalar_binder.bind(expr)?;
+            if !scalar.evaluable() {
+                return Err(ErrorCode::SemanticError(format!(
+                    "Values can't contain subquery, aggregate functions, window functions, or UDFs"
+                ))
+                .set_span(span));
+            }
+
             let used_columns = scalar.used_columns();
             if !used_columns.is_empty() {
                 if let Some(expression_scan_info) = expression_scan_info.as_ref() {

diff --git a/src/query/sql/src/planner/semantic/type_check.rs b/src/query/sql/src/planner/semantic/type_check.rs
@@ -1193,6 +1193,21 @@ impl<'a> TypeChecker<'a> {
 
         let frame =
             self.resolve_window_frame(span, &func, &mut order_by, spec.window_frame.clone())?;
+
+        if matches!(&frame.start_bound, WindowFuncFrameBound::Following(None)) {
+            return Err(ErrorCode::SemanticError(
+                "Frame start cannot be UNBOUNDED FOLLOWING".to_string(),
+            )
+            .set_span(span));
+        }
+
+        if matches!(&frame.end_bound, WindowFuncFrameBound::Preceding(None)) {
+            return Err(ErrorCode::SemanticError(
+                "Frame end cannot be UNBOUNDED PRECEDING".to_string(),
+            )
+            .set_span(span));
+        }
+
         let data_type = func.return_type();
         let window_func = WindowFunc {
             span,

diff --git a/tests/sqllogictests/src/arg.rs b/tests/sqllogictests/src/arg.rs
@@ -121,4 +121,11 @@ pub struct SqlLogicTestArgs {
         help = "Specify the database to connnect, the default database is 'default'"
     )]
     pub database: String,
+
+    #[arg(
+        long = "port",
+        default_value = "8000",
+        help = "The databend server http port"
+    )]
+    pub port: u16,
 }
diff --git a/tests/sqllogictests/src/client/http_client.rs b/tests/sqllogictests/src/client/http_client.rs
@@ -37,6 +37,7 @@ pub struct HttpClient {
     pub session_token: String,
     pub debug: bool,
     pub session: Option<HttpSessionConf>,
+    pub port: u16,
 }
 
 #[derive(serde::Deserialize, Debug)]
@@ -75,7 +76,7 @@ struct LoginResponse {
 }
 
 impl HttpClient {
-    pub async fn create() -> Result<Self> {
+    pub async fn create(port: u16) -> Result<Self> {
         let mut header = HeaderMap::new();
         header.insert(
             "Content-Type",
@@ -94,10 +95,10 @@ impl HttpClient {
             .pool_max_idle_per_host(0)
             .build()?;
 
-        let url = "http://127.0.0.1:8000/v1/session/login";
+        let url = format!("http://127.0.0.1:{}/v1/session/login", port);
 
         let session_token = client
-            .post(url)
+            .post(&url)
             .body("{}")
             .basic_auth("root", Some(""))
             .send()
@@ -119,18 +120,19 @@ impl HttpClient {
             session_token,
             session: None,
             debug: false,
+            port,
         })
     }
 
     pub async fn query(&mut self, sql: &str) -> Result<DBOutput<DefaultColumnType>> {
         let start = Instant::now();
 
-        let url = "http://127.0.0.1:8000/v1/query".to_string();
+        let url = format!("http://127.0.0.1:{}/v1/query", self.port);
         let mut parsed_rows = vec![];
         let mut response = self.post_query(sql, &url).await?;
         self.handle_response(&response, &mut parsed_rows)?;
         while let Some(next_uri) = &response.next_uri {
-            let url = format!("http://127.0.0.1:8000{next_uri}");
+            let url = format!("http://127.0.0.1:{}{next_uri}", self.port);
             let new_response = self.poll_query_result(&url).await?;
             if new_response.next_uri.is_some() {
                 self.handle_response(&new_response, &mut parsed_rows)?;

diff --git a/tests/sqllogictests/src/main.rs b/tests/sqllogictests/src/main.rs
@@ -159,7 +159,7 @@ async fn run_hybrid_client(
         match c.as_ref() {
             ClientType::MySQL | ClientType::Http => {}
             ClientType::Ttc(image, _) => {
-                run_ttc_container(&docker, image, port_start, cs).await?;
+                run_ttc_container(&docker, image, port_start, args.port, cs).await?;
                 port_start += 1;
             }
             ClientType::Hybird => panic!("Can't run hybrid client in hybrid client"),
@@ -184,7 +184,7 @@ async fn create_databend(client_type: &ClientType, filename: &str) -> Result<Dat
             client = Client::MySQL(mysql_client);
         }
         ClientType::Http => {
-            client = Client::Http(HttpClient::create().await?);
+            client = Client::Http(HttpClient::create(args.port).await?);
         }
 
         ClientType::Ttc(image, port) => {

diff --git a/tests/sqllogictests/src/util.rs b/tests/sqllogictests/src/util.rs
@@ -249,6 +249,7 @@ pub async fn run_ttc_container(
     docker: &Docker,
     image: &str,
     port: u16,
+    http_server_port: u16,
     cs: &mut Vec<ContainerAsync<GenericImage>>,
 ) -> Result<()> {
     let mut images = image.split(":");
@@ -269,7 +270,10 @@ pub async fn run_ttc_container(
             .with_network("host")
             .with_env_var(
                 "DATABEND_DSN",
-                "databend://root:@127.0.0.1:8000?sslmode=disable",
+                format!(
+                    "databend://root:@127.0.0.1:{}?sslmode=disable",
+                    http_server_port
+                ),
             )
             .with_env_var("TTC_PORT", format!("{port}"))
             .with_container_name(&container_name)

diff --git a/tests/sqllogictests/suites/base/03_common/03_0016_insert_into_values.test b/tests/sqllogictests/suites/base/03_common/03_0016_insert_into_values.test
@@ -13,6 +13,9 @@ CREATE DATABASE db2;
 statement ok
 USE db1;
 
+statement error 1065
+explain VALUES(2), (3), (COUNT(*) OVER ()), (4), (5)
+
 statement ok
 CREATE TABLE IF NOT EXISTS t1(a Int8 null, b UInt32 null, c Date null, d DateTime null, e String null, f Float64 null) Engine = Fuse
 
@@ -33,6 +36,7 @@ select sum(a),sum(b) from t1
 ----
 401 367
 
+
 statement ok
 CREATE TABLE IF NOT EXISTS t_str(a Varchar)
 

diff --git a/tests/sqllogictests/suites/query/window_function/window_basic.test b/tests/sqllogictests/suites/query/window_function/window_basic.test
@@ -1,6 +1,13 @@
 statement ok
 CREATE DATABASE IF NOT EXISTS test_window_basic
 
+
+statement error 1065
+SELECT last_value(number)  OVER (ORDER BY number ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED PRECEDING) FROM numbers(1);
+
+statement error 1065
+SELECT last_value(number)  OVER (ORDER BY number RANGE BETWEEN UNBOUNDED FOLLOWING AND UNBOUNDED FOLLOWING) FROM numbers(1);
+
 statement ok
 USE test_window_basic