Mcol 2044 update ranges during dml #1771

mariadb-SergeyZefirov · 2021-02-16T17:19:33Z

Relevant task: https://jira.mariadb.org/browse/MCOL-2044?focusedCommentId=178041&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel

dbcon/dmlpackageproc/commandpackageprocessor.cpp

dbcon/dmlpackageproc/dmlpackageprocessor.cpp

mtr/basic/r/mcol2044_signed_bigint_delete-drop-max.result

writeengine/server/we_ddlcommandproc.cpp

writeengine/wrapper/we_colop.cpp

writeengine/wrapper/writeengine.cpp

drrtuy · 2021-02-25T09:42:17Z

writeengine/wrapper/writeengine.cpp

+        uint64_t uvalue, oldUValue;
+
+	// fetching. May not assign value or oldValue variables when array is not present.
+        switch (colType)


Despite the fact that CPU branch prediction should eliminate the negative effect of a switch called in a loop is it possible to move the switch above the loop?

I also don't see int128_t support here.

Now we do support wide decimals.

Plz put the switch statement outside of the loop using std::function wrapper to save needed fetchNewOldValues<T, T> and call it later in the loop.

writeengine/wrapper/writeengine.cpp

drrtuy

In general the approach is OK. There are some comments/suggestions that should be addressed or replied though.

versioning/BRM/brmtypes.h

writeengine/wrapper/we_colop.cpp

writeengine/wrapper/writeengine.cpp

writeengine/shared/we_brm.h

writeengine/wrapper/writeengine.cpp

drrtuy

Plz address the comentaries.

Progress keep and test commit Progress keep and test commit Progress keep and test commit Again, trying to pinpoint problematic part of a change Revert "Again, trying to pinpoint problematic part of a change" This reverts commit 71874e7. Revert "Progress keep and test commit" This reverts commit 63c7bc6. Revert "Progress keep and test commit" This reverts commit 121c09f. Small steps - I walk minefield here Propagating changes - now CPInfo in convertValArray Progress keep commit Restoring old functionality Progress keep commit Small steps to avoid/better locate old problem with the write engine. Progress keeping commit Thread the CPInfo up to convertValArray call in writeColumnRec About to test changes - I should get no regression and no updates in ranges either. Testing out why I get a regression Investigating source of regression Debugging prints Fix compile error Debugging print - debug regression I clearly see calls to writeColumnRec and prints there added to discern between these. Fix warning error Possible culprit Add forgotten default parameter for convertValArray New logic to test Max/min gets updated during value conversion To test results of updates Debug logs Debug logs An attempt to provide proper sequence index Debug logs An attempt to provide proper sequence index - now magic for resetting Debug logs Debug logs Debug logs Trying to perform correct updates Trying to perform correct updates - seqNum woes fight COMMIT after INSERT performs 'mark extent as invalid' operation - investigating To test: cut setting of CPInfo upon commit from DML processor It may be superfluous as write engine does that too Debug logs Debug logs Better interface for CPMaxMin Old interface forgot to set isBinaryColumn field Possible fix for the problems I forgot to reassign the value in cpinfoList Debug logs Computation of 'binary' column property logs indicated that it was not set in getExtentCPMaxMin, and it was impossible to compute there so I had to move that into writeengine. To test: code to allow cross-extent insertion To test: removed another assertion for probable cause of errors Debug logs Dropped excessive logs Better reset code Again, trying to fix ordering Fixing order of rowids for LBID computation Debug logs Remove update of second LBID in split insert I have to validate incorrect behaviour for this test Restoring the case where everything almost worked Tracking changes in newly created extents Progress keeping commit Fixing build errors with recent server An ability to get old values from blocks we update Progress keeping commit Adding analysis of old values to write engine code. It is needed for updates and deletes. Progress keeping commit Moving max/min range update from convertValArray into separate function with simpler logic. To test and debug - logic is there Fix build errors Update logic to debug There is a suspicious write engine method updateColumnRecs which receives a vector of column types but does not iterate over them (otherwise it will be identical to updateColumnRec in logic). Other than that, the updateColumnRec looks like the center of all updates - deleteRow calls it, for example, dml processor also calls it. Debug logs for insert bookkeeping regression Set up operation type in externally-callable interface Internal operations depend on the operation type and consistency is what matters there. Debug logs Fix for extent range update failure during update operation Fix build error Debug logs Fix for update on deletion I am not completely sure in it - to debug. Debug log writeColumnRec cannot set m_opType to UPDATE unconditionally It is called from deleteRow Better diagnostics Debug logs Fixed search condition Debug logs Debugging invalid LBID appearance Debug logs - fixed condition Fix problems with std::vector reallocation during growth Fix growing std::vector data dangling access error Still fixing indexing errors Make in-range update to work Correct sequence numbers Debug logs Debug logs Remove range drop from DML part of write engine A hack to test the culprit of range non-keeping Tests - no results for now MTR-style comments Empty test results To be filled with actual results. Special database and result selects for all tests Pleasing MTR with better folder name Pleasing MTR - testing test result comparison Pleasing MTR by disabling warnings All test results Cleaning up result files Reset ranges before update Remove comments from results - point of failure in MTR Remove empty line from result - another MTR failure point Probably fix for deletes Possible fix for remaining failed delete test Fix a bug in writeRows It should not affect delete-with-range test case, yet it is a bug. Debug logs Debug logs Tests reorganization and description Support for unsigned integer for new tests Fix type omission Fix test failure due to warnings on clean installation Support for bigint to test Fix for failed signed bigint test Set proper unsignedness flag Removed that assignment during refactoring. Tests for types with column width 1 and 2 Support for types in new tests Remove trailing empty lines from results Tests had failed because of extra empty lines. Remove debug logs Update README with info about new tests Move tests for easier testing Add task tag to tests Fix invalid unsaigned range check Fix for signed types Fix regressions - progress keeping commit Do not set invalid ranges into valid state A possible fix for mcs81_self_join test MCOL 2044 test database cleanup Missing expected results Delete extraneous assignment to m_opType nullptr instead of NULL Refactor extended CPInfo with TypeHandler Better handling of ranges - safer types, less copy-paste Fix logic error related to typo Fix logic error related to typo Trying to figure out why invalid ranges aren't displayed as NULL..NULL Debug logs Debug logs Debug logs Debug logs for worker node Debug logs for worker node in extent map Debugging virtual table fill operation Debugging virtual table fill operation Fix for invalid range computation Remove debug logs Change handling of invalid ranges They are also set, but to invalid state. Complete change Fix typo Remove unused code "Fix" for tests - -1..0 instead of NULL..NULL for invalid unsigned ranges Not a good change, yet I cannot do better for now. MTR output requires tabs instead of spaces Debug logs Debug logs Debug logs - fix build Debug logs and logic error fix Fix for clearly incorrect firstLBID in CPInfo being set - to test Fix for system catalog operations suppot Better interface to fix build errors Delete tests we cannot satisfy due to extent rescan due to WHERE Tests for wide decimals Testing support for wide decimals Fix for wide decimals tests Fix for delete within range Memory leak fix and, possible, double free fix Dispatch on CalpontSystemCatalog::ColDataType is more robust Add support for forgotten MEDINT type Add forgottent BIGINT empty() instead of size() > 0 Better layout Remove confusing comment Sensible names for special values of seqNum field Tests for wide decimal support Addressing concerns of drrtuy Remove test we cannot satisfy Final touches for PR Remove unused result file

mariadb-SergeyZefirov · 2021-04-05T18:17:21Z

I cannot correctly answer the concern about branch prediction and switch/case inside a loop using github UI.

Here's my take. First, the code works alongside deserialisation of boost::any and conversion from boost::any with the same and even greater number of elements processed. The loop inside updateMaxMinRanges can bail out earlier than complete set of rows when range becomes invalid. Second, the approach with std::function wrapper would require two additional template functions (set or not set int64_t and/or int128_t values) or use of lambda function and, subsequently, the approach will result in more source code and, possibly, even more confused compiler analysis.

I would prefer to leave what I did. And wait for strength reduction of switch/case target computation to be added to compilers.

mariadb-SergeyZefirov requested a review from drrtuy February 16, 2021 17:19

mariadb-SergeyZefirov force-pushed the MCOL-2044-update-ranges-during-DML branch from 68e535a to 34e7e6f Compare February 24, 2021 11:16