Skip to content

MDEV-8235: Expose check_slave_start_position as SQL function GTID_CHECK_POS()#4956

Open
ayush-jha123 wants to merge 1 commit into
MariaDB:mainfrom
ayush-jha123:mdev-8235-gtid-pos
Open

MDEV-8235: Expose check_slave_start_position as SQL function GTID_CHECK_POS()#4956
ayush-jha123 wants to merge 1 commit into
MariaDB:mainfrom
ayush-jha123:mdev-8235-gtid-pos

Conversation

@ayush-jha123
Copy link
Copy Markdown

@ayush-jha123 ayush-jha123 commented Apr 18, 2026

I’ve been diving deep into the replication source code for the last two weeks,
investigating MDEV-8235. I noticed this task has been open for a while and realized
how useful it would be for DBAs to have a simple way to check GTID availability
directly from the SQL layer.

After studying how check_slave_start_position() works internally, I’ve implemented
a user-facing version called GTID_CHECK_POS().

What this PR does:

  • Extracted the internal GTID binlog-lookup logic into a shared helper function
    in sql_repl.cc to avoid code duplication.
  • Created the Item_func_gtid_check_pos class to wrap the logic for the SQL parser.
  • Added input validation: it returns 1 (exists), 0 (purged), or NULL (invalid input)
    which makes it robust for automation scripts.
  • Included an MTR test main.gtid_check_pos covering standard and edge cases.

I’m really excited to contribute this to MariaDB and would appreciate feedback
on the implementation.

@gkodinov gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Apr 20, 2026
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

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

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

First, the formalities:

  • please keep a single commit, with a commit message in it.
  • please make sure all of the buildbot hosts compile and run successfully

Below are some of my thoughts on the functionality and its implementation and test coverage. You can ignore all of the below and take it with the final reviewer. Or address it and have a faster final review with these fixed.

Comment thread sql/item_func.h Outdated
Comment thread sql/sql_repl.cc Outdated
Comment thread sql/sql_repl.cc Outdated
Comment thread sql/sql_repl.cc Outdated
Comment thread sql/sql_repl.cc Outdated
Comment thread sql/item_func.cc Outdated
Comment thread mysql-test/main/gtid_check_pos.test
Comment thread mysql-test/main/gtid_check_pos.test
@gkodinov gkodinov self-assigned this Apr 20, 2026
@ayush-jha123 ayush-jha123 force-pushed the mdev-8235-gtid-pos branch 3 times, most recently from c4377de to bd30bf6 Compare April 26, 2026 18:30
@vuvova vuvova requested a review from knielsen May 13, 2026 10:22
Copy link
Copy Markdown
Member

@gkodinov gkodinov left a comment

Choose a reason for hiding this comment

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

It's looking much better. Please make sure to address the comments below. This is my last batch of preliminary review comments.
Thanks for your efforts.

Comment thread sql/sql_repl.cc Outdated
}



Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no space only changes please.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

revert these please

Comment thread libmariadb
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

revert this please.

Comment thread sql/item_func.cc
return 0;
#else
bool is_reachable= false;
if (rpl_gtid_pos_check_reachable(gtid_str, &is_reachable))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This fails to compile on buildbot (I'm assuming for the unit tests) as follows:

/home/buildbot/aarch64-debian-11/build/sql/item_func.cc: In member function ‘virtual bool Item_func_gtid_check_pos::val_bool()’:
/home/buildbot/aarch64-debian-11/build/sql/item_func.cc:841:7: error: ‘rpl_gtid_pos_check_reachable’ was not declared in this scope
  841 |   if (rpl_gtid_pos_check_reachable(gtid_str, &is_reachable))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Please have a look.

@ayush-jha123 ayush-jha123 force-pushed the mdev-8235-gtid-pos branch 2 times, most recently from beeba7c to ebb7195 Compare May 14, 2026 16:59
@ayush-jha123
Copy link
Copy Markdown
Author

Hi @gkodinov,

I've just pushed the updated commit addressing your latest feedback:

Compilaton: Added #include "sql_repl.h" to sql/item_func.cc to fix the scope error you observed on Buildbot.
Submodules: Reverted all changes to the libmariadb and storage/columnstore/columnstore submodules.
Whitespace: I've ensured that no space-only changes are included in the diff; the comment lines and empty lines now match the original source exactly.

Thanks for the review, let me know if there's anything else!

@ayush-jha123 ayush-jha123 force-pushed the mdev-8235-gtid-pos branch 3 times, most recently from 9783df1 to ca84483 Compare May 25, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.

Development

Successfully merging this pull request may close these issues.

2 participants