diff --git a/storage/duckdb/CLAUDE.md b/storage/duckdb/CLAUDE.md index b6cea1a72ef66..2f1bdb7de756d 100644 --- a/storage/duckdb/CLAUDE.md +++ b/storage/duckdb/CLAUDE.md @@ -46,6 +46,8 @@ Tests use MariaDB's MTR (MySQL Test Runner) framework. Test files live in `mysql All engine code is in the `myduck` namespace (except `ha_duckdb` which is in global scope per MariaDB handler convention). +> **Doc convention:** when this file (or a code comment) names a specific API to describe control flow (e.g. `SELECT_LEX::print()`), it must also name the file/function where that API is actually called. If no call site can be pointed to, describe the behavior instead of naming the API — do not imply a code path that isn't there. + ### Key components - **`ha_duckdb`** (`ha_duckdb.cc/h`) — MariaDB `handler` subclass. Entry point for all storage engine operations (open, close, read, write, DDL). Implements row-at-a-time interface for MariaDB, translating to DuckDB batch operations. @@ -66,7 +68,16 @@ All engine code is in the `myduck` namespace (except `ha_duckdb` which is in glo ### SQL generation conventions -All generated SQL must use **double quotes** for identifiers (DuckDB follows SQL standard), not backticks. The `SELECT_LEX::print()` output from MariaDB uses backticks and must be post-processed. See `docs/mariadb-duckdb-incompatibilities.md` for known function name rewrites and type mapping issues. +DuckDB follows the SQL standard and delimits identifiers with **double quotes**, not backticks. + +SQL reaches DuckDB by two routes: + +- **Whole queries are forwarded verbatim.** SELECT and INSERT … SELECT are pushed down as the raw `thd->query()` text — see `extract_source_query()` in `ha_duckdb_pushdown.cc`. The engine does **not** re-print the whole query via `SELECT_LEX::print()`. +- **Only fragments are printed.** `Item`/`COND::print()` is used for per-table WHERE conditions in cross-engine scan (`ha_duckdb_pushdown.cc`) and for DDL default / `nextval` expressions (`ddl_convertor.cc`). DDL/DML convertors also build identifier strings directly from `Field`/`TABLE` metadata. + +Both routes then pass through `backticks_to_double_quotes()` (`runtime/duckdb_query.cc`), which is where identifier requoting actually happens: backtick-delimited identifiers are rewritten to double-quoted ones and any embedded double quote is escaped (MDEV-40653). Raw forwarding additionally goes through `mariadb_query_has_lexical_mismatch()`, which refuses to forward SQL whose backslash-escape semantics differ between MariaDB and DuckDB. + +See `docs/mariadb-duckdb-incompatibilities.md` for known function name rewrites and type mapping issues. ### DuckDB source and patches diff --git a/storage/duckdb/README.md b/storage/duckdb/README.md index 842863bc097ba..f72cd9aa0f64a 100644 --- a/storage/duckdb/README.md +++ b/storage/duckdb/README.md @@ -137,6 +137,7 @@ DuckDB handles the join, aggregation, and sorting; InnoDB rows are produced on d - **Some MariaDB functions are yet not pushdown-compatible** — `GROUP_CONCAT()`, `DATE_FORMAT()`, `JSON_CONTAINS()`, `FOUND_ROWS()`, `LAST_INSERT_ID()`, and a few others have no DuckDB equivalent or differ in syntax. Such queries fall back to MariaDB execution. - **Strict GROUP BY** — DuckDB rejects `SELECT` columns not in `GROUP BY` and not aggregated, even when MariaDB's `sql_mode` allows it. - **XA transactions** — `XA PREPARE` is not supported by the engine. +- **Table partitioning** — `CREATE TABLE ... PARTITION BY` and converting a DuckDB table with `ALTER TABLE ... PARTITION BY` are not supported; the engine declares `HTON_NO_PARTITION`. - **Collations** — MariaDB UCA-based collation rules are approximated via DuckDB's built-in `NOCASE`/`NOACCENT` collations for UTF-8 charsets; non-UTF8 charsets fall back to binary comparison. See [`docs/collation-mapping.md`](docs/collation-mapping.md) for the full mapping and known gaps. - **Cross-engine scan is yet single-threaded** — each external (non-DuckDB) table is produced by a single fiber-driven MariaDB query (`_mdb_scan` reports `MaxThreads() == 1`); only the DuckDB side of the query is parallelized. - **ALTER COLUMN DROP DEFAULT** — not propagated to DuckDB catalog. diff --git a/storage/duckdb/convertor/ddl_convertor.cc b/storage/duckdb/convertor/ddl_convertor.cc index 2ce9cab6b8f26..2432a95389d5d 100644 --- a/storage/duckdb/convertor/ddl_convertor.cc +++ b/storage/duckdb/convertor/ddl_convertor.cc @@ -259,17 +259,19 @@ static std::string autoinc_nextval_expr(const std::string &schema_name, Read the literal default value of a field from the default record and return it as a string suitable for DuckDB SQL. - BIT fields are converted to DuckDB blob literal format: '\xHH...'::BLOB. - Other fields use standard quoted literal format: 'value'. + BIT fields are converted to DuckDB blob literal format, optionally with an + explicit BLOB cast. Other fields use standard quoted literal format. @param field Field whose default value to read (must not be at offset) @param offset Offset from record[0] to default_values (s->default_values - record[0]) + @param cast_bit Add an explicit BLOB cast for BIT fields @return Default value string, or "NULL" if field is null at default record */ static std::string get_field_default_for_duckdb(Field *field, - my_ptrdiff_t offset) + my_ptrdiff_t offset, + bool cast_bit= true) { field->move_field_offset(offset); @@ -295,7 +297,9 @@ static std::string get_field_default_for_duckdb(Field *field, ss << hx; } } - ss << "'::BLOB"; + ss << "'"; + if (cast_bit) + ss << "::BLOB"; default_value= ss.str(); } else @@ -303,7 +307,7 @@ static std::string get_field_default_for_duckdb(Field *field, char buf[MAX_FIELD_WIDTH]; String str(buf, sizeof(buf), system_charset_info); String *val= field->val_str(&str); - if (val && val->length() > 0) + if (val) { /* Escape the literal by doubling any embedded single quote so a crafted @@ -312,11 +316,14 @@ static std::string get_field_default_for_duckdb(Field *field, charset aware and performs exactly this doubling. */ std::string escaped(2 * val->length(), '\0'); - my_bool overflow; - size_t escaped_len= escape_quotes_for_mysql(val->charset(), &escaped[0], - 0, val->ptr(), val->length(), - &overflow); - escaped.resize(escaped_len); + if (val->length()) + { + my_bool overflow; + size_t escaped_len= escape_quotes_for_mysql( + val->charset(), &escaped[0], 0, val->ptr(), val->length(), + &overflow); + escaped.resize(escaped_len); + } default_value= "'" + escaped + "'"; } else @@ -848,7 +855,11 @@ void AddColumnConvertor::prepare_columns() m_columns_to_add.emplace_back(new_field, field); if ((new_field->flags & NOT_NULL_FLAG) != 0) + { m_columns_to_set_not_null.emplace_back(new_field, field); + if ((field->flags & NO_DEFAULT_VALUE_FLAG) != 0) + m_columns_to_drop_default.emplace_back(new_field, field); + } } } @@ -896,9 +907,16 @@ std::string AddColumnConvertor::translate() my_ptrdiff_t offset= field->table->s->default_values - field->table->record[0]; has_default= true; - default_value= get_field_default_for_duckdb(field, offset); + default_value= get_field_default_for_duckdb(field, offset, false); } } + else if (field->flags & NOT_NULL_FLAG) + { + my_ptrdiff_t offset= + field->table->s->default_values - field->table->record[0]; + has_default= true; + default_value= get_field_default_for_duckdb(field, offset, false); + } append_stmt_column_add(result, m_schema_name, m_table_name, new_field->field_name.str, type, has_default, @@ -913,6 +931,13 @@ std::string AddColumnConvertor::translate() new_field->field_name.str); } + for (auto &pair : m_columns_to_drop_default) + { + Create_field *new_field= pair.first; + append_stmt_column_drop_default(result, m_schema_name, m_table_name, + new_field->field_name.str); + } + return result.str(); } diff --git a/storage/duckdb/convertor/ddl_convertor.h b/storage/duckdb/convertor/ddl_convertor.h index 75871fc066a17..6cb33633ca173 100644 --- a/storage/duckdb/convertor/ddl_convertor.h +++ b/storage/duckdb/convertor/ddl_convertor.h @@ -245,6 +245,9 @@ class AddColumnConvertor : public AlterTableConvertor /** Columns to set not null */ Columns m_columns_to_set_not_null; + /** Columns whose temporary default must be dropped */ + Columns m_columns_to_drop_default; + /** Prepare columns to add and set not null. */ void prepare_columns(); }; diff --git a/storage/duckdb/docs/architecture.md b/storage/duckdb/docs/architecture.md index 4e773431716f3..b87a027b984fd 100644 --- a/storage/duckdb/docs/architecture.md +++ b/storage/duckdb/docs/architecture.md @@ -190,10 +190,13 @@ check_if_supported_inplace_alter() → HA_ALTER_INPLACE_NO_LOCK commit_inplace_alter_table() → AddColumnConvertor / DropColumnConvertor / ChangeColumnConvertor / ChangeColumnDefaultConvertor / ChangeColumnForPrimaryKeyConvertor - → each operation executes in a separate auto-commit context - (DuckDB v1.5+ disallows compound DDL mixing structural + constraint changes) + → all generated operations execute in one explicit DuckDB transaction ``` +Table partitioning is disabled with `HTON_NO_PARTITION`; both +`CREATE TABLE ... PARTITION BY` and `ALTER TABLE ... PARTITION BY` are rejected +before MariaDB creates a partition handler or starts the table-copy protocol. + DROP DATABASE: `duckdb_drop_database()` → `DROP SCHEMA IF EXISTS "db"`. ### Path 3: Row-by-Row DML diff --git a/storage/duckdb/ha_duckdb.cc b/storage/duckdb/ha_duckdb.cc index e5b3b989e78df..62d08c8a075c2 100644 --- a/storage/duckdb/ha_duckdb.cc +++ b/storage/duckdb/ha_duckdb.cc @@ -229,7 +229,7 @@ static int duckdb_init_func(void *p) duckdb_hton= (handlerton *) p; duckdb_hton->db_type= DB_TYPE_AUTOASSIGN; duckdb_hton->create= duckdb_create_handler; - duckdb_hton->flags= HTON_TEMPORARY_NOT_SUPPORTED; + duckdb_hton->flags= HTON_TEMPORARY_NOT_SUPPORTED | HTON_NO_PARTITION; duckdb_hton->prepare= duckdb_prepare; duckdb_hton->commit= duckdb_commit; duckdb_hton->rollback= duckdb_rollback; @@ -1274,6 +1274,9 @@ ha_duckdb::check_if_supported_inplace_alter(TABLE *altered_table, if (ha_alter_info->alter_info->flags & ALTER_COLUMN_ORDER) DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED); + if (ha_alter_info->error_if_not_empty) + DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED); + /* Reject ALTER on tables without PK when require_primary_key is ON */ if (myduck::require_primary_key && table->s->primary_key == MAX_KEY) { @@ -1353,29 +1356,53 @@ bool ha_duckdb::commit_inplace_alter_table(TABLE *altered_table, if (convertors.empty()) DBUG_RETURN(false); - /* Execute each ALTER operation in its own auto-commit context. - DuckDB v1.5+ does not allow compound DDL that mixes structural - changes (ADD COLUMN) with constraint updates (SET DEFAULT) - within the same transaction. */ - auto con= myduck::DuckdbManager::CreateConnection(); - + std::vector statements; for (auto &conv : convertors) { if (!conv || conv->check()) DBUG_RETURN(true); std::string sql= conv->translate(); - if (sql.empty()) - continue; + if (!sql.empty()) + statements.push_back(std::move(sql)); + } + + if (statements.empty()) + DBUG_RETURN(false); + + /* A single MariaDB ALTER TABLE can produce multiple DuckDB statements. + Execute the generated operations atomically on a dedicated connection. */ + auto con= myduck::DuckdbManager::CreateConnection(); + auto query_result= myduck::duckdb_query(*con, "BEGIN"); + if (query_result->HasError()) + { + my_error(ER_GET_ERRMSG, MYF(0), HA_ERR_GENERIC, + query_result->GetError().c_str(), "DuckDB"); + DBUG_RETURN(true); + } - auto query_result= myduck::duckdb_query(*con, sql); + for (const auto &sql : statements) + { + query_result= myduck::duckdb_query(*con, sql); if (query_result->HasError()) { - my_error(ER_GET_ERRMSG, MYF(0), HA_ERR_GENERIC, query_result->GetError().c_str(), "DuckDB"); + std::string error= query_result->GetError(); + myduck::duckdb_query(*con, "ROLLBACK"); + my_error(ER_GET_ERRMSG, MYF(0), HA_ERR_GENERIC, error.c_str(), "DuckDB"); DBUG_RETURN(true); } } + query_result= myduck::duckdb_query(*con, "COMMIT"); + if (query_result->HasError()) + { + std::string error= query_result->GetError(); + if (con->HasActiveTransaction()) + myduck::duckdb_query(*con, "ROLLBACK"); + my_error(ER_GET_ERRMSG, MYF(0), HA_ERR_GENERIC, error.c_str(), "DuckDB"); + DBUG_RETURN(true); + } + DBUG_RETURN(false); } diff --git a/storage/duckdb/mysql-test/duckdb/r/alter_duckdb_column.result b/storage/duckdb/mysql-test/duckdb/r/alter_duckdb_column.result index 0e3040ddcc742..7be07dd029436 100644 --- a/storage/duckdb/mysql-test/duckdb/r/alter_duckdb_column.result +++ b/storage/duckdb/mysql-test/duckdb/r/alter_duckdb_column.result @@ -625,7 +625,7 @@ VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR db_alter_col t id NULL NO INTEGER NULL db_alter_col t B0 CAST('\x00' AS "BLOB") NO BLOB NULL db_alter_col t B1 CAST('\x00\x00\x0D\x05' AS "BLOB") NO BLOB NULL -db_alter_col t B2 CAST('\x00\x00\x00\x00\x00\x00\x00\x1F' AS "BLOB") NO BLOB NULL +db_alter_col t B2 '\x00\x00\x00\x00\x00\x00\x00\x1F' NO BLOB NULL SELECT TABLE_SCHEMA, TABLE_NAME, COLUMN_NAME, COLUMN_DEFAULT, IS_NULLABLE, DATA_TYPE, COLUMN_COMMENT FROM information_schema.columns WHERE TABLE_NAME = 't'; @@ -1503,7 +1503,7 @@ VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR VARCHAR db_alter_col t id NULL NO INTEGER NULL db_alter_col t B0 CAST('\x00' AS "BLOB") NO BLOB NULL db_alter_col t B1 CAST('\x00\x00\x0D\x05' AS "BLOB") NO BLOB NULL -db_alter_col t B2 CAST('\x00\x00\x00\x00\x00\x00\x00\x1F' AS "BLOB") NO BLOB NULL +db_alter_col t B2 '\x00\x00\x00\x00\x00\x00\x00\x1F' NO BLOB NULL SELECT TABLE_SCHEMA, TABLE_NAME, COLUMN_NAME, COLUMN_DEFAULT, IS_NULLABLE, DATA_TYPE, COLUMN_COMMENT FROM information_schema.columns WHERE TABLE_NAME = 't'; diff --git a/storage/duckdb/mysql-test/duckdb/r/mdev_40651.result b/storage/duckdb/mysql-test/duckdb/r/mdev_40651.result new file mode 100644 index 0000000000000..53ec412f791be --- /dev/null +++ b/storage/duckdb/mysql-test/duckdb/r/mdev_40651.result @@ -0,0 +1,25 @@ +CREATE TABLE t (c1 INT KEY) ENGINE=DuckDB; +INSERT INTO t VALUES (1),(1); +ALTER TABLE t ADD c2 INT NOT NULL; +SELECT * FROM t ORDER BY c1, c2; +c1 c2 +1 0 +1 0 +INSERT INTO t (c1) VALUES (2); +ERROR HY000: Field 'c2' doesn't have a default value +ALTER TABLE t PARTITION BY HASH (c1) (PARTITION p1, PARTITION p2); +ERROR HY000: Engine cannot be used in partitioned tables +SELECT * FROM t ORDER BY c1, c2; +c1 c2 +1 0 +1 0 +DROP TABLE t; +CREATE TABLE t (id INT PRIMARY KEY, c1 INT) ENGINE=DuckDB; +INSERT INTO t VALUES (1, NULL); +ALTER TABLE t ADD c2 INT, MODIFY c1 INT NOT NULL; +ERROR HY000: Got error 168 'Constraint Error: NOT NULL constraint failed: t.c1' from DuckDB +ALTER TABLE t ADD c2 INT; +SELECT * FROM t ORDER BY id; +id c1 c2 +1 NULL NULL +DROP TABLE t; diff --git a/storage/duckdb/mysql-test/duckdb/t/mdev_40651.test b/storage/duckdb/mysql-test/duckdb/t/mdev_40651.test new file mode 100644 index 0000000000000..086242246619a --- /dev/null +++ b/storage/duckdb/mysql-test/duckdb/t/mdev_40651.test @@ -0,0 +1,24 @@ +--source ../include/have_duckdb.inc +--source include/have_partition.inc +--source include/not_msan.inc + +# MDEV-40651: keep DuckDB DDL atomic and preserve implicit defaults. +CREATE TABLE t (c1 INT KEY) ENGINE=DuckDB; +INSERT INTO t VALUES (1),(1); +ALTER TABLE t ADD c2 INT NOT NULL; +SELECT * FROM t ORDER BY c1, c2; +--error ER_NO_DEFAULT_FOR_FIELD +INSERT INTO t (c1) VALUES (2); +--error ER_PARTITION_MERGE_ERROR +ALTER TABLE t PARTITION BY HASH (c1) (PARTITION p1, PARTITION p2); +SELECT * FROM t ORDER BY c1, c2; +DROP TABLE t; + +# A later DuckDB DDL error rolls back preceding generated statements. +CREATE TABLE t (id INT PRIMARY KEY, c1 INT) ENGINE=DuckDB; +INSERT INTO t VALUES (1, NULL); +--error ER_GET_ERRMSG +ALTER TABLE t ADD c2 INT, MODIFY c1 INT NOT NULL; +ALTER TABLE t ADD c2 INT; +SELECT * FROM t ORDER BY id; +DROP TABLE t; diff --git a/storage/duckdb/runtime/duckdb_query.cc b/storage/duckdb/runtime/duckdb_query.cc index a3d9030e3fe76..fc0c39a171597 100644 --- a/storage/duckdb/runtime/duckdb_query.cc +++ b/storage/duckdb/runtime/duckdb_query.cc @@ -135,7 +135,8 @@ bool mariadb_query_has_unsafe_quote_escape(THD *thd, const char *query, } /* - Convert MariaDB's printed SQL (backtick-quoted identifiers) into DuckDB SQL + Convert forwarded MariaDB SQL (the raw thd->query() text, plus any + Item::print() fragments) from backtick-quoted identifiers into DuckDB SQL (double-quoted identifiers). MariaDB delimits identifiers with backticks and doubles an embedded backtick;