databend icon indicating copy to clipboard operation
databend copied to clipboard

parser: table name case sensitive error not right

Open BohuTANG opened this issue 2 years ago • 4 comments

Summary

SQL:

CREATE TABLE `Student`(id int);
INSERT INTO STUDENT VALUES(1);

Error:

1025=> Unknown table 'student'

Expected:

Unknown table 'STUDENT'. Did you mean 'Student'?

We should guide the user to write:

INSERT INTO Student VALUES(1);

BohuTANG avatar Sep 28 '23 01:09 BohuTANG

Maybe We change quoted_ident_case_sensitive default 0

mysql> set quoted_ident_case_sensitive=0;
Query OK, 0 rows affected (0.00 sec)

mysql> CREATE TABLE `Student`(id int);
Query OK, 0 rows affected (0.01 sec)

mysql> INSERT INTO STUDENT VALUES(1);
Query OK, 1 row affected (0.03 sec)

mysql> INSERT INTO Student VALUES(1);
Query OK, 1 row affected (0.04 sec)

mysql> select * from STUDENT;
+------+
| id   |
+------+
|    1 |
|    1 |
+------+
2 rows in set (0.02 sec)
Read 2 rows, 10.00 B in 0.012 sec., 166.38 rows/sec., 831.91 B/sec.

wubx avatar Sep 28 '23 01:09 wubx

quoted_ident_case_sensitive

This is not a solution, we should guide the user to know the right table name Student , because he create the table with Student.

BohuTANG avatar Sep 28 '23 02:09 BohuTANG

I looked a bit more into this issue. The error throwed here

        let table_id = match db_meta.from_share {
            Some(ref share) => {
                get_table_id_from_share_by_name(self, share, &tenant_dbname_tbname.table_name)
                    .await?
            }
            None => {
                // Get table by tenant,db_id, table_name to assert presence.

                let dbid_tbname = DBIdTableName {
                    db_id,
                    table_name: tenant_dbname_tbname.table_name.clone(),
                };

                let (tb_id_seq, table_id) = get_u64_value(self, &dbid_tbname).await?;
                assert_table_exist(tb_id_seq, tenant_dbname_tbname, "get_table")?;  // UnknownTable err returned here

                table_id
            }
        };

The simple solution I can think of is to add a new field similar_table_name in UnknownTable and modify app error message. We retrive all tables and find most similar one(maybe using case-insensitive comparison first).

#[derive(thiserror::Error, Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[error("UnknownTable: `{table_name}` while `{context}`")]
pub struct UnknownTable {
    table_name: String,
    similar_table_name: Option<String>,
    context: String,
}

impl AppErrorMessage for UnknownTable {
    fn message(&self) -> String {
        format!("Unknown table '{}'{}", self.table_name,
        if self.similar_table_name.is_some() {
            format!(". Did you mean '{}'?", self.similar_table_name.as_ref().unwrap())
        } else {
            "".to_string()
        })
    }
}

lewiszlw avatar Sep 29 '23 05:09 lewiszlw

How does solution #15889 look? But it's too cumbersome and feels like it needs a specification and then refactoring.

forsaken628 avatar Jun 25 '24 04:06 forsaken628