impl Display for Statement

[tisonkun]

What type of enhancement is this?

API improvement

What does the enhancement do?

sqlparser's Statement implements Display, we need to implement Display for our Statement, named sql::statement::Statement also.

See

match self.query_statement(stmt, query_ctx.clone()).await {
    Ok(output) => {
        let output_result =
            query_interceptor.post_execute(output, query_ctx.clone());
        results.push(output_result);
    }
    Err(e) => {
        let redacted = sql::util::redact_sql_secrets(query.as_ref());
        error!(e; "Failed to execute query: {redacted}");

        results.push(Err(e));
        break;
    }
}

We currently print all the query if any of the statement failed to execute. We should write instead:

match self.query_statement(stmt, query_ctx.clone()).await {
    Ok(output) => {
        let output_result =
            query_interceptor.post_execute(output, query_ctx.clone());
        results.push(output_result);
    }
    Err(e) => {
        error!(e; "Failed to execute query: {stmt}");

        results.push(Err(e));
        break;
    }
}

with stmt probably redacted. And formatting structural stmt would be more reliable to redact fields, instead of depending on brittle regexps.

Implementation challenges

No response

[irenjj]

I'd like to give it a try.

[tisonkun]

@irenjj Assigned and please go ahead!

You can implement Display for Statement first and we later integrate it with the current logging on error logic. Or you can implmenet both at once if you'd like to. But anyway, please submit a PR and ping me as a reviewer when the basic functionality is ready.

[tisonkun]

@irenjj a reminder to confirm that you still work on this issue - do you find any trouble here?

[irenjj]

hi, @tisonkun, Sorry, I've been a bit busy lately. I have only completed part of it.

impl Display for Statement {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        match self {
            Statement::Query(s) => s.inner.fmt(f),
            Statement::Insert(s) => s.inner.fmt(f),
            Statement::Delete(s) => s.inner.fmt(f),
            Statement::CreateTable(s) => s.fmt(f),
            Statement::CreateExternalTable(s) => todo!(),
            Statement::CreateTableLike(s) => todo!(),
            Statement::DropTable(s) => s.fmt(f),
            Statement::DropDatabase(s) => s.fmt(f),
            Statement::CreateDatabase(s) => s.fmt(f),
            Statement::Alter(s) => todo!(),
            Statement::ShowDatabases(s) => s.fmt(f),
            Statement::ShowTables(s) => s.fmt(f),
            Statement::ShowColumns(s) => s.fmt(f),
            Statement::ShowIndex(s) => s.fmt(f),
            Statement::ShowCreateTable(s) => s.fmt(f),
            Statement::DescribeTable(s) => s.fmt(f),
            Statement::Explain(s) => s.inner.fmt(f),
            Statement::Copy(s) => todo!(),
            Statement::Tql(s) => todo!(),
            Statement::TruncateTable(s) => s.fmt(f),
            Statement::SetVariables(s) => s.fmt(f),
            Statement::ShowVariables(s) => s.fmt(f),
        }
    }
}

I'll complete the remaining logic in the next few days and submit a draft.

[tisonkun]

@irenjj Thanks for your update! That's great.