I guess the proper way of fixing this should be moving spider errors from spd_err.h to errmsg-utf8.txt, which will also make it automatically localised.

no, the proper way would be to extend plugin API to allow plugins to have their own translated error messages.
Proper plugins should have nothing outside of the plugin source directory, and in particular should not leak into server's errmsg-utf8.txt

serg I assume this rule applies to dynamic plugins only, i.e. those not in mysql_mandatory_plugins and mysql_optional_plugins, right? I mean there are plenty of things in the server code helping out builtin plugins like InnoDB, perfschema, aria etc.

it's a general rule with exception. there are few plugins who are more integrated into the server, unfortunately.
unix_socket plugin is compiled statically but it doesn't leak into the server. And rocksdb is compiled dynamically, but it does.

Spider could be, technically, compiled statically too, if you remove MODULE_ONLY from its CMakeLists.txt

> Spider could be, technically, compiled statically too, if you remove MODULE_ONLY from its CMakeLists.txt

or do PLUGIN_SPIDER:STRING=STATIC in CMakeCache.txt

Another example here.

--source plugin/spider/spider/include/init_spider.inc

SET sql_mode='';

SET spider_same_server_link= on;

eval CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (HOST "127.0.0.1", DATABASE "test", USER "root", PORT $MASTER_MYPORT);

CREATE TABLE t (a INT) ENGINE=Spider COMMENT='WRAPPER "mysql",SRV "srv",TABLE "t"';

INSERT INTO t VALUES (1);

# Cleanup

DROP TABLE t;

--source plugin/spider/spider/include/deinit_spider.inc

For the INSERT will produce a correct and expected infinite loop error:

mysqltest: At line 6: query 'INSERT INTO t VALUES (1)' failed: <Unknown> (12719): An infinite loop is detected when opening table test.t

However, this error is not known and reported by mysqltest it as <Unknown>. It would be great to have this error, as well as other Spider errors, added to mariadbd/mysqltest and perror, which currently reports it as illegal error code:

$ ./bin/perror 12719

Illegal error code: 12719

Hi Roel, how about something like this:

68bfd982fbc bb-10.5-mdev-30576 MDEV-30576 [poc] Add a script to output spider errors

After building the script should be available in the same dir as the spider plugin

It is great to see this, and great progress. Thank you ycp

/test/bb-10.5-mdev-30576_opt$ ./storage/spider/pspderr 12714

#define ER_SPIDER_TABLE_OPEN_TIMEOUT_NUM 12714

/test/bb-10.5-mdev-30576_opt$ ./storage/spider/pspderr 12501

#define ER_SPIDER_INVALID_CONNECT_INFO_NUM 12501

/test/bb-10.5-mdev-30576_opt$ ./storage/spider/pspderr 12719

#define ER_SPIDER_INFINITE_LOOP_NUM 12719

Here are my thoughts;
1. This is already very helpful, as it gives some idea what the error means.
2. We can then expand with translating the short error codes like "ER_SPIDER_INFINITE_LOOP_NUM" to a more comprehensive English error message like "[ERROR] Spider: Infinite amount of loops detected", or similar, so it becomes easy for users who use the tool (alike to perror) to understand. I guess those are available already? If so, it would be interesting to find out how they are mapped?
3. Then the ./bin/perror tool can be extended to call ./bin/pspderr (more on the ./bin part in the next point) for Spider errors (for example by defining the applicable range of Spider errors), and thus make that also user friendly. Or, can we simply just do the full work inside perror?
4. I noticed that ./scripts/make_binary_distribution does not copy the pspderr into the resulting build yet. This would be the next thing to add, and then it should go in as ./bin/pspderr (or as mentioned in the last point, the alternative is to put all Spider error resolution code into ./bin/perror if possible, wich would seem to be better).
5. I am not sure how MTR does error resolution, but it would be great if MTR also did not report Spider errors as unknown.

Thank you!

serg Hi! I reviewed your input above and also discussed things with Yuchen. I understand the input that proper plugins should have nothing outside of the plugin source directory. However, in this case I think we have an interesting possibility of helping users with only a very small code hack for perror, while not touching the server's errmsg-utf8.txt:

How about if we add something like the following (pseudo/example code) to the perror source code:

if err_code >= 12000 and err_code <=13000 then

   print ('Apparent Spider plugin error, please check the error using ./bin/pspderr instead')

For this small perror hack, it gives users a way to discover the meaning for the error they are trying to lookup with perror and learn about pspderr at the same time.
Your thoughts?

Roel I updated my patch to incorporate some of your suggestions:

8d1c68edb5a upstream/bb-10.5-mdev-30576 MDEV-30576 Add a script to output spider errors

> 1. This is already very helpful, as it gives some idea what the error means.

Great!

> 2. We can then expand with translating the short error codes like "ER_SPIDER_INFINITE_LOOP_NUM" to a more comprehensive English error message like "[ERROR] Spider: Infinite amount of loops detected", or similar, so it becomes easy for users who use the tool (alike to perror) to understand. I guess those are available already? If so, it would be interesting to find out how they are mapped?

Done. Now you can do

$ ./storage/spider/pspderr 12719

#define ER_SPIDER_INFINITE_LOOP_NUM 12719

#define ER_SPIDER_INFINITE_LOOP_STR "An infinite loop is detected when opening table %s.%s"

$ ./storage/spider/pspderr ER_SPIDER_INFINITE_LOOP

#define ER_SPIDER_INFINITE_LOOP_NUM 12719

#define ER_SPIDER_INFINITE_LOOP_STR "An infinite loop is detected when opening table %s.%s"

> 3. Then the ./bin/perror tool can be extended to call ./bin/pspderr (more on the ./bin part in the next point) for Spider errors (for example by defining the applicable range of Spider errors), and thus make that also user friendly. Or, can we simply just do the full work inside perror?

I agree with serg that spider should not leak into server code. How about something like this:

$ function perrboth() { ./extra/perror "$1" || ./storage/spider/pspderr "$1"; };

$ perrboth 1146

MariaDB error code 1146 (ER_NO_SUCH_TABLE): Table '%-.192s.%-.192s' doesn't exist

Learn more: https://mariadb.com/kb/en/e1146/

$ perrboth 12701

Illegal error code: 12701

#define ER_SPIDER_REMOTE_SERVER_GONE_AWAY_NUM 12701

#define ER_SPIDER_REMOTE_SERVER_GONE_AWAY_STR "Remote MariaDB server has gone away"

> 4. I noticed that ./scripts/make_binary_distribution does not copy the pspderr into the resulting build yet. This would be the next thing to add, and then it should go in as ./bin/pspderr (or as mentioned in the last point, the alternative is to put all Spider error resolution code into ./bin/perror if possible, wich would seem to be better).

Done. You can do

$ ./scripts/make_binary_distribution

CPack: Create package using TGZ

CPack: Install projects

CPack: - Run preinstall target for: MySQL

CPack: - Install project: MySQL []

CPack: Create package

CPack: - package: /home/ycp/source/mariadb-server/10.5/build/mariadb-10.5.27-linux-x86_64.tar.gz generated.

$ tar -xf mariadb-10.5.27-linux-x86_64.tar.gz -C /tmp

$ /tmp/mariadb-10.5.27-linux-x86_64/lib/plugin/pspderr 12719

#define ER_SPIDER_INFINITE_LOOP_NUM 12719

#define ER_SPIDER_INFINITE_LOOP_STR "An infinite loop is detected when opening table %s.%s"

> 5. I am not sure how MTR does error resolution, but it would be great if MTR also did not report Spider errors as unknown.

I agree, but I think this is the same issue as perror not being aware of spider errors.

Ideally, a plugin can provide its own errmsg file with error codes in the non-overlapping range and the server (and perror) can use it automatically. This is an old idea, but we haven't never implemented it yet.

> Ideally, a plugin can provide its own errmsg file with error codes in the non-overlapping range and the server (and perror) can use it automatically. This is an old idea, but we haven't never implemented it yet.

Sure, but if spider is the only plugin having this problem, I am not sure it justifies the effort required to implement this feature, compared to the simple solution in my current patch.

From my side I believe having Spider's own errmsg file is ideal also.