Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PXC-4593: [Dev] Code refresh for - PXC 8.0.41 #2050

Closed
wants to merge 287 commits into from

Conversation

kamil-holubicki
Copy link
Contributor

No description provided.

Anibal Pinto and others added 30 commits September 25, 2024 13:33
…-fix]

Missed check on test to only run on debug builds.

Change-Id: I2aa1c5bed5ad2bf642d77792804ea82b4a75aefd
Change-Id: I3651390d62e8667c42a41e1ceea651852d2a05e1
Change-Id: Ib64c017dc603bce0a04eb26b2d8aa27ac67097ef
DASH IN HINT COMMENT

[post-push fix]
Test fails on --embedded with error:
sh: select#1: command not found
Hence disabling on --embedded

Approved by: Miroslav Rajcic <[email protected]>
Change-Id: I40ab28569c2f40d5f3908832c65edf4e265785e5
Every object in fts_cache_t::get_docs vector should have its cache field
set to point to the containing fts_cache_t object. When fts_reset_get_doc
re-sets the get_docs vector, it does not set the field.

Fix:
Set fts_get_doc_t::cache in fts_reset_get_doc.

Change-Id: I1947462a07ba53ce0e8d5155f220f2a78825e919
Remove text that wasn't supposed to be there in the first place.

Change-Id: I5bd4771f25fefdd4581d8636854e0930eabe345f
(cherry picked from commit 0d1164e80cf5d72fbcff4af7500197d5f08d988e)
(cherry picked from commit ce527f4cd6a573ddfe1ac054b875aee8806c0356)
Define RAPIDJSON_HAS_STDSTRING=1 for the rapidjson interface target.
Remove explicit definitions before including rapidjson headers.

Change-Id: I60aa0ea60e246d4f3638e5c05dec92602f1f74c4
(cherry picked from commit 9ad50ced4d2fdb6a8f5ebb3e28bf8b3ef0994824)
…=1091, page number=0] of

Symptom:
  Test script fails sporadically with the error that table space for the table is not found.

Cause:
  Test case is to test the working for dblwr buffer. It
  1 Stops the checkpointer
  2 Inserts some rows in table (causing changes in pages in the tablespace)
  3 These changes are REDO logged.
  4 Kill the server
  5 corrupt the pages manually
  6 Restart the server. REDOs are applied (can't be truncated as checkpointer is stopped).
  7 Read pages are corrupted, so dblwr copy is used.

 To make the test work, the REDOS generated in step 3, MUST be flushed to disk. If they are not (the failure case)
 during REDO recovery, the corresponding pages are not read. So InnoDB doesn't know that the pages are corrupted.
 And later when they are read and found corrupted, we don't have dblwr copy of page. So InnoDB doesn't load the
 tablespace and error out that tablespace is not found.

Fix:
 Make sure the REDOs generated in step 3 are flushed to disk

Change-Id: I406d93ef4b9cadffbeeae298bf5ed22b913c47e7
…cover

Symptom:
  Test script fails sporadically with the error that table space for the table is not found.

Cause:
  Test case is to test the working for dblwr buffer. It
  1 Stops the checkpointer
  2 Inserts some rows in table (causing changes in pages in the tablespace)
  3 These changes are REDO logged.
  4 Kill the server
  5 corrupt the pages manually
  6 Restart the server. REDOs are applied (can't be truncated as checkpointer is stopped).
  7 Read pages are corrupted, so dblwr copy is used.

 To make the test work, the REDOS generated in step 3, MUST be flushed to disk. If they are not (the failure case)
 during REDO recovery, corresponding pages are not read. So innodb doesn't know that the pages are corrupted. And
 later when the pages are read, they are found corrupted and InnoDB error out saying page can't be read.

Fix:
 Make sure the REDOs generated in step 3 are flushed to disk

Change-Id: I9021db62b01e0be56b921a99d5a4bf185fff7956
… errors

Add testcases to testScan showing problems with Api and Kernel
error interactions in scans.

  testScan -n ScanErrorCleanup T1

    Shows case where API error tramples kernel side error resulting
    in 'nocloseneeded' behaviour for scan, reuse and then TC state
    crash

  testScan -n ScanErrorHandling T1

    More comprehensive testcase covering combinatorial space of
    scan error sources :
      - Type : Table, Range, Range-ordered
      - Locking : Y/N
      - Delay before iteration (racing incoming signals or not)
      - When error occurs : Pre-exec, Pre-iterate, During-iterate, After
      - What error occurs : None, Early scan reject, Late scan fail, Api error

    Test checks :
      - (No crashes / no overall test timeout)
      - Number of API side Transaction objects does not increase
      - Subsequent transactions do not crash (kernel side object state ok)

Change-Id: I47a0733940c94f8e82c600b0242755b1cea68d31

Bug#37022773 (2/2) Separate handling of NdbApi kernel and Api sourced errors

Scan objects allow API interaction concurrently with the reception of
incoming signals related to the scan.

Incoming signals use the scan object's error code to manage scan
protocol error handling and subsequent closure.

Problem

This is flawed as the API may set a scan object's error code so that
subsequent closure does not operate correctly.

This can lead to :
 - Scan protocol timeout waiting for signals that the kernel will
   not send
 - Failure to close the scan resources on the kernel, leading to
   data node side state assertions.

Solution

The signal handling part of the code assumes that any error which
is set has been set by e.g. SCAN_TABREF handling.

Make this true by maintaining separate 'kernel error' and
'API/overall error' objects.

Signal handling sets only the kernel error object.
Signal handling based on the error examines only the kernel error
object
When the user makes a blocking call into the API (nextResult/close),
if the kernel error object is set, then it is copied to the API error
object so that the user becomes aware.

Refactoring

When implementing the fix, it can be seen that an analogous design is
used for SPJ, with the implementation encapsulated rather than implemented
in NdbTransactionScan.

This model is used to extend the 'execCLOSE_SCAN_REP()' method in a symmetric
way, which encapsulates the scan object's handling of errors and completions.

Change-Id: I8b2bc6471e6b4b826a543b51056dac755e25fcb6
…handling

Add transactions_full base table to ndbinfo

The existing cluster_transactions table is filtered at source
in TC to exclude ApiConnectRecords which are seized, but not
in use.
This is likely aligned with the user's expectations, but makes
some resource usage unobservable.

A new base table is added without this filtering, as a
parameterised variant of the existing table.

No user-visible view is added at this time.

Change-Id: I23cff8a6ac0f719e82c843d465504929d7bc733a

Bug#37022901 (2/7) Improve NdbApi Table and OI scan protocol timeout handling

Add reduced timeout DBUG capability to NdbAPI.

New DBUG variable allows tests to be run to show timeout behaviour
without needing to wait 6 minutes for every iteration.

Modify direct access to the value from check_send_timeout() to ensure
that it is affected when the API timeout is reduced.

Change-Id: I7af78a6c70845323ee5e13af0e5b3a8f80791b02

Bug#37022901 (3/7) Improve NdbApi Table and OI scan protocol timeout handling

Add error inserts for testing timeouts

  5112 (LQH) : Range scans get an infinite sequence of MRR ranges so that
               they never terminate.
               With a non-matching pushed filter, they will scan indefinitely
               with no results

  8124 (TC)  : SCAN_FRAGCONF signals stall indefinitely, causing scans to
               timeout, and close() requests are unable to resolve until
               error insertion is cleared

Change-Id: Id7f7f29d2eab5a87e30cef40a02817d4355d5fda

Bug#37022901 (4/7) Improve NdbApi Table and OI scan protocol timeout handling

New MTR testcase ndb_scan_protocol_timeout

New testcase added which tests system behaviour of :
  - Unordered + Ordered scans
  - 'Load' and 'Bug' timeout scenarios (via error inserts)

Test checks :
  - TC resource usage to ensure no 'leaks' of ApiConnectRecords
  - Ability to query after all tests are complete

New include file ndb_get_api_connect_count uses new
ndbinfo.ndb$transactions_full base table to observe seized but
inactice (CS_CONNECTED) ApiConnectRecords.

No result file recorded as test fails without subsequent patches.

Change-Id: I42f920e98117f6f67a3122a2879047e8535a5738

Bug#37022901 (5/7) Improve NdbApi Table and OI scan protocol timeout handling

Remove error code 4008 handling from closeTransaction()

Scan operation variants may set error 4008 on both a 'scan' and 'main'
transaction object when a scan times out.
When that main transaction is closed in closeTransaction(), the presence
of error 4008 causes it not to be closed, but rather 'forgotten'.

This results in both the main transaction object + ApiConnectRecord, and
any related scan transaction objects + their ApiconnectRecords, to be
leaked.

This occurs whether or not the scan encountering 4008 has subsequently
been closed correctly at the kernel.

The leak is detected via  assertion failures in debug builds
(as NdbList detects that some allocated resource has not been returned).

As part of fixing timeout behaviour this special error code based
behaviour is removed.

If there is some situation where the kernel side of a scan cannot be
closed then the kernel side object may be leaked (until API disconnection),
but the api side object is not.

The baseline ndb_scan_protocol_timeout test result is recorded here,
showing 4 situations, each resulting in 1 ApiConnectRecord object being
leaked at TC.

Change-Id: I1ead0a95ebfdb2ab03276df437aa4a6cc0963bd7

Bug#37022901 (6/7) Improve NdbApi Table and OI scan protocol timeout handling

Protocol timeouts mark scans as needing close at kernel

The current timeout handling results in an error being set on the object,
but not in a way that ensures that an attempt will be made to cleanup
the kernel-side of the scan when close() is called.

The behaviour then depends on whether there were any 'confirmed' receivers
at the time the error was received (unlikely).

This is changed to handle an api protocol timeout in a similar way
to receiving a SCAN_TABREF from the kernel with an error code and
the 'need_close' flag set.

The code invoked in this case is reused.

This should result in a clean close of the scan in cases where the
scan is timing out due to 'LOAD' (e.g. the scan is busy but has returned
no results for 6 minutes).

In 'BUG' cases it will result in an attempt to close the scan (which may
help, depending on the problem).

The testcase result is updated to show that the 'LOAD' cases now have no
ApiConnectRecord leak for ordered scans.

Unordered scans continue to leak as they have a further bug

Future ideas :
  - Allow users to control timeout duration
  - Allow users to set a non-error timeout (polling)
  - Have kernel send a scan HB to reset api protocol timeout

Change-Id: I92aad65f6214505cd6abd5f3e97997ac20084c9f

Bug#37022901 (7/7) Improve NdbApi Table and OI scan protocol timeout handling

Only set ReleaseOnClose if close() times out

An NdbTransaction's ReleaseOnClose member is used to indicate that the
TC ApiConnectRecord associated with the NdbTransaction is 'gone', and so
when the transaction is next closed, the API side NdbTransaction object
should return to an anonymous free pool, ready to be associated with a
different ApiConnectRecord object.

The primary use case for this is when a data node fails - all the
ApiConnectRecords it had are now gone / unreachable, so the API must
detach its NdbTransaction objects from those.

Cases where current scan code is setting this member to true :

 - executeCursor()
   Node fail detected when sending : OK

 - In unordered scan nextResult() for 3 cases
   scan timeout (-1) : Maybe
   node failure (-2) : OK
   node seq change / signal send failure (-3) : Maybe

 - In close_impl()
   When node seq is changed : OK
   Wait-for-outstanding
     timeout (-1) : Maybe
     node failure (-2) : OK
   Send-close
     signal send failure : OK
     timeout (-1) : OK
     node-failure (-2) : OK

The ordered scan nextResult() code is not setting this member to true
as it assumes that in cases where the node fails, the sequence
number will change and this will be detected in close_impl()

This explains the test result difference between the ordered and
unordered scan cases.
If there is a timeout in nextResult() in an unordered scan the
nextResult() code sets theReleaseOnClose flag even in cases where
close_impl() manages to close the scan, resulting in an ApiConnectRecord
leak.

This patch aligns the unordered nextResult() code with the ordered
nextResult() code :
  - There is no explicit setting of theReleaseOnClose flag
    for timeout *or* node failure
  - The change in node sequence number is relied on to detect
    the node failure case
  - Extra logging is added so that if there is a timeout case,
    it is logged as either :
    - Timeout occurred and was successfully handled in close()
    - Timeout occurred, and close failed to handle.

The ndb_scan_protocol_timeout testcase result is updated to show
the removal of the unordered scan 'LOAD' scenario ApiConnectRecord
leaks with this patch.

Change-Id: I289d69f8a6bb8728c071d959dde74c7b2c0cfbf0
This test should not have been pushed to the mysql-8.0 branch; it
should exist only in versions 8.4 and up.

Change-Id: I612b0bf76defdd2dfc4b9e2753a476b33f949a8f
Garbage collector number of runs when initialized to same value of
write set counter will delay the removal of values from certification
garbage collect.

This only impact scenarios of test but the solution is simple, initialize
to garbage collect runs to one more.

Change-Id: Ib47b5634fe84c462b5008bd5c210abd3cf88aab1
…=1091, page number=0] of

 Post-push fix

 Error message in error log to be ignored is not being ignored on WINDOWS

Change-Id: I593abb528c8b7a3b45cd0732888aa28672c10d5e
Analysis
--------
1. The parent table has 3 column of datatype (i) int (PK) (ii) varchar
   (iii) a virtual column which is generated by varchar column.
2. The child table has a column which is referring to the primary key
   of the parent table.
3. There is a update in the parent table which changes the primary key
   and the varchar column, so it tries to build the update node to
   propagate to the child table.
4. While building the update node it is trying to build the update
   field for virtual column which is causing the crash.

FIX
===
1. The virtual column update should not be present in the update node,
   because no column in child table can have a foreign key
   relationship with a virtual column.

2. So skip the update to the virtual column when building the update node
   for child table

Change-Id: Iff7d1f32c7c9d846453c587fc0c5b8604ec679ec
Patch 1: git add the original tarball.

https://thrysoee.dk/editline/libedit-20240808-3.1.tar.gz

Remove before checkin: config.guess config.sub, m4/*

Change-Id: I88375b9959e846a87fa62c9147710bb34492af2b
(cherry picked from commit 5b83fbb4cb38eba0448f22e1d9bc060948f273fa)
Patch 2: Add cmake code based on the .ac and .am autotools files.

No relevant changes to .ac and .am files.
Copy from old libedit directory:
  src/CMakeLists.txt
  src/config.h
and add the new files.

Rename src/makelist to src/makelist.in so we can subst @awk@ in it.
Change path to current libedit in cmake/readline.cmake.

Change-Id: I700c63d2c0a957afd35562a2c594529b85d43e62
(cherry picked from commit ae16e9ab60e161cffe3ee750d9b2fbc259d2bfa7)
Patch 3: Local patches
 - parse.c : fix maybe-uninitialized warnings
 - read.c  : handle SIGINT i.e. control-c
 - sys.h   : Get SIZE_MAX from the standard header <limits.h>

Change-Id: Ia923fda20d9d567e8efa063e1cf3d3529b49253f
(cherry picked from commit 685f25fde6fe817eb572cf90ed7c00b7761eaa2d)
Patch 4: git rm -r libedit-20221030-3.1

Change-Id: Ibdf9765c7e7e384b6f71f58488f1e03fba4512d4
(cherry picked from commit 212401300f750fb8c2db8bbef47db0180ae7f54e)
Downgrade a few compiler warnings for LTO builds.

Change-Id: I057fe0e4a4de936b48a6ccb675c2da7001f90439
(cherry picked from commit 5f3f6ebe94183277b9aa1a683552764db0de9460)
(cherry picked from commit 120595d985930d8730b7622e07d4e1c6e9f40557)
…m tablespace

Following two observations are reported in this bug.

1. Change buffer is a B-Tree that is persisted in the System tablespace.
   If innodb_file_per_table is turned OFF then tables are created in the
   system tablespace.
   Any of the above two may cause Innodb to extend the system
   tablespace but InnoDB stopped producing the MLOG_FILE_EXTEND log
   record type for the system tablespaces through WL#13782/RB#23888.
   The rationale of doing this change was clarified in the code
   comments but it doesn't justify of doing this change. We can easily
   see the system tablespace extending second case when
   innodb_file_per_table is turned off.

2. Right after extending the space size we were not updating the
   space and node(s) size variables under the shard mutex. This may
   cause IO thread(s) believing that space size and accumulated nodes
   size differ when it calls Fil_shard::validate() method.

Fix
====
1. Removed the artificial restriction of not producing the
   MLOG_FILE_EXTEND log record for the system tablespace as well.
   Also the tablespace scanning system doesn't track the system
   tablespace. Therefore, added a check to skip looking for
   system tablespace in fil_tablespace_redo_extend method.

2. Update the node and space size together under the shard mutex.
   Since we had shard handle so replaced the fil_flush() call with
   space_flush() that doesn't make any difference.

Added a test case to demonstrate that system tablespace extends when
the change buffer grows.

Change-Id: Icf53861c434565b98a22e949045393f98f9d27a9
… failing rarely

Symptom:
Debug build o mysqld occasionally fails on assertio in
ddl::Key_sort_buffer::serialize (ddl0buffer.cc):
ut_ad(ptr > io_buffer.first);

Cause:
The code around this assertio supports the case where data left in the
buffer after serializing the records would overrun the buffer if aligned
to IO_BLOCK_SIZE. In this case all complete blocks of data are persisted
to make room in the buffer for the final block. The scenario occurs
when the length of that left over data is a multiple of IO_BLOCK_SIZE;
in this case all of it is persisted, and nothing is left. The assertion
does not anticipate that situation, although the code supports it
correctly.

Fix:
Assertion is changed to allow for this scenario - after persisting
the data the buffer may be empty, and "past the end" pointer will
be equal to the start of the buffer.

Change-Id: Icbaef197123f0fc85dbdba7c0259e3dcc9352920
Problem

Testcase was added as part of fix for
  Bug#22602898 NDB : CURIOUS STATE OF TC COMMIT_SENT / COMPLETE_SENT TIMEOUT HANDLING

That fix modified TC timeout handling to avoid the 'switch to
serial' behaviour when detecting a timeout, as it can result
in excessive slowdown for large transaction commits, leading
to long + large epochs, replication problems etc.

Test testNodeRestart -n TransStallTimeout tested this fix by stalling
a transaction in the commit or complete phase and checking that it
was *not* unblocked (and committed) by TC timeout switching to
the serial commit protocol.

However there was an issue with transient transaction states
which were not directly handled as part of node failure, and
so relied on the non node-failure timeout handling to cleanup.

This was detected and fixed in :
  Bug#36028828 testNodeRestart -n MixedReadUpdateScan failures

That fix uses the difference between the TC's global 'failure number'
counter and an ApiConnectRecord's local failure number to detect
when a node failure has occurred since a transaction started, to
detect and cleanup transactions which were in transient states when
the node failure was detected.

However it is observed that testNodeRestart -n TransStallTimeout
fails occasionally in the COMPLETE phase - a transaction blocked
in the COMPLETE phase is unblocked by normal COMPLETE timeout
handling.

Investigation shows that the problem is that in some cases the
ApiConnectRecord's failure number is not initialised.

This is because the COMPLETE phase uses a different ApiConnectRecord
(Copy record) than the PREPARE + COMMIT phases, to allow the PREPARE
+ COMMIT record to be used for a new transaction concurrently with
COMPLETE.

Solution

Ensure that the Copy ApiConnectRecord has an initialised Failure
number.

Change-Id: I725d80bb3230eeeeb3c3e357943738dfdfc3f434
Backport to 7.6

Problem

Testcase was added as part of fix for
  Bug#22602898 NDB : CURIOUS STATE OF TC COMMIT_SENT / COMPLETE_SENT TIMEOUT HANDLING

That fix modified TC timeout handling to avoid the 'switch to
serial' behaviour when detecting a timeout, as it can result
in excessive slowdown for large transaction commits, leading
to long + large epochs, replication problems etc.

Test testNodeRestart -n TransStallTimeout tested this fix by stalling
a transaction in the commit or complete phase and checking that it
was *not* unblocked (and committed) by TC timeout switching to
the serial commit protocol.

However there was an issue with transient transaction states
which were not directly handled as part of node failure, and
so relied on the non node-failure timeout handling to cleanup.

This was detected and fixed in :
  Bug#36028828 testNodeRestart -n MixedReadUpdateScan failures

That fix uses the difference between the TC's global 'failure number'
counter and an ApiConnectRecord's local failure number to detect
when a node failure has occurred since a transaction started, to
detect and cleanup transactions which were in transient states when
the node failure was detected.

However it is observed that testNodeRestart -n TransStallTimeout
fails occasionally in the COMPLETE phase - a transaction blocked
in the COMPLETE phase is unblocked by normal COMPLETE timeout
handling.

Investigation shows that the problem is that in some cases the
ApiConnectRecord's failure number is not initialised.

This is because the COMPLETE phase uses a different ApiConnectRecord
(Copy record) than the PREPARE + COMMIT phases, to allow the PREPARE
+ COMMIT record to be used for a new transaction concurrently with
COMPLETE.

Solution

Ensure that the Copy ApiConnectRecord has an initialised Failure
number.

Change-Id: I725d80bb3230eeeeb3c3e357943738dfdfc3f434
Change-Id: I5b889ae50f69341f051e27bfec3532eded79eada
WHEN SLAVE RECONNECTS

 Description:
 ------------
 In a source-replica setup, when the replica reconnects to the source,
 a "killing the zombie dump thread" message appears in the source's error log.
 Despite this message, both servers continue to operate normally.

 Analysis:
 ------------
 This is expected behavior, but the message could be made clearer:
 The term "zombie dump thread" is likely an internal term and should be
 avoided.
 The message should clearly indicate, through its text (not just the severity
 level), that this is a normal, expected condition.
 The text should make it clear that this most likely indicates the same
 replica is reconnecting.

 Fix:
 ----
 Update the message with clearer and more meaningful text.

Change-Id: Id5808be9cbe9e8fa41da5313ad5e5eeaa37d3bf8
  Merge branch 'mysql-5.7-cluster-7.6' into mysql-8.0
Tusamarco and others added 5 commits February 24, 2025 10:29
PKG-494 Add netcat dependency to debian packages
# Conflicts:
#	mysql-test/r/grant_dynamic.result
#	mysql-test/suite/perfschema/r/privilege_table_io.result
https://perconadev.atlassian.net/browse/PXC-4469

Post push fix.

Problem:
mysql client version check used incorrect way of versions comparison.

Cause:
Comparison metod used -lt that works for integers. We need lexicographic
comparison.

Solution:
Changed to use already existing 'compare_versions' function from
wsrep_sst_common.sh

Additionally fixed MTR test.
@it-percona-cla
Copy link

it-percona-cla commented Feb 26, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
6 out of 11 committers have signed the CLA.

✅ kamil-holubicki
✅ VarunNagaraju
✅ adivinho
✅ surbhat1595
✅ Tusamarco
✅ inikep
❌ percona-ysorokin
❌ lukin-oleksiy
❌ aybek
❌ aibek.bukabayev [email protected]
❌ bjornmu


aibek.bukabayev [email protected] seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/45)

@@ -226,6 +226,25 @@ void Commit_stage_manager::deinit() {
mysql_mutex_destroy(&this->m_lock_wait_for_ticket_turn);
}

bool Commit_stage_manager::is_ticket_on_its_turn_and_back_ticket_incremented(
THD *thd) const {

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method is_ticket_on_its_turn_and_back_ticket_incremented can be made static

Suggested change
THD *thd) const {
THD *thd) {

@return True if the THD session parameter BGC ticket is active and
the BGC back ticket was incremented, false otherwise.
*/
bool is_ticket_on_its_turn_and_back_ticket_incremented(THD *thd) const;

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method is_ticket_on_its_turn_and_back_ticket_incremented can be made static

Suggested change
bool is_ticket_on_its_turn_and_back_ticket_incremented(THD *thd) const;
static bool is_ticket_on_its_turn_and_back_ticket_incremented(THD *thd) ;

Comment on lines +240 to +245
if (ticket == ticket_manager.get_front_ticket() &&
ticket < ticket_manager.get_back_ticket()) {
return true;
}

return false;

Choose a reason for hiding this comment

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

⚠️ readability-simplify-boolean-expr ⚠️
redundant boolean literal in conditional return statement

Suggested change
if (ticket == ticket_manager.get_front_ticket() &&
ticket < ticket_manager.get_back_ticket()) {
return true;
}
return false;
return ticket == ticket_manager.get_front_ticket() &&
ticket < ticket_manager.get_back_ticket();

@@ -930,19 +930,19 @@ class ha_tokudb : public handler {

private:
#if defined(TOKU_INCLUDE_UPSERT) && TOKU_INCLUDE_UPSERT
MY_NODISCARD int fast_update(THD *thd, mem_root_deque<Item *> &update_fields,
[[nodiscard]] int fast_update(THD *thd, mem_root_deque<Item *> &update_fields,
mem_root_deque<Item *> &update_values,
Item *conds);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-explicit-virtual-functions ⚠️
annotate this function with override or (rarely) final

Suggested change
Item *conds);
Item *conds) override;

List<Item> &update_values, Item *conds,
DB_TXN *txn);
MY_NODISCARD int upsert(THD *thd, mem_root_deque<Item *> &update_fields,
[[nodiscard]] int upsert(THD *thd, mem_root_deque<Item *> &update_fields,
mem_root_deque<Item *> &update_values);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-explicit-virtual-functions ⚠️
annotate this function with override or (rarely) final

Suggested change
mem_root_deque<Item *> &update_values);
mem_root_deque<Item *> &update_values) override;

@@ -2363,6 +2363,12 @@ void Dbtup::drop_fragment_fsremove_done(Signal *signal, TablerecPtr tabPtr,
signal->theData[0] = ZREL_FRAG;
signal->theData[1] = tabPtr.i;
signal->theData[2] = logfile_group_id;
if (ERROR_INSERTED(4039)) {

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4039 is a magic number; consider replacing it with a named constant

@@ -84,6 +84,14 @@
} while (0)
#endif

#ifdef HAVE_PSI_METADATA_INTERFACE
#define mysql_mdl_set_type(L, D) inline_mysql_mdl_set_type(L, D)

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro mysql_mdl_set_type used; consider a constexpr template function

@@ -128,6 +136,13 @@ static inline void inline_mysql_mdl_set_duration(
}
}

static inline void inline_mysql_mdl_set_type(PSI_metadata_lock *psi,
enum_mdl_type mdl_type) {

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
unknown type name enum_mdl_type

Comment on lines +139 to +140
static inline void inline_mysql_mdl_set_type(PSI_metadata_lock *psi,
enum_mdl_type mdl_type) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter mdl_type is unused

Suggested change
static inline void inline_mysql_mdl_set_type(PSI_metadata_lock *psi,
enum_mdl_type mdl_type) {
static inline void inline_mysql_mdl_set_type(PSI_metadata_lock *psi) {

@@ -4561,7 +4561,9 @@ static bool is_silent_error(THD *thd) {
int Query_log_event::do_apply_event(Relay_log_info const *rli,

Choose a reason for hiding this comment

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

⚠️ readability-function-cognitive-complexity ⚠️
function do_apply_event has cognitive complexity of 254 (threshold 50)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (2/45)

const_cast<Relay_log_info *>(rli)->get_group_master_log_name_info(),
llstr(const_cast<Relay_log_info *>(rli)
->get_group_master_log_pos_info(),
llbuff));
goto compare_errors;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-avoid-goto ⚠️
avoid using goto for flow control

DBUG_EXECUTE_IF("simulate_error_in_ddl", error_code = 1051;);

if (ignored_error_code((expected_error = error_code)) ||
if (ignored_error_code((expected_error)) ||

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (ignored_error_code((expected_error)) ||
if ((ignored_error_code((expected_error)) != 0) ||

@@ -470,6 +470,132 @@ int runRestarter(NDBT_Context *ctx, NDBT_Step *step) {
return result;
}

int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter ctx is unused

Suggested change
int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {
int runCreateDropTableUntilStopped(NDBT_Context * /*ctx*/, NDBT_Step *step) {

@@ -470,6 +470,132 @@ int runRestarter(NDBT_Context *ctx, NDBT_Step *step) {
return result;
}

int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter step is unused

Suggested change
int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {
int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step * /*step*/) {

@@ -470,6 +470,132 @@ int runRestarter(NDBT_Context *ctx, NDBT_Step *step) {
return result;
}

int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pNdb is not initialized

Suggested change
Ndb *pNdb = GETNDB(step);
Ndb *pNdb = nullptr = GETNDB(step);

@@ -470,6 +470,132 @@ int runRestarter(NDBT_Context *ctx, NDBT_Step *step) {
return result;
}

int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);
NdbRestarter res;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable res is not initialized

Suggested change
NdbRestarter res;
NdbRestarter res = 0;

int runCreateDropTableUntilStopped(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);
NdbRestarter res;
const char *tableName = ctx->getProperty("tableName", (char *)NULL);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable tableName is not initialized

Suggested change
const char *tableName = ctx->getProperty("tableName", (char *)NULL);
const char *tableName = nullptr = ctx->getProperty("tableName", (char *)NULL);

NdbRestarter res;
const char *tableName = ctx->getProperty("tableName", (char *)NULL);

NdbDictionary::Dictionary *pDict = pNdb->getDictionary();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pDict is not initialized

Suggested change
NdbDictionary::Dictionary *pDict = pNdb->getDictionary();
NdbDictionary::Dictionary *pDict = nullptr = pNdb->getDictionary();

const char *tableName = ctx->getProperty("tableName", (char *)NULL);

NdbDictionary::Dictionary *pDict = pNdb->getDictionary();
Uint32 stepNum = step->getStepNo();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable stepNum is not initialized

Suggested change
Uint32 stepNum = step->getStepNo();
Uint32 stepNum = 0 = step->getStepNo();

* TUPFRAGREQ).
* 4039: Error Insert to delay fragment release in TUP.
*/
int error = rand() % 2 ? 3006 : 4039;

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
int error = rand() % 2 ? 3006 : 4039;
int error = (rand() % 2) != 0 ? 3006 : 4039;

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (3/45)

* TUPFRAGREQ).
* 4039: Error Insert to delay fragment release in TUP.
*/
int error = rand() % 2 ? 3006 : 4039;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
3006 is a magic number; consider replacing it with a named constant

* TUPFRAGREQ).
* 4039: Error Insert to delay fragment release in TUP.
*/
int error = rand() % 2 ? 3006 : 4039;

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4039 is a magic number; consider replacing it with a named constant

}

if (pDict->createTable(tab) != 0) {
NdbError err = pDict->getNdbError();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable err is not initialized

Suggested change
NdbError err = pDict->getNdbError();
NdbError err = 0 = pDict->getNdbError();

* test can continue
*/
if (pDict->dropTable(tabName.c_str()) != 0) {
NdbError err = pDict->getNdbError();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable err is not initialized

Suggested change
NdbError err = pDict->getNdbError();
NdbError err = 0 = pDict->getNdbError();

return NDBT_FAILED;
}
}
NdbSleep_MilliSleep(20);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
20 is a magic number; consider replacing it with a named constant

return NDBT_OK;
}

int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter ctx is unused

Suggested change
int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {
int runScanNdbInfoTable(NDBT_Context * /*ctx*/, NDBT_Step *step) {

return NDBT_OK;
}

int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter step is unused

Suggested change
int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {
int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step * /*step*/) {

}

int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {
int result = NDBT_OK;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable result is not initialized

Suggested change
int result = NDBT_OK;
int result = 0 = NDBT_OK;


int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {
int result = NDBT_OK;
int loops = ctx->getNumLoops();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable loops is not initialized

Suggested change
int loops = ctx->getNumLoops();
int loops = 0 = ctx->getNumLoops();

int runScanNdbInfoTable(NDBT_Context *ctx, NDBT_Step *step) {
int result = NDBT_OK;
int loops = ctx->getNumLoops();
const char *tableName = ctx->getProperty("infoTableName", (char *)NULL);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable tableName is not initialized

Suggested change
const char *tableName = ctx->getProperty("infoTableName", (char *)NULL);
const char *tableName = nullptr = ctx->getProperty("infoTableName", (char *)NULL);

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (4/45)

return NDBT_FAILED;
}

const NdbInfo::Table *table;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable table is not initialized

Suggested change
const NdbInfo::Table *table;
const NdbInfo::Table *table = nullptr;


while (loops-- && !ctx->isTestStopped()) {
NdbInfoScanOperation *scanOp = nullptr;
if (ndbinfo.createScanOperation(table, &scanOp)) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (ndbinfo.createScanOperation(table, &scanOp)) {
if (ndbinfo.createScanOperation(table, &scanOp) != 0) {

shard mutex for this table_id.
To stop iteration the visitor can return true.
*/
void find_on_table(const table_id_t table_id,

Choose a reason for hiding this comment

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

⚠️ readability-avoid-const-params-in-decls ⚠️
parameter table_id is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions

Suggested change
void find_on_table(const table_id_t table_id,
void find_on_table(table_id_t table_id,

@@ -314,8 +314,8 @@ class NdbTableImpl : public NdbDictionary::Table, public NdbDictObjectImpl {
Uint8 m_noOfKeys;
// if all pk = dk then this is zero!
Uint8 m_noOfDistributionKeys;
Uint8 m_noOfBlobs;
Uint8 m_noOfDiskColumns;
Uint16 m_noOfBlobs;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable m_noOfBlobs has public visibility

Uint8 m_noOfBlobs;
Uint8 m_noOfDiskColumns;
Uint16 m_noOfBlobs;
Uint16 m_noOfDiskColumns;

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable m_noOfDiskColumns has public visibility

@@ -729,7 +729,8 @@ int Wsrep_applier_service::apply_nbo_begin(const wsrep::ws_meta &ws_meta,
replayer_thd->set_psi(psi);
PSI_THREAD_CALL(set_thread)(psi);
PSI_THREAD_CALL(set_thread_os_id)(psi);
PSI_THREAD_CALL(set_thread_account)("root", strlen("root"), nullptr, 0);
PSI_THREAD_CALL(set_thread_account)
("system user", strlen("system user"), nullptr, 0);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from size_t (aka unsigned long) to signed type int is implementation-defined

@@ -648,3 +648,7 @@ const char *MySQLSession::ssl_cipher() {
void MySQLSession::LoggingStrategyDebugLogger::log(const std::string &msg) {
log_debug("%s", msg.c_str());
}

unsigned long MySQLSession::server_version() {
return connection_ ? mysql_get_server_version(connection_) : 0;

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion MYSQL * -> bool

Suggested change
return connection_ ? mysql_get_server_version(connection_) : 0;
return connection_ != nullptr ? mysql_get_server_version(connection_) : 0;

@@ -137,6 +137,7 @@ CREATE TABLE mysql.masking_dictionaries(
Term VARCHAR(256) NOT NULL,
UNIQUE INDEX dictionary_term_idx (Dictionary, Term)
) ENGINE = InnoDB DEFAULT CHARSET=utf8mb4;
GRANT SELECT, INSERT, UPDATE, DELETE ON mysql.masking_dictionaries TO 'mysql.session'@'localhost';

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
unknown type name GRANT

@@ -137,6 +137,7 @@ CREATE TABLE mysql.masking_dictionaries(
Term VARCHAR(256) NOT NULL,
UNIQUE INDEX dictionary_term_idx (Dictionary, Term)
) ENGINE = InnoDB DEFAULT CHARSET=utf8mb4;
GRANT SELECT, INSERT, UPDATE, DELETE ON mysql.masking_dictionaries TO 'mysql.session'@'localhost';

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
expected ; after top level declarator

Suggested change
GRANT SELECT, INSERT, UPDATE, DELETE ON mysql.masking_dictionaries TO 'mysql.session'@'localhost';
GRANT SELECT, INSERT, UPDATE, DELETE; ON mysql.masking_dictionaries TO 'mysql.session'@'localhost';

@@ -137,6 +137,7 @@ CREATE TABLE mysql.masking_dictionaries(
Term VARCHAR(256) NOT NULL,
UNIQUE INDEX dictionary_term_idx (Dictionary, Term)
) ENGINE = InnoDB DEFAULT CHARSET=utf8mb4;
GRANT SELECT, INSERT, UPDATE, DELETE ON mysql.masking_dictionaries TO 'mysql.session'@'localhost';

--let $current_user = `SELECT USER()`

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
expected unqualified-id

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (5/45)

@return DB_SUCCESS or error code. */
[[nodiscard]] dberr_t handle_autoinc(const dtuple_t *row) noexcept;
[[nodiscard]] dberr_t handle_autoinc(const dtuple_t *row,

Choose a reason for hiding this comment

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

⚠️ readability-inconsistent-declaration-parameter-name ⚠️
function ddl::Context::handle_autoinc has a definition with different parameter names


set_mock_server_version(http_port, GetParam().server_version);

std::vector<std::string> bootstrap_params = {

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable bootstrap_params is not initialized

Suggested change
std::vector<std::string> bootstrap_params = {
std::vector<std::string> bootstrap_params = 0 = {

@@ -2079,6 +2079,8 @@ class Query_block : public Query_term {
*/
uint with_wild{0};

/// Original query table map before aj/sj processing.
table_map original_tables_map{};

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-non-private-member-variables-in-classes ⚠️
member variable original_tables_map has public visibility

}

void Parallel_reader::Thread_ctx::
save_current_user_record_as_last_processed() noexcept {

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method save_current_user_record_as_last_processed can be made const

Suggested change
save_current_user_record_as_last_processed() noexcept {
save_current_user_record_as_last_processed() const noexcept {

/** Save current position, commit any active mtr. */
void savepoint() noexcept;
/** @see PCursor::save_current_user_record_as_last_processed */
void save_current_user_record_as_last_processed() noexcept;

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method save_current_user_record_as_last_processed can be made const

Suggested change
void save_current_user_record_as_last_processed() noexcept;
void save_current_user_record_as_last_processed() const noexcept;

return (!m_pcur->is_after_last_on_page()) ? DB_SUCCESS
: move_to_next_block_user_rec();
void Parallel_reader::Thread_ctx::
restore_to_last_processed_user_record() noexcept {

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method restore_to_last_processed_user_record can be made const

Suggested change
restore_to_last_processed_user_record() noexcept {
restore_to_last_processed_user_record() const noexcept {

@return DB_SUCCESS or error code. */
[[nodiscard]] dberr_t restore_from_savepoint() noexcept;
/** @see PCursor::restore_to_last_processed_user_record */
void restore_to_last_processed_user_record() noexcept;

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method restore_to_last_processed_user_record can be made const

Suggested change
void restore_to_last_processed_user_record() noexcept;
void restore_to_last_processed_user_record() const noexcept;

ut_ad(m_pcursor->read_level() == 0);
return m_pcursor->restore_from_savepoint();
void Parallel_reader::Thread_ctx::
save_previous_user_record_as_last_processed() noexcept {

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method save_previous_user_record_as_last_processed can be made const

Suggested change
save_previous_user_record_as_last_processed() noexcept {
save_previous_user_record_as_last_processed() const noexcept {

void restore_to_last_processed_user_record() noexcept;

/** @see PCursor::save_previous_user_record_as_last_processed */
void save_previous_user_record_as_last_processed() noexcept;

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method save_previous_user_record_as_last_processed can be made const

Suggested change
void save_previous_user_record_as_last_processed() noexcept;
void save_previous_user_record_as_last_processed() const noexcept;

}

void Parallel_reader::Thread_ctx::savepoint() noexcept {
m_pcursor->savepoint();
void Parallel_reader::Thread_ctx::restore_to_first_unprocessed() noexcept {

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method restore_to_first_unprocessed can be made const

Suggested change
void Parallel_reader::Thread_ctx::restore_to_first_unprocessed() noexcept {
void Parallel_reader::Thread_ctx::restore_to_first_unprocessed() const noexcept {

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (6/45)

void save_previous_user_record_as_last_processed() noexcept;

/** @see PCursor::restore_to_first_unprocessed */
void restore_to_first_unprocessed() noexcept;

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method restore_to_first_unprocessed can be made const

Suggested change
void restore_to_first_unprocessed() noexcept;
void restore_to_first_unprocessed() const noexcept;

Comment on lines +555 to +556
} else {
/* This happens if we were descending the B-tree, in which case we try

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else {
/* This happens if we were descending the B-tree, in which case we try
} /* This happens if we were descending the B-tree, in which case we try

to navigate in such a way to arrive at a user record. */
ut_a(m_pcur->is_on_user_rec());
return DB_SUCCESS;
}

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
}

in the context of cascading DML operation, only the referenced
table is relevant for the validation even if the current table
has FTS index.*/
if (!foreign_table->fts || foreign_table == referenced_table) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion fts_t * -> bool

Suggested change
if (!foreign_table->fts || foreign_table == referenced_table) {
if ((foreign_table->fts == nullptr) || foreign_table == referenced_table) {

#include <my_loglevel.h>
#include <my_sys.h>
#include <my_thread.h>
#include <mysqld_error.h>

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
mysqld_error.h file not found

using handler_function_type = std::function<void()>;

// passing 'handler_function' deliberately by value to move from
jthread(handler_function_type handler_function, const char *category_name,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter handler_function is unused

Suggested change
jthread(handler_function_type handler_function, const char *category_name,
jthread(const char *category_name,

}

// initializing internal connection (along with THD)
term_cache_ptr cache;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable cache is not initialized

Suggested change
term_cache_ptr cache;
term_cache_ptr cache = 0;

@@ -4,6 +4,7 @@
#
# Result is placed on var:

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
invalid preprocessing directive

@@ -4,6 +4,7 @@
#
# Result is placed on var:
# - $group_replication_primary_connection_out_var

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
invalid preprocessing directive

@@ -4,6 +4,7 @@
#
# Result is placed on var:
# - $group_replication_primary_connection_out_var
# - $group_replication_primary_connection_number_out_var

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
invalid preprocessing directive

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (7/45)

@@ -4,6 +4,7 @@
#
# Result is placed on var:
# - $group_replication_primary_connection_out_var
# - $group_replication_primary_connection_number_out_var
# - $group_replication_found_primary_out_var

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
invalid preprocessing directive

@@ -4,6 +4,7 @@
#
# Result is placed on var:
# - $group_replication_primary_connection_out_var
# - $group_replication_primary_connection_number_out_var
# - $group_replication_found_primary_out_var
#
# It relies on $rpl_server_count to iterate through

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
invalid preprocessing directive

@@ -24,6 +25,15 @@
# --source include/rpl_connection.inc
# # now on primary
# }
#
# --source include/gr_find_a_primary.inc
# if ($group_replication_primary_connection_number_out_var)

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
unterminated conditional directive

Owned by the receiver thread
Read by API thread under mutex protection
*/
Uint32 m_kernel_error_code;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-non-private-member-variables-in-classes ⚠️
member variable m_kernel_error_code has protected visibility

RecordProperty("Description", GetParam().description);

const size_t kClusterNodes{2};
std::vector<ProcessWrapper *> cluster_nodes;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable cluster_nodes is not initialized

Suggested change
std::vector<ProcessWrapper *> cluster_nodes;
std::vector<ProcessWrapper *> cluster_nodes = 0;


const size_t kClusterNodes{2};
std::vector<ProcessWrapper *> cluster_nodes;
std::vector<uint16_t> md_servers_classic_ports, md_servers_http_ports;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable md_servers_classic_ports is not initialized

Suggested change
std::vector<uint16_t> md_servers_classic_ports, md_servers_http_ports;
std::vector<uint16_t> md_servers_classic_ports = 0, md_servers_http_ports;

if (at == nullptr) {
view_does_not_exist = (at == nullptr);
base_table_with_same_name_exists =
(at && (at->type() == dd::enum_table_type::BASE_TABLE));

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion const dd::Abstract_table * -> bool

Suggested change
(at && (at->type() == dd::enum_table_type::BASE_TABLE));
((at != nullptr) && (at->type() == dd::enum_table_type::BASE_TABLE));

if (at == nullptr) {
view_does_not_exist = (at == nullptr);
base_table_with_same_name_exists =
(at && (at->type() == dd::enum_table_type::BASE_TABLE));

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion const dd::Abstract_table * -> bool

Suggested change
(at && (at->type() == dd::enum_table_type::BASE_TABLE));
((at != nullptr) && (at->type() == dd::enum_table_type::BASE_TABLE));

@@ -8,7 +8,7 @@ if (!$MASKING_FUNCTIONS_COMPONENT) {
}

#
## Check if --plugin-dir was setup for component_encryption_udf
## Check if --plugin-dir was setup for component_masking_functions

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
expected unqualified-id

#include <my_dbug.h>
#include <my_inttypes.h>
#include <my_loglevel.h>
#include <mysqld_error.h>

Choose a reason for hiding this comment

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

⚠️ clang-diagnostic-error ⚠️
mysqld_error.h file not found

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (8/45)

@@ -543,6 +543,16 @@ void Dbacc::set_tup_fragptr(Uint32 fragptr, Uint32 tup_fragptr) {
void Dbacc::execACCFRAGREQ(Signal *signal) {
const AccFragReq *const req = (AccFragReq *)&signal->theData[0];
jamEntry();
if (ERROR_INSERTED(3006)) {

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
3006 is a magic number; consider replacing it with a named constant

@@ -740,5 +740,6 @@ class ha_ndbcluster : public handler, public Partition_handler {
};

int ndb_to_mysql_error(const NdbError *ndberr);
int fail_index_offline(TABLE *t, int index);

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
parameter name t is too short, expected at least 2 characters

PROVIDES_SERVICE(performance_schema, psi_mdl_v2),
PROVIDES_SERVICE(performance_schema, psi_mdl_v3),

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-const-cast ⚠️
do not use const_cast

* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116954
*
*/
#define MY_COMPILER_GCC_WORKAROUND_FALSE_POSITIVE_SUGGEST_ATTRIBUTE_FORMAT() \

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro MY_COMPILER_GCC_WORKAROUND_FALSE_POSITIVE_SUGGEST_ATTRIBUTE_FORMAT used; consider a constexpr template function


extern REQUIRES_PSI_MDL_SERVICE_PLACEHOLDER;

#define PSI_MDL_CALL(M) mysql_service_psi_mdl_v2->M
#define PSI_MDL_CALL(M) mysql_service_psi_mdl_v3->M

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-macro-usage ⚠️
function-like macro PSI_MDL_CALL used; consider a constexpr template function

@@ -51,7 +51,7 @@ int cstrbuf_vsnprintf_noerr(char str[], size_t size, const char fmt[],
#include "util/span.h"

int main() {
plan(39);
plan(40);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
40 is a magic number; consider replacing it with a named constant

@@ -116,6 +116,7 @@ int main() {
ok1(cstrbuf_copy({buf + 3, 5}, "Mugge vigge") == 1);
ok1(cstrbuf_format(ndb::span(buf + 19, buf + 27), "Mugge %zu", sizeof(buf)) ==
1);
ok1(cstrbuf_format(buf, "Mugge %zu", sizeof(buf)) == 0);

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion bool -> int

Suggested change
ok1(cstrbuf_format(buf, "Mugge %zu", sizeof(buf)) == 0);
ok1(static_cast<int>(cstrbuf_format(buf, "Mugge %zu", sizeof(buf)) == 0));

@@ -582,6 +582,7 @@ Dbtc::Dbtc(Block_context &ctx, Uint32 instanceNo)
tcFailRecord = 0;
m_deferred_enabled = ~Uint32(0);
m_max_writes_per_trans = ~Uint32(0);
m_dbinfo_full_apiconnectrecord = false;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
m_dbinfo_full_apiconnectrecord should be initialized in a member initializer of the constructor

Suggested change
m_dbinfo_full_apiconnectrecord = false;

@@ -1874,34 +1874,55 @@ void Dbtc::execTCRELEASEREQ(Signal *signal) {
/****************************************************************************/
// Error Handling for TCKEYREQ messages
/****************************************************************************/
void Dbtc::signalErrorRefuseLab(Signal *signal,
ApiConnectRecordPtr const apiConnectptr) {
void Dbtc::handleSignalStateProblem(Signal *signal,

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method handleSignalStateProblem can be made static

Suggested change
void Dbtc::handleSignalStateProblem(Signal *signal,
static void Dbtc::handleSignalStateProblem(Signal *signal,

@@ -1874,34 +1874,55 @@ void Dbtc::execTCRELEASEREQ(Signal *signal) {
/****************************************************************************/
// Error Handling for TCKEYREQ messages
/****************************************************************************/
void Dbtc::signalErrorRefuseLab(Signal *signal,
ApiConnectRecordPtr const apiConnectptr) {
void Dbtc::handleSignalStateProblem(Signal *signal,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter signal is unused

Suggested change
void Dbtc::handleSignalStateProblem(Signal *signal,
void Dbtc::handleSignalStateProblem(Signal * /*signal*/,

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (9/45)

@@ -13698,6 +13716,14 @@ void Dbtc::execSCAN_FRAGCONF(Signal *signal) {
const Uint32 activeMask =
(sig_len >= ScanFragConf::SignalLength_ext) ? conf->activeMask : 0;

if (ERROR_INSERTED(8124)) {

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
8124 is a magic number; consider replacing it with a named constant

"TC %u setting DBINFO ApiConnectRecord full from %u to %u", instance(),
m_dbinfo_full_apiconnectrecord, newVal);
m_dbinfo_full_apiconnectrecord = newVal;
}
} // Dbtc::execDUMP_STATE_ORD()

void Dbtc::execDBINFO_SCANREQ(Signal *signal) {

Choose a reason for hiding this comment

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

⚠️ readability-function-cognitive-complexity ⚠️
function execDBINFO_SCANREQ has cognitive complexity of 51 (threshold 50)

Uint32 loop_count = 0;
Uint32 api_ptr = cursor->data[0];
const bool full = (m_dbinfo_full_apiconnectrecord ||

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable full is not initialized

Suggested change
const bool full = (m_dbinfo_full_apiconnectrecord ||
const bool full = 0 = (m_dbinfo_full_apiconnectrecord ||

bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row,
ApiConnectRecordPtr transPtr) {
bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row, ApiConnectRecordPtr transPtr,
const bool full) {

Choose a reason for hiding this comment

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

⚠️ readability-make-member-function-const ⚠️
method ndbinfo_write_trans can be made const

Suggested change
const bool full) {
const bool full) const {

@@ -16375,8 +16416,8 @@ void Dbtc::execDBINFO_SCANREQ(Signal *signal) {
ndbinfo_send_scan_conf(signal, req, rl);
}

bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row,
ApiConnectRecordPtr transPtr) {
bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row, ApiConnectRecordPtr transPtr,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter row is unused

Suggested change
bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row, ApiConnectRecordPtr transPtr,
bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row & /*row*/, ApiConnectRecordPtr transPtr,

@@ -16386,8 +16427,9 @@ bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row,
conState = CS_CONNECTED;
}

if (conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
conState == CS_RESTART) {
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
if (((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||

conState == CS_RESTART) {
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
conState == CS_RESTART) &
!full) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
!full) {
!full) != 0) {

@@ -16386,8 +16427,9 @@ bool Dbtc::ndbinfo_write_trans(Ndbinfo::Row &row,
conState = CS_CONNECTED;
}

if (conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
conState == CS_RESTART) {
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion bool -> int

Suggested change
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
if (static_cast<int>(conState == CS_CONNECTED || conState == CS_DISCONNECTED ||

conState == CS_RESTART) {
if ((conState == CS_CONNECTED || conState == CS_DISCONNECTED ||
conState == CS_RESTART) &
!full) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion bool -> int

Suggested change
!full) {
static_cast<int>(!full)) {


protected:
bool contextualize_setop(Parse_context *pc, Query_term_type setop_type,
Surrounding_context context);
PT_query_expression_body *m_lhs;
void merge_children(Query_term_set_op *setop, Query_term_set_op *lower);

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method merge_children can be made static

Suggested change
void merge_children(Query_term_set_op *setop, Query_term_set_op *lower);
static void merge_children(Query_term_set_op *setop, Query_term_set_op *lower);

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (10/45)


pc->thd->lex->pop_context();
List_iterator<PT_query_expression_body> it(m_list);
PT_query_expression_body *elt;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable elt is not initialized

Suggested change
PT_query_expression_body *elt;
PT_query_expression_body *elt = nullptr;

}

bool can_absorb_order_and_limit(bool, bool) const override { return false; }

bool is_table_value_constructor() const override { return false; }
PT_insert_values_list *get_row_value_list() const override { return nullptr; }
bool is_distinct() const { return m_is_distinct; }

List<PT_query_expression_body> m_list;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-non-private-member-variables-in-classes ⚠️
member variable m_list has public visibility

bool is_distinct() const { return m_is_distinct; }

List<PT_query_expression_body> m_list;
void set_is_rhs_in_parentheses(bool v) { m_is_rhs_in_parentheses = v; }

Choose a reason for hiding this comment

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

⚠️ readability-identifier-length ⚠️
parameter name v is too short, expected at least 2 characters

PT_into_destination *m_into;
const bool m_is_rhs_in_parentheses;
bool m_is_rhs_in_parentheses;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-non-private-member-variables-in-classes ⚠️
member variable m_is_rhs_in_parentheses has protected visibility

Comment on lines +5406 to +5407
} else {
/* X op Y */

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
} else {
/* X op Y */
} /* X op Y */

/* X op Y */
return new (mem_root)
Class(left, is_distinct, right, is_right_in_parentheses);
}

Choose a reason for hiding this comment

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

⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change
}

synode_no const config_id_where_members_left,
std::vector<Gcs_member_identifier *> const &members_that_left) {
synode_no const config_id_where_members_under_effect,
std::vector<Gcs_member_identifier *> const &members_under_effect) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter members_under_effect is unused

Suggested change
std::vector<Gcs_member_identifier *> const &members_under_effect) {
std::vector<Gcs_member_identifier *> const & /*members_under_effect*/) {

* @retval true there is an expel in progress for @c member
* @retval false otherwise
*/
bool contains(Gcs_member_identifier const &member) const;

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method contains can be made static

Suggested change
bool contains(Gcs_member_identifier const &member) const;
static bool contains(Gcs_member_identifier const &member) ;


bool Gcs_xcom_expels_in_progress::contains(
Gcs_member_identifier const &member) const {

Choose a reason for hiding this comment

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

⚠️ readability-convert-member-functions-to-static ⚠️
method contains can be made static

Suggested change
Gcs_member_identifier const &member) const {
Gcs_member_identifier const &member) {

@@ -637,7 +637,7 @@ class NdbScanOperation : public NdbOperation {
int send_next_scan(Uint32 cnt, bool close);
void receiver_delivered(NdbReceiver *);
void receiver_completed(NdbReceiver *);
void execCLOSE_SCAN_REP();
void execCLOSE_SCAN_REP(Uint32 errorCode, bool needClose);

Choose a reason for hiding this comment

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

⚠️ readability-inconsistent-declaration-parameter-name ⚠️
function NdbScanOperation::execCLOSE_SCAN_REP has a definition with different parameter names

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (11/45)

@@ -47,6 +47,7 @@ NdbScanOperation::NdbScanOperation(Ndb *aNdb, NdbOperation::Type aType)
m_api_receivers = nullptr;
m_conf_receivers = nullptr;
m_sent_receivers = nullptr;
m_kernel_error_code = 0;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
m_kernel_error_code should be initialized in a member initializer of the constructor

Suggested change
m_kernel_error_code = 0;

@@ -1619,6 +1621,12 @@ int NdbScanOperation::nextResultNdbRecord(const char *&out_row,
return 2;
}

/* Check if api side error already set */
if (theError.code) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (theError.code) {
if (theError.code != 0) {

*/
if (theError.code != Err_scanAlreadyComplete) setErrorCode(theError.code);
setErrorCode(m_kernel_error_code);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from Uint32 (aka unsigned int) to signed type int is implementation-defined

setErrorCode(theError.code);
if (m_kernel_error_code != 0) {
/* Kernel side error set, transfer to the api visible error code */
setErrorCode(m_kernel_error_code);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from Uint32 (aka unsigned int) to signed type int is implementation-defined

@@ -1724,18 +1724,20 @@ int NdbScanOperation::nextResultNdbRecord(const char *&out_row,
g_eventLogger->info(
"NdbScanOperation::nextResultNdbRecord() 1:4008 on connection %d",
theNdbCon->ptr2int());
/* Mark scan as failed, needing close at kernel */
execCLOSE_SCAN_REP(4008, true);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4008 is a magic number; consider replacing it with a named constant

/* Check for kernel side error */
if (m_kernel_error_code != 0) {
/* Transfer error to user side and return */
setErrorCode(m_kernel_error_code);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from Uint32 (aka unsigned int) to signed type int is implementation-defined

int ret_code = poll_guard.wait_scan(3 * timeout, nodeId, forceSend);
if (ret_code == 0 && seq == impl->getNodeSequence(nodeId)) continue;
if (ret_code == -1) {
g_eventLogger->info(
"NdbIndexScanOperation::ordered_send_scan_wait_for_all() "
"2:4008 on connection %d",
theNdbCon->ptr2int());
/* Mark scan as failed, needing close at kernel */
execCLOSE_SCAN_REP(4008, true);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4008 is a magic number; consider replacing it with a named constant

setErrorCode(theError.code);
if (m_kernel_error_code != 0) {
/* Transfer to user side and return */
setErrorCode(m_kernel_error_code);

Choose a reason for hiding this comment

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

⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from Uint32 (aka unsigned int) to signed type int is implementation-defined

@@ -3442,6 +3475,7 @@ int NdbScanOperation::close_impl(bool forceSend, PollGuard *poll_guard) {
Uint32 timeout = impl->get_waitfor_timeout();
Uint32 seq = theNdbCon->theNodeSequence;
Uint32 nodeId = theNdbCon->theDBnode;
bool scanTimeoutCase = (theError.code == 4008);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4008 is a magic number; consider replacing it with a named constant

*/
impl->incClientStat(Ndb::WaitScanResultCount, 1);
while (theError.code == 0 && m_sent_receivers_count) {
while (m_kernel_error_code == 0 && m_sent_receivers_count) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion Uint32 (aka unsigned int) -> bool

Suggested change
while (m_kernel_error_code == 0 && m_sent_receivers_count) {
while (m_kernel_error_code == 0 && (m_sent_receivers_count != 0u)) {

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (12/45)

g_eventLogger->info(
"NdbScanOperation::close_impl() 3:4008 on connection %d",
theNdbCon->ptr2int());
/* Mark scan as failed, needing close at kernel */
execCLOSE_SCAN_REP(4008, true);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
4008 is a magic number; consider replacing it with a named constant


Uint32 cfg_db_lcp_interval = ctx->getProperty(

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable cfg_db_lcp_interval is not initialized

Suggested change
Uint32 cfg_db_lcp_interval = ctx->getProperty(
Uint32 cfg_db_lcp_interval = 0 = ctx->getProperty(

/*
The goal of the following loop is to avoid locking for too long transactions
on servers that have a high rate of trx. Processing 1M GTIDs in the original
code blocked the transaction processing for about 1s.
*/
while (it != certification_info.end()) {
stable_gtid_set_lock->wrlock();
uint64 write_set_counter = it->second->get_garbage_collect_counter();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable write_set_counter is not initialized

Suggested change
uint64 write_set_counter = it->second->get_garbage_collect_counter();
uint64 write_set_counter = 0 = it->second->get_garbage_collect_counter();

};
} // namespace

TEST(fts0fts, fts_doc_id_field_cmp) {

Choose a reason for hiding this comment

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

⚠️ google-readability-avoid-underscore-in-googletest-name ⚠️
avoid using "_" in test name "fts_doc_id_field_cmp" according to Googletest FAQ


/* Insert doc ids into rbtree. */
for (auto doc_id : doc_ids) {
ib_rbt_bound_t parent;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-member-init ⚠️
uninitialized record type: parent

Suggested change
ib_rbt_bound_t parent;
ib_rbt_bound_t parent{};

/* Insert doc ids into rbtree. */
for (auto doc_id : doc_ids) {
ib_rbt_bound_t parent;
dummy obj;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-member-init ⚠️
uninitialized record type: obj

Suggested change
dummy obj;
dummy obj{};

}

/* Check if doc id exists in rbtree */
ib_rbt_bound_t parent;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-member-init ⚠️
uninitialized record type: parent

Suggested change
ib_rbt_bound_t parent;
ib_rbt_bound_t parent{};

}

// RAII wrapper for initialization needed to create and use RW locks
struct os_support_t {

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-special-member-functions ⚠️
class os_support_t defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

}
};

TEST(fts0fts, fts_reset_get_doc) {

Choose a reason for hiding this comment

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

⚠️ google-readability-avoid-underscore-in-googletest-name ⚠️
avoid using "_" in test name "fts_reset_get_doc" according to Googletest FAQ

TEST(fts0fts, fts_reset_get_doc) {
os_support_t dummy;
dict_table_t *table =
static_cast<dict_table_t *>(calloc(1, sizeof(dict_table_t)));

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-no-malloc ⚠️
do not manage memory manually; consider a container or a smart pointer

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (13/45)

dict_table_t *table =
static_cast<dict_table_t *>(calloc(1, sizeof(dict_table_t)));

struct fts_cache_wrapper_t {

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-special-member-functions ⚠️
class fts_cache_wrapper_t defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

static_cast<dict_table_t *>(calloc(1, sizeof(dict_table_t)));

struct fts_cache_wrapper_t {
fts_cache_t *const cache{};

Choose a reason for hiding this comment

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

⚠️ misc-non-private-member-variables-in-classes ⚠️
member variable cache has public visibility


struct fts_cache_wrapper_t {
fts_cache_t *const cache{};
fts_cache_wrapper_t(dict_table_t *table) : cache{fts_cache_create(table)} {}

Choose a reason for hiding this comment

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

⚠️ google-explicit-constructor ⚠️
single-argument constructors must be marked explicit to avoid unintentional implicit conversions

Suggested change
fts_cache_wrapper_t(dict_table_t *table) : cache{fts_cache_create(table)} {}
explicit fts_cache_wrapper_t(dict_table_t *table) : cache{fts_cache_create(table)} {}


rw_lock_x_unlock(&cache->init_lock);

auto index_cache1 =

Choose a reason for hiding this comment

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

⚠️ llvm-qualified-auto ⚠️
auto index_cache1 can be declared as auto *index_cache1

Suggested change
auto index_cache1 =
auto *index_cache1 =


auto index_cache1 =
static_cast<fts_index_cache_t *>(ib_vector_get(cache->indexes, 0));
auto index_cache2 =

Choose a reason for hiding this comment

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

⚠️ llvm-qualified-auto ⚠️
auto index_cache2 can be declared as auto *index_cache2

Suggested change
auto index_cache2 =
auto *index_cache2 =

EXPECT_EQ(get_doc->get_document_graph, nullptr);
EXPECT_EQ(get_doc->cache, cache);

free(table);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-no-malloc ⚠️
do not manage memory manually; use RAII

@@ -2686,6 +2686,318 @@ int runScanLateUnlock(NDBT_Context *ctx, NDBT_Step *step) {
}
return NDBT_OK;
}

int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter ctx is unused

Suggested change
int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {
int runScanErrorCleanup(NDBT_Context * /*ctx*/, NDBT_Step *step) {

@@ -2686,6 +2686,318 @@ int runScanLateUnlock(NDBT_Context *ctx, NDBT_Step *step) {
}
return NDBT_OK;
}

int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter step is unused

Suggested change
int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {
int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step * /*step*/) {

@@ -2686,6 +2686,318 @@ int runScanLateUnlock(NDBT_Context *ctx, NDBT_Step *step) {
}
return NDBT_OK;
}

int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {
const NdbDictionary::Table *pTab = ctx->getTab();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pTab is not initialized

Suggested change
const NdbDictionary::Table *pTab = ctx->getTab();
const NdbDictionary::Table *pTab = nullptr = ctx->getTab();


int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {
const NdbDictionary::Table *pTab = ctx->getTab();
Ndb *pNdb = GETNDB(step);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pNdb is not initialized

Suggested change
Ndb *pNdb = GETNDB(step);
Ndb *pNdb = nullptr = GETNDB(step);

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (14/45)

int runScanErrorCleanup(NDBT_Context *ctx, NDBT_Step *step) {
const NdbDictionary::Table *pTab = ctx->getTab();
Ndb *pNdb = GETNDB(step);
NdbTransaction *pTrans;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pTrans is not initialized

Suggested change
NdbTransaction *pTrans;
NdbTransaction *pTrans = nullptr;

NdbTransaction *pTrans;
CHECKE((pTrans = pNdb->startTransaction()) != NULL, (*pNdb));

NdbScanOperation *pScan = pTrans->getNdbScanOperation(pTab);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pScan is not initialized

Suggested change
NdbScanOperation *pScan = pTrans->getNdbScanOperation(pTab);
NdbScanOperation *pScan = nullptr = pTrans->getNdbScanOperation(pTab);


g_err << "Wait for results to come in before iterating" << endl;

NdbSleep_MilliSleep(5000);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
5000 is a magic number; consider replacing it with a named constant

<< "released ApiConnectRecord objects" << endl;
{
/* Now do some other transactions */
HugoTransactions hugoTrans(*pTab);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable hugoTrans is not initialized

Suggested change
HugoTransactions hugoTrans(*pTab);
HugoTransactions hugoTrans = 0(*pTab);

}

static Uint32 getTransactionObjectCount(Ndb *pNdb) {
Ndb::Free_list_usage flu;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable flu is not initialized

Suggested change
Ndb::Free_list_usage flu;
Ndb::Free_list_usage flu = 0;

Comment on lines +2771 to +2772
static void checkScanErrorAction(NdbTransaction *pTrans,
NdbScanOperation *pScan,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter pTrans is unused

Suggested change
static void checkScanErrorAction(NdbTransaction *pTrans,
NdbScanOperation *pScan,
static void checkScanErrorAction(NdbScanOperation *pScan,

}

static void checkScanErrorAction(NdbTransaction *pTrans,
NdbScanOperation *pScan,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter pScan is unused

Suggested change
NdbScanOperation *pScan,


static void checkScanErrorAction(NdbTransaction *pTrans,
NdbScanOperation *pScan,
NdbRestarter *restarter, int when, int what) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter restarter is unused

Suggested change
NdbRestarter *restarter, int when, int what) {
int when, int what) {

case 3:
if (when > 0) {
ndbout << "3 NdbApi error on scan ";
int rc = pScan->deleteCurrentTuple();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable rc is not initialized

Suggested change
int rc = pScan->deleteCurrentTuple();
int rc = 0 = pScan->deleteCurrentTuple();

Comment on lines +2812 to +2813
static int checkScanErrorCase(Ndb *pNdb, const NdbDictionary::Table *pTab,
const NdbDictionary::Index *pIdx,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter pTab is unused

Suggested change
static int checkScanErrorCase(Ndb *pNdb, const NdbDictionary::Table *pTab,
const NdbDictionary::Index *pIdx,
static int checkScanErrorCase(Ndb *pNdb, const NdbDictionary::Index *pIdx,

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (15/45)

}

static int checkScanErrorCase(Ndb *pNdb, const NdbDictionary::Table *pTab,
const NdbDictionary::Index *pIdx,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter pIdx is unused

Suggested change
const NdbDictionary::Index *pIdx,


static int checkScanErrorCase(Ndb *pNdb, const NdbDictionary::Table *pTab,
const NdbDictionary::Index *pIdx,
NdbRestarter *restarter, int records,

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter restarter is unused

Suggested change
NdbRestarter *restarter, int records,
int records,

NdbRestarter *restarter, int records,
int scanType, int lock, int delay, int when,
int what) {
NdbTransaction *pTrans = nullptr;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pTrans is not initialized

Suggested change
NdbTransaction *pTrans = nullptr;
NdbTransaction *pTrans = nullptr = nullptr;


ndbout << "checkScanErrorCase : ";

NdbScanOperation *pScan = nullptr;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pScan is not initialized

Suggested change
NdbScanOperation *pScan = nullptr;
NdbScanOperation *pScan = nullptr = nullptr;

bool ordered = false;
ndbout << scanType << " ";
switch (scanType) {
case 0:

Choose a reason for hiding this comment

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

⚠️ bugprone-branch-clone ⚠️
switch has 2 consecutive identical branches


CHECKE(pScan != nullptr, (*pTrans));

NdbOperation::LockMode lm = NdbOperation::LM_CommittedRead;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable lm is not initialized

Suggested change
NdbOperation::LockMode lm = NdbOperation::LM_CommittedRead;
NdbOperation::LockMode lm = 0 = NdbOperation::LM_CommittedRead;


ndbout << lock << " ";
switch (lock) {
case 0:

Choose a reason for hiding this comment

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

⚠️ bugprone-branch-clone ⚠️
switch has 2 consecutive identical branches

if (delay == 1) {
ndbout << "with delay, ";

NdbSleep_MilliSleep(1000);

Choose a reason for hiding this comment

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

⚠️ readability-magic-numbers ⚠️
1000 is a magic number; consider replacing it with a named constant

return NDBT_OK;
}

static int runScanErrorHandling(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter ctx is unused

return NDBT_OK;
}

static int runScanErrorHandling(NDBT_Context *ctx, NDBT_Step *step) {

Choose a reason for hiding this comment

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

⚠️ misc-unused-parameters ⚠️
parameter step is unused

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (16/45)

}

static int runScanErrorHandling(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pNdb is not initialized

Suggested change
Ndb *pNdb = GETNDB(step);
Ndb *pNdb = nullptr = GETNDB(step);


static int runScanErrorHandling(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);
const NdbDictionary::Table *pTab = ctx->getTab();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pTab is not initialized

Suggested change
const NdbDictionary::Table *pTab = ctx->getTab();
const NdbDictionary::Table *pTab = nullptr = ctx->getTab();

static int runScanErrorHandling(NDBT_Context *ctx, NDBT_Step *step) {
Ndb *pNdb = GETNDB(step);
const NdbDictionary::Table *pTab = ctx->getTab();
const NdbDictionary::Index *pIdx = pNdb->getDictionary()->getIndex(

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable pIdx is not initialized

Suggested change
const NdbDictionary::Index *pIdx = pNdb->getDictionary()->getIndex(
const NdbDictionary::Index *pIdx = nullptr = pNdb->getDictionary()->getIndex(

const NdbDictionary::Table *pTab = ctx->getTab();
const NdbDictionary::Index *pIdx = pNdb->getDictionary()->getIndex(
orderedPkIdxName, ctx->getTab()->getName());
NdbRestarter restarter;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable restarter is not initialized

Suggested change
NdbRestarter restarter;
NdbRestarter restarter = 0;

const NdbDictionary::Index *pIdx = pNdb->getDictionary()->getIndex(
orderedPkIdxName, ctx->getTab()->getName());
NdbRestarter restarter;
const int records = ctx->getNumRecords();

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable records is not initialized

Suggested change
const int records = ctx->getNumRecords();
const int records = 0 = ctx->getNumRecords();

const int someRecords = MIN(records, 100);

/* Pre-warm transaction objects in Ndb object */
HugoTransactions hugoTrans(*pTab, pIdx);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable hugoTrans is not initialized

Suggested change
HugoTransactions hugoTrans(*pTab, pIdx);
HugoTransactions hugoTrans = 0(*pTab, pIdx);

ndbout << "------------------------------------" << endl;
const Uint32 startTransactionCount =
getTransactionObjectCount(pNdb);
int rc = checkScanErrorCase(pNdb, pTab, pIdx, &restarter, records,

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable rc is not initialized

Suggested change
int rc = checkScanErrorCase(pNdb, pTab, pIdx, &restarter, records,
int rc = 0 = checkScanErrorCase(pNdb, pTab, pIdx, &restarter, records,

@@ -21867,7 +21889,10 @@ int ha_innobase::cmp_ref(
}

if (result) {
return (result);
if (key_part->key_part_flag & HA_REVERSE_SORT) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion int -> bool

Suggested change
if (key_part->key_part_flag & HA_REVERSE_SORT) {
if ((key_part->key_part_flag & HA_REVERSE_SORT) != 0) {

que_thr_t *thr, /*!< in: query thread */
mtr_t *mtr) /*!< in: mtr; gets committed here */
{
[[nodiscard]] static dberr_t row_upd_clust_rec(ulint flags, upd_node_t *node,

Choose a reason for hiding this comment

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

⚠️ readability-function-cognitive-complexity ⚠️
function row_upd_clust_rec has cognitive complexity of 64 (threshold 50)

@@ -37,6 +37,7 @@
#include "sql/srv_session.h"

extern SERVICE_TYPE_NO_CONST(registry) * srv_registry;
extern SERVICE_TYPE_NO_CONST(registry) * srv_registry_no_lock;

Choose a reason for hiding this comment

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

⚠️ readability-redundant-declaration ⚠️
redundant srv_registry_no_lock declaration

Suggested change
extern SERVICE_TYPE_NO_CONST(registry) * srv_registry_no_lock;

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (17/45)

MYSQL *mysql = ctx->mysql;
Mysql_handle mysql_handle;
mysql_handle.mysql = mysql;
auto mcs_extn = MYSQL_COMMAND_SERVICE_EXTN(mysql);

Choose a reason for hiding this comment

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

⚠️ llvm-qualified-auto ⚠️
auto mcs_extn can be declared as auto *mcs_extn

Suggested change
auto mcs_extn = MYSQL_COMMAND_SERVICE_EXTN(mysql);
auto *mcs_extn = MYSQL_COMMAND_SERVICE_EXTN(mysql);

MYSQL *mysql = ctx->mysql;
Mysql_handle mysql_handle;
mysql_handle.mysql = mysql;
auto mcs_extn = MYSQL_COMMAND_SERVICE_EXTN(mysql);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

MYSQL *mysql = ctx->mysql;
Mysql_handle mysql_handle;
mysql_handle.mysql = mysql;
auto mcs_extn = MYSQL_COMMAND_SERVICE_EXTN(mysql);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

const char *host = ctx->host;
const char *user = ctx->user;
const char *db = ctx->db;
MYSQL_THD thd;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable thd is not initialized

Suggested change
MYSQL_THD thd;
MYSQL_THD thd = nullptr;

MYSQL_SESSION mysql_session = nullptr;

if (mysql_command_services_imp::get(
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion mysql_service_status_t (aka int) -> bool

Suggested change
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry) != 0)

MYSQL_SESSION mysql_session = nullptr;

if (mysql_command_services_imp::get(
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

MYSQL_SESSION mysql_session = nullptr;

if (mysql_command_services_imp::get(
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))

Choose a reason for hiding this comment

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

⚠️ google-readability-casting ⚠️
C-style casts are discouraged; use reinterpret_cast

Suggested change
(MYSQL_H)&mysql_handle, MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))
reinterpret_cast<MYSQL_H>(&mysql_handle), MYSQL_NO_LOCK_REGISTRY, &no_lock_registry))

if (mysql_session == nullptr) return STATE_MACHINE_FAILED;
thd = mysql_session->get_thd();
mcs_extn->is_thd_associated = false;
Security_context_handle sc;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-init-variables ⚠️
variable sc is not initialized

Suggested change
Security_context_handle sc;
Security_context_handle sc = nullptr;

thd = mysql_session->get_thd();
mcs_extn->is_thd_associated = false;
Security_context_handle sc;
if (mysql_security_context_imp::get(thd, &sc)) return STATE_MACHINE_FAILED;

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion mysql_service_status_t (aka int) -> bool

Suggested change
if (mysql_security_context_imp::get(thd, &sc)) return STATE_MACHINE_FAILED;
if (mysql_security_context_imp::get(thd, &sc) != 0) return STATE_MACHINE_FAILED;

mcs_extn->is_thd_associated = false;
Security_context_handle sc;
if (mysql_security_context_imp::get(thd, &sc)) return STATE_MACHINE_FAILED;
if (mysql_security_context_imp::lookup(sc, user, host, nullptr, db))

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion mysql_service_status_t (aka int) -> bool

Suggested change
if (mysql_security_context_imp::lookup(sc, user, host, nullptr, db))
if (mysql_security_context_imp::lookup(sc, user, host, nullptr, db) != 0)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (18/45)

mysql->thd = thd;
mcs_extn->session_svc = mysql_session;
} else {
mysql->thd = reinterpret_cast<void *>(mcs_extn->mcs_thd);

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

mcs_extn->command_consumer_services = new mysql_command_consumer_refs();
}
mysql_command_consumer_refs *consumer_refs =
(mysql_command_consumer_refs *)mcs_extn->command_consumer_services;

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

mcs_extn->command_consumer_services = new mysql_command_consumer_refs();
}
mysql_command_consumer_refs *consumer_refs =
(mysql_command_consumer_refs *)mcs_extn->command_consumer_services;

Choose a reason for hiding this comment

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

⚠️ google-readability-casting ⚠️
C-style casts are discouraged; use static_cast

Suggested change
(mysql_command_consumer_refs *)mcs_extn->command_consumer_services;
static_cast<mysql_command_consumer_refs *>(mcs_extn->command_consumer_services);

else if (((class mysql_command_consumer_refs *)(command_consumer_srv))
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle)) {
if (mcs_extn->consumer_srv_data != nullptr) {
((class mysql_command_consumer_refs *)(command_consumer_srv))

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

else if (((class mysql_command_consumer_refs *)(command_consumer_srv))
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle)) {
if (mcs_extn->consumer_srv_data != nullptr) {
((class mysql_command_consumer_refs *)(command_consumer_srv))

Choose a reason for hiding this comment

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

⚠️ google-readability-casting ⚠️
C-style casts are discouraged; use static_cast

Suggested change
((class mysql_command_consumer_refs *)(command_consumer_srv))
(static_cast<class mysql_command_consumer_refs *>(command_consumer_srv))

if (mcs_extn->consumer_srv_data != nullptr) {
((class mysql_command_consumer_refs *)(command_consumer_srv))
->factory_srv->end(
reinterpret_cast<SRV_CTX_H>(mcs_extn->consumer_srv_data));

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-reinterpret-cast ⚠️
do not use reinterpret_cast

reinterpret_cast<SRV_CTX_H>(mcs_extn->consumer_srv_data));
}
if (((class mysql_command_consumer_refs *)(command_consumer_srv))
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle)) {

Choose a reason for hiding this comment

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

⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion mysql_service_status_t (aka int) -> bool

Suggested change
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle)) {
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle) != 0) {

->factory_srv->end(
reinterpret_cast<SRV_CTX_H>(mcs_extn->consumer_srv_data));
}
if (((class mysql_command_consumer_refs *)(command_consumer_srv))

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

->factory_srv->end(
reinterpret_cast<SRV_CTX_H>(mcs_extn->consumer_srv_data));
}
if (((class mysql_command_consumer_refs *)(command_consumer_srv))

Choose a reason for hiding this comment

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

⚠️ google-readability-casting ⚠️
C-style casts are discouraged; use static_cast

Suggested change
if (((class mysql_command_consumer_refs *)(command_consumer_srv))
if ((static_cast<class mysql_command_consumer_refs *>(command_consumer_srv))

reinterpret_cast<SRV_CTX_H>(mcs_extn->consumer_srv_data));
}
if (((class mysql_command_consumer_refs *)(command_consumer_srv))
->factory_srv->start(&srv_ctx_h, (MYSQL_H *)&mysql_handle)) {

Choose a reason for hiding this comment

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

⚠️ cppcoreguidelines-pro-type-cstyle-cast ⚠️
do not use C-style cast to convert between unrelated types

It produces unmanagable flood of warnings.
The issue should be addressed separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.