Skip to content

[POC] Prototyping - MySQL User Defined Types - #707

Open
marcalff wants to merge 16 commits into
mysql:trunkfrom
marcalff:mysql-udt
Open

[POC] Prototyping - MySQL User Defined Types#707
marcalff wants to merge 16 commits into
mysql:trunkfrom
marcalff:mysql-udt

Conversation

@marcalff

@marcalff marcalff commented Aug 3, 2026

Copy link
Copy Markdown
Member

Proof Of Concepts - MySQL User Defined Types

Contributes to #674

Caution

Proof of concepts code, do not merge, do not use in production.

This code is dirty and to throw away.
The point of this PR is to do prototyping, to identify what needs to be changed in the server, to start a technical discussion.

Quick start

Build

Compile with -DWITH_EXPERIMENTAL_UDT=ON -DWITH_DEBUG=ON in CMake.

Run

mtr --suite=udt

Current progress

Declare a UDT and provide an implementation:

CREATE TYPE test.complex_number AS BINARY(16);

INSTALL COMPONENT "file://component_udt_example";

Declare a UDT variable, perform operations provided by the component:

delimiter $$

CREATE PROCEDURE test.complex()
BEGIN
  DECLARE a test.complex_number;
  DECLARE b test.complex_number;
  DECLARE c test.complex_number;
  SET a = complex_number_from_string("1+2i");
  SET b = complex_number_from_string("3+4i");
  SET c = complex_number_add(a, b);
  # SELECT complex_number_to_string(c);
END$$

delimiter ;

Invoke the UDT implementation at runtime:

call test.complex();

Cleanup:

UNINSTALL COMPONENT "file://component_udt_example";

DROP PROCEDURE test.complex;

Component implementation

Check files under components/udt_example.

@marcalff
marcalff requested a review from seemasundara as a code owner August 3, 2026 14:30
@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 3, 2026
@github-actions github-actions Bot added InnoDB Changes touching InnoDB storage engine code Pluggable Changes touching plugins or components Build Changes touching build or GitHub automation Tests Changes touching test code or test data Build Failed PR build failed labels Aug 3, 2026
@github-actions
github-actions Bot requested a review from gopshank August 3, 2026 14:31
@github-actions github-actions Bot added Review Requested Review requested from code owners MTR Failed MTR suite failed labels Aug 3, 2026
@marcalff marcalff self-assigned this Aug 3, 2026
@marcalff marcalff added the User Defined Types User Defined Types project label Aug 3, 2026
@marcalff

Copy link
Copy Markdown
Member Author

Can now add complex numbers.
@marcalff
marcalff requested a review from a team August 18, 2026 22:42
@ogrovlen ogrovlen added the Optimizer Changes touching optimizer code label Aug 20, 2026
Run untrusted pull request builds with restricted permissions against
validated revisions, and publish statuses and labels only from trusted
workflows that revalidate the repository, workflow run, PR head, and
ordering.

Replace the custom review client with the pinned OpenAI Codex Action,
bound its input to a validated PR diff, pin third-party actions, and
add dependency maintenance for GitHub Actions.

Retry a failed or empty Codex review once after a delay with a
configurable fallback model while preserving the same read-only
isolation boundary and structured output contract.

Publish structured Codex findings as one commit-bound GitHub review.
Validate each file and right-side line range against the current diff,
keep unanchored findings in the summary, prevent duplicate reviews, and
revalidate both reviewed revisions before posting.

Warm trusted Boost and ccache entries, align the MTR compiler cache
with the GCC build, shard MTR suites across runners, run tests in
parallel  with bounded retries, and retain diagnostics. Safely reset
head-scoped CI state, standardize labels, and remove the obsolete OCA
checkbox.

Require both the OCA Verified label and a current trusted approval
before adding Integrate. Revalidate both conditions around label
publication and remove Integrate if either condition no longer holds.

Temporarily disable parallel-run failures tracked by Bug#39882117 and
restore the required restart and expected output for the
buffer-pool-load MTR.

Change-Id: I7393e75cab3afa172a99237337c26e3974f955fa
@RidhaOracle

Copy link
Copy Markdown
Member

@marcalff The fix for issue #715 has been merged, can you please rebase your changes to latest trunk ?

@github-actions github-actions Bot removed Build Failed PR build failed MTR Failed MTR suite failed Optimizer Changes touching optimizer code labels Aug 21, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (99% confidence)

The change introduces server-crash paths, stale callbacks after component unload, missing authorization and replication handling, unbounded runtime leaks, and discards the type definition itself.

