From d86b3386dc1ed1be53964e1eed9e27f4efcd8fe5 Mon Sep 17 00:00:00 2001
From: Niwaka <61189782+NiwakaDev@users.noreply.github.com>
Date: Fri, 5 May 2023 12:29:09 +0900
Subject: [PATCH] fix: incorrect show create table output (#1514)

* fix: incorrect show create table output

* feat: change CreateTable's Display if table is external

* feat: change CreateTable's Display if table is external
---
 src/query/src/sql/show.rs                     | 77 ++++++++++++++++++-
 src/sql/src/statements/create.rs              | 12 ++-
 .../cases/standalone/show/show_create.result  |  4 +-
 3 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/src/query/src/sql/show.rs b/src/query/src/sql/show.rs
index f301674782..051874a48b 100644
--- a/src/query/src/sql/show.rs
+++ b/src/query/src/sql/show.rs
@@ -25,6 +25,7 @@ use sql::parser::ParserContext;
 use sql::statements::create::{CreateTable, TIME_INDEX};
 use sql::statements::{self};
 use table::metadata::{TableInfoRef, TableMeta};
+use table::requests::IMMUTABLE_TABLE_META_KEY;
 
 use crate::error::{ConvertSqlTypeSnafu, ConvertSqlValueSnafu, Result, SqlSnafu};
 
@@ -74,7 +75,11 @@ fn create_sql_options(table_meta: &TableMeta) -> Vec<SqlOption> {
         options.push(sql_option("compaction_time_window", number_value(w)));
     }
 
-    for (k, v) in &table_opts.extra_options {
+    for (k, v) in table_opts
+        .extra_options
+        .iter()
+        .filter(|(k, _)| k != &IMMUTABLE_TABLE_META_KEY)
+    {
         options.push(sql_option(k, string_value(v)));
     }
 
@@ -184,6 +189,10 @@ mod tests {
     use datatypes::prelude::ConcreteDataType;
     use datatypes::schema::{Schema, SchemaRef};
     use table::metadata::*;
+    use table::requests::{
+        TableOptions, IMMUTABLE_TABLE_FORMAT_KEY, IMMUTABLE_TABLE_LOCATION_KEY,
+        IMMUTABLE_TABLE_META_KEY,
+    };
 
     use super::*;
 
@@ -255,6 +264,72 @@ CREATE TABLE IF NOT EXISTS system_metrics (
 ENGINE=mito
 WITH(
   regions = 3
+)"#,
+            sql
+        );
+    }
+
+    #[test]
+    fn test_show_create_external_table_sql() {
+        let schema = vec![
+            ColumnSchema::new("host", ConcreteDataType::string_datatype(), true),
+            ColumnSchema::new("cpu", ConcreteDataType::float64_datatype(), true),
+        ];
+        let table_schema = SchemaRef::new(Schema::new(schema));
+        let table_name = "system_metrics";
+        let schema_name = "public".to_string();
+        let catalog_name = "greptime".to_string();
+        let mut options: TableOptions = Default::default();
+        options.extra_options.insert(
+            IMMUTABLE_TABLE_LOCATION_KEY.to_string(),
+            "foo.csv".to_string(),
+        );
+        options.extra_options.insert(
+            IMMUTABLE_TABLE_META_KEY.to_string(),
+            "{{\"files\":[\"foo.csv\"]}}".to_string(),
+        );
+        options
+            .extra_options
+            .insert(IMMUTABLE_TABLE_FORMAT_KEY.to_string(), "csv".to_string());
+        let meta = TableMetaBuilder::default()
+            .schema(table_schema)
+            .primary_key_indices(vec![])
+            .engine("file".to_string())
+            .next_column_id(0)
+            .engine_options(Default::default())
+            .options(options)
+            .created_on(Default::default())
+            .build()
+            .unwrap();
+
+        let info = Arc::new(
+            TableInfoBuilder::default()
+                .table_id(1024)
+                .table_version(0 as TableVersion)
+                .name(table_name)
+                .schema_name(schema_name)
+                .catalog_name(catalog_name)
+                .desc(None)
+                .table_type(TableType::Base)
+                .meta(meta)
+                .build()
+                .unwrap(),
+        );
+
+        let stmt = create_table_stmt(&info).unwrap();
+
+        let sql = format!("\n{}", stmt);
+        assert_eq!(
+            r#"
+CREATE EXTERNAL TABLE IF NOT EXISTS system_metrics (
+  host STRING NULL,
+  cpu DOUBLE NULL,
+
+)
+ENGINE=file
+WITH(
+  FORMAT = 'csv',
+  LOCATION = 'foo.csv'
 )"#,
             sql
         );
diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs
index 4a9eb00337..431742db6c 100644
--- a/src/sql/src/statements/create.rs
+++ b/src/sql/src/statements/create.rs
@@ -15,6 +15,7 @@
 use std::collections::HashMap;
 use std::fmt::{Display, Formatter};
 
+use common_catalog::consts::IMMUTABLE_FILE_ENGINE;
 use itertools::Itertools;
 
 use crate::ast::{ColumnDef, Ident, ObjectName, SqlOption, TableConstraint, Value as SqlValue};
@@ -110,7 +111,8 @@ impl CreateTable {
         if self.options.is_empty() {
             "".to_string()
         } else {
-            let options = format_list_indent!(self.options);
+            let options: Vec<&SqlOption> = self.options.iter().sorted().collect();
+            let options = format_list_indent!(options);
             format!(
                 r#"WITH(
 {options}
@@ -165,10 +167,14 @@ impl Display for CreateTable {
         let partitions = self.format_partitions();
         let engine = &self.engine;
         let options = self.format_options();
-
+        let maybe_external = if self.engine == IMMUTABLE_FILE_ENGINE {
+            "EXTERNAL "
+        } else {
+            ""
+        };
         write!(
             f,
-            r#"CREATE TABLE {if_not_exists} {name} (
+            r#"CREATE {maybe_external}TABLE {if_not_exists} {name} (
 {columns},
 {constraints}
 )
diff --git a/tests/cases/standalone/show/show_create.result b/tests/cases/standalone/show/show_create.result
index f6b07ba640..763bf3e9d6 100644
--- a/tests/cases/standalone/show/show_create.result
+++ b/tests/cases/standalone/show/show_create.result
@@ -32,8 +32,8 @@ SHOW CREATE TABLE system_metrics;
 |                | ENGINE=mito                                             |
 |                | WITH(                                                   |
 |                |   regions = 1,                                          |
-|                |   write_buffer_size = '1.0KiB',                         |
-|                |   ttl = '7days'                                         |
+|                |   ttl = '7days',                                        |
+|                |   write_buffer_size = '1.0KiB'                          |
 |                | )                                                       |
 +----------------+---------------------------------------------------------+