MDEV-40972: Apply JSON_TABLE ON ERROR to scalar conversion failures by DerZc · Pull Request #5714 · MariaDB/server

DerZc · 2026-09-21T04:47:22Z

JSON_TABLE discards the return status from scalar Field::store() calls. For example, converting {"a":"str"} to an INT column can return zero even with NULL ON ERROR. This change propagates conversion failures so the column applies its NULL, DEFAULT or ERROR response.

Bug report: https://jira.mariadb.org/browse/MDEV-40972

The helper now returns a boolean failure status. Its result is combined with per-column error tracking using the existing Counting_error_handler. The custom conversion handler is removed; warnings and successful rounding remain visible, and arrays/objects retain their existing diagnostic.

Regression coverage includes invalid integers, overflow, NULL/DEFAULT/ERROR policies, column and row isolation, ON EMPTY, JSON null and booleans, structural errors, rounding and strict SQL mode. The existing JSON_TABLE assertions are updated for the conversion policy, with deterministic ordering for tied values.

Validation on the final patch:

Incremental Release build passed; no install was run.
10 tests passed in normal mode and the same 10 passed with --ps-protocol: json.json_table, json.json_table_mysql, json.json_table_notembedded, json.json_table_binlog, json.json_no_table, json.type_json, main.func_json, main.select, main.type_decimal and main.type_float.
git diff --check passed.

The changes are consolidated into one commit. The full regression suite and upstream Debug/Embedded/MSan/Windows configurations were not run locally; their CI results remain pending.

CLAassistant · 2026-09-21T04:47:29Z

All committers have signed the CLA.

gkodinov

Thank you for your contribution! This is a preliminary review.

Please squash the two commits into a single one. Also, please fix the failing tests in the buildbot run.

Some optional (mostly personal opinion really) hints below. These should help you with the final review.

gkodinov · 2026-09-23T12:01:50Z


-static void store_json_in_field(Field *f, const json_engine_t *je)
+static void store_json_in_field(Field *f, const json_engine_t *je,
+                                int *store_error)


you do not need to pass up different errors. error is an error.

Thank you for the suggestion. I changed store_json_in_field() to return a boolean failure status and removed the extra output parameter. The caller only needs to know whether conversion failed; it selects the appropriate JSON_TABLE diagnostic.

gkodinov · 2026-09-23T12:03:08Z

+              error= store_json_in_json(*f, &je);
+              thd->pop_internal_handler();
+              if (!error)
+                error= error_handler.errors;


the error should be the union of all errors IMHO.

Thanks for pointing this out. The conversion status is now combined unconditionally with the handler's error flag using logical OR. The existing Counting_error_handler is scoped to each column conversion, and the regression coverage checks that a handled failure does not affect another column or the next row.

gkodinov · 2026-09-23T12:04:03Z

    if (rt == INT_RESULT || rt == DECIMAL_RESULT || rt == REAL_RESULT)
    {
-      f->store(je->value_type == JSON_VALUE_TRUE, false);
+      *store_error= f->store(je->value_type == JSON_VALUE_TRUE, false);


this is the actual fix: the code now processes the conversion error instead of dropping it onto the floor.

Thank you for identifying the core fix. The revised code directly propagates the nonzero Field::store() result to ON ERROR. The tests cover NULL, DEFAULT and ERROR responses, including invalid integers and overflow, while successful decimal rounding still returns the rounded value.

gkodinov · 2026-09-23T12:04:36Z

 }


+class Json_table_conversion_error_handler : public Internal_error_handler


I believe this is redundant and needs to go.

Thank you for the review. I removed the custom conversion-error handler and reused Counting_error_handler. Normal warnings and notes are preserved, and a rounding note alone does not trigger ON ERROR when Field::store() succeeds.

Propagate Field::store() failures to the column's ON ERROR response. Combine conversion status with per-column error tracking, using the existing handler and preserving warnings and successful rounding. Cover NULL, DEFAULT and ERROR responses, row/column isolation, ON EMPTY, JSON null and booleans, structural errors and strict SQL mode. Update existing JSON_TABLE expectations for the changed conversion policy.

DerZc · 2026-09-23T13:12:39Z

Thank you for taking the time to review this PR and for the detailed suggestions. I have simplified the error propagation, removed the custom conversion-error handler, and consolidated the changes into a single commit: 8682522. I have also addressed each inline comment.

The five failing Buildbot configurations all reported json.json_table and json.json_table_mysql. I reproduced both failures locally, updated the assertions for the conversion policy, and added focused coverage for NULL/DEFAULT/ERROR responses, overflow, row/column isolation, ON EMPTY, JSON null and booleans, structural errors, rounding and strict SQL mode. Normal warnings and notes are preserved.

Validation on the final patch: the incremental Release build passed, and all 10 selected tests passed both normally and with --ps-protocol (20 successful test executions). git diff --check also passed. No install was run.

The new upstream CI results are still pending. The Windows ARM64 workflow is awaiting maintainer approval and has not run any jobs; could you please approve it when appropriate? The local results do not replace the upstream Debug, Embedded, MSan or Windows builds.

DerZc force-pushed the fix-mdev-40972 branch from 190c5c5 to 21761be Compare September 21, 2026 05:59

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Sep 23, 2026

gkodinov requested changes Sep 23, 2026

View reviewed changes

gkodinov self-assigned this Sep 23, 2026

DerZc force-pushed the fix-mdev-40972 branch from 09388da to 8682522 Compare September 23, 2026 13:10

DerZc changed the title ~~MDEV-40972 JSON_TABLE's ON ERROR / ON EMPTY / DEFAULT Clauses Are All Silently Ignored~~ MDEV-40972: Apply JSON_TABLE ON ERROR to scalar conversion failures Sep 23, 2026

Navigation Menu

Uh oh!

MDEV-40972: Apply JSON_TABLE ON ERROR to scalar conversion failures - #5714

MDEV-40972: Apply JSON_TABLE ON ERROR to scalar conversion failures#5714
DerZc wants to merge 1 commit into
MariaDB:11.4from
DerZc:fix-mdev-40972

DerZc commented Sep 21, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Sep 21, 2026 •

edited

Loading

Uh oh!

gkodinov left a comment

Uh oh!

gkodinov Sep 23, 2026

Uh oh!

DerZc Sep 23, 2026

Uh oh!

gkodinov Sep 23, 2026

Uh oh!

DerZc Sep 23, 2026

Uh oh!

gkodinov Sep 23, 2026

Uh oh!

DerZc Sep 23, 2026

Uh oh!

gkodinov Sep 23, 2026

Uh oh!

DerZc Sep 23, 2026

Uh oh!

DerZc commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		}


		class Json_table_conversion_error_handler : public Internal_error_handler

Uh oh!

Conversation

DerZc commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkodinov left a comment

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!

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!

DerZc commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

DerZc commented Sep 21, 2026 •

edited

Loading

CLAassistant commented Sep 21, 2026 •

edited

Loading