MDEV-38574 Rename cloning functions of class Item and descendants #4557

Olernov · 2026-01-19T10:52:30Z

Rename the cloning methods of class Item and its descendants in the following way:

   do_build_clone  -> deep_copy
      build_clone  -> deep_copy_with_checks
   
   do_get_copy  -> shallow_copy
      get_copy  -> shallow_copy_with_checks

to better reflect their functionality.

This change also makes sure deep_copy_with_checks() and shallow_copy_with_checks() comprise a public interface to
the functionality while deep_copy() and shallow_copy() are protected and not accessible from outside of Item subclasses.

Rename method clone_item() of class Items and its descendants to clone_constant() to better clarity.

This commit renames the cloning methods of class Item and its descendants in the following way: (from) (to) do_build_clone -> deep_copy build_clone -> deep_copy_with_checks do_get_copy -> shallow_copy get_copy -> shallow_copy_with_checks to better reflect their functionality. This change also makes sure `deep_copy_with_checks()` and `shallow_copy_with_checks()` comprise a public interface to the functionality while `deep_copy()` and `shallow_copy()` are protected and not accessible from outside of Item subclasses.

This commit renames method `clone_item()` of class Items and its descendants to `clone_constant()` to better clarity.

spetrunia

diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl
index 59f4fa56861..b077c81eb63 160000
--- a/extra/wolfssl/wolfssl
+++ b/extra/wolfssl/wolfssl
@@ -1 +1 @@
-Subproject commit 59f4fa568615396fbf381b073b220d1e8d61e4c2
+Subproject commit b077c81eb635392e694ccedbab8b644297ec0285

Why this change? I think it's not intentional?

Olernov · 2026-01-20T11:34:31Z

diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl
index 59f4fa56861..b077c81eb63 160000
--- a/extra/wolfssl/wolfssl
+++ b/extra/wolfssl/wolfssl
@@ -1 +1 @@
-Subproject commit 59f4fa568615396fbf381b073b220d1e8d61e4c2
+Subproject commit b077c81eb635392e694ccedbab8b644297ec0285

Why this change? I think it's not intentional?

Oops, I missed that. Looks like an issue with rebase . Will roll it back

spetrunia · 2026-01-20T11:42:46Z

sql/item_cmpfunc.h


+#define PCRE2_STATIC 1             /* Important on Windows */
+#include "pcre2.h"                 /* pcre2 header file */
+


What's the above? why in this patch?

It's moved from .cc file. But why?

Otherwise I get a bunch of build errors looking like:

[build] In file included from /home/oleg/10.6-MDEV-38574-rename/sql/item.h:6681, [build] from /home/oleg/10.6-MDEV-38574-rename/sql/sql_lex.h:35, [build] from /home/oleg/10.6-MDEV-38574-rename/sql/sql_class.h:668, [build] from /home/oleg/10.6-MDEV-38574-rename/storage/connect/user_connect.cc:40: [build] /home/oleg/10.6-MDEV-38574-rename/sql/item_cmpfunc.h:3099:3: error: ‘pcre2_code’ does not name a type [build] 3099 | pcre2_code *m_pcre; [build] | ^~~~~~~~~~

It's not surprising, asitem_cmpfunc.h operates some pcre structures. Not clear how it managed to compile before, probably the order of includes was different.

spetrunia · 2026-01-20T11:57:17Z

sql/item_strfunc.h

-  { return get_item_copy<Item_func_replace_oracle>(thd, this); }
+
+protected:
+  Item *shallow_copy(THD *thd) const override { return nullptr; }


Why change the implementation?

It's a mistake, will fix.

spetrunia · 2026-01-20T12:13:56Z

I see that this commit didn't change any semantics in Item_avg_field_decimal.

But now when I see this:

  Item *deep_copy(THD *thd) const override
  { return shallow_copy_with_checks(thd); }

it looks concerning. Why do we not clone the arguments?
(I see similar thing in Items representing literals. They also have deep_copy() calling shallow_copy(). It seems fine as literals do not have any arguments...)

This question is outside of scope of this PR. It won't prevent this PR from being pushed.

spetrunia · 2026-01-20T12:16:36Z

This commit renames method clone_item() of class Items and its
descendants to clone_constant() to better clarity.

Rephrase to something like:

  Rename Item::clone() to clone_constant(), and do the same for 
  any overloads in descendant items.
  The function returns non-NULL only for items that represent constant 
  literals.

spetrunia · 2026-01-20T12:18:32Z

Same with the other commit:

This commit renames the cloning methods of class Item and
its descendants in the following way:

Rename cloning methods in class Item:

...

This change also makes sure deep_copy_with_checks() and
shallow_copy_with_checks() comprise a public interface to
the functionality while deep_copy() and shallow_copy()
are protected and not accessible from outside of Item subclasses.

Also make Item::deep_copy() and shallow_copy() protected.
Outside users should call deep_copy_with_checks() and shallow_copy_with_checks().

spetrunia · 2026-01-20T12:20:03Z

Please also apply this and squash into the first commit: more-protected.patch

Olernov · 2026-01-20T14:59:04Z

Item_avg_field_decimal

Apparently, instances of this classes are prone to failures like #4529.

Olernov added the MariaDB Corporation label Jan 19, 2026

Olernov force-pushed the 10.6-MDEV-38574-rename branch 2 times, most recently from 2b19dc1 to 3235437 Compare January 19, 2026 18:24

Olernov added 2 commits January 20, 2026 16:25

MDEV-38574 Rename cloning functions of class Item and descendants

a692ee2

This commit renames method `clone_item()` of class Items and its descendants to `clone_constant()` to better clarity.

Olernov force-pushed the 10.6-MDEV-38574-rename branch from 3235437 to a692ee2 Compare January 20, 2026 09:26

spetrunia requested changes Jan 20, 2026

View reviewed changes

spetrunia reviewed Jan 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

MDEV-38574 Rename cloning functions of class Item and descendants #4557

MDEV-38574 Rename cloning functions of class Item and descendants #4557

Olernov commented Jan 19, 2026

Uh oh!

spetrunia left a comment

Uh oh!

Olernov commented Jan 20, 2026

Uh oh!

spetrunia Jan 20, 2026

Uh oh!

spetrunia Jan 20, 2026

Uh oh!

Olernov Jan 20, 2026

Uh oh!

spetrunia Jan 20, 2026

Uh oh!

Olernov Jan 20, 2026

Uh oh!

spetrunia commented Jan 20, 2026 •

edited

Loading

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

Olernov commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants


		#define PCRE2_STATIC 1 /* Important on Windows */
		#include "pcre2.h" /* pcre2 header file */

Uh oh!

MDEV-38574 Rename cloning functions of class Item and descendants #4557

Are you sure you want to change the base?

MDEV-38574 Rename cloning functions of class Item and descendants #4557

Conversation

Olernov commented Jan 19, 2026

Uh oh!

spetrunia left a comment

Choose a reason for hiding this comment

Uh oh!

Olernov commented Jan 20, 2026

Uh oh!

spetrunia Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

spetrunia Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Olernov Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

spetrunia Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Olernov Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

spetrunia commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

spetrunia commented Jan 20, 2026

Uh oh!

Olernov commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

spetrunia commented Jan 20, 2026 •

edited

Loading