7 finding(s) were posted inline.

Comment thread sql/sql_cmd_ddl_type.cc
Comment thread sql/sql_cmd_ddl_type.cc
return true;
}

if (dd::create_udt_type(thd, *existing_schema, type_name)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Enforce privileges before storing the type

This is the point where persistent dictionary state is created, but execute() performs no access check first. Any authenticated user able to issue the statement can therefore create types in schemas where they have no DDL privileges. Check the appropriate schema-level privilege before acquiring locks or modifying the dictionary.

Confidence: 99%

Comment thread sql/sql_cmd_ddl_type.cc
return true;
}

if (trans_commit_stmt(thd) || trans_commit(thd)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Binlog CREATE TYPE before committing

The new dictionary object is committed without writing the statement to the binary log. On replicated or point-in-time-recovered systems, the type will exist only on the originating server and later routines referring to it will fail elsewhere. Binlog the DDL before committing and roll back both statement and transaction if logging or commit fails.

Confidence: 98%

Comment thread sql/sql_udt.cc
Comment thread sql/sql_udt.cc
Comment thread sql/sql_udt.cc
Comment thread sql/sql_yacc.yy Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (99% confidence)

The change introduces server-crash paths, missing DDL authorization, unsafe component unload behavior, and fails to retain the defining type metadata.

6 finding(s) were posted inline.

Comment thread sql/sql_udt.cc
Comment thread sql/sql_udt.cc
Comment thread sql/sql_yacc.yy Outdated
Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +35 to +36
#ifdef WITH_EXPERIMENTAL_UDT
WARN_NOT_IMPLEMENTED(thd, "Sql_cmd_create_type::execute()");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Check CREATE TYPE privileges before modifying the dictionary

The command locks and stores a dictionary object without any authorization check. Consequently, a user able to issue SQL can create types in schemas where they lack CREATE/appropriate DDL privileges. Perform the schema-level privilege check before acquiring locks or writing the DD object.

Confidence: 94%

Comment thread sql/sql_cmd_ddl_type.cc
Comment thread sql/sql_udt.cc
@github-actions github-actions Bot added the Build Passed PR build passed label Aug 28, 2026
@github-actions github-actions Bot removed the Build Passed PR build passed label Aug 28, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (99% confidence)

The change has authorization and replication gaps, unsafe component-unload behavior, and several defects that prevent declared and returned UDT values from retaining their intended types.

6 finding(s) were posted inline.

Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +36 to +38
WARN_NOT_IMPLEMENTED(thd, "Sql_cmd_create_type::execute()");

if (m_type_ident->db.length == 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Enforce privileges before creating a type

This execution path never performs an access check before writing directly to the data dictionary. Consequently, a user without an appropriate CREATE privilege can create types in any existing schema. Check the relevant schema-level privilege before acquiring locks or storing the object.

Confidence: 99%

Comment thread sql/sql_cmd_ddl_type.cc
return true;
}

if (trans_commit_stmt(thd) || trans_commit(thd)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Write CREATE TYPE to the binary log

The new dictionary object is committed without first logging the CREATE TYPE statement. On a binlogged server, replicas therefore never create the type and subsequently fail to resolve routines or declarations that reference it. Binlog the statement before committing and roll back the dictionary transaction if logging fails.

Confidence: 99%

Comment thread sql/sql_udt.cc
Comment thread sql/sql_user_defined_type.cc
Comment thread sql/sql_udt.cc
Comment on lines +367 to +368
// to->m_length = from->length;
// to->m_dec = from->decimals;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Apply component descriptor lengths to result fields

The conversion drops from->length and from->decimals, leaving the TypeDescriptor defaults. In particular, a registered VARCHAR return type creates a zero-width result field, so storing a nonempty component result truncates it; other size- or precision-sensitive types are likewise misrepresented. Convert these numeric descriptor values into the representation expected by Create_field::init, or make TypeDescriptor retain them numerically.

Confidence: 96%

Comment thread sql/sql_udt.cc
Comment on lines +514 to +517
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Return the actual save result

Even when evaluate_to_field succeeds, this method always reports TYPE_ERR_BAD_VALUE. Callers that honor conversion status, such as key materialization paths, consequently treat valid UDT function values as fatal conversion failures. Propagate evaluation failure appropriately and return TYPE_OK after a successful save.

Confidence: 95%

@oracle-contributor-agreement

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement Bot added OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. and removed OCA Verified All contributors have signed the Oracle Contributor Agreement. labels Sep 2, 2026
@github-actions github-actions Bot added Build Failed PR build failed and removed Build Passed PR build passed Build Failed PR build failed labels Sep 2, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (99% confidence)

The new UDT implementation permits unauthorized, unreplicated DDL and contains fundamental type-resolution and component-lifetime defects that can produce incorrect schemas or server crashes.

6 finding(s) were posted inline.

Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +32 to +36
bool Sql_cmd_create_type::execute(THD *thd) {
bool rc;

#ifdef WITH_EXPERIMENTAL_UDT
WARN_NOT_IMPLEMENTED(thd, "Sql_cmd_create_type::execute()");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Enforce authorization before creating a type

This execution path writes directly to the data dictionary without checking any global or schema privilege. Consequently, an authenticated user able to issue the statement can create a type in any existing schema. Check the appropriate CREATE privilege before acquiring locks or storing the object.

Confidence: 96%

Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +114 to +118
if (dd::create_udt_type(thd, *existing_schema, type_name)) {
return true;
}

if (trans_commit_stmt(thd) || trans_commit(thd)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Write CREATE TYPE to the binary log

The successful DDL path commits the dictionary transaction without calling thd->binlog_query. On a binlogged server, replicas therefore never create the type and subsequently diverge or reject statements that reference it. Log the statement before committing and roll back if logging fails.

Confidence: 98%

Comment thread sql/sql_udt.cc
Comment thread sql/sql_user_defined_type.cc
Comment on lines +1021 to +1024
explicit PT_user_defined_type(const POS &pos, Type_ident *ident)
: PT_type(pos, MYSQL_TYPE_INVALID), type_ident(ident) {}

const Type_ident *get_type_ident() const override { return type_ident; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Resolve UDTs used in ordinary column definitions

This node leaves its native type as MYSQL_TYPE_INVALID, while the existing PT_field_def path reads only that native type and never consumes get_type_ident(). Thus the newly accepted CREATE TABLE ... (c schema.type) syntax passes an invalid type into Alter_info::add_field rather than resolving the UDT. Carry the identifier through field contextualization and invoke resolve_type_descriptor before constructing the Create_field.

Confidence: 97%

Comment thread sql/sql_udt.cc
Comment on lines +514 to +517
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Return success after saving a successful UDT value

This method ignores whether evaluation succeeded and always returns TYPE_ERR_BAD_VALUE. Callers using the normal save_in_field path therefore treat successfully stored UDT-function results as conversion failures. Return TYPE_OK after successful evaluation and an error status only when evaluate_to_field fails.

Confidence: 98%

@oracle-contributor-agreement oracle-contributor-agreement Bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Sep 2, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (98% confidence)

The patch introduces server-crash lifecycle/NULL paths and leaves core UDT storage, column resolution, and DD upgrade behavior incomplete.

6 finding(s) were posted inline.

Comment thread sql/sql_udt.cc
Comment thread sql/sql_udt.cc
Comment on lines +251 to +252
String *data = m_item->val_str(&m_string_data);
*val = reinterpret_cast<unsigned char *>(data->ptr());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Handle NULL results before dereferencing val_str

val_str() legitimately returns nullptr for SQL NULL, but get_blob() immediately dereferences it. For example, nesting a NULL-producing UDT function as an argument makes Item_udt_func::is_null() initially report false, then this path crashes when evaluating the value. Check the returned pointer and propagate NULL through the value API; get_null() must also evaluate the current item reliably.

Confidence: 97%

Comment thread sql/sql_udt.cc
Comment on lines +514 to +517
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Return success after saving a valid UDT value

The function result is written to the destination field, but this method unconditionally returns TYPE_ERR_BAD_VALUE and even ignores evaluation failure. Any execution path using save_in_field() can therefore reject or warn about every valid UDT result. Return TYPE_OK when evaluate_to_field() succeeds and an appropriate error status when it fails.

Confidence: 99%

Comment thread sql/sql_yacc.yy Outdated
Comment thread sql/sql_yacc.yy

field_def:
type opt_column_attribute_list
broken_type /* FIXME: opt_collate */ opt_column_attribute_list

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Resolve UDTs used in table column definitions

Changing field_def to accept broken_type exposes user-defined types in CREATE TABLE, but PT_user_defined_type supplies MYSQL_TYPE_INVALID and the existing field contextualization neither retains get_type_ident() nor calls resolve_type_descriptor(). Thus accepted syntax such as CREATE TABLE t(c test.usbn13) reaches field creation with an invalid type. Propagate and resolve the type identifier for columns, or do not add UDTs to this grammar path yet.

Confidence: 94%

Comment thread sql/dd/impl/system_registry.cc
@github-actions github-actions Bot added the Build Passed PR build passed label Sep 2, 2026
@github-actions github-actions Bot removed the Build Passed PR build passed label Sep 4, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (99% confidence)

The new UDT implementation contains an authorization bypass, multiple server-crash paths, and incorrect type and conversion behavior.

5 finding(s) were posted inline.

Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +38 to +39
if (m_type_ident->db.length == 0) {
m_type_ident->db = thd->db();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Enforce privileges before creating a type

This execution path performs no access check before storing an object in the target schema, allowing an authenticated user without CREATE privileges to create types. Check the appropriate schema-level privilege (for example, CREATE_ACL) before acquiring locks or modifying the dictionary.

Confidence: 99%

Comment thread sql/sql_udt.cc
Comment on lines +313 to +314
// FIXME: lifecycle, may be in use
udt_function_hash->erase(key);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Keep registered functions alive while statements reference them

Unregistration erases the function even when ref_count is nonzero. Prepared statements and cached stored-program item trees retain pointers to the component-owned descriptor and implementation, so uninstalling the component and later executing them can call unloaded code. Items must release their references during cleanup, and unregistration must defer removal/unloading or fail while references remain.

Confidence: 98%

Comment thread sql/sql_udt.cc
Comment on lines +239 to +241
String *data = m_item->val_str(&m_string_data);
*str = data->ptr();
*length = data->length();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Handle dynamically evaluated NULL arguments

The null service uses Item::is_null(), whose base implementation returns false without evaluating most expressions. A dynamically null string expression therefore reaches this code with data == nullptr, and dereferencing it can crash the server. Evaluate the argument and propagate its null_value, checking the returned String * before dereferencing; the blob getter has the same issue.

Confidence: 97%

Comment on lines +142 to +145
// FIXME: forged BINARY(16)
td->m_type = MYSQL_TYPE_BLOB;
td->m_type_flags = 0;
td->m_length = "16";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Resolve the type definition stored by CREATE TYPE

Every user-defined type is resolved as BLOB(16) regardless of its declared base type. For example, CREATE TYPE ... AS CHAR(13) subsequently creates binary blob variables rather than character variables. Persist the parsed m_type descriptor in the DD object and reconstruct TypeDescriptor from that data here.

Confidence: 100%

Comment thread sql/sql_udt.cc
Comment on lines +524 to +527
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Return the actual field-evaluation status

This method always reports TYPE_ERR_BAD_VALUE, even when evaluate_to_field() succeeds. Callers that honor the conversion status will treat valid UDT function assignments as failed. Return TYPE_OK on a false evaluation result and TYPE_ERR_BAD_VALUE only on failure.

Confidence: 98%

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (98% confidence)

Core UDT metadata is lost, table-column UDTs are not resolved, result stores signal failure, and runtime registration has unsafe type and lifetime handling.

4 finding(s) were posted inline.

Findings without a current diff anchor

These findings remain in the summary because their locations could not be verified against the current GitHub diff.

P1: [P1] Resolve UDTs before constructing table columns

Adding user_defined_type to field_def leaves PT_field_def_base::type as MYSQL_TYPE_INVALID; the existing table-column contextualization passes that directly to Create_field::init and never propagates or resolves Type_ident. Thus CREATE/ALTER TABLE ... (c schema.type) cannot construct a valid column. Carry the identifier into the field definition, resolve it, and call the descriptor-aware initializer.

Confidence: 96%

sql/sql_yacc.yy:7206-7206

Comment thread sql/sql_cmd_ddl_type.cc
return true;
}

if (dd::create_udt_type(thd, *existing_schema, type_name)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Persist the declared underlying type

m_type from CREATE TYPE ... AS ... is discarded here, and the DD object stores only the type name. Consequently CHAR(13) and BINARY(16) declarations become indistinguishable, while resolution later hard-codes every UDT to BLOB(16). Store the parsed base-type descriptor in the DD object and restore it during resolution.

Confidence: 99%

Comment thread sql/sql_udt.cc
Comment on lines +524 to +527
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Do not report successful stores as bad values

The function result is written to field, but this method unconditionally returns TYPE_ERR_BAD_VALUE, even when evaluation succeeds. Callers such as assignments, inserts, and materialization therefore treat valid UDT results as conversion failures. Return an error only when evaluate_to_field fails, otherwise return TYPE_OK.

Confidence: 99%

Comment thread sql/sql_udt.cc
Comment on lines +313 to +314
// FIXME: lifecycle, may be in use
udt_function_hash->erase(key);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Prevent unloading functions that still have live items

Parsing retains a pointer to the registry record and component-owned descriptor/callback, but unregister erases it regardless of ref_count; moreover Item_udt_func never releases its acquired reference. Uninstalling a component while a prepared statement or concurrent execution still references its function can subsequently call unloaded code or dereference unloaded descriptor memory. Release references from item destruction and reject/defer unregister while references remain.

Confidence: 94%

Comment thread sql/sql_udt.cc
Comment on lines +500 to +503
for (size_t i = 0; i < count; i++) {
// FIXME: build proper value
item = that->get_arg(i);
array[i] = new UDT_value_in(item);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Enforce registered argument descriptors

Argument construction checks only the count and wraps each raw Item; fd->argument_type_array is never consulted. A call with incompatible SQL values therefore reaches component code as if correctly typed—for example the example blob decoder receives short strings and consumes an uninitialized 16-byte buffer. Resolve/coerce each argument against its registered descriptor or reject the call before invoking the callback.

Confidence: 91%

@github-actions github-actions Bot added the Build Passed PR build passed label Sep 8, 2026
@github-actions github-actions Bot removed the Build Passed PR build passed label Sep 9, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex PR review

Overall: patch is incorrect (98% confidence)

The change permits unauthorized DD writes, loses declared type information, and introduces unsafe component-function lifetimes plus broken persistence paths.

5 finding(s) were posted inline.

Comment thread sql/sql_cmd_ddl_type.cc
Comment on lines +35 to +36
#ifdef WITH_EXPERIMENTAL_UDT
WARN_NOT_IMPLEMENTED(thd, "Sql_cmd_create_type::execute()");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Enforce privileges before creating a type

This execution path writes a persistent DD object without any schema privilege check. Consequently, a user lacking CREATE privileges can issue CREATE TYPE in any existing schema. Call check_access() with the appropriate CREATE privilege before acquiring locks or modifying the dictionary.

Confidence: 98%

Comment thread sql/sql_cmd_ddl_type.cc
return true;
}

if (dd::create_udt_type(thd, *existing_schema, type_name)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Persist the declared underlying type

Only the schema and type name are stored; m_type is never used and the DD object has no underlying-type attributes. Resolution subsequently hard-codes every alias as BINARY(16), so declarations such as CREATE TYPE ... AS CHAR(13) have incorrect storage and semantics. Persist the complete declared type descriptor and reconstruct it during resolution.

Confidence: 99%

Comment thread sql/sql_udt.cc
Comment on lines +313 to +314
// FIXME: lifecycle, may be in use
udt_function_hash->erase(key);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: [P1] Prevent unloading functions that are still referenced

Unregistration erases the function unconditionally despite acquire_udt_function() incrementing ref_count, and Item_udt_func never releases that reference. A prepared statement can retain the record, unload the component, and later dereference its unloaded descriptor/callback, causing a crash. Release references from item teardown and reject or defer unregistration while references remain.

Confidence: 95%

Comment on lines +71 to +73
m_target_def.add_foreign_key(FK_SCHEMA_ID, "FK_SCHEMA_ID",
"FOREIGN KEY (schema_id) "
"REFERENCES schemata(id)");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Remove UDT rows when dropping their schema

The new DD table has a restrictive foreign key to schemata, but the DROP DATABASE path has no corresponding cleanup for UDT objects. A schema containing a created type therefore retains a child row and prevents deletion of the schema record. Explicitly drop all schema-owned UDT objects during DROP DATABASE or define appropriate cascading behavior.

Confidence: 90%

Comment thread sql/sql_udt.cc
Comment on lines +524 to +527
evaluate_to_field(field);

// FIXME
return TYPE_ERR_BAD_VALUE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: [P2] Return the actual field-save status

save_in_field_inner() reports TYPE_ERR_BAD_VALUE unconditionally and ignores whether evaluation succeeded. Contexts that save a UDT function directly into a field, such as INSERT/UPDATE assignments, therefore report a conversion failure even after producing a valid value. Propagate evaluation failure and return TYPE_OK on success.

Confidence: 98%

@github-actions github-actions Bot added the Build Passed PR build passed label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Passed PR build passed Build Changes touching build or GitHub automation InnoDB Changes touching InnoDB storage engine code OCA Verified All contributors have signed the Oracle Contributor Agreement. Pluggable Changes touching plugins or components Review Requested Review requested from code owners Tests Changes touching test code or test data User Defined Types User Defined Types project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants