MDEV-40383:innodb_gis.point_basic fails on replay by bsrikanth-mariadb · Pull Request #5446 · MariaDB/server

bsrikanth-mariadb · 2026-07-23T09:21:07Z

There are 2 problems: -

The REPLACE statement that is recorded doesn't store the
value of geometry type field correctly.
The table definition that got recorded has fields with non-null constraint,
and no default value is specified.
Also, the "REPLACE INTO" statement that gets stored in the context,
doesn't have any value specified for these non-null fields.

Solution is to: -

When using REPLACE INTO statement, store all the non-numeric values in HEX,
whenever conversion from field's charset to output's charset is lossy.
Instead of storing only the column values that were projected in the
query, store all the non-virtual column values into the recorded
REPLACE INTO statement.

Implementation details: -

Introduce a new method is_target_cs_superset() in filesort.cc, to check
if the output charset to which field's data is being written to, a superset
of it. If so, non-numeric values being witten using REPLACE INTO
statement are stored in string representation, else they are converted to
HEX.
From join_read_const(), and join_read_system() methods in sql_select.cc,
re-read the const row for all the non-virtual fields in the table.
After the row is re-read and recorded, restore the table->read_set,
table->status, and the const row, to the value that was before.

gemini-code-assist · 2026-07-23T09:29:07Z

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

mariadb-pavithrapandith · 2026-07-24T06:36:08Z

Test failure is fixed with the changes

vuvova · 2026-07-24T15:57:42Z

+      if (require_quote && pref_str_in_hex && tmp.length())
+      {
+        output.append(STRING_WITH_LEN("0x"));
+        output.append_hex(tmp.ptr(), tmp.length());


you know the charset of tmp and charset of output. You must always use hex if tmp cannot be converted to output charset without losses. Otherwise you don't need hex. It's not an external property that you need to pass as an argument.

May be I should narrow down the scope of change. For Geometry types I can use HEX, and for others, existing string representation should work, as it was already taken care to use charsets.

No! This is the whole point. Geometry is just a special case of a binary string that cannot necessarily be converted to the target charset. It is not a property of a type, it's a property of the charset. You must use hex in all cases where you cannot perform a lossless conversion from the source to the target charset.

updated it now.

vuvova · 2026-07-24T15:59:19Z

+  join_read_system()/join_read_const() only fetch the columns present in
+  table->read_set. That's sufficient for execution, but when we record the
+  const row for replay it yields an incomplete REPLACE INTO -- columns that
+  are NOT NULL and have no default then depend on a relaxed sql_mode (see


just make sure all columns always have defaults and you won't need any of the below!

@vuvova , do you suggest generating default values for any possible column type? How feasible is this given that there may be something fancy like GEOMETRY?

Olernov · 2026-07-24T18:24:09Z

    {
      Field *field= *pfield;
+      /* Generated columns cannot be assigned a value in REPLACE INTO */
+      if (field->vcol_info)


This function is also used by dbug_format_row() / dbug_print_row(), so this brings in a regression for those (same applies below too)

Agree. However, I believe dbug_format_row() / dbug_print_row() are only used when debug is enabled. Since it is low risk method, we clubbed our changes with this. How, about if we always set print_names=true?

At least, I'd suggest adding an argument like print_virtual_cols to format_and_store_row(), otherwise silently dropping vcols looks unexpected

if we always set print_names=true

Yes, this looks more robust than positional printing

Olernov · 2026-07-26T18:58:40Z

+  table->column_bitmaps_set(&table->s->all_set, table->write_set);
+
+  int error;
+  if (is_system)


A small comment why system and const tables differ in the way of retrieving data would help

Olernov · 2026-07-26T19:13:15Z

+  Optimizer_context_recorder *rec= tab->join->thd->opt_ctx_recorder;
+
+  MY_BITMAP *save_read_set= table->read_set;
+  table->column_bitmaps_set(&table->s->all_set, table->write_set);


Widening to &table->s->all_set pulls in virtual/generated columns which then will be dropped at format_and_store_row(). If you filter those fields out here, there'll be no need to change format_and_store_row() as well as no overhead from computing the unnecesary vcols.

used this approach, and updated the PR.

gemini-code-assist · 2026-07-27T12:29:09Z

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

There are 2 problems: - 1. The REPLACE statement that is recorded doesn't store the value of geometry type field correctly. 2. The table definition that got recorded has fields with non-null constraint, and no default value is specified. Also, the "REPLACE INTO" statement that gets stored in the context, doesn't have any value specified for these non-null fields. Solution is to: - 1. When using REPLACE INTO statement, store all the non-numeric values in HEX, whenever conversion from field's charset to output's charset is lossy. 2. Instead of storing only the column values that were projected in the query, store all the non-virtual column values into the recorded REPLACE INTO statement. Implementation details: - 1. Introduce a new method is_target_cs_superset() in filesort.cc, to check if the output charset to which field's data is being written to, a superset of it. If so, non-numeric values being witten using REPLACE INTO statement are stored in string representation, else they are converted to HEX. 2. From join_read_const(), and join_read_system() methods in sql_select.cc, re-read the const row for all the non-virtual fields in the table. After the row is re-read and recorded, restore the table->read_set, table->status, and the const row, to the value that was before.

bsrikanth-mariadb added the MariaDB Corporation label Jul 23, 2026

bsrikanth-mariadb force-pushed the 13.1-MDEV-40383-innodb_gis.point_basic-fails-on-replay branch 2 times, most recently from d7ceeb9 to 12df834 Compare July 24, 2026 02:51

bsrikanth-mariadb requested a review from spetrunia July 24, 2026 09:10

vuvova reviewed Jul 24, 2026

View reviewed changes

Olernov reviewed Jul 24, 2026

View reviewed changes

Comment thread sql/filesort.cc Outdated

Olernov reviewed Jul 26, 2026

View reviewed changes

Comment thread sql/sql_select.cc Outdated

Olernov reviewed Jul 26, 2026

View reviewed changes

bsrikanth-mariadb force-pushed the 13.1-MDEV-40383-innodb_gis.point_basic-fails-on-replay branch from 12df834 to 4ebbfec Compare July 27, 2026 11:29

bsrikanth-mariadb marked this pull request as draft July 27, 2026 11:47

bsrikanth-mariadb force-pushed the 13.1-MDEV-40383-innodb_gis.point_basic-fails-on-replay branch 3 times, most recently from 9f50d3c to cf67fb5 Compare July 27, 2026 12:28

bsrikanth-mariadb marked this pull request as ready for review July 27, 2026 12:29

bsrikanth-mariadb force-pushed the 13.1-MDEV-40383-innodb_gis.point_basic-fails-on-replay branch from cf67fb5 to d817206 Compare July 27, 2026 12:35

Uh oh!

Uh oh!

Conversation

bsrikanth-mariadb commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot commented Jul 23, 2026

Uh oh!

mariadb-pavithrapandith commented Jul 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olernov Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olernov Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

bsrikanth-mariadb commented Jul 23, 2026 •

edited

Loading

Olernov Jul 24, 2026 •

edited

Loading

Olernov Jul 26, 2026 •

edited

Loading